fix: keep anchor links aligned after layout changes#2731
fix: keep anchor links aligned after layout changes#2731apples-kksk wants to merge 2 commits intodocsifyjs:developfrom
Conversation
|
@apples-kksk is attempting to deploy a commit to the Docsify Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
Fixes anchor deep-link scrolling (#/?id=...) landing too early when late-loading content (e.g., images) above the target heading changes layout after the initial scroll, by re-aligning the target for a short settling window and stopping once the user interacts.
Changes:
- Refactors anchor navigation scrolling into a dedicated
#scrollToHeading()helper that re-syncs scroll position during a brief post-scroll window usingResizeObserver. - Adds a Playwright E2E regression test that simulates a delayed image above the anchor target.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/e2e/anchor-scroll.test.js | Adds an E2E regression test for anchor alignment when an image above the target loads late. |
| src/core/event/index.js | Implements post-scroll re-alignment logic for anchor targets during late layout changes and cancels on user interaction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await expect | ||
| .poll(async () => { | ||
| return page.locator('#target-section').evaluate(el => { | ||
| return el.getBoundingClientRect().top; | ||
| }); | ||
| }) | ||
| .toBeLessThan(80); |
| window.addEventListener('load', resync, { once: true }); | ||
| timers.max = setTimeout(cancel, 3000); | ||
| requestAnimationFrame(() => requestAnimationFrame(resync)); | ||
|
|
| /** @param {ScrollBehavior} [behavior] */ | ||
| const scrollToHeading = (behavior = 'smooth') => { | ||
| if (!document.contains(headingElm)) { | ||
| cancel(); | ||
| return; | ||
| } | ||
|
|
||
| this.#watchNextScroll(); | ||
| headingElm.scrollIntoView({ | ||
| behavior, | ||
| block: 'start', | ||
| }); | ||
| }; |
|
Addressed the review feedback in cf22daf: the regression test now waits for the delayed image to finish loading before checking the target, anchor resync stops after cancellation, and instant resyncs no longer register extra scroll watchers.\n\nVerified with:\n- npx prettier --check src/core/event/index.js test/e2e/anchor-scroll.test.js\n- npx eslint src/core/event/index.js test/e2e/anchor-scroll.test.js\n- npm run build:js\n- npm run test:e2e:chromium -- anchor-scroll.test.js |
Summary
Direct links like
#/?id=targetcan land too early when images or other content above the heading are still changing the page height. This keeps the requested heading aligned for a short settling window after the first scroll, and stops doing that as soon as the user starts interacting with the page.I also added a Playwright regression that delays an image above the target heading and checks that the heading still ends up near the top of the viewport.
Related issue, if any:
Fixes #351
What kind of change does this PR introduce?
For any code change,
Does this PR introduce a breaking change?
Tested in the following browsers:
Checked locally with
npm run lint,npm run build:js,npm run test:e2e:chromium -- anchor-scroll.test.js, andgit diff --check.