ScrollSpy: improve active feedback

[guilhas07]

Motivation & Context

Current ScrollSpy behavior had some shortcomings when it comes to detecting the correct active class.

• Sometimes none of the classes are active
• After clicking to go to a section, it immediately shows as non active

Solution

To fix this I simplified the current logic, by adding an active state for all sections and keeping track of the previous highlighted section. The current rational is to highlight the first section that is intersecting at all times.

Different logic for scrolling up or down and custom thresholds are is removed due to not being needed anymore.

Edit: My previous implementation only considered header elements, which are small due to not wrapping the text in their section. However, when using entire div elements, it became impossible to activate the next element because, in many cases, two elements would be visible simultaneously. To address this, I now activate the element with the larger intersection ratio. To achieve this, I keep track of each element's intersection ratio and have increased the thresholds to recalculate the intersection ratios at every 10% increment.

Changes

Scroll Behaviour:

• Before ( Note that sometimes the active class disappears and other times it jumps to early ):

old-scroll.mp4

• After:

new-scroll.mp4

Click Behaviour:

• Before (Note the section clicked isn't active after auto scroll)

old-click.mp4

• After

new-click.mp4

Type of changes

• Bug fix (non-breaking change which fixes an issue)
• Refactoring (non-breaking change)

Checklist

• I have read the contributing guidelines
• My code follows the code style of the project (using npm run lint)
• My change introduces changes to the documentation
• I have updated the documentation accordingly
• I have added tests to cover my changes
• All new and existing tests passed

Live Previews

https://deploy-preview-41016--twbs-bootstrap.netlify.app/docs/5.3/getting-started/introduction/

[Manchmal1]

Was muß ich machen um die auf dem Fernseher zu sehen bitte

[julien-deramond]

Sorry for the delay, @guilhas07, and thank you for taking the time to work on this PR!
I haven't digged into the history of the implementation of the Scrollspy component yet, nor looked at your code.
Here's my feedback after testing and comparing the behavior with the current implementation of the Scrollspy component.

• General improvement: Overall, this feels less buggy, as all items are active while scrolling. This is a great enhancement!
• Behavior on specific pages: On the Reboot documentation page, for example, the last element is active in the current implementation but not in your branch. Personally, I don’t mind this change—it aligns with how some other frameworks (e.g., Astro/Starlight) handle small sections that "stick" to the bottom of the page.
• In the Scrollspy Navbar example from your branch, selecting an item from the dropdown and then scrolling results in two .active elements simultaneously (which shouldn't happen IMO).

• Possibly linked to one of my previous comments, in the Scrollspy Nested Nav example from your branch, the last element (3.2) never becomes active, even when clicked.

To wrap up, I think this PR has potential, but we need to address these edge cases. Once finalized, it would be great to add strict unit tests to enforce the new behavior. These tests could also act as documentation for the expected behavior, helping prevent regressions in the future.

Edit: be careful, the tests fail right now. You can run npm run js-test locally to replicate the issues.

Anyone interested in this topic, feel free to jump in and share your feedback! (/cc @twbs/js-review, you have more expertise on this than I do).

(Note for us, maintainers: if this PR ends up being the main one to fix Scrollspy issues, let's add the references of all the issues related to the Scrollspy)

[guilhas07]

First of all thank you for your feedback!

General improvement: Overall, this feels less buggy, as all items are active while scrolling. This is a great enhancement!

Thank you!

Behavior on specific pages: On the Reboot documentation page, for example, the last element is active in the current implementation but not in your branch. Personally, I don’t mind this change—it aligns with how some other frameworks (e.g., Astro/Starlight) handle small sections that "stick" to the bottom of the page.

I'm afraid as you already mentioned that is hard to do because technically there are two elements fully shown.

In the Scrollspy Navbar example from your branch, selecting an item from the dropdown and then scrolling results in two .active elements simultaneously (which shouldn't happen IMO).

Fixed.

Possibly linked to one of my previous comments, in the Scrollspy Nested Nav example from your branch, the last element (3.2) never becomes active, even when clicked.

Also fixed but this led me to change the current implementation.

To wrap up, I think this PR has potential, but we need to address these edge cases. Once finalized, it would be great to add strict unit tests to enforce the new behavior. These tests could also act as documentation for the expected behavior, helping prevent regressions in the future.
Edit: be careful, the tests fail right now. You can run npm run js-test locally to replicate the issues.

I'm looking into it. I've modified some tests to align with the new behavior, and I will try to come up with new ones (if you have any suggestions, I'm all ears). Currently, there is one test failing: ScrollSpy constructor should not process data if the activeTarget is the same as the given target. Do we need to keep this behavior? Restored the behavior, so now every test is passing locally.

Changes:

My previous implementation only considered header elements, which are small due to not wrapping the text in their section. However, when using entire div elements, it became impossible to activate the next element because, in many cases, two elements would be visible simultaneously. To address this, I now activate the element with the larger intersection ratio. To achieve this, I keep track of each element's intersection ratio and have increased the thresholds to recalculate the intersection ratios at every 10% increment.

[Manchmal1]

Danke für die Mühe und ganze Arbeit 👍👍

[codyrobbins]

Thanks for taking a stab at finally fixing this! In case it’s helpful to cross-reference, there’s a long thread at #36431 discussing various aspects of the current buggy behavior.

[Manchmal1]

Ost es aktiv was muss ich machen um es zu aktivieren das es auf meinem zandy oder tv läuft

[guilhas07]

@julien-deramond Hey! I've pushed some changes and separated them into commits for easier review. When you have a moment, PTAL.

@codyrobbins Thanks for linking the issue! Would you mind testing this version and sharing your thoughts on it?

---

I found the wrong smoothscroll behaviour in my implementation going over the local website build, but couldn't recreate a test due to being dependent on multiple factors. I think the gist is that smoothscroll behaviour, unlike auto, doesn't scroll the correct amount of pixels which ends up not triggering the intersection observer API. That is why I introduced a scrollOffset.

[DenisLopatin]

Hi. Will this close #36431? Or is it an offshoot of it, which doesn't fix everything?

[guilhas07]

Hi. Will this close #36431? Or is it an offshoot of it, which doesn't fix everything?

For me it fixed all of my problems, but I will not make any assumptions. Feedback always welcome!

[dgatla]

Hi @julien-deramond and @guilhas07,

What are the remaining tasks for this PR? Please let me know if there’s any way I can help—I'm dependent on this fix. 🙂

Edit: Also, I noticed that this is going to target #36431. This was a "nice to have" in 5.3.4, and since it is now done, is there a plan to add this to an upcoming 5.4 project or any other bugfix version?

[julien-deramond]

@dgatla At the moment I'm unfortunately swamped with both work commitments and personal matters. Aside from bumping dependencies, I won’t be very available until early December. Around that time, I’ll need to do a first overall review of the v6 draft branch.

For this specific PR, if anyone is able to help, the most valuable contribution would be to thoroughly test the new implementation: try to break the component in as many usage scenarios as you can think of. This will help us understand whether the change is a real improvement, determine the limitations of this improvement, and also determine if it could be released as a patch, minor, or major version, depending on any breaking changes.

Thanks to everyone involved in this effort.

[DenisLopatin]

I'll just attach the task numbers here, which, in theory, this PR should ideally fix (so that it doesn't hang in a vacuum):

#41361
#36912
#36431

[mdo]

Put some time in with Claude to see if we can get a rewrite in for v6—this wouldn't be able to land in v5 unfortunately. See #42557 for that. Happy to review any feedback there, or adjust as needed. Let me know what you think!

In the mean time, I'll leave this open, but I don't expect to see it merged at this point for v5.

[DenisLopatin]

Put some time in with Claude to see if we can get a rewrite in for v6—this wouldn't be able to land in v5 unfortunately. See #42557 for that. Happy to review any feedback there, or adjust as needed. Let me know what you think!

In the mean time, I'll leave this open, but I don't expect to see it merged at this point for v5.

I have already rewrote the component, keeping the basic functionality. I rewrote all the tests and spent a week on it (some tests were just for show), but they didn't review this commit because there were too many changes. However, it is necessary to rewrite the module. Maybe we should give this commit a chance? Or at least lean on it, or do you see it somehow differently? #41914