-
-
Notifications
You must be signed in to change notification settings - Fork 78.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use scrollspy on docs pages #33428
Use scrollspy on docs pages #33428
Conversation
6f99f9e
to
aaad5be
Compare
Oh, nice! I'll check it out after beta3 to see what we can improve (not a huge fan of the replacements, but I guess there's no other solution right now since Hugo doesn't allow us to customize the ToC template). Also, sort of looks like it's a few pixels off. I'm pretty sure @mdo will tweak the CSS as needed because currently it looks a little weird 😛 |
aaad5be
to
3d460e7
Compare
3d460e7
to
0f2c5ce
Compare
0f2c5ce
to
b02e142
Compare
Redesigned things a bit to add a hover state and restore the v3-ish sidebar highlighting. Here's an example of nested active items plus a hovered item down the page. @GeoSot Is it possible to implement an offset for the scrollspy anchors? Our sticky subnav gets in the way I think, so when I click on the ToC item, it doesn't get highlighted until I scroll. |
Btw, I'm game for this in v5.1 (or v5.1.1) or v5.2 depending on everyone else :). |
Glad to hear it, cause I cannot do much more on the current implementation 😢 I 'll give it a try but my real hope is on Intersection observer #33421 |
@mdo on the CSS side, can't we have a transparent left border in the ToC items so that they don't jump when the active class is added? |
We may also use BTW I really like it, thanks @GeoSot :) |
The accuracy of the highlighting feels a little off in at least Safari for me. Anything we can do to help with that? |
This is a little stale, but this was how I implemented it already :). |
Any chance we can check in on this one though? |
b02e142
to
43d9219
Compare
43d9219
to
9e39327
Compare
9e39327
to
b84d966
Compare
b84d966
to
b95a1a4
Compare
b95a1a4
to
2ac2fb2
Compare
2ac2fb2
to
a4a0c05
Compare
Played with this more locally and it's a little off—sections are getting skipped. Is there anything else to improve here or in the scrollspy implementation itself? |
dfdebd7
to
a983f6f
Compare
a983f6f
to
ce2015e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for this work! Is there a chance to see it live in Bootstrap anytime soon ?
I don't know if the design is up to date but it looks fine to me !
The only issues I can see so far are due to the scrollspy component itself so I think it could be merged as-is ?
@@ -1,3 +1,4 @@ | |||
{{ define "body_override" }}<body data-bs-spy="scroll" tabindex="0" data-bs-target="#TableOfContents">{{ end }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that there should be a block {{ if (eq .Page.Params.toc true) }}
around to avoid trigger the js when there's no need.
That said, I wonder if these attributes should be on .bd-main
, .bd-content
or keep it here ?
Some sections are off indeed but it's more likely coming from the Scrollscpy component itself. |
523b75b
to
637a112
Compare
413999b
to
03b5528
Compare
closes #30977
Maybe can have better choices on css and classes
Preview: https://deploy-preview-33428--twbs-bootstrap.netlify.app/docs/5.2/getting-started/introduction/