From 89187b111c15eef9d43af2279c11c452a99fb0d3 Mon Sep 17 00:00:00 2001 From: "Mr.Hope" Date: Mon, 28 Sep 2026 00:28:06 +0800 Subject: [PATCH] fix(client): honor scroll-margin-top when scrolling to hash --- e2e/docs/router/scroll-to-hash-target.md | 16 ++++ e2e/docs/router/scroll-to-hash.md | 56 +++++++++++++ e2e/tests/router/scroll-to-hash.spec.ts | 81 +++++++++++++++++++ packages/client/src/router/createVueRouter.ts | 20 ++++- 4 files changed, 172 insertions(+), 1 deletion(-) create mode 100644 e2e/docs/router/scroll-to-hash-target.md create mode 100644 e2e/docs/router/scroll-to-hash.md create mode 100644 e2e/tests/router/scroll-to-hash.spec.ts diff --git a/e2e/docs/router/scroll-to-hash-target.md b/e2e/docs/router/scroll-to-hash-target.md new file mode 100644 index 0000000000..5e05e6d72f --- /dev/null +++ b/e2e/docs/router/scroll-to-hash-target.md @@ -0,0 +1,16 @@ +## Target + +Target content + +
+ + diff --git a/e2e/docs/router/scroll-to-hash.md b/e2e/docs/router/scroll-to-hash.md new file mode 100644 index 0000000000..5a3eff905a --- /dev/null +++ b/e2e/docs/router/scroll-to-hash.md @@ -0,0 +1,56 @@ + + + + + +## Links + +- [Another page target](./scroll-to-hash-target.md#target) + +
+ +## Target + +Target content + +
+ +

Target with a complex id

+ +Target content + +
+ + + + diff --git a/e2e/tests/router/scroll-to-hash.spec.ts b/e2e/tests/router/scroll-to-hash.spec.ts new file mode 100644 index 0000000000..740d034410 --- /dev/null +++ b/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 => + 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 => + 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) +}) diff --git a/packages/client/src/router/createVueRouter.ts b/packages/client/src/router/createVueRouter.ts index d1331bf86d..827be86913 100644 --- a/packages/client/src/router/createVueRouter.ts +++ b/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 } }, })