-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Framework: Bundle the BlockTools component within BlockCanvas #56996
Conversation
@@ -60,7 +59,6 @@ export default function EditorWithUndoRedo() { | |||
label="Redo" | |||
/> | |||
<BlockToolbar hideDragHandle /> |
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.
I noticed that hideDragHandle is always passed to this component. It's only left as false in the case of the "popover toolbar" which is rendered by the framework, this means that we should probably change the default value of this prop to "true".
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.
Agreed.
Size Change: -216 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
> | ||
{ children } | ||
</WritingFlow> | ||
</> | ||
</BlockTools> |
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.
Only downside here is that we'll have two divs (BlockTools div and WritingFlow div), we could potentially merge the two later.
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.
For non iframe, maybe, but I don't think it's that important. For iframe writing flow already does not produce a div.
<EditorStyles styles={ styles } /> | ||
{ children } | ||
</Iframe> | ||
</BlockTools> |
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.
The BlockTools div could be optional maybe, its behavior could be automatically added to the Iframe wrapper or something.
Flaky tests detected in b446ac2. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7188481224
|
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.
Nice!
name="editor-canvas" | ||
<BlockTools | ||
__unstableContentRef={ localRef } | ||
style={ { height, display: 'flex' } } |
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.
Why did you decide to add flex
here? This is strange
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.
I can't really tell at this point 😬
Related to #53874
What?
The
BlockTools
component has always been a weird component that is a bit hard to describe for third-party block editor developers. It's used to render the block toolbar in the "default block toolbar mode" but also attaches some mandatory event handlers for the block editor. It's hard to explain how to use it and where to use it properlyThis PR solves that issue by bundling it within
BlockCanvas
component. It also updates the platform docs to explain how to use the related BlockToolbar component.Testing Instructions
Check the post, site and widget editors. Check things like device preview, resizing... Things should work as expected.