fix(docs): fix scroll selector to scroll to top on page change #4364
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mcoker
approved these changes
Nov 11, 2024
|
@mcoker good callout, I've moved the page scrolling inside the promise to trigger after page navigation 👍 |
srambach
approved these changes
Nov 15, 2024
| @@ -117,7 +91,7 @@ export const TableOfContents = ({ items }) => { | |||
| <JumpLinks | |||
| label="Table of contents" | |||
| isVertical | |||
| scrollableSelector=".pf-v6-c-page__main-container" | |||
| scrollableSelector="#ws-page-main" | |||
| className="ws-toc" | |||
| style={{ top: stickyNavHeight }} | |||
| offset={width > 1450 ? 108 + stickyNavHeight : 148 + stickyNavHeight} | |||
There was a problem hiding this comment.
Probably a new issue - but with the jumplinks collapsed, I notice that the scroll position puts the title underneath the jumplink toggle. The page switch looks 👌🏻 though!
There was a problem hiding this comment.
@srambach I'm only able to recreate if I navigate to a jumplink on desktop layout and then shrink the screen down so the jumplinks move from the side to the top as seen in the video, did you see it anywhere else? Navigating from this point forward seems to work as expected at the new screen size.
Screen.Recording.2024-11-15.at.11.55.57.AM.mov
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #4363
This PR fixes pages not scrolling to top when navigating to a new page.
It updates the Link component and TableOfContents component to revert the scrollSelector change from #4197 which is no longer needed after Core update patternfly/patternfly#7025 was merged.