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
Fix scroll restoration for layout components #26861
Fix scroll restoration for layout components #26861
Conversation
this is required to match the type of `useRef` which requires the target element type to be specified. The hook can be used like the following: ```typescript const MyComponent: React.FunctionComponent = () => { const scrollRestorationProps = useScrollRestoration<HTMLDivElement>(`some-key`) return ( <div {...scrollRestorationProps} style={{ overflow: `auto` }} > Test </div> ) } ``` Fixes: gatsbyjs#26458
Previously, the scroll position was only updated when the using component was re-rendered. This did not work when the hook is used in a wrapping Layout component, becaus its persitent over location changes. Adding the location key to useEffect will cause the scroll position is updated every time the key changes.
330ac40
to
987d950
Compare
@wardpeet Would you like to add the Can you estimate when this PR will be reviewed? I had to add a really nasty workaround to our website to be able to go live and would like to remove it as soon as possible. |
I don't know if it works that way but i'll review it tomorrow, if not feel free to re-open and re-create it |
Unfortunately it did not work. Anyway, I don't want to interrupt the review process. Thank you for trying… I am looking forward to the outcome of the review! |
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.
Thank you for the PR! It looks good to me 💜
Sorry for not coming back to this in time - we are a bit short on time dedicated to PR reviews 😞
…ents (#26861) * add generic type to useScrollRestoration hook this is required to match the type of `useRef` which requires the target element type to be specified. The hook can be used like the following: ```typescript const MyComponent: React.FunctionComponent = () => { const scrollRestorationProps = useScrollRestoration<HTMLDivElement>(`some-key`) return ( <div {...scrollRestorationProps} style={{ overflow: `auto` }} > Test </div> ) } ``` Fixes: #26458 * add tests for useScrollRestoration hook * fix useScrollResotration to update position on location change Previously, the scroll position was only updated when the using component was re-rendered. This did not work when the hook is used in a wrapping Layout component, becaus its persitent over location changes. Adding the location key to useEffect will cause the scroll position is updated every time the key changes. * lint/ts fixes Co-authored-by: Vladimir Razuvaev <vladimir.razuvaev@gmail.com> (cherry picked from commit f57efab)
…ents (#26861) (#31079) * add generic type to useScrollRestoration hook this is required to match the type of `useRef` which requires the target element type to be specified. The hook can be used like the following: ```typescript const MyComponent: React.FunctionComponent = () => { const scrollRestorationProps = useScrollRestoration<HTMLDivElement>(`some-key`) return ( <div {...scrollRestorationProps} style={{ overflow: `auto` }} > Test </div> ) } ``` Fixes: #26458 * add tests for useScrollRestoration hook * fix useScrollResotration to update position on location change Previously, the scroll position was only updated when the using component was re-rendered. This did not work when the hook is used in a wrapping Layout component, becaus its persitent over location changes. Adding the location key to useEffect will cause the scroll position is updated every time the key changes. * lint/ts fixes Co-authored-by: Vladimir Razuvaev <vladimir.razuvaev@gmail.com> (cherry picked from commit f57efab) Co-authored-by: Michael van Engelshoven <michael@van-engelshoven.de>
Description
Main focus of this PR is to fix the
useScrollRestoration
hook to work in layout components which are not re-rendered on a location change.To be able to check the results I added previously missing tests. For this I addded some generic typings to the hook to be able to use it well typed in the tests on a div element (have a look at #26458).
Documentation
This should not affect the currently documented usage.
Related Issues
Fixes #26458