Skip to content

Conversation

@DenisLopatin
Copy link

@DenisLopatin DenisLopatin commented Sep 5, 2025

Closes: #39198
Closes: #37858
Closes: #39248
Fix: #35566 (Hope, see below)

Description

Allows you to select the elements in the Scrollspy component that need to be escaped.

Motivation & Context

Users complain that they can't always control how their IDs for a component are filled in.

Type of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would change existing functionality)

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

Related issues

#39198
#37858
#32586
#35566 (very important)

In PR #35566, @pierresouchay added a selector search mechanism when escaping selector = selector.replaceAll(/#([^\s"#']+)/g, (match, id) => '#' + CSS.escape(id)). I removed the # symbol from there, as it interfered with the current implementation, but did not affect the previous functionality (during manual and automatic testing). I do not know why it was not necessary to escape the identifier inside the expression (#firistid#secondId is invalid), but the functionality works. In the worst case, it was necessary, but they just forgot to write the tests. At best, everything is fine.

P.S. A lot of questions, so I will be glad to receive feedback))

@DenisLopatin DenisLopatin requested a review from a team as a code owner September 5, 2025 21:05
@XhmikosR XhmikosR changed the title normalizes the selector operation in scrollspy Normalize selector operation in scrollspy Sep 7, 2025
@chalin
Copy link
Contributor

chalin commented Nov 16, 2025

Thanks for the fix @DenisLopatin.

@XhmikosR et all: What can be done to help get this merged and released? What are the roadblocks, if any?

FYI, in the meantime I've implemented a (partial) workaround in Hugo/Docsy (via google/docsy#2370), but I'd love to see this complete solution land.

@chalin
Copy link
Contributor

chalin commented Nov 24, 2025

@DenisLopatin - your solution works nicely!

I couldn't wait for this to land, so I created a runtime patcher. Linking to it here in case it can be helpful to others in the meantime: https://www.docsy.dev/site/implementation/scrollspy-patch/

@DenisLopatin
Copy link
Author

@DenisLopatin - your solution works nicely!

It's good. The bootstrap team is probably busy with their own business right now).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

4 participants