Skip to content

Block editor: use React events for shortcuts (portal bubbles & contextual) #32323

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.

Sign up for GitHub

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

Merged
ellatrix merged 6 commits into from
Jul 22, 2021
Merged

Block editor: use React events for shortcuts (portal bubbles & contextual) #32323

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d4294d1
Block editor: use React events for shortcuts (portal bubbles & contex…
ellatrix May 28, 2021
120f227
Reduce divs
ellatrix May 28, 2021
bd596fc
Fix rebase error
ellatrix Jul 16, 2021
9f0d7a0
Fix access shortcut
ellatrix Jul 18, 2021
2d6754a
Fix unit tests
ellatrix Jul 22, 2021
bce98da
Fix e2e test
ellatrix Jul 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Reduce divs
  • Loading branch information
@ellatrix
ellatrix committed Jul 22, 2021
commit 120f227ac9ddb91654c99c28e63fa0bdcdfbaf3c
8 changes: 6 additions & 2 deletions packages/block-editor/src/components/block-tools/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ import { usePopoverScroll } from './use-popover-scroll';
* @param {Object} $0.children The block content and style container.
* @param {Object} $0.__unstableContentRef Ref holding the content scroll container.
*/
export default function BlockTools( { children, __unstableContentRef } ) {
export default function BlockTools( {
children,
__unstableContentRef,
...props
} ) {
const isLargeViewport = useViewportMatch( 'medium' );
const hasFixedToolbar = useSelect(
( select ) => select( blockEditorStore ).getSettings().hasFixedToolbar,
Expand Down Expand Up @@ -110,7 +114,7 @@ export default function BlockTools( { children, __unstableContentRef } ) {

return (
// eslint-disable-next-line jsx-a11y/no-static-element-interactions
<div onKeyDown={ onKeyDown }>
<div { ...props } onKeyDown={ onKeyDown }>
<InsertionPoint __unstableContentRef={ __unstableContentRef }>
{ ( hasFixedToolbar || ! isLargeViewport ) && (
<BlockContextualToolbar isFixed />
Expand Down
72 changes: 63 additions & 9 deletions packages/edit-post/src/components/visual-editor/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,20 +192,37 @@ export default function VisualEditor( { styles } ) {
}, [ isTemplateMode, themeSupportsLayout, contentSize, wideSize ] );

return (
<div
<BlockTools
Copy link
Contributor

@youknowriad youknowriad Jul 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me if this needs to be a parent of the block editor, it needs to be documented properly in the block-editor package and the documentation we have to build third-party block editors.

That said, the naming is not great at all, shouldn't be BlockEditorWrapper or something like that? Also, if it's a wrapper, why not just bundle it in BlockEditorProvider

Copy link
Member Author

@ellatrix ellatrix Jul 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does it need to be higher up the tree? Agreed on the documentation, I should add some.

Copy link
Contributor

@youknowriad youknowriad Jul 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the question. My main motivation here is that it seems we're requiring two wrappers where we can require just one. Unless I'm not understanding properly both these components should wrap the editor canvas? Maybe more granularity is needed, but it's not clear from the user's perspective where to put each one of these components.

__unstableContentRef={ ref }
className={ classnames( 'edit-post-visual-editor', {
'is-template-mode': isTemplateMode,
} ) }
>
<VisualEditorGlobalKeyboardShortcuts />
<BlockTools __unstableContentRef={ ref }>
<motion.div
className="edit-post-visual-editor__content-area"
animate={ {
padding: isTemplateMode ? '48px 48px 0' : '0',
} }
ref={ blockSelectionClearerRef }
>
{ isTemplateMode && (
<Button
className="edit-post-visual-editor__exit-template-mode"
icon={ arrowLeft }
onClick={ () => {
clearSelectedBlock();
setIsEditingTemplate( false );
} }
>
{ __( 'Back' ) }
</Button>
) }
<motion.div
className="edit-post-visual-editor__content-area"
animate={ {
padding: isTemplateMode ? '48px 48px 0' : '0',
} }
ref={ blockSelectionClearerRef }
animate={ animatedStyles }
initial={ desktopCanvasStyles }
>
<<<<<<< HEAD
{ isTemplateMode && (
<Button
className="edit-post-visual-editor__exit-template-mode"
Expand Down Expand Up @@ -245,13 +262,50 @@ export default function VisualEditor( { styles } ) {
</RecursionProvider>
</MaybeIframe>
</motion.div>
=======
<MaybeIframe
isTemplateMode={ isTemplateMode }
contentRef={ contentRef }
styles={ styles }
style={ { paddingBottom } }
>
{ themeSupportsLayout && (
<LayoutStyle
selector=".edit-post-visual-editor__post-title-wrapper, .block-editor-block-list__layout.is-root-container"
layout={ defaultLayout }
/>
) }
<WritingFlow>
{ ! isTemplateMode && (
<div className="edit-post-visual-editor__post-title-wrapper">
<PostTitle />
</div>
) }
<RecursionProvider>
<BlockList
__experimentalLayout={
themeSupportsLayout
? {
type: 'default',
// Find a way to inject this in the support flag code (hooks).
alignments: themeSupportsLayout
? alignments
: undefined,
}
: undefined
}
/>
</RecursionProvider>
</WritingFlow>
</MaybeIframe>
>>>>>>> 4c1f6c502b (Reduce divs)
</motion.div>
</BlockTools>
</motion.div>
<__unstableBlockSettingsMenuFirstItem>
{ ( { onClose } ) => (
<BlockInspectorButton onClick={ onClose } />
) }
</__unstableBlockSettingsMenuFirstItem>
</div>
</BlockTools>
);
}