Site Editor - add query args for current context.#27124
Site Editor - add query args for current context.#27124Addison-Stavlo merged 6 commits intomasterfrom
Conversation
|
Size Change: +329 B (0%) Total Size: 1.19 MB
ℹ️ View Unchanged
|
packages/edit-site/src/components/post-router/update-query-params.js
Outdated
Show resolved
Hide resolved
ntsekouras
left a comment
There was a problem hiding this comment.
Thanks for working on this! I believe this will be useful for detecting the context for Query block as well.
This seems to be in a good direction but I need to test it more. I noticed a couple of things for start:
- In FSE navigation(sidebar) the description displays html tags. In
templates-> indexdescription. - Having
contentas object in url makes it unreadable. I don't have something to propose yet but will look in more depth in general.
david-szabo97
left a comment
There was a problem hiding this comment.
Love this! Thank you for working on it! ✅ 🙇
packages/edit-site/src/components/post-router/update-query-params.js
Outdated
Show resolved
Hide resolved
6288aaf to
2296e01
Compare
|
In 8468ac7, I switched to use @wordpress/url instead of the build-in methods. I think with these changes all PR feedback should be addressed! It seems to be working pretty well. |
|
Might need to update some e2e tests, but rerunning the failing ones first to make sure. |
|
I verified that the e2e failure started happening elsewhere before this PR (see here) |
There was a problem hiding this comment.
I tested this and works well for now. I think we can merge and continue on follow ups, especially for handling taxonomies. The change to not having in URL all the context seems right to me and makes it readable as well. Will need a rebase for green CI though (thanks to @noahtallen ) :)
Thanks for working on this 👍 !
|
|
||
| return ( | ||
| <> | ||
| <PostRouter /> |
There was a problem hiding this comment.
I'm not sure if this naming is very helpful. as in my mind this is something like FSERouter or SiteEditorRouter... It can be addressed later though but wanted to get some thoughts from others..
There was a problem hiding this comment.
Yeah, I don't have any strong reasons for choosing PostRouter other than lack of creativity. EditorRouter or something along the flavors you mentioned may be more fitting.
There was a problem hiding this comment.
Or would QueryController, or URLQueryController make more sense? Any thoughts?
There was a problem hiding this comment.
Having
contentas object in url makes it unreadable. I don't have something to propose yet but will look in more depth in general.
Agreed, however I was thinking it would be preferable to have an unreadable value for one of these over a simplification that only supports a subset of editor states. However, I think we can still have the best of both worlds.
continue on follow ups, especially for handling
taxonomies.
We should still be able to support taxonomies without that unreadable value by adding a few more readable query args and using them to rebuild the page object we set on state. Or are there any other ideas on how to handle them? 🤔 Either way, I also think that will make sense as a follow up.
Thanks for working on this one @noahtallen ! Its looking good 😁
1f0f6fa to
ed2a1cd
Compare
|
Is something blocking this PR? |
Nope, sorry! We have been distracted with some unrelated duties this week. 😢 Il rebase. |
ed2a1cd to
b074dec
Compare
|
Rebased and updated the name. Just need tests to pass and a 👍 on the new component name and this should be good to merge. (also feel free to change it if you think EditorRouter or SiteEditorRouter would still be more fitting). |
Description
Initial stab at #22238. This PR allows us to load the site editor to a specific context via query args
postTypeandpostIdBased on @vindl's comment here, this PR has been changed from the original implementation.
The idea is that
postTypecan be one of: "page", "post", "wp_template", "wp_template_part". Then the ID would be whichever post we mean to display. This covers several scenarios:postType=page&postId=2. This is enough information for the site editor to resolve the correct template to display. Same for "post"postType=wp_template&postId=2. This will load the specified template without any page context set. Same for "wp_template_part".Limitations:
I think this is simple enough for a round one without having to cover the edge cases by serializing the JSON state into the URL. What do you think?
How has this been tested?
Verify this works for various contexts including: categories, pages, posts, templates, and template parts.
Verify creating a new template updates the url as well.
Screenshots
Types of changes
Checklist: