Fix browser history when synchronising state with urls#48731
Merged
youknowriad merged 11 commits intotrunkfrom Mar 6, 2023
Merged
Fix browser history when synchronising state with urls#48731youknowriad merged 11 commits intotrunkfrom
youknowriad merged 11 commits intotrunkfrom
Conversation
Contributor
|
Seems to fix the issue on my end! Nice one @youknowriad! Out of curiosity, on a separate note (not caused by this PR), when I click "Template Parts" and click "Footer", for example, I need to click the browser's back button twice to actually go back. Is this related or a separate issue? |
|
Size Change: +710 B (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
Mamaduka
reviewed
Mar 3, 2023
Member
Mamaduka
left a comment
There was a problem hiding this comment.
This partially fixes the reported issue, but when I navigate around in Site Editor and then use the Back button, I end up with wp-admin/site-editor.php?canvas=init&path=%2Fwp_template%2Ftwentytwentythree%252F%252Fhome URL, which produces white screen.
Contributor
|
@Mamaduka I can't reproduce this myself. Do you have some steps I could follow to see if I can reproduce? |
Contributor
Author
|
@Mamaduka which browser are you using? |
Contributor
Author
I can reproduce this issue, it seems there's still an extra history.push. Let's see if I can find it. |
Member
|
My bad! It looks like this was a browser cache issue. Sorry for the confusion. P.S. I'm testing with Chrome. |
Contributor
Author
|
It turns out that when you do |
Contributor
Author
|
While the previous fix works, I'm also exploring if I can change how "push" works instead. |
Contributor
|
85f2370 fixes the double back button issue but also appears to flash the "Home" submenu briefly. |
Mamaduka
reviewed
Mar 3, 2023
|
|
||
| // TODO: We should update this to filter by template part's areas as well. | ||
| const browseAllLinkProps = useLink( { | ||
| canvas: 'view', |
Contributor
Author
There was a problem hiding this comment.
it should be "undefined" now because "view" is the default value. But I guess it suffers from the issue I talk about here #48731 (comment)
Member
|
@youknowriad, I also noticed that if I refresh |
Contributor
Author
|
Ok, I think I've fixed all the issues. I'm not entirely satisfied with these syncing hooks and I believe we need e2e tests for all these flows but this should be good maybe for now? |
Contributor
|
This is working perfectly when going straight from the dashboard to the site editor then hitting the back button. One thing I noticed is when you go dashboard > site editor > [back to templates sidebar view] then try the browser back button, there's a couple of extra dead back button clicks in there between the site editor and the dashboard. I tried to capture it in this gif:
|
Contributor
|
@apeatling Noticing that too when I do the following:
I'm also still noticing the flash of the "Home" sidebar view when I do the following:
|
Contributor
Author
|
I think I fixed both of these issues. I'd appreciate some checks. @apeatling I also noticed that on your video you land directly on a sub menu when opening the site editor. I'm aware of this issue but it's specific to old WP versions (there's a redirect that we can't override in Gutenberg). If you test with WP trunk, you'll notice that you land on the "root" of the site editor. |
ntsekouras
reviewed
Mar 6, 2023
packages/edit-site/src/components/sync-state-with-url/use-sync-canvas-mode-with-url.js
Outdated
Show resolved
Hide resolved
Contributor
Author
|
I added some e2e tests to cover these flows. |
ntsekouras
reviewed
Mar 6, 2023
kevin940726
reviewed
Mar 6, 2023
ntsekouras
approved these changes
Mar 6, 2023
Contributor
ntsekouras
left a comment
There was a problem hiding this comment.
I've tested this PR quite a lot and everything seems to work well. Thanks Riad!
packages/edit-site/src/components/sidebar-navigation-screen-templates/index.js
Outdated
Show resolved
Hide resolved
|
Flaky tests detected in ee28018. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4341886921
|
Contributor
|
I just cherry-picked this PR to the wp/6.2 branch to get it included in the next release: e064813 |
ntsekouras
reviewed
Oct 31, 2023
packages/edit-site/src/components/sync-state-with-url/use-sync-canvas-mode-with-url.js
Show resolved
Hide resolved
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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 #48726
What?
This PRs fixes the browser history when navigating the site editor.
How?
Testing Instructions
1- Open the site editor
2- Click browser "back" button
3- you should be back in the dashboard properly
4- Testing using Chrome, Firefox or Edge (in trunk it's broken with these)