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

List View: allow expanding and collapsing of nested blocks #32117

Merged
merged 16 commits into from
Jun 15, 2021
Merged

List View: allow expanding and collapsing of nested blocks #32117

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
b440453
List View: allow expanding and collapsing of nested blocks
gwwar May 20, 2021
2e30a75
start adding left and right keyboard events in treegrid
gwwar May 26, 2021
747348d
prettier
gwwar May 26, 2021
e4ea861
fix expand and collapse shortcuts
gwwar May 27, 2021
c63a159
prettier
gwwar May 27, 2021
8bcdeb4
remove todo
gwwar May 27, 2021
c8c03e0
Polish.
jasmussen May 28, 2021
7f8901e
Try a smaller chevron
jameskoster May 28, 2021
f05cff0
use useReducer to clean up handling
gwwar May 28, 2021
92f353c
Fix border radius on selected+collapsed items
jameskoster Jun 2, 2021
c36de9e
Centrally align the chevron
jameskoster Jun 2, 2021
7fab9c8
move indentation to sass for loop
gwwar Jun 11, 2021
1b8b13d
split out select and toggle handlers
gwwar Jun 11, 2021
f4b5a5f
Hide expander from accessibility tree and add comment
gwwar Jun 14, 2021
90dd73f
Tree Grid: pass back full row element in expand collapse handers for …
gwwar Jun 14, 2021
7065ab6
fix expand behavior when multiple focusable items in row
gwwar Jun 14, 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
4 changes: 3 additions & 1 deletion packages/block-editor/src/components/block-navigation/block-contents.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const BlockNavigationBlockContents = forwardRef(
(
{
onClick,
onToggleExpanded,
block,
isSelected,
position,
Expand Down Expand Up @@ -98,7 +99,7 @@ const BlockNavigationBlockContents = forwardRef(
ref={ ref }
className={ className }
block={ block }
onClick={ onClick }
onToggleExpanded={ onToggleExpanded }
isSelected={ isSelected }
position={ position }
siblingBlockCount={ siblingBlockCount }
Expand All @@ -114,6 +115,7 @@ const BlockNavigationBlockContents = forwardRef(
className={ className }
block={ block }
onClick={ onClick }
onToggleExpanded={ onToggleExpanded }
isSelected={ isSelected }
position={ position }
siblingBlockCount={ siblingBlockCount }
Expand Down
3 changes: 3 additions & 0 deletions packages/block-editor/src/components/block-navigation/block-select-button.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@ import BlockIcon from '../block-icon';
import useBlockDisplayInformation from '../use-block-display-information';
import { getBlockPositionDescription } from './utils';
import BlockTitle from '../block-title';
import BlockNavigationExpander from './expander';

function BlockNavigationBlockSelectButton(
{
className,
block: { clientId },
isSelected,
onClick,
onToggleExpanded,
position,
siblingBlockCount,
level,
Expand Down Expand Up @@ -61,6 +63,7 @@ function BlockNavigationBlockSelectButton(
onDragEnd={ onDragEnd }
draggable={ draggable }
>
<BlockNavigationExpander onClick={ onToggleExpanded } />
<BlockIcon icon={ blockInformation?.icon } showColors />
<BlockTitle clientId={ clientId } />
{ blockInformation?.anchor && (
Expand Down
5 changes: 5 additions & 0 deletions packages/block-editor/src/components/block-navigation/block-slot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { BlockListBlockContext } from '../block-list/block';
import BlockNavigationBlockSelectButton from './block-select-button';
import { getBlockPositionDescription } from './utils';
import { store as blockEditorStore } from '../../store';
import BlockNavigationExpander from './expander';

const getSlotName = ( clientId ) => `BlockNavigationBlock-${ clientId }`;

Expand Down Expand Up @@ -57,6 +58,7 @@ function BlockNavigationBlockSlot( props, ref ) {
level,
tabIndex,
onFocus,
onToggleExpanded,
} = props;

const blockType = getBlockType( name );
Expand Down Expand Up @@ -86,6 +88,9 @@ function BlockNavigationBlockSlot( props, ref ) {
className
) }
>
<BlockNavigationExpander
onClick={ onToggleExpanded }
/>
<BlockIcon icon={ blockType.icon } showColors />
{ Children.map( fills, ( fill ) =>
cloneElement( fill, {
Expand Down
6 changes: 5 additions & 1 deletion packages/block-editor/src/components/block-navigation/block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,14 @@ export default function BlockNavigationBlock( {
isBranchSelected,
isLastOfSelectedBranch,
onClick,
onToggleExpanded,
position,
level,
rowCount,
siblingBlockCount,
showBlockMovers,
path,
isExpanded,
} ) {
const cellRef = useRef( null );
const [ isHovered, setIsHovered ] = useState( false );
Expand Down Expand Up @@ -142,6 +144,7 @@ export default function BlockNavigationBlock( {
path={ path }
id={ `block-navigation-block-${ clientId }` }
data-block={ clientId }
isExpanded={ isExpanded }
>
<TreeGridCell
className="block-editor-block-navigation-block__contents-cell"
Expand All @@ -152,7 +155,8 @@ export default function BlockNavigationBlock( {
<div className="block-editor-block-navigation-block__contents-container">
<BlockNavigationBlockContents
block={ block }
onClick={ () => onClick( block.clientId ) }
onClick={ onClick }
onToggleExpanded={ onToggleExpanded }
isSelected={ isSelected }
position={ position }
siblingBlockCount={ siblingBlockCount }
Expand Down
28 changes: 26 additions & 2 deletions packages/block-editor/src/components/block-navigation/branch.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import { Fragment } from '@wordpress/element';
import BlockNavigationBlock from './block';
import BlockNavigationAppender from './appender';
import { isClientIdSelected } from './utils';
import { useBlockNavigationContext } from './context';

export default function BlockNavigationBranch( props ) {
const {
blocks,
Expand Down Expand Up @@ -42,6 +44,8 @@ export default function BlockNavigationBranch( props ) {
const rowCount = hasAppender ? blockCount + 1 : blockCount;
const appenderPosition = rowCount;

const { expandedState, expand, collapse } = useBlockNavigationContext();

return (
<>
{ map( filteredBlocks, ( block, index ) => {
Expand Down Expand Up @@ -71,11 +75,30 @@ export default function BlockNavigationBranch( props ) {
const isLastOfSelectedBranch =
isLastOfBranch && ! hasNestedBranch && isLastBlock;

const isExpanded = hasNestedBranch
? expandedState[ clientId ] ?? true
: undefined;

const selectBlockWithClientId = ( event ) => {
event.stopPropagation();
selectBlock( clientId );
};

const toggleExpanded = ( event ) => {
event.stopPropagation();
if ( isExpanded === true ) {
collapse( clientId );
} else if ( isExpanded === false ) {
expand( clientId );
}
};

return (
<Fragment key={ clientId }>
<BlockNavigationBlock
block={ block }
onClick={ selectBlock }
onClick={ selectBlockWithClientId }
onToggleExpanded={ toggleExpanded }
isSelected={ isSelected }
isBranchSelected={ isSelectedBranch }
isLastOfSelectedBranch={ isLastOfSelectedBranch }
Expand All @@ -86,8 +109,9 @@ export default function BlockNavigationBranch( props ) {
showBlockMovers={ showBlockMovers }
terminatedLevels={ terminatedLevels }
path={ updatedPath }
isExpanded={ isExpanded }
/>
{ hasNestedBranch && (
{ hasNestedBranch && isExpanded && (
<BlockNavigationBranch
blocks={ innerBlocks }
selectedBlockClientIds={
Expand Down
16 changes: 16 additions & 0 deletions packages/block-editor/src/components/block-navigation/expander.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* WordPress dependencies
*/
import { chevronRightSmall, Icon } from '@wordpress/icons';
export default function BlockNavigationExpander( { onClick } ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally in the future we don't need a component like this, and can instead use CSS rules to point at an SVG image.

return (
// Keyboard events are handled by TreeGrid see: components/src/tree-grid/index.js
// eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions
<span
Copy link
Contributor

@talldan talldan Jun 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The implementation as a pseudo-element in the w3 example was really surprising to me.

I guess that makes it completely hidden from the accessibility tree, in which case the span could have aria-hidden="true" specified to replicate that.

It might be worth adding some comments to the component explaining why this is implemented like it is—I imagine many devs with good intentions might be tempted to change it to a Button which wouldn't be good given it's already rendered in a Button.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, I've updated this in f4b5a5f

For folks 👀 we can see the pseudo element in https://www.w3.org/TR/wai-aria-practices/examples/treegrid/treegrid-1.html

Screen Shot 2021-06-14 at 9 25 57 AM

className="block-editor-block-navigation__expander"
onClick={ ( event ) => onClick( event, { forceToggle: true } ) }
>
<Icon icon={ chevronRightSmall } />
</span>
);
}
60 changes: 52 additions & 8 deletions packages/block-editor/src/components/block-navigation/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@
&.is-branch-selected.is-selected .block-editor-block-navigation-block-contents {
border-radius: 2px 2px 0 0;
}

&[aria-expanded="false"] {
&.is-branch-selected.is-selected .block-editor-block-navigation-block-contents {
border-radius: 2px;
}
}
&.is-branch-selected:not(.is-selected) .block-editor-block-navigation-block-contents {
// Lighten a CSS variable without introducing a new SASS variable
background:
Expand All @@ -60,7 +66,7 @@
align-items: center;
width: 100%;
height: auto;
padding: ($grid-unit-15 / 2) $grid-unit-15;
padding: ($grid-unit-15 / 2) $grid-unit-15 ($grid-unit-15 / 2) 0;
text-align: left;
color: $gray-900;
border-radius: 2px;
Expand Down Expand Up @@ -121,8 +127,8 @@

.block-editor-block-icon {
align-self: flex-start;
margin-right: ( $grid-unit-05 * 2.5 ); // 10px is off the 4px grid, but required for visual alignment between block name and subsequent nested icon
Copy link
Contributor

Choose a reason for hiding this comment

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

CC: @jameskoster, the icon was scaled down to 20px, so it didn't show true pixels. I shifted the metrics around a bit, but would appreciate you taking a look when you have a chance, thank you.

Copy link
Contributor

Choose a reason for hiding this comment

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

Screenshot 2021-05-28 at 10 41 48

Looks good to me.

width: 20px;
margin-right: $grid-unit-10;
width: $icon-size;
}

.block-editor-block-navigation-block__menu-cell,
Expand Down Expand Up @@ -274,15 +280,53 @@
}
}

// Chevron container metrics.
.block-editor-block-navigation__expander {
height: $icon-size;
margin-left: $grid-unit-05;
width: $icon-size;
}

// First level of indentation is aria-level 2, max indent is 8.
// Indent is a full icon size, plus 4px which optically aligns child icons to the text label above.
$block-navigation-max-indent: 8;
.block-editor-block-navigation-leaf[aria-level] .block-editor-block-icon {
margin-left: ( $grid-unit-30 + $grid-unit-05 ) * $block-navigation-max-indent;
.block-editor-block-navigation-leaf[aria-level] .block-editor-block-navigation__expander {
margin-left: ( $icon-size ) * $block-navigation-max-indent + 4 * ( $block-navigation-max-indent - 1 );
}

.block-editor-block-navigation-leaf:not([aria-level="1"]) {
.block-editor-block-navigation__expander {
margin-right: 4px;
}
}
@for $i from 0 through $block-navigation-max-indent {
.block-editor-block-navigation-leaf[aria-level="#{ $i + 1 }"] .block-editor-block-icon {
margin-left: ( $grid-unit-30 + $grid-unit-05 ) * $i;

@for $i from 0 to $block-navigation-max-indent {
.block-editor-block-navigation-leaf[aria-level="#{ $i + 1 }"] .block-editor-block-navigation__expander {
@if $i - 1 >= 0 {
margin-left: ( $icon-size * $i ) + 4 * ($i - 1);
}
@else {
margin-left: ( $icon-size * $i );
}
}
}

.block-editor-block-navigation-leaf .block-editor-block-navigation__expander {
visibility: hidden;
}

// Point downwards when open.
.block-editor-block-navigation-leaf[aria-expanded="true"] .block-editor-block-navigation__expander svg {
visibility: visible;
transition: transform 0.2s ease;
transform: rotate(90deg);
@include reduce-motion("transition");
}

// Point rightwards when closed
.block-editor-block-navigation-leaf[aria-expanded="false"] .block-editor-block-navigation__expander svg {
visibility: visible;
transform: rotate(0deg);
transition: transform 0.2s ease;
@include reduce-motion("transition");
}
32 changes: 31 additions & 1 deletion packages/block-editor/src/components/block-navigation/tree.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,13 @@

import { __experimentalTreeGrid as TreeGrid } from '@wordpress/components';
import { useDispatch } from '@wordpress/data';
import { useCallback, useEffect, useMemo, useRef } from '@wordpress/element';
import {
useCallback,
useEffect,
useMemo,
useRef,
useReducer,
} from '@wordpress/element';
import { __ } from '@wordpress/i18n';

/**
Expand All @@ -17,6 +23,16 @@ import useBlockNavigationDropZone from './use-block-navigation-drop-zone';
import { store as blockEditorStore } from '../../store';

const noop = () => {};
const expanded = ( state, action ) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We only record state when someone toggles an expand/collapse chevron.

One case I haven't covered is when blocks are deleted, we might have some extra state left over eg:

state = { 
   'block-client-id-a': false,
   'block-client-id-b': true,
   'deleted-client-id': false
};

It is possible to prune this by dispatching an event as a useEffect side effect when clientIdsTree, or something similar like listening to a block delete event. I chose to leave this out since it does feel a bit unnecessary since this is likely to remain pretty small. Let me know if others feel otherwise and I can add handling.

Copy link
Contributor

@talldan talldan Jun 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, agreed, it's probably not a big issue. I doubt this will ever become a huge amount of data.

I have ideas but I think it's an effort vs. reward question more than anything:

  • BlockNavigation could receive stable block client ids from a selector (e.g. from getBlockOrder), and reconcile it against the expanded state in an effect.
  • It could be possible to move the state to the treegrid component entirely. Could each TreeGridRow manage its own state? I think it'd then have to render its own expander component, but that seems potentially ok. I might be missing some complications.

switch ( action.type ) {
case 'expand':
return { ...state, ...{ [ action.clientId ]: true } };
case 'collapse':
return { ...state, ...{ [ action.clientId ]: false } };
default:
return state;
}
};

/**
* Wrap `BlockNavigationRows` with `TreeGrid`. BlockNavigationRows is a
Expand Down Expand Up @@ -52,6 +68,7 @@ export default function BlockNavigationTree( {
},
[ selectBlock, onSelect ]
);
const [ expandedState, setExpandedState ] = useReducer( expanded, {} );

let {
ref: treeGridRef,
Expand All @@ -67,18 +84,29 @@ export default function BlockNavigationTree( {
blockDropTarget = undefined;
}

const expand = ( clientId ) =>
setExpandedState( { type: 'expand', clientId } );
const collapse = ( clientId ) =>
setExpandedState( { type: 'collapse', clientId } );

const contextValue = useMemo(
() => ( {
__experimentalFeatures,
__experimentalPersistentListViewFeatures,
blockDropTarget,
isTreeGridMounted: isMounted.current,
expandedState,
expand,
collapse,
} ),
[
__experimentalFeatures,
__experimentalPersistentListViewFeatures,
blockDropTarget,
isMounted.current,
expandedState,
expand,
collapse,
]
);

Expand All @@ -87,6 +115,8 @@ export default function BlockNavigationTree( {
className="block-editor-block-navigation-tree"
aria-label={ __( 'Block navigation structure' ) }
ref={ treeGridRef }
onCollapseRow={ collapse }
onExpandRow={ expand }
>
<BlockNavigationContext.Provider value={ contextValue }>
<BlockNavigationBranch
Expand Down
Loading