fix(v2): do not focus on skip link if page refreshed - #4797
Conversation
|
[V1] Built with commit ef838fe |
|
[V2] Built with commit ef838fe |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4797--docusaurus-2.netlify.app/ |
|
Size Change: +128 B (0%) Total Size: 640 kB
ℹ️ View Unchanged
|
| const initialPathname = useRef(pathname); | ||
|
|
||
| useEffect(() => { | ||
| if (pathname !== initialPathname.current) { |
There was a problem hiding this comment.
That seems to work on most cases but what if user is on /a, navigates to /b, and then navigates back to /a?
I think you should rather use a hook like usePrevious to make sure that the callback fires even if the user goes back to the page that was initially loaded
There was a problem hiding this comment.
Otherwise that looks like to fix the problem! thanks
There was a problem hiding this comment.
Could you reproduce this issue on the preview site? Currently, when navigating back to initially loaded page, useEffect fires as expected.
There was a problem hiding this comment.
Oh I'm sorry, didn't notice that you reassigned initialPathname to latest route.
So somehow you already handle that case, it's just that I'm not fan of the naming "initialPathname" considering its value is changed over time. Will change change the name and merge
* fix(v2): do not focus on skip link if page refreshed * rename ref Co-authored-by: slorber <lorber.sebastien@gmail.com>
Motivation
Fixes #4794
Using more reliable logic to determine that the route has actually changed (currently, there are false positives on reloads of same page).
As a result, the scroll position is lost during hot reloading.
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
See To Reproduce section in #4794
Related PRs
(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)