Skip to content
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

Docs(Scrollspy): minor changes to be aligned with new version of javascript #36260

Merged
merged 4 commits into from May 7, 2022

Conversation

GeoSot
Copy link
Member

@GeoSot GeoSot commented May 2, 2022

Changes:

  • first example: add proper root-margin
  • 'nested nav' example: add more content to sections & enable smooth-scroll

closes #36198

@louismaximepiton please, you may take a look and feedback me

@GeoSot GeoSot added this to In progress in v5.2.0 via automation May 2, 2022
@twbs twbs deleted a comment from 2030811fla May 2, 2022
Copy link
Member

@louismaximepiton louismaximepiton left a 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 these corrections, they look good to me,

However :

  • when a spy is clicked on the third example, the component moves under the header
  • there are still some issues with the scroll on the nested nav

site/content/docs/5.1/components/scrollspy.md Outdated Show resolved Hide resolved
site/content/docs/5.1/components/scrollspy.md Outdated Show resolved Hide resolved
@GeoSot
Copy link
Member Author

GeoSot commented May 3, 2022

Thank you a lot for your help 😄

when a spy is clicked on the third example, the component moves under the header

It was the default mode, ( I didn't like it either ) but I've changed it

there are still some issues with the scroll on the nested nav

I am aware of them, but as before, the same problem comes in front. As javascript itself is not so much clever to decide which visible point has to be active (plus the observer restrictions), all functionality sums up to developer thoughts and skills (in this situation, mine's 😋 )

So it would be great if anyone else take a look and come up with a more precise and of course minimalist solution. Ps: Passing the js tests is one more hidden challenge, in this specific component

@GeoSot GeoSot marked this pull request as ready for review May 3, 2022 09:08
@GeoSot GeoSot requested a review from a team as a code owner May 7, 2022 03:38
@mdo
Copy link
Member

mdo commented May 7, 2022

@GeoSot I pushed a bunch of updates here to the docs to clean up some of the new copy, but also rewrite a lot of content to better explain how it all works up front. Should be good to go!

GeoSot and others added 4 commits May 6, 2022 21:10
…script code

* first example: add proper root-margin
* 'nested nav' example: add more content to sections & enable smooth-scroll
@mdo mdo merged commit 92e6856 into main May 7, 2022
v5.2.0 automation moved this from In progress to Done May 7, 2022
@mdo mdo deleted the gs/scrollspy-patches branch May 7, 2022 04:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
v5.2.0
  
Done
Development

Successfully merging this pull request may close these issues.

Bug on Scrollspy
3 participants