From a7131e3ad5df0f9ded52b6eff6bf384b8b18cdab Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Wed, 4 Jan 2023 20:27:04 -0600 Subject: [PATCH 1/8] Refactor getBlockContent to a component --- packages/block-library/src/page-list/edit.js | 117 ++++++++++--------- 1 file changed, 65 insertions(+), 52 deletions(-) diff --git a/packages/block-library/src/page-list/edit.js b/packages/block-library/src/page-list/edit.js index 219822ed87944f..35aee139e275e1 100644 --- a/packages/block-library/src/page-list/edit.js +++ b/packages/block-library/src/page-list/edit.js @@ -41,6 +41,63 @@ import { convertDescription } from './constants'; const MAX_PAGE_COUNT = 100; const NOOP = () => {}; +function BlockContent( { + blockProps, + innerBlocksProps, + hasResolvedPages, + totalPages, + blockList, + pages, +} ) { + if ( ! hasResolvedPages ) { + return ( +
+ +
+ ); + } + + if ( totalPages === null ) { + return ( +
+ + { __( 'Page List: Cannot retrieve Pages.' ) } + +
+ ); + } + + if ( totalPages === 0 ) { + return ( +
+ + { __( 'Page List: Cannot retrieve Pages.' ) } + +
+ ); + } + + if ( blockList.length === 0 ) { + const parentPageDetails = + pages && pages.find( ( page ) => page.id === parentPageID ); + return ( +
+ + { sprintf( + // translators: %s: Page title. + __( '"%s" page has no children.' ), + parentPageDetails.title.rendered + ) } + +
+ ); + } + + if ( totalPages > 0 ) { + return ; + } +} + export default function PageListEdit( { context, clientId, @@ -137,56 +194,6 @@ export default function PageListEdit( { value: blockList, } ); - const getBlockContent = () => { - if ( ! hasResolvedPages ) { - return ( -
- -
- ); - } - - if ( totalPages === null ) { - return ( -
- - { __( 'Page List: Cannot retrieve Pages.' ) } - -
- ); - } - - if ( totalPages === 0 ) { - return ( -
- - { __( 'Page List: Cannot retrieve Pages.' ) } - -
- ); - } - - if ( blockList.length === 0 ) { - const parentPageDetails = - pages && pages.find( ( page ) => page.id === parentPageID ); - return ( -
- - { sprintf( - // translators: %s: Page title. - __( '"%s" page has no children.' ), - parentPageDetails.title.rendered - ) } - -
- ); - } - - if ( totalPages > 0 ) { - return ; - } - }; - const { replaceBlock, selectBlock } = useDispatch( blockEditorStore ); const { parentNavBlockClientId, isNested } = useSelect( @@ -271,8 +278,14 @@ export default function PageListEdit( { clientId={ clientId } /> ) } - - { getBlockContent() } + ); } From 921161ee91a45161a2558ed01acad6a9a5bac8f1 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Thu, 5 Jan 2023 10:10:40 -0600 Subject: [PATCH 2/8] Inline custom hooks --- packages/block-library/src/page-list/edit.js | 88 +++++++++----------- 1 file changed, 38 insertions(+), 50 deletions(-) diff --git a/packages/block-library/src/page-list/edit.js b/packages/block-library/src/page-list/edit.js index 35aee139e275e1..160d5cd1ccee13 100644 --- a/packages/block-library/src/page-list/edit.js +++ b/packages/block-library/src/page-list/edit.js @@ -105,8 +105,44 @@ export default function PageListEdit( { setAttributes, } ) { const { parentPageID } = attributes; - const [ pages ] = useGetPages(); - const { pagesByParentId, totalPages, hasResolvedPages } = usePageData(); + const { records: pages, hasResolved: hasResolvedPages } = useEntityRecords( + 'postType', + 'page', + { + orderby: 'menu_order', + order: 'asc', + _fields: [ 'id', 'link', 'parent', 'title', 'menu_order' ], + per_page: -1, + context: 'view', + } + ); + useMemo( () => { + // TODO: Once the REST API supports passing multiple values to + // 'orderby', this can be removed. + // https://core.trac.wordpress.org/ticket/39037 + + const sortedPages = [ ...( pages ?? [] ) ].sort( ( a, b ) => { + if ( a.menu_order === b.menu_order ) { + return a.title.rendered.localeCompare( b.title.rendered ); + } + return a.menu_order - b.menu_order; + } ); + const pagesByParentId = sortedPages.reduce( ( accumulator, page ) => { + const { parent } = page; + if ( accumulator.has( parent ) ) { + accumulator.get( parent ).push( page ); + } else { + accumulator.set( parent, [ page ] ); + } + return accumulator; + }, new Map() ); + + return { + pagesByParentId, + hasResolvedPages, + totalPages: pages?.length ?? null, + }; + }, [ pages, hasResolvedPages ] ); const isNavigationChild = 'showSubmenuIcon' in context; const allowConvertToLinks = @@ -289,51 +325,3 @@ export default function PageListEdit( { ); } - -function useGetPages() { - const { records: pages, hasResolved: hasResolvedPages } = useEntityRecords( - 'postType', - 'page', - { - orderby: 'menu_order', - order: 'asc', - _fields: [ 'id', 'link', 'parent', 'title', 'menu_order' ], - per_page: -1, - context: 'view', - } - ); - - return [ pages, hasResolvedPages ]; -} - -function usePageData( pageId = 0 ) { - const [ pages, hasResolvedPages ] = useGetPages(); - - return useMemo( () => { - // TODO: Once the REST API supports passing multiple values to - // 'orderby', this can be removed. - // https://core.trac.wordpress.org/ticket/39037 - - const sortedPages = [ ...( pages ?? [] ) ].sort( ( a, b ) => { - if ( a.menu_order === b.menu_order ) { - return a.title.rendered.localeCompare( b.title.rendered ); - } - return a.menu_order - b.menu_order; - } ); - const pagesByParentId = sortedPages.reduce( ( accumulator, page ) => { - const { parent } = page; - if ( accumulator.has( parent ) ) { - accumulator.get( parent ).push( page ); - } else { - accumulator.set( parent, [ page ] ); - } - return accumulator; - }, new Map() ); - - return { - pagesByParentId, - hasResolvedPages, - totalPages: pages?.length ?? null, - }; - }, [ pageId, pages, hasResolvedPages ] ); -} From 93a89a7be48182fe791f6e438855d2d6cbb7fe78 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Thu, 5 Jan 2023 10:11:45 -0600 Subject: [PATCH 3/8] Optimize useMemo with default empty array const --- packages/block-library/src/page-list/edit.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/block-library/src/page-list/edit.js b/packages/block-library/src/page-list/edit.js index 160d5cd1ccee13..493239831b3eaf 100644 --- a/packages/block-library/src/page-list/edit.js +++ b/packages/block-library/src/page-list/edit.js @@ -40,6 +40,7 @@ import { convertDescription } from './constants'; // Performance of Navigation Links is not good past this value. const MAX_PAGE_COUNT = 100; const NOOP = () => {}; +const EMPTY_ARR = []; function BlockContent( { blockProps, @@ -105,23 +106,20 @@ export default function PageListEdit( { setAttributes, } ) { const { parentPageID } = attributes; - const { records: pages, hasResolved: hasResolvedPages } = useEntityRecords( - 'postType', - 'page', - { + const { records: pages = EMPTY_ARR, hasResolved: hasResolvedPages } = + useEntityRecords( 'postType', 'page', { orderby: 'menu_order', order: 'asc', _fields: [ 'id', 'link', 'parent', 'title', 'menu_order' ], per_page: -1, context: 'view', - } - ); + } ); useMemo( () => { // TODO: Once the REST API supports passing multiple values to // 'orderby', this can be removed. // https://core.trac.wordpress.org/ticket/39037 - const sortedPages = [ ...( pages ?? [] ) ].sort( ( a, b ) => { + const sortedPages = pages.sort( ( a, b ) => { if ( a.menu_order === b.menu_order ) { return a.title.rendered.localeCompare( b.title.rendered ); } From 78e5b1d4dcf80348b9f968b35a94012462d7db85 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Thu, 5 Jan 2023 13:32:35 -0600 Subject: [PATCH 4/8] Refactor and unify convertToNavigtionLinks behavior --- .../block-library/src/page-list/constants.js | 8 - .../src/page-list/convert-to-links-modal.js | 70 ------- packages/block-library/src/page-list/edit.js | 176 ++++++++++-------- ....js => use-convert-to-navigation-links.js} | 42 ++++- 4 files changed, 136 insertions(+), 160 deletions(-) delete mode 100644 packages/block-library/src/page-list/constants.js delete mode 100644 packages/block-library/src/page-list/convert-to-links-modal.js rename packages/block-library/src/page-list/{convert-to-navigation-links.js => use-convert-to-navigation-links.js} (59%) diff --git a/packages/block-library/src/page-list/constants.js b/packages/block-library/src/page-list/constants.js deleted file mode 100644 index 9322fa345321c4..00000000000000 --- a/packages/block-library/src/page-list/constants.js +++ /dev/null @@ -1,8 +0,0 @@ -/** - * WordPress dependencies - */ -import { __ } from '@wordpress/i18n'; - -export const convertDescription = __( - 'This menu is automatically kept in sync with pages on your site. You can manage the menu yourself by clicking customize below.' -); diff --git a/packages/block-library/src/page-list/convert-to-links-modal.js b/packages/block-library/src/page-list/convert-to-links-modal.js deleted file mode 100644 index dbc76fc77cdb1e..00000000000000 --- a/packages/block-library/src/page-list/convert-to-links-modal.js +++ /dev/null @@ -1,70 +0,0 @@ -/** - * WordPress dependencies - */ -import { Button, Modal } from '@wordpress/components'; -import { __ } from '@wordpress/i18n'; -import { useDispatch } from '@wordpress/data'; -import { useEntityRecords } from '@wordpress/core-data'; -import { store as blockEditorStore } from '@wordpress/block-editor'; -/** - * Internal dependencies - */ -import { convertToNavigationLinks } from './convert-to-navigation-links'; - -/** - * Internal dependencies - */ -import { convertDescription } from './constants'; - -const PAGE_FIELDS = [ 'id', 'title', 'link', 'type', 'parent' ]; -const MAX_PAGE_COUNT = 100; - -export default function ConvertToLinksModal( { onClose, clientId } ) { - const { records: pages, hasResolved: pagesFinished } = useEntityRecords( - 'postType', - 'page', - { - per_page: MAX_PAGE_COUNT, - _fields: PAGE_FIELDS, - // TODO: When https://core.trac.wordpress.org/ticket/39037 REST API support for multiple orderby - // values is resolved, update 'orderby' to [ 'menu_order', 'post_title' ] to provide a consistent - // sort. - orderby: 'menu_order', - order: 'asc', - } - ); - - const { replaceBlock } = useDispatch( blockEditorStore ); - - return ( - -

- { convertDescription } -

-
- - -
- - ); -} diff --git a/packages/block-library/src/page-list/edit.js b/packages/block-library/src/page-list/edit.js index 493239831b3eaf..d48802e0117430 100644 --- a/packages/block-library/src/page-list/edit.js +++ b/packages/block-library/src/page-list/edit.js @@ -23,30 +23,31 @@ import { Notice, ComboboxControl, Button, + Modal, } from '@wordpress/components'; import { __, sprintf } from '@wordpress/i18n'; import { useMemo, useState, useEffect } from '@wordpress/element'; import { useEntityRecords } from '@wordpress/core-data'; -import { useSelect, useDispatch } from '@wordpress/data'; +import { useSelect } from '@wordpress/data'; /** * Internal dependencies */ -import ConvertToLinksModal from './convert-to-links-modal'; -import { convertToNavigationLinks } from './convert-to-navigation-links'; -import { convertDescription } from './constants'; +import { useConvertToNavigationLinks } from './use-convert-to-navigation-links'; // We only show the edit option when page count is <= MAX_PAGE_COUNT // Performance of Navigation Links is not good past this value. const MAX_PAGE_COUNT = 100; const NOOP = () => {}; -const EMPTY_ARR = []; + +const convertDescription = __( + 'This menu is automatically kept in sync with pages on your site. You can manage the menu yourself by clicking customize below.' +); function BlockContent( { blockProps, innerBlocksProps, hasResolvedPages, - totalPages, blockList, pages, } ) { @@ -58,7 +59,7 @@ function BlockContent( { ); } - if ( totalPages === null ) { + if ( pages === null ) { return (
@@ -68,7 +69,7 @@ function BlockContent( { ); } - if ( totalPages === 0 ) { + if ( pages.length === 0 ) { return (
@@ -79,8 +80,9 @@ function BlockContent( { } if ( blockList.length === 0 ) { - const parentPageDetails = - pages && pages.find( ( page ) => page.id === parentPageID ); + const parentPageDetails = pages.find( + ( page ) => page.id === parentPageID + ); return (
@@ -94,11 +96,54 @@ function BlockContent( { ); } - if ( totalPages > 0 ) { + if ( pages.length > 0 ) { return
    ; } } +function ConvertToLinksModal( { onClick, disabled } ) { + const [ isOpen, setOpen ] = useState( false ); + const openModal = () => setOpen( true ); + const closeModal = () => setOpen( false ); + + return ( + <> + + + { __( 'Edit' ) } + + + { isOpen && ( + +

    + { convertDescription } +

    +
    + + +
    +     + ) } + + ); +} + export default function PageListEdit( { context, clientId, @@ -106,26 +151,47 @@ export default function PageListEdit( { setAttributes, } ) { const { parentPageID } = attributes; - const { records: pages = EMPTY_ARR, hasResolved: hasResolvedPages } = - useEntityRecords( 'postType', 'page', { + + const { records: pages, hasResolved: hasResolvedPages } = useEntityRecords( + 'postType', + 'page', + { + per_page: MAX_PAGE_COUNT, + _fields: [ 'id', 'link', 'menu_order', 'parent', 'title', 'type' ], + // TODO: When https://core.trac.wordpress.org/ticket/39037 REST API support for multiple orderby + // values is resolved, update 'orderby' to [ 'menu_order', 'post_title' ] to provide a consistent + // sort. orderby: 'menu_order', order: 'asc', - _fields: [ 'id', 'link', 'parent', 'title', 'menu_order' ], - per_page: -1, - context: 'view', - } ); - useMemo( () => { + } + ); + + const allowConvertToLinks = + 'showSubmenuIcon' in context && + pages?.length > 0 && + pages?.length <= MAX_PAGE_COUNT; + + const convertToNavigationLinks = useConvertToNavigationLinks( { + clientId, + pages, + } ); + + const pagesByParentId = useMemo( () => { + if ( pages === null ) { + return new Map(); + } + // TODO: Once the REST API supports passing multiple values to // 'orderby', this can be removed. // https://core.trac.wordpress.org/ticket/39037 - const sortedPages = pages.sort( ( a, b ) => { if ( a.menu_order === b.menu_order ) { return a.title.rendered.localeCompare( b.title.rendered ); } return a.menu_order - b.menu_order; } ); - const pagesByParentId = sortedPages.reduce( ( accumulator, page ) => { + + return sortedPages.reduce( ( accumulator, page ) => { const { parent } = page; if ( accumulator.has( parent ) ) { accumulator.get( parent ).push( page ); @@ -134,21 +200,7 @@ export default function PageListEdit( { } return accumulator; }, new Map() ); - - return { - pagesByParentId, - hasResolvedPages, - totalPages: pages?.length ?? null, - }; - }, [ pages, hasResolvedPages ] ); - - const isNavigationChild = 'showSubmenuIcon' in context; - const allowConvertToLinks = - isNavigationChild && totalPages <= MAX_PAGE_COUNT; - - const [ isOpen, setOpen ] = useState( false ); - const openModal = () => setOpen( true ); - const closeModal = () => setOpen( false ); + }, [ pages ] ); const blockProps = useBlockProps( { className: classnames( 'wp-block-page-list', { @@ -228,27 +280,16 @@ export default function PageListEdit( { value: blockList, } ); - const { replaceBlock, selectBlock } = useDispatch( blockEditorStore ); - - const { parentNavBlockClientId, isNested } = useSelect( + const { isNested } = useSelect( ( select ) => { - const { getSelectedBlockClientId, getBlockParentsByBlockName } = - select( blockEditorStore ); - - const _selectedBlockClientId = getSelectedBlockClientId(); - + const { getBlockParentsByBlockName } = select( blockEditorStore ); + const blockParents = getBlockParentsByBlockName( + clientId, + 'core/navigation-submenu', + true + ); return { - parentNavBlockClientId: getBlockParentsByBlockName( - _selectedBlockClientId, - 'core/navigation', - true - )[ 0 ], - isNested: - getBlockParentsByBlockName( - clientId, - 'core/navigation-submenu', - true - ).length > 0, + isNested: blockParents.length > 0, }; }, [ clientId ] @@ -261,22 +302,13 @@ export default function PageListEdit( { return ( <> - { isNavigationChild && pages?.length > 0 && ( + { allowConvertToLinks && (

    { convertDescription }

    @@ -299,24 +331,16 @@ export default function PageListEdit( {     ) }     - { allowConvertToLinks && totalPages > 0 && ( - - - { __( 'Edit' ) } - - - ) } - { allowConvertToLinks && isOpen && ( + { allowConvertToLinks && ( ) } diff --git a/packages/block-library/src/page-list/convert-to-navigation-links.js b/packages/block-library/src/page-list/use-convert-to-navigation-links.js similarity index 59% rename from packages/block-library/src/page-list/convert-to-navigation-links.js rename to packages/block-library/src/page-list/use-convert-to-navigation-links.js index 64b1dbf1c7d4d6..a5decd6377a54d 100644 --- a/packages/block-library/src/page-list/convert-to-navigation-links.js +++ b/packages/block-library/src/page-list/use-convert-to-navigation-links.js @@ -2,12 +2,10 @@ * WordPress dependencies */ import { createBlock } from '@wordpress/blocks'; +import { useSelect, useDispatch } from '@wordpress/data'; +import { store as blockEditorStore } from '@wordpress/block-editor'; -export const convertToNavigationLinks = ( pages ) => { - if ( ! pages ) { - return; - } - +function convertToNavigationLinks( pages = [] ) { const linkMap = {}; const navigationLinks = []; pages.forEach( ( { id, title, link: url, type, parent } ) => { @@ -57,4 +55,36 @@ export const convertToNavigationLinks = ( pages ) => { transformSubmenus( navigationLinks ); return navigationLinks; -}; +} + +export function useConvertToNavigationLinks( { clientId, pages } ) { + const { replaceBlock, selectBlock } = useDispatch( blockEditorStore ); + + const { parentNavBlockClientId } = useSelect( + ( select ) => { + const { getSelectedBlockClientId, getBlockParentsByBlockName } = + select( blockEditorStore ); + + const _selectedBlockClientId = getSelectedBlockClientId(); + + return { + parentNavBlockClientId: getBlockParentsByBlockName( + _selectedBlockClientId, + 'core/navigation', + true + )[ 0 ], + }; + }, + [ clientId ] + ); + + return () => { + const navigationLinks = convertToNavigationLinks( pages ); + + // Replace the Page List block with the Navigation Links. + replaceBlock( clientId, navigationLinks ); + + // Select the Navigation block to reveal the changes. + selectBlock( parentNavBlockClientId ); + }; +} From 75814e5ec7bbacc9b7ebd99e10bc94e103027ef8 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Mon, 9 Jan 2023 09:51:38 -0600 Subject: [PATCH 5/8] Fix missing parentPageID --- packages/block-library/src/page-list/edit.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/block-library/src/page-list/edit.js b/packages/block-library/src/page-list/edit.js index d48802e0117430..8a18b79ce58507 100644 --- a/packages/block-library/src/page-list/edit.js +++ b/packages/block-library/src/page-list/edit.js @@ -50,6 +50,7 @@ function BlockContent( { hasResolvedPages, blockList, pages, + parentPageID, } ) { if ( ! hasResolvedPages ) { return ( @@ -343,6 +344,7 @@ export default function PageListEdit( { hasResolvedPages={ hasResolvedPages } blockList={ blockList } pages={ pages } + parentPageID={ parentPageID } /> ); From 4b977fc25a77e6774866bee9109ca98a1715ff09 Mon Sep 17 00:00:00 2001 From: Alex Lende Date: Mon, 9 Jan 2023 09:53:32 -0600 Subject: [PATCH 6/8] Fix convertToNavigationLinksTests --- .../block-library/src/page-list/test/convert-to-links-modal.js | 2 +- .../src/page-list/use-convert-to-navigation-links.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/page-list/test/convert-to-links-modal.js b/packages/block-library/src/page-list/test/convert-to-links-modal.js index 4c83b64f9bb9f9..d7907cbf2e4d4a 100644 --- a/packages/block-library/src/page-list/test/convert-to-links-modal.js +++ b/packages/block-library/src/page-list/test/convert-to-links-modal.js @@ -2,7 +2,7 @@ * Internal dependencies */ -import { convertToNavigationLinks } from '../convert-to-navigation-links'; +import { convertToNavigationLinks } from '../use-convert-to-navigation-links'; // Mock createBlock to avoid creating the blocks in test environment // as convertToNavigationLinks calls this method internally. diff --git a/packages/block-library/src/page-list/use-convert-to-navigation-links.js b/packages/block-library/src/page-list/use-convert-to-navigation-links.js index a5decd6377a54d..929fb706e01bab 100644 --- a/packages/block-library/src/page-list/use-convert-to-navigation-links.js +++ b/packages/block-library/src/page-list/use-convert-to-navigation-links.js @@ -5,7 +5,7 @@ import { createBlock } from '@wordpress/blocks'; import { useSelect, useDispatch } from '@wordpress/data'; import { store as blockEditorStore } from '@wordpress/block-editor'; -function convertToNavigationLinks( pages = [] ) { +export function convertToNavigationLinks( pages = [] ) { const linkMap = {}; const navigationLinks = []; pages.forEach( ( { id, title, link: url, type, parent } ) => { From 86abead22ecdcaa4de4cecc85e8a265cc74c52ed Mon Sep 17 00:00:00 2001 From: Ben Dwyer Date: Thu, 12 Jan 2023 11:33:06 +0000 Subject: [PATCH 7/8] Remove the limit on the number of pages to show --- packages/block-library/src/page-list/edit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/page-list/edit.js b/packages/block-library/src/page-list/edit.js index 8a18b79ce58507..94102736236bf8 100644 --- a/packages/block-library/src/page-list/edit.js +++ b/packages/block-library/src/page-list/edit.js @@ -157,7 +157,7 @@ export default function PageListEdit( { 'postType', 'page', { - per_page: MAX_PAGE_COUNT, + per_page: -1, _fields: [ 'id', 'link', 'menu_order', 'parent', 'title', 'type' ], // TODO: When https://core.trac.wordpress.org/ticket/39037 REST API support for multiple orderby // values is resolved, update 'orderby' to [ 'menu_order', 'post_title' ] to provide a consistent From dc83eba18f0afedc3d010c01c1a744b647687a0f Mon Sep 17 00:00:00 2001 From: Ben Dwyer Date: Thu, 26 Jan 2023 10:08:32 +0000 Subject: [PATCH 8/8] Limit the number of pages to display to 100 --- packages/block-library/src/page-list/edit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/page-list/edit.js b/packages/block-library/src/page-list/edit.js index 94102736236bf8..8a18b79ce58507 100644 --- a/packages/block-library/src/page-list/edit.js +++ b/packages/block-library/src/page-list/edit.js @@ -157,7 +157,7 @@ export default function PageListEdit( { 'postType', 'page', { - per_page: -1, + per_page: MAX_PAGE_COUNT, _fields: [ 'id', 'link', 'menu_order', 'parent', 'title', 'type' ], // TODO: When https://core.trac.wordpress.org/ticket/39037 REST API support for multiple orderby // values is resolved, update 'orderby' to [ 'menu_order', 'post_title' ] to provide a consistent