Editor: Move the InterfaceSkeleton to the editor package #62118
Merged
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
the
[Type] Code Quality
youknowriad
requested review from
draganescu,
jameskoster,
jasmussen,
ellatrix,
jorgefilipecosta and
ntsekouras
|
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 29, 2024
| variants={ slideX } | ||
| transition={ { type: 'tween', delay: 0.8 } } | ||
| > | ||
| <FullscreenModeClose showTooltip initialPost={ initialPost } /> |
There was a problem hiding this comment.
I'm using a Slot to inject the "W" back button to the post editor. In fact the Slot already exited before for third-party usage, I moved it to the Header component in this PR to be able to inject a different button there in the site editor (different behavior there to exit edit mode)
youknowriad
commented
May 29, 2024
| /** | ||
| * Show icon label overrides. | ||
| */ | ||
| .show-icon-labels .editor-header { |
There was a problem hiding this comment.
This is just copied from another CSS file. I didn't touch this CSS at all.
youknowriad
commented
May 29, 2024
| ! DESIGN_POST_TYPES.includes( postType ) | ||
| ) { | ||
| baseStyles.push( { | ||
| css: 'body{padding-bottom: 40vh}', |
There was a problem hiding this comment.
This style was already applied in the custom "VisualEditor" component that existed in the post editor. But I removed that component (it was mostly useless).
That said, this particular style might need to move to the editor package at some point to apply to "posts" when they load in the site editor context as well.
youknowriad
commented
May 29, 2024
| <MetaBoxes location="advanced" /> | ||
| </div> | ||
| ) } | ||
| </EditorCanvas> |
There was a problem hiding this comment.
I wish we didn't need all these props (forceIsDirty, children, disableIframe, autoFocus...) but I think there's no way around them for now (mostly to inject metaboxes UI...)
youknowriad
commented
May 29, 2024
| return; | ||
| } | ||
| titleRef?.current?.focus(); | ||
| }, [ autoFocus ] ); |
There was a problem hiding this comment.
This actually existed before, but I removed it inadvertently in one of the previous PRs.
|
Size Change: +460 B (+0.03%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
youknowriad
force-pushed
the
update/remove-useless-props
branch
from
b027005
to
9a9e93b
Compare
youknowriad
force-pushed
the
move/editor-skeleton
branch
3 times, most recently
from
9eb6c2b
to
9eec8aa
Compare
draganescu
approved these changes
May 30, 2024
youknowriad
force-pushed
the
update/remove-useless-props
branch
from
9a9e93b
to
da569a9
Compare
youknowriad
force-pushed
the
move/editor-skeleton
branch
from
9eec8aa
to
ddf142f
Compare
youknowriad
force-pushed
the
move/editor-skeleton
branch
from
ddf142f
to
845e82d
Compare
stokesman
reviewed
May 30, 2024
There was a problem hiding this comment.
Do excuse/ignore my bikeshedding but EditorCanvas seems odd to me. Is it a terrible idea to simply call it Editor? Other ideas: EditorShell, EditorEasel, Editorscape.
There was a problem hiding this comment.
Is it a terrible idea to simply call it Editor?
Well we already have an editor which is the underlying system implementing a block editor specific for WP, upon which the post and the site editors are built.
@youknowriad why not preserve Interface and name it EditorInterface?
There was a problem hiding this comment.
So I agree that Editor would be the best name but I didn't use it yet because I plan for Editor to be the only component one would use to render an "postType/postId" editor.
The component here requires an EditorProvider to work still, so it corresponds more to the UI of the editor only.
I think any of the ideas shared above can work: Canvas is already heavily used in the code base but I'm actually not opinionated EditorInterface sound good to me as well. But worth noting it's just an internal component (and may stay that way forever so the naming is not that important). I may go with EditorInterface for now.
The Editor component will come later.
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Jun 4, 2024
…2118) Co-authored-by: youknowriad <[email protected]> Co-authored-by: draganescu <[email protected]>
patil-vipul
pushed a commit
to patil-vipul/gutenberg
that referenced
this pull request
Jun 17, 2024
…2118) Co-authored-by: youknowriad <[email protected]> Co-authored-by: draganescu <[email protected]>
2 tasks
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
What?
This is one of the latest pieces of the puzzle of the unification between post and site editors. This PR moves the "InterfaceSkeleton" (the whole UI basically) of the editor to the editor package (from the edit-post package) under the EditorInterface component. To ease with review and avoid breakage and regressions, I chose not to update the site editor to use this component yet, it will require some more changes there, so for now I just focused on the post editor..
Testing Instructions
1- There should be no functional change or UI change at all but smoke testing the post editor is necessary to check for potential unintended regressions.