fix(client): honor scroll-margin-top when scrolling to hash

e2e/docs/router/scroll-to-hash-target.md

@@ -0,0 +1,16 @@
+## Target
+
+Target content
+
+<div class="spacer"></div>
+
+<style>
+.spacer {
+  height: 1000px;
+}
+
+#target {
+  /* keep the anchor target away from the top of the viewport */
+  scroll-margin-top: 100px;
+}
+</style>

e2e/docs/router/scroll-to-hash.md

@@ -0,0 +1,56 @@
+<button id="same-page-target" @click="goSamePageTarget">Same page target</button>
+<button id="same-page-complex-target" @click="goSamePageComplexTarget">Same page target with a complex id</button>
+<button id="another-page-target" @click="goAnotherPageTarget">Another page target</button>
+<button id="another-page" @click="goAnotherPage">Another page without a hash</button>
+
+## Links
+
+- [Another page target](./scroll-to-hash-target.md#target)
+
+<div class="spacer"></div>
+
+## Target
+
+Target content
+
+<div class="spacer"></div>
+
+<h2 id="target:1">Target with a complex id</h2>
+
+Target content
+
+<div class="spacer"></div>
+
+<script setup lang="ts">
+import { useRouter } from 'vuepress/client';
+
+const router = useRouter();
+
+const goSamePageTarget = () => {
+  router.push('#target');
+}
+
+const goSamePageComplexTarget = () => {
+  router.push('#target:1');
+}
+
+const goAnotherPageTarget = () => {
+  router.push('/router/scroll-to-hash-target.html#target');
+}
+
+const goAnotherPage = () => {
+  router.push('/router/scroll-to-hash-target.html');
+}
+</script>
+
+<style>
+.spacer {
+  height: 1000px;
+}
+
+#target,
+#target\:1 {
+  /* keep the anchor target away from the top of the viewport */
+  scroll-margin-top: 100px;
+}
+</style>

e2e/tests/router/scroll-to-hash.spec.ts

@@ -0,0 +1,81 @@
+import type { Page } from '@playwright/test'
+import { expect, test } from '@playwright/test'
+
+import { BASE } from '../../utils/env'
+
+/**
+ * The target elements of these pages have a `scroll-margin-top` of `100px`, so
+ * scrolling to them should leave them `100px` below the top of the viewport.
+ */
+const SCROLL_MARGIN_TOP = 100
+
+const getTargetTop = (page: Page, id: string): Promise<number> =>
+  page.evaluate(
+    (targetId) =>
+      document.getElementById(targetId)!.getBoundingClientRect().top,
+    id,
+  )
+
+/**
+ * The scroll is deferred to the next frame, so the assertion has to wait for it
+ * instead of reading the position right after the URL has changed.
+ */
+const expectTargetTop = (page: Page, id: string): Promise<void> =>
+  expect
+    .poll(async () => Math.round(await getTargetTop(page, id)))
+    .toBe(SCROLL_MARGIN_TOP)
+
+test.beforeEach(async ({ page }) => {
+  await page.goto('router/scroll-to-hash.html')
+})
+
+test('should honor scroll-margin-top when navigating to a hash on the same page', async ({
+  page,
+}) => {
+  await page.locator('#same-page-target').click()
+
+  await expect(page).toHaveURL(`${BASE}router/scroll-to-hash.html#target`)
+  await expectTargetTop(page, 'target')
+})
+
+test('should honor scroll-margin-top when the hash contains special characters', async ({
+  page,
+}) => {
+  await page.locator('#same-page-complex-target').click()
+
+  await expect(page).toHaveURL(`${BASE}router/scroll-to-hash.html#target:1`)
+  await expectTargetTop(page, 'target:1')
+})
+
+test('should honor scroll-margin-top when navigating to a hash on another page', async ({
+  page,
+}) => {
+  await page.locator('#another-page-target').click()
+
+  await expect(page).toHaveURL(
+    `${BASE}router/scroll-to-hash-target.html#target`,
+  )
+  await expectTargetTop(page, 'target')
+})
+
+test('should honor scroll-margin-top when navigating by a markdown link', async ({
+  page,
+}) => {
+  await page.locator('#links + ul > li > a').click()
+
+  await expect(page).toHaveURL(
+    `${BASE}router/scroll-to-hash-target.html#target`,
+  )
+  await expectTargetTop(page, 'target')
+})
+
+test('should scroll to the top when navigating to another page without a hash', async ({
+  page,
+}) => {
+  await page.locator('#same-page-target').click()
+  await expect(page).toHaveURL(`${BASE}router/scroll-to-hash.html#target`)
+
+  await page.locator('#another-page').click()
+  await expect(page).toHaveURL(`${BASE}router/scroll-to-hash-target.html`)
+  await expect.poll(async () => page.evaluate(() => window.scrollY)).toBe(0)
+})

packages/client/src/router/createVueRouter.ts

@@ -32,7 +32,25 @@ export const createVueRouter = (): Router => {
     ],
     scrollBehavior: (to, _from, savedPosition) => {
       if (savedPosition) return savedPosition
-      if (to.hash) return { el: to.hash }
+      if (to.hash) {
+        const id = decodeURIComponent(to.hash).slice(1)
+
+        // scroll with `scrollIntoView` instead of letting vue-router resolve
+        // the element, so that `scroll-margin-top` is honored. Themes rely on
+        // it to keep anchor targets clear of a fixed navbar.
+        //
+        // It is also deferred to the next frame, because the target element of
+        // a hash link pointing to another page has not been rendered yet when
+        // the scroll behavior is called.
+        requestAnimationFrame(() => {
+          // `getElementById` is used instead of `querySelector`, as an id may
+          // contain characters that are invalid in a CSS selector, e.g. the
+          // space in `#foo bar` or the `:` in `#foo:1`
+          document.getElementById(id)?.scrollIntoView({ block: 'start' })
+        })
+
+        return false
+      }
       return { top: 0 }
     },
   })