ScrollSpy: deterministic active-section detection#42557
Merged
Conversation
7b08736 to
7aefcb2
Compare
Replace the scroll-direction + sequential-offsetTop heuristic (which depended on IntersectionObserver entry order and mis-highlighted when several sections shared the viewport) with a deterministic geometric model: the active section is the deepest one whose top has scrolled to or above an activation line near the top of the scroll root, read fresh from getBoundingClientRect on each (rAF-throttled) scroll. At the bottom the last section wins; above the first nothing is active. Also: settle on scrollend (with a scroll-idle fallback) to restore the URL hash via replaceState and move focus to the target after a smooth-scroll navigation; escape section ids via parseSelector so dotted /special-character ids no longer throw; add a topMargin option for the activation line. Supersedes #41016. Fixes #37858, #39198, #39248, #36387, #40526.
ddcd3ec to
29d393f
Compare
- Add 'Fpath' to the cspell dictionary (from the #%2Fpath encoded-id example) - Bump the four JS bundlewatch thresholds for the rewritten ScrollSpy - Add option-derived activation-line tests (topMargin %/px, derived rootMargin with/without a scroll root, non-numeric topMargin, explicit rootMargin, overflow state, observer rebuild no-op, resize listener add/remove) to lift branch coverage from 88.63% back over the 89% threshold (now 89.21%)
Mirror the migration.mdx ScrollSpy changelog in the migration skill: IntersectionObserver rewrite / activation line, removed offset+method options, new topMargin, rootMargin default null, threshold default [0], smoothScroll hash+focus restore, and getElementById id resolution. Keeps the skill and guide consistent as this PR lands ScrollSpy.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ScrollSpy chose the active section with a scroll-direction flag + a sequential
offsetToptiebreaker that mutated state while walking IntersectionObserver entries. IO delivers entries in no guaranteed order, so when several sections shared the viewport (e.g. the docs right-rail TOC) the highlight was wrong. Coarse defaults also left stale gaps, smooth scroll suppressed the URL hash, focus wasn't managed, and ids with dots/special characters threw.Change
Detection is now IntersectionObserver-native and deterministic — no scroll-position polling and no per-frame
getBoundingClientRect()sweep:rootMargin) to a strip from the top of the scroll root down to an activation line. A smallSetof currently-intersecting sections is maintained from the IO entries, and the active section is the deepest of them. Work happens only when a section crosses the line — a handful of times per page traversal, not every animation frame.aria-hiddenbottom sentinel (its own observer) activates the last section even when its top never reaches the line — replacing the oldscrollTopmath.decodeURIComponent+getElementById(scoped withcontains), so dotted/colon/slash/percent-encoded ids resolve without escaping.scrollendis now registered as a real native event (added toEventHandler), with a transient scroll-idle fallback. On a smooth-scroll click the settle restores the URL hash viahistory.replaceStateand moves focus to the section (tabindex="-1"+focus({ preventScroll: true })). When the target needs no scroll (already there / reduced motion) it settles immediately instead of stranding the pending navigation.topMarginoption (default12%, accepts%orpx, e.g.96pxbelow a sticky navbar) positions the activation line;rootMarginis an advanced override that takes precedence and is passed straight to the observer.thresholddefault is now[0]. The long-deprecatedoffset/methodoptions are removed. A debouncedresizerebuild keeps apxactivation line accurate.Issues / PRs
getElementById).Verification
rootMarginpassthrough, resize rebuild, immediate settle when already at target, and the settle -> hash/focus behavior.data-bs-top-margin="96px"(continuous scroll advances cleanly through every section with nested h2/h3 highlighting); smooth-scroll click restores the hash and moves focus.The docs right-rail TOC uses Bootstrap ScrollSpy directly, so it's fixed transitively.