-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Refactor the site editor URLs for better backward compatibility #48063
Conversation
Size Change: +119 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
Flaky tests detected in 558a900. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4183686050
|
I can't speak about the code, but everything seems to be working just fine on this branch. |
1f19b45
to
558a900
Compare
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.
LGTM and worked well on my tests 👍
@@ -128,12 +128,9 @@ export default function NewTemplate( { | |||
setCanvasMode( 'edit' ); | |||
|
|||
// Navigate to the created template editor. | |||
window.queueMicrotask( () => { |
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.
Any idea of why we were using window.queueMicrotask and not it is not needed?
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.
These were needed previously because we had two synchronous changes causing effects in hooks that update the URL (canvas change and template navigation) and these two changes to the "history" path were overriding each other, the micro task ensured that they were done one after the other.
Now that the url is the source of truth of everything, it's way simpler.
I just cherry-picked this PR to the wp/6.2 branch to get it included in the next release: 5719ffb |
What?
This PR refactors how we deal with URLs in the site editor to avoid the duplication between "path", "postId" and "postType" URLs.
In trunk, for a given postType/postId couple to load in the site editor we'd need both of these in the URL and also a path=/navigation/postType/postId otherwise, the path is considered wrong and the post doesn't load, this PR simplifies that by inverting the source of truth.
1- The URL is the actual source of truth. If there's a postId and postType in the URL, there's no need to provide a "path". It's automatically generated from these arguments.
2- If one of these is missing, it means we need to provide a "path" to avoid ambiguity.
3- The useSyncPathFromURL makes sure to compute the right path based on the url params to initialize the sidebar navigation properly.
Why?
This ensures the URLs don't change between WP 6.1 and WP 6.2 and also avoids race conditions between hooks updated the URL.
Testing Instructions
1- Navigate all the different pages and flows of the site editor.