Skip to content
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

Add block appender to Nav block offcanvas experiment #45905

Closed
wants to merge 7 commits into from
Closed

Add block appender to Nav block offcanvas experiment #45905

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
50daa93
WIP add block appender
Nov 18, 2022
a016ad5
WIP try using BlockListAppender instead
Nov 21, 2022
1d71883
Add basic custom appender to match visual design
getdave Nov 21, 2022
bf288ef
Include offcanvas specific styles
getdave Nov 21, 2022
b6eebc0
Provide offcanvas specific CSS classes
getdave Nov 21, 2022
206bb7d
Conditionalise display of menu appender
getdave Nov 21, 2022
e13e62b
Correctly pass rootClientId into appender
getdave Nov 21, 2022
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
3 changes: 2 additions & 1 deletion packages/block-editor/src/components/button-block-appender/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import deprecated from '@wordpress/deprecated';
import Inserter from '../inserter';

function ButtonBlockAppender(
{ rootClientId, className, onFocus, tabIndex },
{ rootClientId, className, onFocus, tabIndex, children },
ref
) {
return (
Expand Down Expand Up @@ -67,6 +67,7 @@ function ButtonBlockAppender(
<VisuallyHidden as="span">{ label }</VisuallyHidden>
) }
<Icon icon={ plus } />
{ children }
Copy link
Contributor

@getdave getdave Nov 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This may need to be a separate PR. Basically it allows us to do this

<ButtonBlockAppender
	className="offcanvas-editor__list-appender__toggle"
	rootClientId={ rootClientId }
>
	{ __( 'Add Menu Item' ) }
</ButtonBlockAppender>

Otherwise we're going to have to reimplement all of <ButtonBlockAppender> as a custom component. Possible but will require additional time.

</Button>
);

Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/components/off-canvas-editor/branch.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { AsyncModeProvider, useSelect } from '@wordpress/data';
import ListViewBlock from './block';
import { useListViewContext } from './context';
import { isClientIdSelected } from './utils';
import ButtonBlockAppender from '../button-block-appender';
import { store as blockEditorStore } from '../../store';

/**
Expand Down
101 changes: 65 additions & 36 deletions packages/block-editor/src/components/off-canvas-editor/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import useListViewClientIds from './use-list-view-client-ids';
import useListViewDropZone from './use-list-view-drop-zone';
import useListViewExpandSelectedItem from './use-list-view-expand-selected-item';
import { store as blockEditorStore } from '../../store';
import BlockListAppender from '../block-list-appender';
import ButtonBlockAppender from '../button-block-appender';

const expanded = ( state, action ) => {
if ( Array.isArray( action.clientIds ) ) {
Expand All @@ -52,6 +54,7 @@ export const BLOCK_LIST_ITEM_HEIGHT = 36;
*
* @param {Object} props Components props.
* @param {string} props.id An HTML element id for the root element of ListView.
* @param {string} props.clientId Client ID of the root navigation block.
* @param {Array} props.blocks Custom subset of block client IDs to be used instead of the default hierarchy.
* @param {boolean} props.showBlockMovers Flag to enable block movers
* @param {boolean} props.isExpanded Flag to determine whether nested levels are expanded by default.
Expand All @@ -61,6 +64,7 @@ export const BLOCK_LIST_ITEM_HEIGHT = 36;
function __ExperimentalOffCanvasEditor(
{
id,
clientId,
blocks,
showBlockMovers = false,
isExpanded = false,
Expand Down Expand Up @@ -104,9 +108,9 @@ function __ExperimentalOffCanvasEditor(
setExpandedState,
} );
const selectEditorBlock = useCallback(
( event, clientId ) => {
updateBlockSelection( event, clientId );
setSelectedTreeId( clientId );
( event, _clientId ) => {
Comment on lines -107 to +111
Copy link
Contributor

Choose a reason for hiding this comment

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

These changes to clientID were to appease the linter which was telling me it was shadowing an existing var.

updateBlockSelection( event, _clientId );
setSelectedTreeId( _clientId );
},
[ setSelectedTreeId, updateBlockSelection ]
);
Expand All @@ -128,20 +132,20 @@ function __ExperimentalOffCanvasEditor(
);

const expand = useCallback(
( clientId ) => {
if ( ! clientId ) {
( _clientId ) => {
if ( ! _clientId ) {
return;
}
setExpandedState( { type: 'expand', clientIds: [ clientId ] } );
setExpandedState( { type: 'expand', clientIds: [ _clientId ] } );
},
[ setExpandedState ]
);
const collapse = useCallback(
( clientId ) => {
if ( ! clientId ) {
( _clientId ) => {
if ( ! _clientId ) {
return;
}
setExpandedState( { type: 'collapse', clientIds: [ clientId ] } );
setExpandedState( { type: 'collapse', clientIds: [ _clientId ] } );
},
[ setExpandedState ]
);
Expand Down Expand Up @@ -183,34 +187,59 @@ function __ExperimentalOffCanvasEditor(

return (
<AsyncModeProvider value={ true }>
<ListViewDropIndicator
listViewRef={ elementRef }
blockDropTarget={ blockDropTarget }
/>
<TreeGrid
id={ id }
className="block-editor-list-view-tree"
aria-label={ __( 'Block navigation structure' ) }
ref={ treeGridRef }
onCollapseRow={ collapseRow }
onExpandRow={ expandRow }
onFocusRow={ focusRow }
applicationAriaLabel={ __( 'Block navigation structure' ) }
>
<ListViewContext.Provider value={ contextValue }>
<ListViewBranch
blocks={ clientIdsTree }
selectBlock={ selectEditorBlock }
showBlockMovers={ showBlockMovers }
fixedListWindow={ fixedListWindow }
selectedClientIds={ selectedClientIds }
isExpanded={ isExpanded }
shouldShowInnerBlocks={ shouldShowInnerBlocks }
selectBlockInCanvas={ selectBlockInCanvas }
/>
</ListViewContext.Provider>
</TreeGrid>
<div className="offcanvas-editor">
Copy link
Contributor

Choose a reason for hiding this comment

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

Wrapped in custom class

<ListViewDropIndicator
listViewRef={ elementRef }
blockDropTarget={ blockDropTarget }
/>
<TreeGrid
id={ id }
className="block-editor-list-view-tree"
aria-label={ __( 'Block navigation structure' ) }
ref={ treeGridRef }
onCollapseRow={ collapseRow }
onExpandRow={ expandRow }
onFocusRow={ focusRow }
applicationAriaLabel={ __( 'Block navigation structure' ) }
>
<ListViewContext.Provider value={ contextValue }>
<ListViewBranch
blocks={ clientIdsTree }
selectBlock={ selectEditorBlock }
showBlockMovers={ showBlockMovers }
fixedListWindow={ fixedListWindow }
selectedClientIds={ selectedClientIds }
isExpanded={ isExpanded }
shouldShowInnerBlocks={ shouldShowInnerBlocks }
selectBlockInCanvas={ selectBlockInCanvas }
/>
</ListViewContext.Provider>
<tr>
<BlockListAppender
Comment on lines +217 to +218
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed from an a11y POV the arrow key navigation will not focus this element. I think that's probably legit as it's not a "block" per se but FYI.

tagName="td"
rootClientId={ clientId }
renderAppender={ () => (
<OffCanvasEditorAppender
rootClientId={ clientId }
/>
Comment on lines +222 to +224
Copy link
Contributor

Choose a reason for hiding this comment

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

The internal implementation of BlockListAppender doesn't pass the rootClientId to the CustomComponent provided by renderAppender. So we have to manually do it here.

) }
/>
</tr>
</TreeGrid>
</div>
</AsyncModeProvider>
);
}

function OffCanvasEditorAppender( { rootClientId } ) {
return (
<ButtonBlockAppender
className="offcanvas-editor__list-appender__toggle"
rootClientId={ rootClientId }
>
{ __( 'Add Menu Item' ) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Annoyingly the internal implementation of ButtonBlockAppender doesn't allow us to customise the label. So we have one text based label here and another provided by ButtonBlockAppender.

</ButtonBlockAppender>
);
}

export default forwardRef( __ExperimentalOffCanvasEditor );
Loading