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.

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: Add private appender prop #49137

Merged
merged 7 commits into from
Mar 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
101 changes: 101 additions & 0 deletions packages/block-editor/src/components/list-view/appender.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/**
* WordPress dependencies
*/
import { useInstanceId } from '@wordpress/compose';
import { speak } from '@wordpress/a11y';
import { useSelect } from '@wordpress/data';
import { forwardRef, useState, useEffect } from '@wordpress/element';
import { __, sprintf } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import { store as blockEditorStore } from '../../store';
import useBlockDisplayTitle from '../block-title/use-block-display-title';
import Inserter from '../inserter';

export const Appender = forwardRef(
( { nestingLevel, blockCount, clientId, ...props }, ref ) => {
const [ insertedBlock, setInsertedBlock ] = useState( null );

const instanceId = useInstanceId( Appender );
const { hideInserter } = useSelect(
( select ) => {
const { getTemplateLock, __unstableGetEditorMode } =
select( blockEditorStore );

return {
hideInserter:
!! getTemplateLock( clientId ) ||
__unstableGetEditorMode() === 'zoom-out',
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the need for __unstableGetEditorMode() === 'zoom-out'?

It stands out, because zoom out mode is something that applies to the editor canvas, and List View is separate to the editor canvas, so I'm wondering how the two might be linked.

It looks like it isn't possible to access List view in zoomed out mode (though it does stay open when already open, which seems like a bug).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks to me like it's copy/pasted from packages/block-editor/src/components/block-list-appender/index.js. Maybe there's some benefit to keeping these files in sync where possible even though this line isn't needed, with the hope that we can share more code in the future?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would probably lean towards removing it, but it's not critical and definitely not a blocker.

};
},
[ clientId ]
);

const blockTitle = useBlockDisplayTitle( {
clientId,
context: 'list-view',
} );

const insertedBlockTitle = useBlockDisplayTitle( {
clientId: insertedBlock?.clientId,
context: 'list-view',
} );

useEffect( () => {
if ( ! insertedBlockTitle?.length ) {
return;
}

speak(
sprintf(
// translators: %s: name of block being inserted (i.e. Paragraph, Image, Group etc)
__( '%s block inserted' ),
insertedBlockTitle
),
'assertive'
);
}, [ insertedBlockTitle ] );
Comment on lines +46 to +59
Copy link
Contributor

Choose a reason for hiding this comment

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

This effect feels unnecessary, when it could something along the lines of:

onSelectOrClose={ ( block ) => {
    if ( block ) {
        const insertedBlockTitle = getBlockDisplayTitle( block );
        speak( sprintf( __( '%s block inserted' ), insertedBlockTitle), 'assertive' );
    }
} )

It looks like it's all down to the fact that the only way to get the de-facto block title is using a react hook, when really it should be possible using a simple function call (getBlockDisplayTitle( block ) or similar), so that means introducing more hooks.

It's not straightforward to solve unfortunately, but might be something to think about if we keep hitting this problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. The process of getting the block title is very hook dependant. Would need to decouple all that first.


if ( hideInserter ) {
return null;
}

const descriptionId = `list-view-appender__${ instanceId }`;
const description = sprintf(
/* translators: 1: The name of the block. 2: The numerical position of the block. 3: The level of nesting for the block. */
__( 'Append to %1$s block at position %2$d, Level %3$d' ),
blockTitle,
blockCount + 1,
nestingLevel
);

return (
<div className="list-view-appender">
<Inserter
ref={ ref }
rootClientId={ clientId }
position="bottom right"
isAppender
selectBlockOnInsert={ false }
shouldDirectInsert={ false }
__experimentalIsQuick
{ ...props }
toggleProps={ { 'aria-describedby': descriptionId } }
onSelectOrClose={ ( maybeInsertedBlock ) => {
if ( maybeInsertedBlock?.clientId ) {
setInsertedBlock( maybeInsertedBlock );
}
} }
/>
<div
className="list-view-appender__description"
id={ descriptionId }
>
{ description }
</div>
</div>
);
}
);
31 changes: 30 additions & 1 deletion packages/block-editor/src/components/list-view/branch.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
/**
* WordPress dependencies
*/
import {
__experimentalTreeGridRow as TreeGridRow,
__experimentalTreeGridCell as TreeGridCell,
} from '@wordpress/components';
import { memo } from '@wordpress/element';
import { AsyncModeProvider, useSelect } from '@wordpress/data';

/**
* Internal dependencies
*/
import { Appender } from './appender';
import ListViewBlock from './block';
import { useListViewContext } from './context';
import { isClientIdSelected } from './utils';
Expand Down Expand Up @@ -93,6 +98,7 @@ function ListViewBranch( props ) {
parentId,
shouldShowInnerBlocks = true,
isSyncedBranch = false,
showAppender: showAppenderProp = true,
} = props;

const parentBlockInformation = useBlockDisplayInformation( parentId );
Expand Down Expand Up @@ -120,8 +126,12 @@ function ListViewBranch( props ) {
return null;
}

// Only show the appender at the first level.
const showAppender = showAppenderProp && level === 1;
const filteredBlocks = blocks.filter( Boolean );
const blockCount = filteredBlocks.length;
// The appender means an extra row in List View, so add 1 to the row count.
const rowCount = showAppender ? blockCount + 1 : blockCount;
let nextPosition = listPosition;

return (
Expand Down Expand Up @@ -175,7 +185,7 @@ function ListViewBranch( props ) {
isDragged={ isDragged }
level={ level }
position={ position }
rowCount={ blockCount }
rowCount={ rowCount }
siblingBlockCount={ blockCount }
showBlockMovers={ showBlockMovers }
path={ updatedPath }
Expand Down Expand Up @@ -209,6 +219,25 @@ function ListViewBranch( props ) {
</AsyncModeProvider>
);
} ) }
{ showAppender && (
<TreeGridRow
level={ level }
setSize={ rowCount }
positionInSet={ rowCount }
isExpanded={ true }
>
<TreeGridCell>
{ ( treeGridCellProps ) => (
<Appender
clientId={ parentId }
nestingLevel={ level }
blockCount={ blockCount }
{ ...treeGridCellProps }
/>
) }
</TreeGridCell>
</TreeGridRow>
) }
</>
);
}
Expand Down
18 changes: 15 additions & 3 deletions packages/block-editor/src/components/list-view/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,17 @@ export const BLOCK_LIST_ITEM_HEIGHT = 36;
* @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.
* @param {boolean} props.showAppender Flag to show or hide the block appender.
* @param {Object} ref Forwarded ref
*/
function ListView(
{ id, blocks, showBlockMovers = false, isExpanded = false },
function ListViewComponent(
{
id,
blocks,
showBlockMovers = false,
isExpanded = false,
showAppender = false,
},
ref
) {
const { clientIdsTree, draggedClientIds, selectedClientIds } =
Expand Down Expand Up @@ -204,10 +211,15 @@ function ListView(
selectedClientIds={ selectedClientIds }
isExpanded={ isExpanded }
shouldShowInnerBlocks={ shouldShowInnerBlocks }
showAppender={ showAppender }
/>
</ListViewContext.Provider>
</TreeGrid>
</AsyncModeProvider>
);
}
export default forwardRef( ListView );
export const PrivateListView = forwardRef( ListViewComponent );

export default forwardRef( ( props, ref ) => {
return <PrivateListView ref={ ref } { ...props } showAppender={ false } />;
} );
19 changes: 19 additions & 0 deletions packages/block-editor/src/components/list-view/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -410,3 +410,22 @@ $block-navigation-max-indent: 8;
height: 36px;
}

.list-view-appender .block-editor-inserter__toggle {
background-color: #1e1e1e;
color: #fff;
margin: $grid-unit-10 0 0 24px;
border-radius: 2px;
height: 24px;
min-width: 24px;
padding: 0;

&:hover,
&:focus {
background: var(--wp-admin-theme-color);
color: #fff;
}
}

.list-view-appender__description {
display: none;
}
2 changes: 2 additions & 0 deletions packages/block-editor/src/private-apis.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { lock } from './lock-unlock';
import OffCanvasEditor from './components/off-canvas-editor';
import LeafMoreMenu from './components/off-canvas-editor/leaf-more-menu';
import { ComposedPrivateInserter as PrivateInserter } from './components/inserter';
import { PrivateListView } from './components/list-view';

/**
* Private @wordpress/block-editor APIs.
Expand All @@ -18,4 +19,5 @@ lock( privateApis, {
LeafMoreMenu,
OffCanvasEditor,
PrivateInserter,
PrivateListView,
} );
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* WordPress dependencies
*/
import { __experimentalListView as ListView } from '@wordpress/block-editor';
import { privateApis as blockEditorPrivateApis } from '@wordpress/block-editor';
import { Button } from '@wordpress/components';
import {
useFocusOnMount,
Expand All @@ -18,6 +18,7 @@ import { ESCAPE } from '@wordpress/keycodes';
* Internal dependencies
*/
import { store as editSiteStore } from '../../store';
import { unlock } from '../../private-apis';

export default function ListViewSidebar() {
const { setIsListViewOpened } = useDispatch( editSiteStore );
Expand All @@ -33,7 +34,7 @@ export default function ListViewSidebar() {

const instanceId = useInstanceId( ListViewSidebar );
const labelId = `edit-site-editor__list-view-panel-label-${ instanceId }`;

const { PrivateListView } = unlock( blockEditorPrivateApis );
return (
// eslint-disable-next-line jsx-a11y/no-static-element-interactions
<div
Expand All @@ -59,7 +60,7 @@ export default function ListViewSidebar() {
focusOnMountRef,
] ) }
>
<ListView />
<PrivateListView />
</div>
</div>
);
Expand Down