-
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,11 +2,13 @@ | |
* WordPress dependencies | ||
*/ | ||
import { useMergeRefs } from '@wordpress/compose'; | ||
import { useRef } from '@wordpress/element'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import BlockList from '../block-list'; | ||
import BlockTools from '../block-tools'; | ||
import EditorStyles from '../editor-styles'; | ||
import Iframe from '../iframe'; | ||
import WritingFlow from '../writing-flow'; | ||
|
@@ -23,11 +25,15 @@ export function ExperimentalBlockCanvas( { | |
} ) { | ||
const resetTypingRef = useMouseMoveTypingReset(); | ||
const clearerRef = useBlockSelectionClearer(); | ||
const contentRef = useMergeRefs( [ contentRefProp, clearerRef ] ); | ||
const localRef = useRef(); | ||
const contentRef = useMergeRefs( [ contentRefProp, clearerRef, localRef ] ); | ||
|
||
if ( ! shouldIframe ) { | ||
return ( | ||
<> | ||
<BlockTools | ||
__unstableContentRef={ localRef } | ||
style={ { height, display: 'flex' } } | ||
> | ||
<EditorStyles | ||
styles={ styles } | ||
scope=".editor-styles-wrapper" | ||
|
@@ -36,29 +42,37 @@ export function ExperimentalBlockCanvas( { | |
ref={ contentRef } | ||
className="editor-styles-wrapper" | ||
tabIndex={ -1 } | ||
style={ { height } } | ||
style={ { | ||
height: '100%', | ||
width: '100%', | ||
} } | ||
> | ||
{ children } | ||
</WritingFlow> | ||
</> | ||
</BlockTools> | ||
); | ||
} | ||
|
||
return ( | ||
<Iframe | ||
{ ...iframeProps } | ||
ref={ resetTypingRef } | ||
contentRef={ contentRef } | ||
style={ { | ||
width: '100%', | ||
height, | ||
...iframeProps?.style, | ||
} } | ||
name="editor-canvas" | ||
<BlockTools | ||
__unstableContentRef={ localRef } | ||
style={ { height, display: 'flex' } } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did you decide to add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't really tell at this point 😬 |
||
> | ||
<EditorStyles styles={ styles } /> | ||
{ children } | ||
</Iframe> | ||
<Iframe | ||
{ ...iframeProps } | ||
ref={ resetTypingRef } | ||
contentRef={ contentRef } | ||
style={ { | ||
width: '100%', | ||
height: '100%', | ||
...iframeProps?.style, | ||
} } | ||
name="editor-canvas" | ||
> | ||
<EditorStyles styles={ styles } /> | ||
{ children } | ||
</Iframe> | ||
</BlockTools> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,6 @@ import { | |
BlockEditorProvider, | ||
BlockCanvas, | ||
BlockToolbar, | ||
BlockTools, | ||
} from '@wordpress/block-editor'; | ||
import { Button } from '@wordpress/components'; | ||
import { undo as undoIcon, redo as redoIcon } from '@wordpress/icons'; | ||
|
@@ -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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. |
||
<BlockTools /> | ||
</div> | ||
<BlockCanvas height="100%" styles={ editorStyles } /> | ||
</BlockEditorProvider> | ||
|
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.