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

Block Editor: Add client ID trees selectors #29902

Merged
merged 7 commits into from
Mar 22, 2021
Merged
Show file tree
Hide file tree
Changes from 6 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
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import classnames from 'classnames';
import { getBlockType } from '@wordpress/blocks';
import { Fill, Slot, VisuallyHidden } from '@wordpress/components';
import { useInstanceId } from '@wordpress/compose';
import { useSelect } from '@wordpress/data';
import {
Children,
cloneElement,
Expand All @@ -24,12 +25,17 @@ import BlockIcon from '../block-icon';
import { BlockListBlockContext } from '../block-list/block';
import BlockNavigationBlockSelectButton from './block-select-button';
import { getBlockPositionDescription } from './utils';
import { store as blockEditorStore } from '../../store';

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

function BlockNavigationBlockSlot( props, ref ) {
const instanceId = useInstanceId( BlockNavigationBlockSlot );
const { clientId } = props.block;
const { name } = useSelect(
( select ) => select( blockEditorStore ).getBlockName( clientId ),
[ clientId ]
);
const instanceId = useInstanceId( BlockNavigationBlockSlot );

return (
<Slot name={ getSlotName( clientId ) }>
Expand All @@ -45,7 +51,6 @@ function BlockNavigationBlockSlot( props, ref ) {

const {
className,
block,
isSelected,
position,
siblingBlockCount,
Expand All @@ -54,7 +59,6 @@ function BlockNavigationBlockSlot( props, ref ) {
onFocus,
} = props;

const { name } = block;
const blockType = getBlockType( name );
const descriptionId = `block-navigation-block-slot__${ instanceId }`;
const blockPositionDescription = getBlockPositionDescription(
Expand Down
72 changes: 35 additions & 37 deletions packages/block-editor/src/components/block-navigation/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ import { noop } from 'lodash';
/**
* WordPress dependencies
*/
import { withSelect, withDispatch } from '@wordpress/data';
import { compose } from '@wordpress/compose';
import { useDispatch, useSelect } from '@wordpress/data';
import { __ } from '@wordpress/i18n';

/**
Expand All @@ -16,13 +15,41 @@ import { __ } from '@wordpress/i18n';
import BlockNavigationTree from './tree';
import { store as blockEditorStore } from '../../store';

function BlockNavigation( {
rootBlock,
rootBlocks,
selectedBlockClientId,
selectBlock,
export default function BlockNavigation( {
onSelect = noop,
__experimentalFeatures,
} ) {
const { rootBlock, rootBlocks, selectedBlockClientId } = useSelect(
( select ) => {
const {
getBlockHierarchyRootClientId,
getSelectedBlockClientId,
__unstableGetClientIdsTree,
__unstableGetClientIdWithClientIdsTree,
} = select( blockEditorStore );

const _selectedBlockClientId = getSelectedBlockClientId();
const _rootBlocks = __unstableGetClientIdsTree();
const _rootBlock = _selectedBlockClientId
Copy link
Contributor

Choose a reason for hiding this comment

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

General question, why the _ prefix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's because those names are already declared in the upper scope.
The _ is just simpler than figure out some naming switcheroo that might compromise the readability.

(Also, I've seen it used plenty of times in Gutenberg, sometimes to mark a constant as "private", but often in this exact same context of select hooks.)

? __unstableGetClientIdWithClientIdsTree(
getBlockHierarchyRootClientId( _selectedBlockClientId )
)
: null;

return {
rootBlock: _rootBlock,
rootBlocks: _rootBlocks,
selectedBlockClientId: _selectedBlockClientId,
};
}
);
const { selectBlock } = useDispatch( blockEditorStore );

function selectEditorBlock( clientId ) {
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to use function vs arrow function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really, no. I don't think there's any practical difference in this case.

selectBlock( clientId );
onSelect( clientId );
}

if ( ! rootBlocks || rootBlocks.length === 0 ) {
return null;
}
Expand All @@ -41,39 +68,10 @@ function BlockNavigation( {
<BlockNavigationTree
blocks={ hasHierarchy ? [ rootBlock ] : rootBlocks }
selectedBlockClientId={ selectedBlockClientId }
selectBlock={ selectBlock }
selectBlock={ selectEditorBlock }
__experimentalFeatures={ __experimentalFeatures }
showNestedBlocks
/>
</div>
);
}

export default compose(
withSelect( ( select ) => {
const {
getSelectedBlockClientId,
getBlockHierarchyRootClientId,
__unstableGetBlockWithBlockTree,
__unstableGetBlockTree,
} = select( blockEditorStore );
const selectedBlockClientId = getSelectedBlockClientId();
return {
rootBlocks: __unstableGetBlockTree(),
rootBlock: selectedBlockClientId
? __unstableGetBlockWithBlockTree(
getBlockHierarchyRootClientId( selectedBlockClientId )
)
: null,
selectedBlockClientId,
};
} ),
withDispatch( ( dispatch, { onSelect = noop } ) => {
return {
selectBlock( clientId ) {
dispatch( blockEditorStore ).selectBlock( clientId );
onSelect( clientId );
},
};
} )
)( BlockNavigation );
35 changes: 35 additions & 0 deletions packages/block-editor/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,41 @@ export const __unstableGetBlockTree = createSelector(
]
);

/**
* Returns a stripped down block object containing only its client ID,
* and its inner blocks' client IDs.
*
* @param {Object} state Editor state.
* @param {string} clientId Client ID of the block to get.
*
* @return {Object} Client IDs of the post blocks.
*/
export const __unstableGetClientIdWithClientIdsTree = createSelector(
( state, clientId ) => ( {
clientId,
innerBlocks: __unstableGetClientIdsTree( state, clientId ),
} ),
( state ) => [ state.blocks.order ]
);

/**
* Returns the block tree represented in the block-editor store from the
* given root, consisting of stripped down block objects containing only
* their client IDs, and their inner blocks' client IDs.
*
* @param {Object} state Editor state.
* @param {?string} rootClientId Optional root client ID of block list.
*
* @return {Object[]} Client IDs of the post blocks.
*/
export const __unstableGetClientIdsTree = createSelector(
( state, rootClientId = '' ) =>
map( getBlockOrder( state, rootClientId ), ( clientId ) =>
__unstableGetClientIdWithClientIdsTree( state, clientId )
),
( state ) => [ state.blocks.order ]
);

/**
* Returns an array containing the clientIds of all descendants
* of the blocks given.
Expand Down
75 changes: 75 additions & 0 deletions packages/block-editor/src/store/test/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ const {
__experimentalGetParsedReusableBlock,
__experimentalGetAllowedPatterns,
__experimentalGetScopedBlockPatterns,
__unstableGetClientIdWithClientIdsTree,
__unstableGetClientIdsTree,
} = selectors;

describe( 'selectors', () => {
Expand Down Expand Up @@ -3535,3 +3537,76 @@ describe( 'getInserterItems with core blocks prioritization', () => {
expect( items.map( ( { name } ) => name ) ).toEqual( expectedResult );
} );
} );

describe( '__unstableGetClientIdWithClientIdsTree', () => {
it( "should return a stripped down block object containing only its client ID and its inner blocks' client IDs", () => {
const state = {
blocks: {
order: {
'': [ 'foo' ],
foo: [ 'bar', 'baz' ],
bar: [ 'qux' ],
},
},
};

expect(
__unstableGetClientIdWithClientIdsTree( state, 'foo' )
).toEqual( {
clientId: 'foo',
innerBlocks: [
{
clientId: 'bar',
innerBlocks: [ { clientId: 'qux', innerBlocks: [] } ],
},
{ clientId: 'baz', innerBlocks: [] },
],
} );
} );
} );
describe( '__unstableGetClientIdsTree', () => {
it( "should return the full content tree starting from the given root, consisting of stripped down block object containing only its client ID and its inner blocks' client IDs", () => {
const state = {
blocks: {
order: {
'': [ 'foo' ],
foo: [ 'bar', 'baz' ],
bar: [ 'qux' ],
},
},
};

expect( __unstableGetClientIdsTree( state, 'foo' ) ).toEqual( [
{
clientId: 'bar',
innerBlocks: [ { clientId: 'qux', innerBlocks: [] } ],
},
{ clientId: 'baz', innerBlocks: [] },
] );
} );

it( "should return the full content tree starting from the root, consisting of stripped down block object containing only its client ID and its inner blocks' client IDs", () => {
const state = {
blocks: {
order: {
'': [ 'foo' ],
foo: [ 'bar', 'baz' ],
bar: [ 'qux' ],
},
},
};

expect( __unstableGetClientIdsTree( state ) ).toEqual( [
{
clientId: 'foo',
innerBlocks: [
{
clientId: 'bar',
innerBlocks: [ { clientId: 'qux', innerBlocks: [] } ],
},
{ clientId: 'baz', innerBlocks: [] },
],
},
] );
} );
} );
13 changes: 7 additions & 6 deletions packages/block-library/src/navigation/block-navigation-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@ export default function BlockNavigationList( {
clientId,
__experimentalFeatures,
} ) {
const { block, selectedBlockClientId } = useSelect(
const { blocks, selectedBlockClientId } = useSelect(
( select ) => {
const { getSelectedBlockClientId, getBlock } = select(
blockEditorStore
);
const {
getSelectedBlockClientId,
__unstableGetClientIdsTree,
} = select( blockEditorStore );

return {
block: getBlock( clientId ),
blocks: __unstableGetClientIdsTree( clientId ),
selectedBlockClientId: getSelectedBlockClientId(),
};
},
Expand All @@ -29,7 +30,7 @@ export default function BlockNavigationList( {

return (
<__experimentalBlockNavigationTree
blocks={ block.innerBlocks }
blocks={ blocks }
selectedBlockClientId={ selectedBlockClientId }
selectBlock={ selectBlock }
__experimentalFeatures={ __experimentalFeatures }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
useInstanceId,
useMergeRefs,
} from '@wordpress/compose';
import { useDispatch, useSelect } from '@wordpress/data';
import { AsyncModeProvider, useDispatch, useSelect } from '@wordpress/data';
import { __ } from '@wordpress/i18n';
import { closeSmall } from '@wordpress/icons';
import { ESCAPE } from '@wordpress/keycodes';
Expand All @@ -23,12 +23,12 @@ import { ESCAPE } from '@wordpress/keycodes';
import { store as editSiteStore } from '../../store';

export default function ListViewSidebar() {
const { rootBlocks, selectedBlockClientId } = useSelect( ( select ) => {
const { getSelectedBlockClientId, __unstableGetBlockTree } = select(
const { clientIdsTree, selectedBlockClientId } = useSelect( ( select ) => {
const { __unstableGetClientIdsTree, getSelectedBlockClientId } = select(
blockEditorStore
);
return {
rootBlocks: __unstableGetBlockTree(),
clientIdsTree: __unstableGetClientIdsTree(),
selectedBlockClientId: getSelectedBlockClientId(),
};
} );
Expand Down Expand Up @@ -71,13 +71,15 @@ export default function ListViewSidebar() {
className="edit-site-editor__list-view-panel-content"
ref={ useMergeRefs( [ focusReturnRef, focusOnMountRef ] ) }
>
<BlockNavigationTree
blocks={ rootBlocks }
selectBlock={ selectEditorBlock }
selectedBlockClientId={ selectedBlockClientId }
showNestedBlocks
__experimentalPersistentListViewFeatures
/>
<AsyncModeProvider value="true">
Copons marked this conversation as resolved.
Show resolved Hide resolved
<BlockNavigationTree
blocks={ clientIdsTree }
selectBlock={ selectEditorBlock }
selectedBlockClientId={ selectedBlockClientId }
showNestedBlocks
__experimentalPersistentListViewFeatures
/>
</AsyncModeProvider>
</div>
</div>
);
Expand Down