Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/fix-scroll-spy-race.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"@patternfly/pfe-core": patch
---
`ScrollSpyController`: fix race conditions on rapid and smooth scroll navigation

- Fix rapid clicks leaving stale force-release listeners that caused the active
state to fall "one click behind"
- Release force on `scrollend` instead of first IntersectionObserver callback,
preventing intermediate sections from stealing active state during smooth scroll
- Sort passed links by DOM order instead of Set insertion order, fixing incorrect
active state with non-contiguous content sections
56 changes: 47 additions & 9 deletions core/pfe-core/controllers/scroll-spy-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ export class ScrollSpyController implements ReactiveController {
/** Ignore intersections? */
#force = false;

/** AbortController to cancel previous force-release listeners */
#forceAbort?: AbortController;

/** Timeout handle for force-release safety valve */
#forceTimeout?: ReturnType<typeof setTimeout>;

/** Has the intersection observer found an element? */
#intersected = false;

Expand Down Expand Up @@ -144,10 +150,23 @@ export class ScrollSpyController implements ReactiveController {
hostDisconnected(): void {
ScrollSpyController.#instances.delete(this);
this.#io?.disconnect();
this.#releaseForce();
}

#initializing = true;

/** Cancel force mode and clean up associated listeners */
#releaseForce() {
if (!this.#force) {
return;
}
this.#force = false;
this.#forceAbort?.abort();
this.#forceAbort = undefined;
clearTimeout(this.#forceTimeout);
this.#forceTimeout = undefined;
}

async #initIo() {
const rootNode = this.#getRootNode();
if (rootNode instanceof Document || rootNode instanceof ShadowRoot) {
Expand Down Expand Up @@ -193,25 +212,34 @@ export class ScrollSpyController implements ReactiveController {

async #nextIntersection() {
this.#intersected = false;
// safeguard the loop
setTimeout(() => this.#intersected = false, 3000);
// safeguard: break the loop after 3s even if no intersection fires
const timer = setTimeout(() => this.#intersected = true, 3000);
while (!this.#intersected) {
await new Promise(requestAnimationFrame);
}
clearTimeout(timer);
}

async #onIo(entries: IntersectionObserverEntry[]) {
if (!this.#force) {
for (const { target, boundingClientRect, intersectionRect } of entries) {
for (const entry of entries) {
const { target, boundingClientRect } = entry;
const selector = `:is(${this.#tagNames.join(',')})[href="#${target.id}"]`;
const link = this.host.querySelector(selector);
if (link) {
this.#markPassed(link, boundingClientRect.top < intersectionRect.top);
// Mark as passed if the element's top has reached the root's top edge.
// Using rootBounds (not intersectionRect) so that elements exactly AT the
// viewport top are correctly considered "passed" (the current section).
const rootTop = entry.rootBounds?.top ?? 0;
this.#markPassed(link, boundingClientRect.top <= rootTop + 2);
}
}
const link = [...this.#passedLinks];
const last = link.at(-1);
this.#setActive(last ?? this.#linkChildren.at(0));
// Sort passed links by DOM order rather than Set insertion order
const linkOrder = this.#linkChildren;
const passed = [...this.#passedLinks]
.sort((a, b) => linkOrder.indexOf(a) - linkOrder.indexOf(b));
const last = passed.at(-1);
this.#setActive(last ?? linkOrder.at(0));
}
this.#intersected = true;
this.#intersectingTargets.clear();
Expand Down Expand Up @@ -242,16 +270,26 @@ export class ScrollSpyController implements ReactiveController {
* @param link usually an `<a>`
*/
public async setActive(link: EventTarget | null): Promise<void> {
// Cancel any previous programmatic scroll's force state
this.#forceAbort?.abort();
clearTimeout(this.#forceTimeout);

this.#force = true;
this.#setActive(link);

let sawActive = false;
for (const child of this.#linkChildren) {
this.#markPassed(child, !sawActive);
if (child === link) {
sawActive = true;
}
}
await this.#nextIntersection();
this.#force = false;

// Force is released when the scroll completes (scrollend event),
// or after a 3-second safety timeout
this.#forceAbort = new AbortController();
const { signal } = this.#forceAbort;
addEventListener('scrollend', () => this.#releaseForce(), { once: true, signal });
this.#forceTimeout = setTimeout(() => this.#releaseForce(), 3000);
}
}
Loading
Loading