From 093c453d4f851a0accc3bb87ad3e0e418de622df Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Fri, 11 Dec 2020 14:29:30 +0200 Subject: [PATCH 1/8] Refactor BlockSwitcher - function component and break into more files --- .../block-switcher/block-styles-menu.js | 36 ++ .../src/components/block-switcher/index.js | 352 +++++++----------- .../block-switcher/preview-block-popover.js | 57 +++ .../components/block-switcher/test/index.js | 95 +++-- 4 files changed, 262 insertions(+), 278 deletions(-) create mode 100644 packages/block-editor/src/components/block-switcher/block-styles-menu.js create mode 100644 packages/block-editor/src/components/block-switcher/preview-block-popover.js diff --git a/packages/block-editor/src/components/block-switcher/block-styles-menu.js b/packages/block-editor/src/components/block-switcher/block-styles-menu.js new file mode 100644 index 0000000000000..01d47a66e462f --- /dev/null +++ b/packages/block-editor/src/components/block-switcher/block-styles-menu.js @@ -0,0 +1,36 @@ +/** + * WordPress dependencies + */ +import { __ } from '@wordpress/i18n'; +import { MenuGroup } from '@wordpress/components'; +import { useState } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import BlockStyles from '../block-styles'; +import PreviewBlockPopover from './preview-block-popover'; + +const BlockStylesMenu = ( { hoveredBlock, onSwitch } ) => { + const [ hoveredClassName, setHoveredClassName ] = useState(); + return ( + + { hoveredClassName && ( + + ) } + + + ); +}; +export default BlockStylesMenu; diff --git a/packages/block-editor/src/components/block-switcher/index.js b/packages/block-editor/src/components/block-switcher/index.js index 7ebb59c16cda3..748a0f8e9e072 100644 --- a/packages/block-editor/src/components/block-switcher/index.js +++ b/packages/block-editor/src/components/block-switcher/index.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { castArray, filter, mapKeys, orderBy, uniq, map } from 'lodash'; +import { castArray, filter, mapKeys, orderBy, uniq } from 'lodash'; /** * WordPress dependencies @@ -12,252 +12,154 @@ import { ToolbarButton, ToolbarGroup, ToolbarItem, - MenuGroup, - Popover, } from '@wordpress/components'; import { getBlockType, getPossibleBlockTransformations, switchToBlockType, - cloneBlock, - getBlockFromExample, store as blocksStore, } from '@wordpress/blocks'; -import { Component } from '@wordpress/element'; -import { withSelect, withDispatch } from '@wordpress/data'; -import { compose } from '@wordpress/compose'; +import { useSelect, useDispatch } from '@wordpress/data'; import { stack } from '@wordpress/icons'; /** * Internal dependencies */ import BlockIcon from '../block-icon'; -import BlockStyles from '../block-styles'; -import BlockPreview from '../block-preview'; import BlockTransformationsMenu from './block-transformations-menu'; +import BlockStylesMenu from './block-styles-menu'; -function PreviewBlockPopover( { hoveredBlock, hoveredClassName } ) { - const hoveredBlockType = getBlockType( hoveredBlock.name ); - return ( -
-
- -
-
- { __( 'Preview' ) } -
- -
-
-
-
+const BlockSwitcher = ( { clientIds } ) => { + const { replaceBlocks } = useDispatch( 'core/block-editor' ); + const { blocks, inserterItems, hasBlockStyles } = useSelect( + ( select ) => { + const { + getBlocksByClientId, + getBlockRootClientId, + getInserterItems, + } = select( 'core/block-editor' ); + const { getBlockStyles } = select( blocksStore ); + const rootClientId = getBlockRootClientId( + castArray( clientIds )[ 0 ] + ); + const _blocks = getBlocksByClientId( clientIds ); + const firstBlock = _blocks?.length === 1 ? _blocks[ 0 ] : null; + const styles = firstBlock && getBlockStyles( firstBlock.name ); + return { + blocks: _blocks, + inserterItems: getInserterItems( rootClientId ), + hasBlockStyles: !! styles?.length, + }; + }, + [ clientIds ] ); -} - -export class BlockSwitcher extends Component { - constructor() { - super( ...arguments ); - this.state = { - hoveredClassName: null, - }; - this.onHoverClassName = this.onHoverClassName.bind( this ); - } - - onHoverClassName( className ) { - this.setState( { hoveredClassName: className } ); - } - - render() { - const { - blocks, - onTransform, - inserterItems, - hasBlockStyles, - } = this.props; - const { hoveredClassName } = this.state; - - if ( ! Array.isArray( blocks ) || ! blocks.length ) { - return null; - } - - const [ hoveredBlock ] = blocks; - const itemsByName = mapKeys( inserterItems, ( { name } ) => name ); - const possibleBlockTransformations = orderBy( - filter( - getPossibleBlockTransformations( blocks ), - ( block ) => block && !! itemsByName[ block.name ] - ), - ( block ) => itemsByName[ block.name ].frecency, - 'desc' - ); - - // When selection consists of blocks of multiple types, display an - // appropriate icon to communicate the non-uniformity. - const isSelectionOfSameType = - uniq( map( blocks, 'name' ) ).length === 1; - - let icon; - if ( isSelectionOfSameType ) { - const sourceBlockName = hoveredBlock.name; - const blockType = getBlockType( sourceBlockName ); - icon = blockType.icon; - } else { - icon = stack; - } - const hasPossibleBlockTransformations = !! possibleBlockTransformations.length; + if ( ! blocks?.length ) return null; - if ( ! hasBlockStyles && ! hasPossibleBlockTransformations ) { - return ( - - } - /> - - ); - } - - const blockSwitcherLabel = - 1 === blocks.length - ? __( 'Change block type or style' ) - : sprintf( - /* translators: %s: number of blocks. */ - _n( - 'Change type of %d block', - 'Change type of %d blocks', - blocks.length - ), - blocks.length - ); + const onTransform = ( name ) => + replaceBlocks( clientIds, switchToBlockType( blocks, name ) ); + const [ hoveredBlock ] = blocks; + // When selection consists of blocks of multiple types, display an + // appropriate icon to communicate the non-uniformity. + const isSelectionOfSameType = + uniq( blocks.map( ( { name } ) => name ) ).length === 1; + let icon; + if ( isSelectionOfSameType ) { + const sourceBlockName = hoveredBlock.name; + const blockType = getBlockType( sourceBlockName ); + icon = blockType.icon; + } else { + icon = stack; + } + const itemsByName = mapKeys( inserterItems, ( { name } ) => name ); + const possibleBlockTransformations = orderBy( + filter( + getPossibleBlockTransformations( blocks ), + ( block ) => block && !! itemsByName[ block.name ] + ), + ( block ) => itemsByName[ block.name ].frecency, + 'desc' + ); + const hasPossibleBlockTransformations = !! possibleBlockTransformations.length; + if ( ! hasBlockStyles && ! hasPossibleBlockTransformations ) { return ( - - { ( toggleProps ) => ( - - } - toggleProps={ toggleProps } - menuProps={ { orientation: 'both' } } - > - { ( { onClose } ) => - ( hasBlockStyles || - hasPossibleBlockTransformations ) && ( -
- { hasPossibleBlockTransformations && ( - { - onTransform( blocks, name ); - onClose(); - } } - /> - ) } - { hasBlockStyles && ( - - { hoveredClassName !== null && ( - - ) } - - - ) } -
- ) - } -
- ) } -
+ } + />
); } -} -export default compose( - withSelect( ( select, { clientIds } ) => { - const { - getBlocksByClientId, - getBlockRootClientId, - getInserterItems, - } = select( 'core/block-editor' ); - const { getBlockStyles } = select( blocksStore ); - const rootClientId = getBlockRootClientId( - castArray( clientIds )[ 0 ] - ); - const blocks = getBlocksByClientId( clientIds ); - const firstBlock = blocks && blocks.length === 1 ? blocks[ 0 ] : null; - const styles = firstBlock && getBlockStyles( firstBlock.name ); - return { - blocks, - inserterItems: getInserterItems( rootClientId ), - hasBlockStyles: styles && styles.length > 0, - }; - } ), - withDispatch( ( dispatch, ownProps ) => ( { - onTransform( blocks, name ) { - dispatch( 'core/block-editor' ).replaceBlocks( - ownProps.clientIds, - switchToBlockType( blocks, name ) - ); - }, - } ) ) -)( BlockSwitcher ); + const blockSwitcherLabel = + 1 === blocks.length + ? __( 'Change block type or style' ) + : sprintf( + /* translators: %s: number of blocks. */ + _n( + 'Change type of %d block', + 'Change type of %d blocks', + blocks.length + ), + blocks.length + ); + + return ( + + + { ( toggleProps ) => ( + + } + toggleProps={ toggleProps } + menuProps={ { orientation: 'both' } } + > + { ( { onClose } ) => + ( hasBlockStyles || + hasPossibleBlockTransformations ) && ( +
+ { hasPossibleBlockTransformations && ( + { + onTransform( name ); + onClose(); + } } + /> + ) } + { hasBlockStyles && ( + + ) } +
+ ) + } +
+ ) } +
+
+ ); +}; + +export default BlockSwitcher; diff --git a/packages/block-editor/src/components/block-switcher/preview-block-popover.js b/packages/block-editor/src/components/block-switcher/preview-block-popover.js new file mode 100644 index 0000000000000..4e8e0cd0b6684 --- /dev/null +++ b/packages/block-editor/src/components/block-switcher/preview-block-popover.js @@ -0,0 +1,57 @@ +/** + * WordPress dependencies + */ +import { __ } from '@wordpress/i18n'; +import { Popover } from '@wordpress/components'; +import { + getBlockType, + cloneBlock, + getBlockFromExample, +} from '@wordpress/blocks'; +/** + * Internal dependencies + */ +import BlockPreview from '../block-preview'; + +export default function PreviewBlockPopover( { + hoveredBlock, + hoveredClassName, +} ) { + const hoveredBlockType = getBlockType( hoveredBlock.name ); + return ( +
+
+ +
+
+ { __( 'Preview' ) } +
+ +
+
+
+
+ ); +} diff --git a/packages/block-editor/src/components/block-switcher/test/index.js b/packages/block-editor/src/components/block-switcher/test/index.js index a5879995c271d..b1cd0d3374764 100644 --- a/packages/block-editor/src/components/block-switcher/test/index.js +++ b/packages/block-editor/src/components/block-switcher/test/index.js @@ -6,6 +6,7 @@ import { shallow, mount } from 'enzyme'; /** * WordPress dependencies */ +import { useSelect } from '@wordpress/data'; import { registerBlockType, unregisterBlockType } from '@wordpress/blocks'; import { DOWN } from '@wordpress/keycodes'; import { Button } from '@wordpress/components'; @@ -13,7 +14,9 @@ import { Button } from '@wordpress/components'; /** * Internal dependencies */ -import { BlockSwitcher } from '../'; +import BlockSwitcher from '../'; + +jest.mock( '@wordpress/data/src/components/use-select', () => jest.fn() ); describe( 'BlockSwitcher', () => { const headingBlock1 = { @@ -94,75 +97,61 @@ describe( 'BlockSwitcher', () => { } ); test( 'should not render block switcher without blocks', () => { + useSelect.mockImplementation( () => ( {} ) ); const wrapper = shallow( ); - expect( wrapper.html() ).toBeNull(); } ); test( 'should render switcher with blocks', () => { - const blocks = [ headingBlock1 ]; - const inserterItems = [ - { name: 'core/heading', frecency: 1 }, - { name: 'core/paragraph', frecency: 1 }, - ]; - - const wrapper = shallow( - - ); - + useSelect.mockImplementation( () => ( { + blocks: [ headingBlock1 ], + inserterItems: [ + { name: 'core/heading', frecency: 1 }, + { name: 'core/paragraph', frecency: 1 }, + ], + } ) ); + const wrapper = shallow( ); expect( wrapper ).toMatchSnapshot(); } ); test( 'should render disabled block switcher with multi block of different types when no transforms', () => { - const blocks = [ headingBlock1, textBlock ]; - const inserterItems = [ - { name: 'core/heading', frecency: 1 }, - { name: 'core/paragraph', frecency: 1 }, - ]; - - const wrapper = shallow( - - ); - + useSelect.mockImplementation( () => ( { + blocks: [ headingBlock1, textBlock ], + inserterItems: [ + { name: 'core/heading', frecency: 1 }, + { name: 'core/paragraph', frecency: 1 }, + ], + } ) ); + const wrapper = shallow( ); expect( wrapper ).toMatchSnapshot(); } ); test( 'should render enabled block switcher with multi block when transforms exist', () => { - const blocks = [ headingBlock1, headingBlock2 ]; - const inserterItems = [ - { name: 'core/heading', frecency: 1 }, - { name: 'core/paragraph', frecency: 1 }, - ]; - - const wrapper = shallow( - - ); - + useSelect.mockImplementation( () => ( { + blocks: [ headingBlock1, headingBlock2 ], + inserterItems: [ + { name: 'core/heading', frecency: 1 }, + { name: 'core/paragraph', frecency: 1 }, + ], + } ) ); + const wrapper = shallow( ); expect( wrapper ).toMatchSnapshot(); } ); describe( 'Dropdown', () => { - const blocks = [ headingBlock1 ]; - - const inserterItems = [ - { name: 'core/quote', frecency: 1 }, - { name: 'core/cover-image', frecency: 2 }, - { name: 'core/paragraph', frecency: 3 }, - { name: 'core/heading', frecency: 4 }, - { name: 'core/text', frecency: 5 }, - ]; - - const onTransformStub = jest.fn(); - const getDropdown = () => { - const blockSwitcher = mount( - - ); - return blockSwitcher.find( 'Dropdown' ); - }; + beforeAll( () => { + useSelect.mockImplementation( () => ( { + blocks: [ headingBlock1 ], + inserterItems: [ + { name: 'core/quote', frecency: 1 }, + { name: 'core/cover-image', frecency: 2 }, + { name: 'core/paragraph', frecency: 3 }, + { name: 'core/heading', frecency: 4 }, + { name: 'core/text', frecency: 5 }, + ], + } ) ); + } ); + const getDropdown = () => mount( ).find( 'Dropdown' ); test( 'should dropdown exist', () => { expect( getDropdown() ).toHaveLength( 1 ); From 3691602f98a608c7aaec8ce231d17c667604f105 Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Fri, 11 Dec 2020 17:23:14 +0200 Subject: [PATCH 2/8] new selector for block transforms --- .../developers/data/data-core-block-editor.md | 4 + .../block-transformations-menu.js | 3 +- .../src/components/block-switcher/index.js | 19 +- packages/block-editor/src/store/selectors.js | 176 ++++++++++++------ 4 files changed, 129 insertions(+), 73 deletions(-) diff --git a/docs/designers-developers/developers/data/data-core-block-editor.md b/docs/designers-developers/developers/data/data-core-block-editor.md index 3ab9d0bd1cd3e..ba1275aa6f2c4 100644 --- a/docs/designers-developers/developers/data/data-core-block-editor.md +++ b/docs/designers-developers/developers/data/data-core-block-editor.md @@ -344,6 +344,10 @@ _Returns_ - `?string`: Client ID of block selection start. +# **getBlockTransformItems** + +Undocumented declaration. + # **getClientIdsOfDescendants** Returns an array containing the clientIds of all descendants diff --git a/packages/block-editor/src/components/block-switcher/block-transformations-menu.js b/packages/block-editor/src/components/block-switcher/block-transformations-menu.js index fb70c053dac78..e8e4566112b4a 100644 --- a/packages/block-editor/src/components/block-switcher/block-transformations-menu.js +++ b/packages/block-editor/src/components/block-switcher/block-transformations-menu.js @@ -18,7 +18,7 @@ const BlockTransformationsMenu = ( { return ( { possibleBlockTransformations.map( ( item ) => { - const { name, icon, title } = item; + const { name, icon, title, isDisabled } = item; return ( { title } diff --git a/packages/block-editor/src/components/block-switcher/index.js b/packages/block-editor/src/components/block-switcher/index.js index 748a0f8e9e072..783dda2cb396f 100644 --- a/packages/block-editor/src/components/block-switcher/index.js +++ b/packages/block-editor/src/components/block-switcher/index.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { castArray, filter, mapKeys, orderBy, uniq } from 'lodash'; +import { castArray, uniq } from 'lodash'; /** * WordPress dependencies @@ -15,7 +15,6 @@ import { } from '@wordpress/components'; import { getBlockType, - getPossibleBlockTransformations, switchToBlockType, store as blocksStore, } from '@wordpress/blocks'; @@ -31,12 +30,12 @@ import BlockStylesMenu from './block-styles-menu'; const BlockSwitcher = ( { clientIds } ) => { const { replaceBlocks } = useDispatch( 'core/block-editor' ); - const { blocks, inserterItems, hasBlockStyles } = useSelect( + const { blocks, possibleBlockTransformations, hasBlockStyles } = useSelect( ( select ) => { const { getBlocksByClientId, getBlockRootClientId, - getInserterItems, + getBlockTransformItems, } = select( 'core/block-editor' ); const { getBlockStyles } = select( blocksStore ); const rootClientId = getBlockRootClientId( @@ -47,7 +46,8 @@ const BlockSwitcher = ( { clientIds } ) => { const styles = firstBlock && getBlockStyles( firstBlock.name ); return { blocks: _blocks, - inserterItems: getInserterItems( rootClientId ), + possibleBlockTransformations: + _blocks && getBlockTransformItems( _blocks, rootClientId ), hasBlockStyles: !! styles?.length, }; }, @@ -72,15 +72,6 @@ const BlockSwitcher = ( { clientIds } ) => { } else { icon = stack; } - const itemsByName = mapKeys( inserterItems, ( { name } ) => name ); - const possibleBlockTransformations = orderBy( - filter( - getPossibleBlockTransformations( blocks ), - ( block ) => block && !! itemsByName[ block.name ] - ), - ( block ) => itemsByName[ block.name ].frecency, - 'desc' - ); const hasPossibleBlockTransformations = !! possibleBlockTransformations.length; if ( ! hasBlockStyles && ! hasPossibleBlockTransformations ) { return ( diff --git a/packages/block-editor/src/store/selectors.js b/packages/block-editor/src/store/selectors.js index c2153ee33cb4b..88c1046e8d6d7 100644 --- a/packages/block-editor/src/store/selectors.js +++ b/packages/block-editor/src/store/selectors.js @@ -13,6 +13,8 @@ import { some, find, filter, + mapKeys, + orderBy, } from 'lodash'; import createSelector from 'rememo'; @@ -23,6 +25,7 @@ import { getBlockType, getBlockTypes, hasBlockSupport, + getPossibleBlockTransformations, parse, } from '@wordpress/blocks'; import { SVG, Rect, G, Path } from '@wordpress/components'; @@ -1400,6 +1403,75 @@ const getItemFromVariation = ( item ) => ( variation ) => ( { keywords: variation.keywords || item.keywords, } ); +/** + * The 'frecency' property is a heuristic (https://en.wikipedia.org/wiki/Frecency) + * that combines block usage frequenty and recency. + * + * @param {*} time + * @param {*} count + */ +// TODO jsdoc +const calculateFrecency = ( time, count ) => { + if ( ! time ) { + return count; + } + + // The selector is cached, which means Date.now() is the last time that the + // relevant state changed. This suits our needs. + const duration = Date.now() - time; + + switch ( true ) { + case duration < MILLISECONDS_PER_HOUR: + return count * 4; + case duration < MILLISECONDS_PER_DAY: + return count * 2; + case duration < MILLISECONDS_PER_WEEK: + return count / 2; + default: + return count / 4; + } +}; + +// TODO jsdoc +const buildBlockTypeItem = ( state, { buildScope = 'inserter' } ) => ( + blockType +) => { + const id = blockType.name; + + let isDisabled = false; + if ( ! hasBlockSupport( blockType.name, 'multiple', true ) ) { + isDisabled = some( + getBlocksByClientId( state, getClientIdsWithDescendants( state ) ), + { name: blockType.name } + ); + } + + const { time, count = 0 } = getInsertUsage( state, id ) || {}; + const blockItemBase = { + id, + name: blockType.name, + title: blockType.title, + icon: blockType.icon, + isDisabled, + frecency: calculateFrecency( time, count ), + }; + if ( buildScope === 'transform' ) return blockItemBase; + + const inserterVariations = blockType.variations.filter( + ( { scope } ) => ! scope || scope.includes( 'inserter' ) + ); + return { + ...blockItemBase, + initialAttributes: {}, + description: blockType.description, + category: blockType.category, + keywords: blockType.keywords, + variations: inserterVariations, + example: blockType.example, + utility: 1, // deprecated + }; +}; + /** * Determines the items that appear in the inserter. Includes both static * items (e.g. a regular block type) and dynamic items (e.g. a reusable block). @@ -1431,64 +1503,9 @@ const getItemFromVariation = ( item ) => ( variation ) => ( { */ export const getInserterItems = createSelector( ( state, rootClientId = null ) => { - const calculateFrecency = ( time, count ) => { - if ( ! time ) { - return count; - } - - // The selector is cached, which means Date.now() is the last time that the - // relevant state changed. This suits our needs. - const duration = Date.now() - time; - - switch ( true ) { - case duration < MILLISECONDS_PER_HOUR: - return count * 4; - case duration < MILLISECONDS_PER_DAY: - return count * 2; - case duration < MILLISECONDS_PER_WEEK: - return count / 2; - default: - return count / 4; - } - }; - - const buildBlockTypeInserterItem = ( blockType ) => { - const id = blockType.name; - - let isDisabled = false; - if ( ! hasBlockSupport( blockType.name, 'multiple', true ) ) { - isDisabled = some( - getBlocksByClientId( - state, - getClientIdsWithDescendants( state ) - ), - { - name: blockType.name, - } - ); - } - - const { time, count = 0 } = getInsertUsage( state, id ) || {}; - const inserterVariations = blockType.variations.filter( - ( { scope } ) => ! scope || scope.includes( 'inserter' ) - ); - - return { - id, - name: blockType.name, - initialAttributes: {}, - title: blockType.title, - description: blockType.description, - icon: blockType.icon, - category: blockType.category, - keywords: blockType.keywords, - variations: inserterVariations, - example: blockType.example, - isDisabled, - utility: 1, // deprecated - frecency: calculateFrecency( time, count ), - }; - }; + const buildBlockTypeInserterItem = buildBlockTypeItem( state, { + buildScope: 'inserter', + } ); const buildReusableBlockInserterItem = ( reusableBlock ) => { const id = `core/block/${ reusableBlock.id }`; @@ -1572,6 +1589,49 @@ export const getInserterItems = createSelector( ] ); +// TODO jsdoc test etc.. +export const getBlockTransformItems = createSelector( + ( state, blocks, rootClientId = null ) => { + const buildBlockTypeTransformItem = buildBlockTypeItem( state, { + buildScope: 'transform', + } ); + + const blockTypeTransformItems = getBlockTypes() + .filter( ( blockType ) => + canIncludeBlockTypeInInserter( state, blockType, rootClientId ) + ) + .map( buildBlockTypeTransformItem ); + + const itemsByName = mapKeys( + blockTypeTransformItems, + ( { name } ) => name + ); + const possibleTransforms = getPossibleBlockTransformations( + blocks + ).reduce( ( accumulator, block ) => { + if ( itemsByName[ block?.name ] ) { + accumulator.push( itemsByName[ block.name ] ); + } + return accumulator; + }, [] ); + const possibleBlockTransformations = orderBy( + possibleTransforms, + ( block ) => itemsByName[ block.name ].frecency, + 'desc' + ); + return possibleBlockTransformations; + }, + ( state, rootClientId ) => [ + state.blockListSettings[ rootClientId ], + state.blocks.byClientId, + state.blocks.order, + state.preferences.insertUsage, + state.settings.allowedBlockTypes, + state.settings.templateLock, + getBlockTypes(), + ] +); + /** * Determines whether there are items to show in the inserter. * From 046e01318f4cdccd701101b950eb54e8e65e805c Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Sat, 12 Dec 2020 13:04:25 +0200 Subject: [PATCH 3/8] jsdoc selectors --- .../developers/data/data-core-block-editor.md | 32 +++++++++++- packages/block-editor/src/store/selectors.js | 52 +++++++++++++++---- 2 files changed, 73 insertions(+), 11 deletions(-) diff --git a/docs/designers-developers/developers/data/data-core-block-editor.md b/docs/designers-developers/developers/data/data-core-block-editor.md index ba1275aa6f2c4..ded41c1a63dfc 100644 --- a/docs/designers-developers/developers/data/data-core-block-editor.md +++ b/docs/designers-developers/developers/data/data-core-block-editor.md @@ -346,7 +346,37 @@ _Returns_ # **getBlockTransformItems** -Undocumented declaration. +Determines the items that appear in the available block transforms list. + +Each item object contains what's necessary to display a menu item in the +transform list and handle its selection. + +The 'frecency' property is a heuristic () +that combines block usage frequenty and recency. + +Items are returned ordered descendingly by their 'frecency'. + +_Parameters_ + +- _state_ `Object`: Editor state. +- _rootClientId_ `?string`: Optional root client ID of block list. + +_Returns_ + +- `Array`: Items that appear in inserter. + +_Type Definition_ + +- _WPEditorTransformItem_ `Object` + +_Properties_ + +- _id_ `string`: Unique identifier for the item. +- _name_ `string`: The type of block to create. +- _title_ `string`: Title of the item, as it appears in the inserter. +- _icon_ `string`: Dashicon for the item, as it appears in the inserter. +- _isDisabled_ `boolean`: Whether or not the user should be prevented from inserting this item. +- _frecency_ `number`: Heuristic that combines frequency and recency. # **getClientIdsOfDescendants** diff --git a/packages/block-editor/src/store/selectors.js b/packages/block-editor/src/store/selectors.js index 88c1046e8d6d7..764f8350b986d 100644 --- a/packages/block-editor/src/store/selectors.js +++ b/packages/block-editor/src/store/selectors.js @@ -1404,22 +1404,23 @@ const getItemFromVariation = ( item ) => ( variation ) => ( { } ); /** - * The 'frecency' property is a heuristic (https://en.wikipedia.org/wiki/Frecency) + * Returns the calculated frecency. + * + * 'frecency' is a heuristic (https://en.wikipedia.org/wiki/Frecency) * that combines block usage frequenty and recency. * - * @param {*} time - * @param {*} count + * @param {number} time When the last insert occurred as a UNIX epoch + * @param {number} count The number of inserts that have occurred. + * + * @return {number} The calculated frecency. */ -// TODO jsdoc const calculateFrecency = ( time, count ) => { if ( ! time ) { return count; } - // The selector is cached, which means Date.now() is the last time that the // relevant state changed. This suits our needs. const duration = Date.now() - time; - switch ( true ) { case duration < MILLISECONDS_PER_HOUR: return count * 4; @@ -1432,7 +1433,16 @@ const calculateFrecency = ( time, count ) => { } }; -// TODO jsdoc +/** + * Returns a function that accepts a block type and builds an item to be shown + * in a specific context. It's used for building items for Inserter and available + * block Transfroms list. + * + * @param {Object} state Editor state. + * @param {Object} options Options object for handling the building of a block type. + * @param {string} options.buildScope The scope for which the item is going to be used. + * @return {Function} Function returns an item to be shown in a specific context (Inserter|Transforms list). + */ const buildBlockTypeItem = ( state, { buildScope = 'inserter' } ) => ( blockType ) => { @@ -1589,13 +1599,36 @@ export const getInserterItems = createSelector( ] ); -// TODO jsdoc test etc.. +/** + * Determines the items that appear in the available block transforms list. + * + * Each item object contains what's necessary to display a menu item in the + * transform list and handle its selection. + * + * The 'frecency' property is a heuristic (https://en.wikipedia.org/wiki/Frecency) + * that combines block usage frequenty and recency. + * + * Items are returned ordered descendingly by their 'frecency'. + * + * @param {Object} state Editor state. + * @param {?string} rootClientId Optional root client ID of block list. + * + * @return {WPEditorTransformItem[]} Items that appear in inserter. + * + * @typedef {Object} WPEditorTransformItem + * @property {string} id Unique identifier for the item. + * @property {string} name The type of block to create. + * @property {string} title Title of the item, as it appears in the inserter. + * @property {string} icon Dashicon for the item, as it appears in the inserter. + * @property {boolean} isDisabled Whether or not the user should be prevented from inserting + * this item. + * @property {number} frecency Heuristic that combines frequency and recency. + */ export const getBlockTransformItems = createSelector( ( state, blocks, rootClientId = null ) => { const buildBlockTypeTransformItem = buildBlockTypeItem( state, { buildScope: 'transform', } ); - const blockTypeTransformItems = getBlockTypes() .filter( ( blockType ) => canIncludeBlockTypeInInserter( state, blockType, rootClientId ) @@ -1624,7 +1657,6 @@ export const getBlockTransformItems = createSelector( ( state, rootClientId ) => [ state.blockListSettings[ rootClientId ], state.blocks.byClientId, - state.blocks.order, state.preferences.insertUsage, state.settings.allowedBlockTypes, state.settings.templateLock, From 1de63d133e4fd66dffa4324d0c49afc92d904ece Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Sat, 12 Dec 2020 13:22:16 +0200 Subject: [PATCH 4/8] fix BlockSwitcher tests --- .../src/components/block-switcher/test/index.js | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/packages/block-editor/src/components/block-switcher/test/index.js b/packages/block-editor/src/components/block-switcher/test/index.js index b1cd0d3374764..40a1e28bc8119 100644 --- a/packages/block-editor/src/components/block-switcher/test/index.js +++ b/packages/block-editor/src/components/block-switcher/test/index.js @@ -105,7 +105,7 @@ describe( 'BlockSwitcher', () => { test( 'should render switcher with blocks', () => { useSelect.mockImplementation( () => ( { blocks: [ headingBlock1 ], - inserterItems: [ + possibleBlockTransformations: [ { name: 'core/heading', frecency: 1 }, { name: 'core/paragraph', frecency: 1 }, ], @@ -117,10 +117,7 @@ describe( 'BlockSwitcher', () => { test( 'should render disabled block switcher with multi block of different types when no transforms', () => { useSelect.mockImplementation( () => ( { blocks: [ headingBlock1, textBlock ], - inserterItems: [ - { name: 'core/heading', frecency: 1 }, - { name: 'core/paragraph', frecency: 1 }, - ], + possibleBlockTransformations: [], } ) ); const wrapper = shallow( ); expect( wrapper ).toMatchSnapshot(); @@ -129,7 +126,7 @@ describe( 'BlockSwitcher', () => { test( 'should render enabled block switcher with multi block when transforms exist', () => { useSelect.mockImplementation( () => ( { blocks: [ headingBlock1, headingBlock2 ], - inserterItems: [ + possibleBlockTransformations: [ { name: 'core/heading', frecency: 1 }, { name: 'core/paragraph', frecency: 1 }, ], @@ -142,12 +139,8 @@ describe( 'BlockSwitcher', () => { beforeAll( () => { useSelect.mockImplementation( () => ( { blocks: [ headingBlock1 ], - inserterItems: [ - { name: 'core/quote', frecency: 1 }, - { name: 'core/cover-image', frecency: 2 }, + possibleBlockTransformations: [ { name: 'core/paragraph', frecency: 3 }, - { name: 'core/heading', frecency: 4 }, - { name: 'core/text', frecency: 5 }, ], } ) ); } ); From 086b72ba54529ed28293578461ea9f9dde148205 Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Mon, 14 Dec 2020 15:02:18 +0200 Subject: [PATCH 5/8] getBlockTransformItems tests --- .../block-editor/src/store/test/selectors.js | 217 ++++++++++++++++++ 1 file changed, 217 insertions(+) diff --git a/packages/block-editor/src/store/test/selectors.js b/packages/block-editor/src/store/test/selectors.js index d11e53a9e7485..a0803a0e5b2c2 100644 --- a/packages/block-editor/src/store/test/selectors.js +++ b/packages/block-editor/src/store/test/selectors.js @@ -61,6 +61,7 @@ const { canInsertBlockType, canInsertBlocks, getInserterItems, + getBlockTransformItems, isValidTemplate, getTemplate, getTemplateLock, @@ -2675,6 +2676,222 @@ describe( 'selectors', () => { } ); } ); + describe( 'getBlockTransformItems', () => { + beforeAll( () => { + registerBlockType( 'core/with-tranforms-a', { + category: 'text', + title: 'Tranforms a', + edit: () => {}, + save: () => {}, + transforms: { + to: [ + { + type: 'block', + blocks: [ 'core/with-tranforms-b' ], + transform: () => {}, + }, + { + type: 'block', + blocks: [ 'core/with-tranforms-c' ], + transform: () => {}, + }, + { + type: 'block', + blocks: [ + 'core/with-tranforms-b', + 'core/with-tranforms-c', + ], + transform: () => {}, + isMultiBlock: true, + }, + ], + }, + } ); + registerBlockType( 'core/with-tranforms-b', { + category: 'text', + title: 'Tranforms b', + edit: () => {}, + save: () => {}, + transforms: { + to: [ + { + type: 'block', + blocks: [ 'core/with-tranforms-a' ], + transform: () => {}, + }, + ], + }, + } ); + registerBlockType( 'core/with-tranforms-c', { + category: 'text', + title: 'Tranforms c', + edit: () => {}, + save: () => {}, + transforms: { + to: [ + { + type: 'block', + blocks: [ 'core/with-tranforms-a' ], + transform: () => {}, + }, + ], + }, + supports: { multiple: false }, + } ); + } ); + afterAll( () => { + [ + 'core/with-tranforms-a', + 'core/with-tranforms-b', + 'core/with-tranforms-c', + ].forEach( unregisterBlockType ); + } ); + it( 'should properly return block type items', () => { + const state = { + blocks: { + byClientId: {}, + attributes: {}, + order: {}, + parents: {}, + cache: {}, + }, + settings: {}, + preferences: {}, + blockListSettings: {}, + }; + const blocks = [ { name: 'core/with-tranforms-a' } ]; + const items = getBlockTransformItems( state, blocks ); + expect( items ).toHaveLength( 2 ); + const returnedProps = Object.keys( items[ 0 ] ); + // Verify we have only the wanted props. + expect( returnedProps ).toHaveLength( 6 ); + expect( returnedProps ).toEqual( + expect.arrayContaining( [ + 'id', + 'name', + 'title', + 'icon', + 'frecency', + 'isDisabled', + ] ) + ); + expect( items ).toEqual( + expect.arrayContaining( [ + expect.objectContaining( { + name: 'core/with-tranforms-b', + } ), + expect.objectContaining( { + name: 'core/with-tranforms-c', + } ), + ] ) + ); + } ); + it( 'should return only eligible blocks for transformation - `allowedBlocks`', () => { + const state = { + blocks: { + byClientId: { + block1: { name: 'core/with-tranforms-b' }, + block2: { name: 'core/with-tranforms-a' }, + }, + attributes: { + block1: {}, + block2: {}, + }, + order: {}, + parents: { + block1: '', + block2: 'block1', + }, + cache: {}, + controlledInnerBlocks: {}, + }, + settings: {}, + preferences: {}, + blockListSettings: { + block1: { + allowedBlocks: [ 'core/with-tranforms-c' ], + }, + block2: {}, + }, + }; + const blocks = [ + { clientId: 'block2', name: 'core/with-tranforms-a' }, + ]; + const items = getBlockTransformItems( state, blocks, 'block1' ); + expect( items ).toHaveLength( 1 ); + expect( items[ 0 ].name ).toEqual( 'core/with-tranforms-c' ); + } ); + it( 'should take into account the usage of blocks settings `multiple` - if multiple blocks of the same type are allowed', () => { + const state = { + blocks: { + byClientId: { + block1: { + clientId: 'block1', + name: 'core/with-tranforms-c', + }, + }, + attributes: { + block1: { attribute: {} }, + }, + order: { + '': [ 'block1' ], + }, + cache: { + block1: {}, + }, + controlledInnerBlocks: {}, + }, + preferences: { + insertUsage: {}, + }, + blockListSettings: {}, + settings: {}, + }; + const blocks = [ { name: 'core/with-tranforms-a' } ]; + const items = getBlockTransformItems( state, blocks ); + expect( items ).toHaveLength( 2 ); + expect( items ).toEqual( + expect.arrayContaining( [ + expect.objectContaining( { + name: 'core/with-tranforms-b', + isDisabled: false, + } ), + expect.objectContaining( { + name: 'core/with-tranforms-c', + isDisabled: true, + } ), + ] ) + ); + } ); + it( 'should set frecency', () => { + const state = { + blocks: { + byClientId: {}, + attributes: {}, + order: {}, + parents: {}, + cache: {}, + }, + preferences: { + insertUsage: { + 'core/with-tranforms-a': { count: 10, time: 1000 }, + }, + }, + blockListSettings: {}, + settings: {}, + }; + const blocks = [ { name: 'core/with-tranforms-c' } ]; + const items = getBlockTransformItems( state, blocks ); + expect( items ).toHaveLength( 1 ); + expect( items[ 0 ] ).toEqual( + expect.objectContaining( { + name: 'core/with-tranforms-a', + frecency: 2.5, + } ) + ); + } ); + } ); + describe( 'isValidTemplate', () => { it( 'should return true if template is valid', () => { const state = { From 82e719e8dcfa9336d1061d74afc3035213b8d207 Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Thu, 17 Dec 2020 11:23:30 +0200 Subject: [PATCH 6/8] make BlockStylesMenu named function --- .../src/components/block-switcher/block-styles-menu.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/block-editor/src/components/block-switcher/block-styles-menu.js b/packages/block-editor/src/components/block-switcher/block-styles-menu.js index 01d47a66e462f..e600f062423df 100644 --- a/packages/block-editor/src/components/block-switcher/block-styles-menu.js +++ b/packages/block-editor/src/components/block-switcher/block-styles-menu.js @@ -11,7 +11,7 @@ import { useState } from '@wordpress/element'; import BlockStyles from '../block-styles'; import PreviewBlockPopover from './preview-block-popover'; -const BlockStylesMenu = ( { hoveredBlock, onSwitch } ) => { +export default function BlockStylesMenu( { hoveredBlock, onSwitch } ) { const [ hoveredClassName, setHoveredClassName ] = useState(); return ( { /> ); -}; -export default BlockStylesMenu; +} From 0a68ebd5b441426c514ef0b000fe070a3beb9c0a Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Thu, 17 Dec 2020 12:00:18 +0200 Subject: [PATCH 7/8] move icon logic in `useSelect` --- .../src/components/block-switcher/index.js | 44 +++++++++---------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/packages/block-editor/src/components/block-switcher/index.js b/packages/block-editor/src/components/block-switcher/index.js index 783dda2cb396f..b789dd7348c14 100644 --- a/packages/block-editor/src/components/block-switcher/index.js +++ b/packages/block-editor/src/components/block-switcher/index.js @@ -13,11 +13,7 @@ import { ToolbarGroup, ToolbarItem, } from '@wordpress/components'; -import { - getBlockType, - switchToBlockType, - store as blocksStore, -} from '@wordpress/blocks'; +import { switchToBlockType, store as blocksStore } from '@wordpress/blocks'; import { useSelect, useDispatch } from '@wordpress/data'; import { stack } from '@wordpress/icons'; @@ -30,25 +26,40 @@ import BlockStylesMenu from './block-styles-menu'; const BlockSwitcher = ( { clientIds } ) => { const { replaceBlocks } = useDispatch( 'core/block-editor' ); - const { blocks, possibleBlockTransformations, hasBlockStyles } = useSelect( + const { + blocks, + possibleBlockTransformations, + hasBlockStyles, + icon, + } = useSelect( ( select ) => { const { getBlocksByClientId, getBlockRootClientId, getBlockTransformItems, } = select( 'core/block-editor' ); - const { getBlockStyles } = select( blocksStore ); + const { getBlockStyles, getBlockType } = select( blocksStore ); const rootClientId = getBlockRootClientId( castArray( clientIds )[ 0 ] ); const _blocks = getBlocksByClientId( clientIds ); - const firstBlock = _blocks?.length === 1 ? _blocks[ 0 ] : null; - const styles = firstBlock && getBlockStyles( firstBlock.name ); + const isSingleBlockSelected = _blocks?.length === 1; + const firstBlock = !! _blocks?.length ? _blocks[ 0 ] : null; + const styles = + isSingleBlockSelected && getBlockStyles( firstBlock.name ); + // When selection consists of blocks of multiple types, display an + // appropriate icon to communicate the non-uniformity. + const isSelectionOfSameType = + uniq( ( _blocks || [] ).map( ( { name } ) => name ) ).length === + 1; return { blocks: _blocks, possibleBlockTransformations: _blocks && getBlockTransformItems( _blocks, rootClientId ), hasBlockStyles: !! styles?.length, + icon: isSelectionOfSameType + ? getBlockType( firstBlock.name )?.icon + : stack, }; }, [ clientIds ] @@ -59,19 +70,6 @@ const BlockSwitcher = ( { clientIds } ) => { const onTransform = ( name ) => replaceBlocks( clientIds, switchToBlockType( blocks, name ) ); - const [ hoveredBlock ] = blocks; - // When selection consists of blocks of multiple types, display an - // appropriate icon to communicate the non-uniformity. - const isSelectionOfSameType = - uniq( blocks.map( ( { name } ) => name ) ).length === 1; - let icon; - if ( isSelectionOfSameType ) { - const sourceBlockName = hoveredBlock.name; - const blockType = getBlockType( sourceBlockName ); - icon = blockType.icon; - } else { - icon = stack; - } const hasPossibleBlockTransformations = !! possibleBlockTransformations.length; if ( ! hasBlockStyles && ! hasPossibleBlockTransformations ) { return ( @@ -139,7 +137,7 @@ const BlockSwitcher = ( { clientIds } ) => { ) } { hasBlockStyles && ( ) } From 8c07b302add3705b16db49bc2d4706abe8feb9cd Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Thu, 17 Dec 2020 12:20:39 +0200 Subject: [PATCH 8/8] fix unit tests --- .../block-editor/src/components/block-switcher/test/index.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/block-editor/src/components/block-switcher/test/index.js b/packages/block-editor/src/components/block-switcher/test/index.js index 40a1e28bc8119..b18f72b4d5938 100644 --- a/packages/block-editor/src/components/block-switcher/test/index.js +++ b/packages/block-editor/src/components/block-switcher/test/index.js @@ -10,6 +10,7 @@ import { useSelect } from '@wordpress/data'; import { registerBlockType, unregisterBlockType } from '@wordpress/blocks'; import { DOWN } from '@wordpress/keycodes'; import { Button } from '@wordpress/components'; +import { stack } from '@wordpress/icons'; /** * Internal dependencies @@ -118,6 +119,7 @@ describe( 'BlockSwitcher', () => { useSelect.mockImplementation( () => ( { blocks: [ headingBlock1, textBlock ], possibleBlockTransformations: [], + icon: stack, } ) ); const wrapper = shallow( ); expect( wrapper ).toMatchSnapshot();