Site Editor: Update to use the EditorInterface component from the editor package. #62146
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] Code Quality
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
youknowriad
commented
May 30, 2024
| @@ -293,6 +294,7 @@ function Layout( { initialPost } ) { | |||
| ) } | |||
| <PostEditorMoreMenu /> | |||
| <BackButton initialPost={ initialPost } /> | |||
| <EditorSnackbars /> | |||
There was a problem hiding this comment.
The snackbars need to be rendered only once in the site editor (or the post editor) so I left this responsibility out of the EditorCanvas component for now.
youknowriad
commented
May 30, 2024
| } | ||
| bottom: 24px; | ||
| padding-left: 24px; | ||
| padding-right: 24px; |
There was a problem hiding this comment.
Simplified the position of the snackbars in the post editor (to be a fixed position like the site editor)
youknowriad
commented
May 30, 2024
| // The button element easily inherits styles that are meant for the editor style. | ||
| // These rules enhance the specificity to reduce that inheritance. | ||
| // This is duplicated in visual-editor. | ||
| .edit-site-block-editor__editor-styles-wrapper .components-button { |
There was a problem hiding this comment.
This style is already in the editor package (so inherited in the site editor)
youknowriad
commented
May 30, 2024
| } | ||
| } | ||
|
|
||
| .edit-site-visual-editor { |
There was a problem hiding this comment.
This style moved to the specific editor-canvas-container component. It's only useful there.
youknowriad
commented
May 30, 2024
| const { VisualEditor } = unlock( editorPrivateApis ); | ||
|
|
||
| function EditorCanvas( { settings } ) { | ||
| export default function useEditorIframeProps() { |
There was a problem hiding this comment.
Mostly this enables the site editor to have the "click on canvas" behavior that is not present and not needed in the post editor. I wish it could be just an onClick prop to the EditorCanvas component but for now, I just left it as is and just passed down the iframeProps prop.
youknowriad
commented
May 30, 2024
| privateKey, | ||
| Slot: EditorCanvasContainerSlot, | ||
| Fill: EditorCanvasContainerFill, | ||
| } = createPrivateSlotFill( SLOT_FILL_NAME ); |
There was a problem hiding this comment.
The creation and the rendering of the slot has moved to the editor package. So any editor (post or site editor) can decide to actually update the "content" are using a fill. (like we do for style book and style revisions). There are probably better ways to do implement this behavior but I didn't want to change too much here.
There was a problem hiding this comment.
There are probably better ways to do implement this behavior but I didn't want to change too much here.
What do you think? Do you mean in terms of restricting the behavior to the post editor or finding a different way to swap editor content in and out?
There was a problem hiding this comment.
There are a few thoughts that I have here:
-
I don't like that we're switching the canvas using a Slot, I'd prefer a children prop or something maybe.
-
I wonder if we're actually switching canvas here, I wonder if stylebook or style revisions are actually the same "editor", maybe it's the whole thing that should be swapped (so no need for Slot or anything). This is clearer for things like:
-
Why I have an inserter and the different block tools for stylebook and style revisions (we have a dedicated prop to remove these but the prop shouldn't exist in the first place) => in other words the header is completely different
-
The "document" sidebar or the "block" sidebar don't make sense anymore. In these modes. We're focused on "global styles". => in other words, the left sidebar is completely different
-
There's no need for list view or inserter => right sidebar is completely different.
This is to say, in my mind, there are two valid paths forward:
- Build a completely different UI (the whole thing, not just the canvas) for these modes. (Could share some very small things like save button and undo/redo)
- Continue with the "editor" but instead of hacking things around like we're doing now, switch the "active entity" (currentPostType, and currentPostId) by passing a different "post" object to EditorProvider, and that post would be the "global styles" post. In which case, the "editor" would know that we're editing the global styles and would switch the UI accordingly.
Both of these can be valid, right now, I'm thinking the former is probably better but it's a very loosely held opinion.
There was a problem hiding this comment.
Thanks a lot for the thorough feedback!
Build a completely different UI (the whole thing, not just the canvas) for these modes. (Could share some very small things like save button and undo/redo)
I could see this as an alternative, particularly if we could isolate it to the site editor.
I guess we have access to context/edited post in the site editor Editor component.
Worth an experiment.
I'll create an issue and put it on the backlog.
Thank you!
There was a problem hiding this comment.
The important feature, at least for global style revisions, was to have a "separate" editor view that could be edited independently of the main editor canvas.
So changes could be made to styles etc in the alt. canvas view, and when it closes, the user is returned to the main editor canvas in whatever state they left it.
youknowriad
commented
May 30, 2024
| customSaveButton, | ||
| forceDisableBlockTools, | ||
| title, | ||
| iframeProps, |
There was a problem hiding this comment.
I wish we didn't need all these props to customize the behavior of the editor between post and site editors. But maybe we could iterate (remove some of these) in dedicated PRs.
|
Size Change: -815 B (-0.05%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
youknowriad
commented
May 30, 2024
|
|
||
| // This is a temporary private API while we're updating the site editor to use EditorProvider. | ||
| useAutoSwitchEditorSidebars, |
There was a problem hiding this comment.
These changes here just clean up the non-used private editor APIs (since now we're using the higher level components)
youknowriad
force-pushed
the
update/site-editor-skeleton
branch
from
6f71d9d
to
362ccf0
Compare
youknowriad
changed the title
ramonjd
reviewed
May 31, 2024
There was a problem hiding this comment.
I mainly did a smoke test in a web browser (not mobile app):
✅ Site Editor: style revisions and style book functionality
✅ Site Editor: click on canvas from view mode (pages, templates, patterns)
✅ Both editors: snackbars (work and look okay in desktop/mobile viewports)
✅ Editor preferences work as expected across both editors
| privateKey, | ||
| Slot: EditorCanvasContainerSlot, | ||
| Fill: EditorCanvasContainerFill, | ||
| } = createPrivateSlotFill( SLOT_FILL_NAME ); |
There was a problem hiding this comment.
There are probably better ways to do implement this behavior but I didn't want to change too much here.
What do you think? Do you mean in terms of restricting the behavior to the post editor or finding a different way to swap editor content in and out?
youknowriad
force-pushed
the
update/site-editor-skeleton
branch
from
362ccf0
to
20cd981
Compare
ellatrix
approved these changes
May 31, 2024
|
Flaky tests detected in 7e94d0c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9315053555
|
|
This PR seems to have caused the Style Book button in the navigation sidebar (not the global styles sidebar) to stop working. See #62204. |
This was referenced Jun 3, 2024
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Jun 4, 2024
…tor package. (WordPress#62146) Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org>
patil-vipul
pushed a commit
to patil-vipul/gutenberg
that referenced
this pull request
Jun 17, 2024
…tor package. (WordPress#62146) Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org>
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.
Related #52632
Similar to #62118
What?
This is one of the latest pieces of the puzzle of the unification between post and site editors. This PR uses the EditorInterface from the editor package in the site editor instead of having a custom "InterfaceSkeleton" (the whole UI basically) usage. In #62118 I had updated the post editor already, this PR follows-up with the site editor changes.
Testing Instructions
1- There should be no functional change or UI change at all but smoke testing the site editor is necessary to check for potential unintended regressions.