Skip to content

fix(core): scroll-spy for non-contiguous regions#2920

Closed
bennypowers wants to merge 7 commits intomainfrom
fix/scrollspy/noncongiguous
Closed

fix(core): scroll-spy for non-contiguous regions#2920
bennypowers wants to merge 7 commits intomainfrom
fix/scrollspy/noncongiguous

Conversation

@bennypowers
Copy link
Member

See RedHat-UX/red-hat-design-system#2471

What I did

  1. Simplify the scroll-spy logic, and reconcile scroll-spy active link on scroll

Testing Instructions

  1. Check out existing jump-links elements

Notes to Reviewers

  1. There might be serious perf problems with the approach taken here so it should be validated against realworld pages

@changeset-bot
Copy link

changeset-bot bot commented Jul 22, 2025

🦋 Changeset detected

Latest commit: 616bd75

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@patternfly/pfe-core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@netlify
Copy link

netlify bot commented Jul 22, 2025

Deploy Preview for patternfly-elements ready!

Name Link
🔨 Latest commit e046932
😎 Deploy Preview https://deploy-preview-2920--patternfly-elements.netlify.app/

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 22, 2025

✅ Commitlint tests passed!

More Info
{
  "valid": true,
  "errors": [],
  "warnings": [],
  "input": "fix(core): scroll-spy for non-contiguous regions"
}

@github-actions github-actions bot added the AT passed Automated testing has passed label Jul 22, 2025
@github-actions
Copy link
Contributor

SSR Test Run for 5350d24: Report

@github-actions
Copy link
Contributor

SSR Test Run for ce07c70: Report

@github-actions
Copy link
Contributor

SSR Test Run for 000acaa: Report

@zeroedin zeroedin self-requested a review July 22, 2025 14:39
Copy link
Contributor

@zeroedin zeroedin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

linking gaps, targets managed

@github-actions
Copy link
Contributor

SSR Test Run for bd870c4: Report

@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2025

SSR Test Run for 1c05b38: Report

@github-actions
Copy link
Contributor

SSR Test Run for e046932: Report

bennypowers added a commit that referenced this pull request Mar 8, 2026
Fixes three bugs in ScrollSpyController:

1. Rapid clicks: setActive() now cancels previous force state via
   AbortController before establishing new force, preventing stale
   listeners from releasing force prematurely.

2. Force release timing: replaced await-first-intersection approach with
   scrollend event listener (+ 3s safety timeout). The old approach
   released force when any IO fired during smooth scroll, causing
   intermediate sections to steal the active state.

3. Non-contiguous sections: passedLinks are now sorted by DOM order
   instead of relying on Set insertion order, which was unreliable
   when sections had untracked content between them.

Also fixes a safeguard bug where the #nextIntersection timeout set
`#intersected = false` (should be `true`), which could cause an infinite
rAF loop, and fixes the boundary comparison to use rootBounds.top
instead of intersectionRect.top so elements exactly at the viewport
top are correctly considered "passed".

Closes #2911
Closes #2920

Assisted-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AT passed Automated testing has passed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants