Site Editor: Refactor the site editor initialization to avoid editedEntity state #55844
Conversation
This file contains 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
youknowriad
added
[Type] Enhancement
youknowriad
requested review from
mcsf,
noisysocks,
jorgefilipecosta,
ntsekouras and
aaronrobertshaw
youknowriad
requested review from
nerrad,
ajitbohra,
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
youknowriad
commented
Nov 3, 2023
| @@ -32,9 +25,6 @@ export default function ResetDefaultTemplate( { onClick } ) { | |||
| onClick={ async () => { | |||
| entity.edit( { template: '' }, { undoIgnore: true } ); | |||
| onClick(); | |||
| await setPage( { | |||
There was a problem hiding this comment.
This is the kind of weird action calls that were needed because of the broken synchronization.
youknowriad
commented
Nov 3, 2023
| 'wp_navigation', | ||
| ]; | ||
|
|
||
| export default function useEditedEntityForParams( params ) { |
There was a problem hiding this comment.
This is the hook that can be used to resolve what postType and postId to use to render an editor frame for the given (postType, postId) tuple. If none is provided we fallback to the home page.
youknowriad
commented
Nov 3, 2023
| @@ -150,6 +150,10 @@ _Returns_ | |||
|
|
|||
| - `undefined< 'edit' >`: Current user object. | |||
|
|
|||
| ### getDefaultTemplateId | |||
There was a problem hiding this comment.
This will be extracted to its own PR, it just replaces the adhoc template lookup calls with proper core-data selector/resolver.
|
Size Change: +73 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
|
ok, so I've thought about this more during the weekend, and I think I've come to an important realization. First, it's not just the "getEditedPostType" and "getEditedPostId" that are specific to a given "frame/editor". There are more things that are not really global to the whole site editor at the moment: all the UI state (inserter, panels...) are also specific to a given frame and not something site editor state. I've come to realize that there's a better path forward here: I think we need to split the site editor in two parts:
Also that second package can actually be used within edit-post exactly as is. (maybe some config to say hide global styles...) and we'll end up with the exact same editor in both finally closing any gaps between these two. |
@youknowriad that sounds pretty great to me. |
In my mind most of the UI is coupled with current frame, except of course the main navigation sidebar. Can you elaborate a bit more about what this package should contain? |
IMO, and AFAIK, there's no coupling at all right now between the frame and the sidebar. They only communicate through core-data entities. Both depend on the "post type" "post id" from the URL though |
24 tasks
|
closing this PR as this work has shifted to another approach described in #52632 (comment) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What?
Note This should be considered as a prototype at the moment. It's not yet clear to me if we should move forward with this refactor.
This is a refactor that deprecates the "editedEntity" state from the site editor store and replaces it with regular selector from core-data (a hook is used to simplify) and props being passed top down. (A context can be considered for the props passed top down)
Why?
The reasons I considered this refactor are multiple:
setPagein random components.getEditedPostType... selectors. We want to provide the post type and post id to use to render such a frame. The current PR allows that by making these postType and postID as props.While I'm not sure yet that we should land it as is, I'm confident that something like this is needed because the site editor as it exists today is no longer a page that renders a site editor for a given entity in which case an "editedEntity" would make sense in its store, today the site editor is comparable to wp-admin as it can render any kind of page.
Testing Instructions
This PR impacts the whole site editor so we should test everything. We have tests for most flows but there's a lot to test here but I also want to get opinions on the PR.