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

Block Quick Inserter: Prioritize showing patterns instead of blocks. #38709

Merged
merged 3 commits into from
Feb 17, 2022
Merged

Block Quick Inserter: Prioritize showing patterns instead of blocks. #38709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ff1f5d3
Update: Prioritize patterns on quick inserter instead of blocks at th…
jorgefilipecosta Feb 9, 2022
460be3c
feedback
jorgefilipecosta Feb 14, 2022
cf7437d
update tests
jorgefilipecosta Feb 15, 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
40 changes: 31 additions & 9 deletions packages/block-editor/src/components/inserter/quick-inserter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { store as blockEditorStore } from '../../store';
const SEARCH_THRESHOLD = 6;
const SHOWN_BLOCK_TYPES = 6;
const SHOWN_BLOCK_PATTERNS = 2;
const SHOWN_BLOCK_PATTERNS_WITH_PRIORITIZATION = 4;

export default function QuickInserter( {
onSelect,
Expand All @@ -46,26 +47,39 @@ export default function QuickInserter( {
onInsertBlocks,
destinationRootClientId
);
const showPatterns = patterns.length && !! filterValue;
const showSearch =
( showPatterns && patterns.length > SEARCH_THRESHOLD ) ||
blockTypes.length > SEARCH_THRESHOLD;

const { setInserterIsOpened, insertionIndex } = useSelect(
const {
setInserterIsOpened,
insertionIndex,
prioritizePatterns,
} = useSelect(
( select ) => {
const { getSettings, getBlockIndex, getBlockCount } = select(
blockEditorStore
);
const settings = getSettings();
const index = getBlockIndex( clientId );
const blockCount = getBlockCount();

return {
setInserterIsOpened: getSettings()
.__experimentalSetIsInserterOpened,
insertionIndex: index === -1 ? getBlockCount() : index,
setInserterIsOpened: settings.__experimentalSetIsInserterOpened,
prioritizePatterns:
settings.__experimentalPreferPatternsOnRoot &&
! rootClientId &&
index > 0 &&
Copy link
Contributor

Choose a reason for hiding this comment

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

What about these positions? If I create a new empty template I should see patterns in the inserter, no? Same goes for the last index if it's top level.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @ntsekouras,
The conditions are following what @mtias specified on the issue:

The top-level means it's a direct descendant of the document, between any root template parts or existing blocks

I added a condition to also show the behavior when the template is empty.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, we'll need to do some extra work on empty templates regardless.

( index < blockCount || blockCount === 0 ),
insertionIndex: index === -1 ? blockCount : index,
};
},
[ clientId, rootClientId ]
);

const showPatterns =
patterns.length && ( !! filterValue || prioritizePatterns );
const showSearch =
( showPatterns && patterns.length > SEARCH_THRESHOLD ) ||
Copy link
Contributor

Choose a reason for hiding this comment

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

Edge case here in the case of prioritizing patterns where the SEARCH_THRESHOLD is more than SHOWN_BLOCK_PATTERNS_WITH_PRIORITIZATION, in combination of course with only a few allowed blocks. I think it will be a super rare use case if it ever occurs, but just noting.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @ntsekouras, I think here SHOWN_BLOCK_PATTERNS_WITH_PRIORITIZATION is not relevant, patterns.length contains the full number of patterns e.g: if we have 34 patterns and SHOWN_BLOCK_PATTERNS_WITH_PRIORITIZATION is 2 and SEARCH_THRESHOLD is 3 we are going to show search anyway as expected because we have 34 patterns.
Let me know if I am not understanding correctly the issue.

blockTypes.length > SEARCH_THRESHOLD;

useEffect( () => {
if ( setInserterIsOpened ) {
setInserterIsOpened( false );
Expand All @@ -78,6 +92,13 @@ export default function QuickInserter( {
setInserterIsOpened( { rootClientId, insertionIndex, filterValue } );
};

let maxBlockPatterns = 0;
if ( showPatterns ) {
maxBlockPatterns = prioritizePatterns
? SHOWN_BLOCK_PATTERNS_WITH_PRIORITIZATION
: SHOWN_BLOCK_PATTERNS;
}

return (
<div
className={ classnames( 'block-editor-inserter__quick-inserter', {
Expand All @@ -104,9 +125,10 @@ export default function QuickInserter( {
rootClientId={ rootClientId }
clientId={ clientId }
isAppender={ isAppender }
maxBlockPatterns={ showPatterns ? SHOWN_BLOCK_PATTERNS : 0 }
maxBlockPatterns={ maxBlockPatterns }
maxBlockTypes={ SHOWN_BLOCK_TYPES }
isDraggable={ false }
prioritizePatterns={ prioritizePatterns }
/>
</div>

Expand Down
96 changes: 54 additions & 42 deletions packages/block-editor/src/components/inserter/search-results.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ function InserterSearchResults( {
showBlockDirectory = false,
isDraggable = true,
shouldFocusBlock = true,
prioritizePatterns,
} ) {
const debouncedSpeak = useDebounce( speak, 500 );

Expand All @@ -70,16 +71,34 @@ function InserterSearchResults( {
destinationRootClientId
);

const filteredBlockPatterns = useMemo( () => {
if ( maxBlockPatterns === 0 ) {
return [];
}
const results = searchItems( patterns, filterValue );
return maxBlockPatterns !== undefined
? results.slice( 0, maxBlockPatterns )
: results;
}, [ filterValue, patterns, maxBlockPatterns ] );

let maxBlockTypesToShow = maxBlockTypes;
if ( prioritizePatterns && filteredBlockPatterns.length > 2 ) {
maxBlockTypesToShow = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: For both filteredPatterns and filteredBlocks, we could bail early if the max is 0 (for perf reasons but really not sure whether it has any impact)

}

const filteredBlockTypes = useMemo( () => {
if ( maxBlockTypesToShow === 0 ) {
return [];
}
const results = searchBlockItems(
orderBy( blockTypes, [ 'frecency' ], [ 'desc' ] ),
blockTypeCategories,
blockTypeCollections,
filterValue
);

return maxBlockTypes !== undefined
? results.slice( 0, maxBlockTypes )
return maxBlockTypesToShow !== undefined
? results.slice( 0, maxBlockTypesToShow )
: results;
}, [
filterValue,
Expand All @@ -89,13 +108,6 @@ function InserterSearchResults( {
maxBlockTypes,
] );

const filteredBlockPatterns = useMemo( () => {
const results = searchItems( patterns, filterValue );
return maxBlockPatterns !== undefined
? results.slice( 0, maxBlockPatterns )
: results;
}, [ filterValue, patterns, maxBlockPatterns ] );

// Announce search results on change
useEffect( () => {
if ( ! filterValue ) {
Expand All @@ -122,49 +134,49 @@ function InserterSearchResults( {
const hasItems =
! isEmpty( filteredBlockTypes ) || ! isEmpty( filteredBlockPatterns );

const blocksUI = !! filteredBlockTypes.length && (
<InserterPanel
title={ <VisuallyHidden>{ __( 'Blocks' ) }</VisuallyHidden> }
>
<BlockTypesList
items={ currentShownBlockTypes }
onSelect={ onSelectBlockType }
onHover={ onHover }
label={ __( 'Blocks' ) }
isDraggable={ isDraggable }
/>
</InserterPanel>
);

const patternsUI = !! filteredBlockPatterns.length && (
<InserterPanel
title={
<VisuallyHidden>{ __( 'Block Patterns' ) }</VisuallyHidden>
}
>
<div className="block-editor-inserter__quick-inserter-patterns">
<BlockPatternsList
shownPatterns={ currentShownPatterns }
blockPatterns={ filteredBlockPatterns }
onClickPattern={ onSelectBlockPattern }
isDraggable={ isDraggable }
/>
</div>
</InserterPanel>
);

return (
<InserterListbox>
{ ! showBlockDirectory && ! hasItems && <InserterNoResults /> }

{ !! filteredBlockTypes.length && (
<InserterPanel
title={
<VisuallyHidden>{ __( 'Blocks' ) }</VisuallyHidden>
}
>
<BlockTypesList
items={ currentShownBlockTypes }
onSelect={ onSelectBlockType }
onHover={ onHover }
label={ __( 'Blocks' ) }
isDraggable={ isDraggable }
/>
</InserterPanel>
) }
{ prioritizePatterns ? patternsUI : blocksUI }

{ !! filteredBlockTypes.length &&
!! filteredBlockPatterns.length && (
<div className="block-editor-inserter__quick-inserter-separator" />
) }

{ !! filteredBlockPatterns.length && (
<InserterPanel
title={
<VisuallyHidden>
{ __( 'Block Patterns' ) }
</VisuallyHidden>
}
>
<div className="block-editor-inserter__quick-inserter-patterns">
<BlockPatternsList
shownPatterns={ currentShownPatterns }
blockPatterns={ filteredBlockPatterns }
onClickPattern={ onSelectBlockPattern }
isDraggable={ isDraggable }
/>
</div>
</InserterPanel>
) }
{ prioritizePatterns ? blocksUI : patternsUI }

{ showBlockDirectory && (
<__unstableInserterMenuExtension.Slot
Expand Down
3 changes: 3 additions & 0 deletions packages/edit-site/src/store/selectors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ export const getSettings = createSelector(
hasFixedToolbar: isFeatureActive( state, 'fixedToolbar' ),
__experimentalSetIsInserterOpened: setIsInserterOpen,
__experimentalReusableBlocks: getReusableBlocks( state ),
__experimentalPreferPatternsOnRoot:
'wp_template' === getEditedPostType( state ),
};

const canUserCreateMedia = getCanUserCreateMedia( state );
Expand All @@ -120,6 +122,7 @@ export const getSettings = createSelector(
isFeatureActive( state, 'focusMode' ),
isFeatureActive( state, 'fixedToolbar' ),
getReusableBlocks( state ),
getEditedPostType( state ),
]
);

Expand Down
9 changes: 8 additions & 1 deletion packages/edit-site/src/store/test/selectors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,19 @@ describe( 'selectors', () => {
it( "returns the settings when the user can't create media", () => {
canUser.mockReturnValueOnce( false );
canUser.mockReturnValueOnce( false );
const state = { settings: {}, preferences: {} };
const state = {
settings: {},
preferences: {},
editedPost: { type: 'wp_template' },
};
const setInserterOpened = () => {};
expect( getSettings( state, setInserterOpened ) ).toEqual( {
outlineMode: true,
focusMode: false,
hasFixedToolbar: false,
__experimentalSetIsInserterOpened: setInserterOpened,
__experimentalReusableBlocks: [],
__experimentalPreferPatternsOnRoot: true,
} );
} );

Expand All @@ -126,6 +131,7 @@ describe( 'selectors', () => {
fixedToolbar: true,
},
},
editedPost: { type: 'wp_template_part' },
};
const setInserterOpened = () => {};

Expand All @@ -137,6 +143,7 @@ describe( 'selectors', () => {
__experimentalSetIsInserterOpened: setInserterOpened,
__experimentalReusableBlocks: [],
mediaUpload: expect.any( Function ),
__experimentalPreferPatternsOnRoot: false,
} );
} );
} );
Expand Down
1 change: 1 addition & 0 deletions packages/editor/src/components/provider/use-block-editor-settings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ function useBlockEditorSettings( settings, hasTemplate ) {
__experimentalCreatePageEntity: createPageEntity,
__experimentalUserCanCreatePages: userCanCreatePages,
pageOnFront,
__experimentalPreferPatternsOnRoot: hasTemplate,
} ),
[
settings,
Expand Down