From 904b211c206a9b3ebea638279214081f4af37550 Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Thu, 25 Jan 2024 14:50:14 +0400 Subject: [PATCH] Block Editor: Optimize the 'useBlockDisplayTitle' hook --- .../src/components/block-title/test/index.js | 171 +++++++++--------- .../block-title/use-block-display-title.js | 64 +++---- 2 files changed, 119 insertions(+), 116 deletions(-) diff --git a/packages/block-editor/src/components/block-title/test/index.js b/packages/block-editor/src/components/block-title/test/index.js index 7031b0b197bf4..8507d5c4c94ca 100644 --- a/packages/block-editor/src/components/block-title/test/index.js +++ b/packages/block-editor/src/components/block-title/test/index.js @@ -13,65 +13,39 @@ import { useSelect } from '@wordpress/data'; */ import BlockTitle from '../'; +const blockTypeMap = { + 'name-not-exists': null, + 'name-exists': { title: 'Block Title' }, + 'name-with-label': { title: 'Block With Label' }, + 'name-with-custom-label': { title: 'Block With Custom Label' }, + 'name-with-long-label': { title: 'Block With Long Label' }, + 'reusable-block': { title: 'Reusable Block' }, +}; + +const blockLabelMap = { + 'Block With Label': 'Test Label', + 'Block With Long Label': + 'This is a longer label than typical for blocks to have.', + 'Block With Custom Label': 'A Custom Label like a Block Variation Label', +}; + jest.mock( '@wordpress/blocks', () => { return { - getBlockType( name ) { - switch ( name ) { - case 'name-not-exists': - return null; - - case 'name-exists': - return { title: 'Block Title' }; - - case 'name-with-label': - return { title: 'Block With Label' }; - - case 'name-with-custom-label': - return { title: 'Block With Custom Label' }; - - case 'name-with-long-label': - return { title: 'Block With Long Label' }; - } + isReusableBlock( { title } ) { + return title === 'Reusable Block'; }, __experimentalGetBlockLabel( { title } ) { - switch ( title ) { - case 'Block With Label': - return 'Test Label'; - - case 'Block With Long Label': - return 'This is a longer label than typical for blocks to have.'; - - case 'Block With Custom Label': - return 'A Custom Label like a Block Variation Label'; - - default: - return title; - } + return blockLabelMap[ title ] || title; }, }; } ); -jest.mock( '../../use-block-display-information', () => { - const resultsMap = { - 'id-name-exists': { title: 'Block Title' }, - 'id-name-with-label': { title: 'Block With Label' }, - 'id-name-with-long-label': { title: 'Block With Long Label' }, - }; - return jest.fn( ( clientId ) => resultsMap[ clientId ] ); -} ); - -jest.mock( '@wordpress/data/src/components/use-select', () => { - // This allows us to tweak the returned value on each test. - const mock = jest.fn(); - return mock; -} ); +// This allows us to tweak the returned value on each test. +jest.mock( '@wordpress/data/src/components/use-select', () => jest.fn() ); describe( 'BlockTitle', () => { it( 'renders nothing if name is falsey', () => { - useSelect.mockImplementation( () => ( { - name: null, - attributes: null, - } ) ); + useSelect.mockImplementation( () => null ); const { container } = render( ); @@ -79,10 +53,12 @@ describe( 'BlockTitle', () => { } ); it( 'renders nothing if block type does not exist', () => { - useSelect.mockImplementation( () => ( { - name: 'name-not-exists', - attributes: null, - } ) ); + useSelect.mockImplementation( ( mapSelect ) => + mapSelect( () => ( { + getBlockName: () => 'name-not-exists', + getBlockType: () => null, + } ) ) + ); const { container } = render( @@ -92,10 +68,14 @@ describe( 'BlockTitle', () => { } ); it( 'renders title if block type exists', () => { - useSelect.mockImplementation( () => ( { - name: 'name-exists', - attributes: null, - } ) ); + useSelect.mockImplementation( ( mapSelect ) => + mapSelect( () => ( { + getBlockName: () => 'name-exists', + getBlockType: ( name ) => blockTypeMap[ name ], + getBlockAttributes: () => null, + getActiveBlockVariation: () => null, + } ) ) + ); render( ); @@ -103,10 +83,13 @@ describe( 'BlockTitle', () => { } ); it( 'renders label if it is set', () => { - useSelect.mockImplementation( () => ( { - name: 'name-with-label', - attributes: null, - } ) ); + useSelect.mockImplementation( ( mapSelect ) => + mapSelect( () => ( { + getBlockName: () => 'name-with-label', + getBlockType: ( name ) => blockTypeMap[ name ], + getBlockAttributes: () => null, + } ) ) + ); render( ); @@ -114,23 +97,29 @@ describe( 'BlockTitle', () => { } ); it( 'should prioritize reusable block title over title', () => { - useSelect.mockImplementation( () => ( { - name: 'name-with-label', - reusableBlockTitle: 'Reuse me!', - attributes: null, - } ) ); + useSelect.mockImplementation( ( mapSelect ) => + mapSelect( () => ( { + getBlockName: () => 'reusable-block', + getBlockType: ( name ) => blockTypeMap[ name ], + getBlockAttributes: () => ( { ref: 1 } ), + __experimentalGetReusableBlockTitle: () => 'Reuse me!', + } ) ) + ); - render( ); + render( ); expect( screen.queryByText( 'Test Label' ) ).not.toBeInTheDocument(); expect( screen.getByText( 'Reuse me!' ) ).toBeVisible(); } ); it( 'should prioritize block label over title', () => { - useSelect.mockImplementation( () => ( { - name: 'name-with-custom-label', - attributes: null, - } ) ); + useSelect.mockImplementation( ( mapSelect ) => + mapSelect( () => ( { + getBlockName: () => 'name-with-custom-label', + getBlockType: ( name ) => blockTypeMap[ name ], + getBlockAttributes: () => null, + } ) ) + ); render( ); @@ -140,22 +129,31 @@ describe( 'BlockTitle', () => { ).toBeVisible(); } ); - it( 'should default to block information title if no reusable title or block name is available', () => { - useSelect.mockImplementation( () => ( { - name: 'some-rando-name', - attributes: null, - } ) ); + it( 'should default to block variation title if no reusable title or block name is available', () => { + useSelect.mockImplementation( ( mapSelect ) => + mapSelect( () => ( { + getBlockName: () => 'name-exists', + getBlockType: ( name ) => blockTypeMap[ name ], + getBlockAttributes: () => null, + getActiveBlockVariation: () => ( { + title: 'Block Variation Label', + } ), + } ) ) + ); - render( ); + render( ); - expect( screen.getByText( 'Block With Label' ) ).toBeVisible(); + expect( screen.getByText( 'Block Variation Label' ) ).toBeVisible(); } ); it( 'truncates the label with custom truncate length', () => { - useSelect.mockImplementation( () => ( { - name: 'name-with-long-label', - attributes: null, - } ) ); + useSelect.mockImplementation( ( mapSelect ) => + mapSelect( () => ( { + getBlockName: () => 'name-with-long-label', + getBlockType: ( name ) => blockTypeMap[ name ], + getBlockAttributes: () => null, + } ) ) + ); render( { } ); it( 'should not truncate the label if maximum length is undefined', () => { - useSelect.mockImplementation( () => ( { - name: 'name-with-long-label', - attributes: null, - } ) ); + useSelect.mockImplementation( ( mapSelect ) => + mapSelect( () => ( { + getBlockName: () => 'name-with-long-label', + getBlockType: ( name ) => blockTypeMap[ name ], + getBlockAttributes: () => null, + } ) ) + ); render( ); diff --git a/packages/block-editor/src/components/block-title/use-block-display-title.js b/packages/block-editor/src/components/block-title/use-block-display-title.js index be1e26319d671..1e4578630b472 100644 --- a/packages/block-editor/src/components/block-title/use-block-display-title.js +++ b/packages/block-editor/src/components/block-title/use-block-display-title.js @@ -3,15 +3,14 @@ */ import { useSelect } from '@wordpress/data'; import { - getBlockType, - __experimentalGetBlockLabel as getBlockLabel, isReusableBlock, + __experimentalGetBlockLabel as getBlockLabel, + store as blocksStore, } from '@wordpress/blocks'; /** * Internal dependencies */ -import useBlockDisplayInformation from '../use-block-display-information'; import { store as blockEditorStore } from '../../store'; /** @@ -35,49 +34,52 @@ export default function useBlockDisplayTitle( { maximumLength, context, } ) { - const { attributes, name, reusableBlockTitle } = useSelect( + const blockTitle = useSelect( ( select ) => { if ( ! clientId ) { - return {}; + return null; } + const { getBlockName, getBlockAttributes, __experimentalGetReusableBlockTitle, } = select( blockEditorStore ); + const { getBlockType, getActiveBlockVariation } = + select( blocksStore ); + const blockName = getBlockName( clientId ); - if ( ! blockName ) { - return {}; + const blockType = getBlockType( blockName ); + if ( ! blockType ) { + return null; } - const isReusable = isReusableBlock( getBlockType( blockName ) ); - return { - attributes: getBlockAttributes( clientId ), - name: blockName, - reusableBlockTitle: - isReusable && - __experimentalGetReusableBlockTitle( - getBlockAttributes( clientId ).ref - ), - }; + + const attributes = getBlockAttributes( clientId ); + const isReusable = isReusableBlock( blockType ); + const reusableBlockTitle = isReusable + ? __experimentalGetReusableBlockTitle( attributes.ref ) + : null; + + if ( reusableBlockTitle ) { + return reusableBlockTitle; + } + + const label = getBlockLabel( blockType, attributes, context ); + // If the label is defined we prioritize it over a possible block variation title match. + if ( label !== blockType.title ) { + return label; + } + + const match = getActiveBlockVariation( blockName, attributes ); + // Label will fallback to the title if no label is defined for the current label context. + return match?.title || blockType.title; }, - [ clientId ] + [ clientId, context ] ); - const blockInformation = useBlockDisplayInformation( clientId ); - if ( ! name || ! blockInformation ) { + if ( ! blockTitle ) { return null; } - const blockType = getBlockType( name ); - const blockLabel = blockType - ? getBlockLabel( blockType, attributes, context ) - : null; - - const label = reusableBlockTitle || blockLabel; - // Label will fallback to the title if no label is defined for the current - // label context. If the label is defined we prioritize it over a - // possible block variation title match. - const blockTitle = - label && label !== blockType.title ? label : blockInformation.title; if ( maximumLength &&