From fd5d963ffae907f475c524c8bcc5ba53ce7d31d3 Mon Sep 17 00:00:00 2001 From: George Hotelling Date: Mon, 25 Oct 2021 19:41:13 -0400 Subject: [PATCH] Create a JS version of Page List for the editor (#31670) * Create a JS version of Page List for the editor * Fix lint errors and remove setAttributes() calls * Split page fetching into a hook * Add navigation submenu markup * Use per_page=-1 * Sort by menu_order,post_title * Add back 'No pages to show' message * Fix component not updating after fetch * Remove TODOs * Remove unused context attributes * Update snapshot * Fix typo in comment * Remove unnecessary isNavigationChild checks * Access navigation settings via context, not attributes * Add loading state * Memoize PageItems Co-authored-by: Robert Anderson --- .../block-library/src/page-list/block.json | 33 -- packages/block-library/src/page-list/edit.js | 281 +++++++++++------- .../block-library/src/page-list/index.php | 46 ++- .../__snapshots__/navigation.test.js.snap | 2 +- 4 files changed, 189 insertions(+), 173 deletions(-) diff --git a/packages/block-library/src/page-list/block.json b/packages/block-library/src/page-list/block.json index a16a0b0abeb0fa..146f236fca84f8 100644 --- a/packages/block-library/src/page-list/block.json +++ b/packages/block-library/src/page-list/block.json @@ -7,39 +7,6 @@ "keywords": [ "menu", "navigation" ], "textdomain": "default", "attributes": { - "textColor": { - "type": "string" - }, - "customTextColor": { - "type": "string" - }, - "backgroundColor": { - "type": "string" - }, - "customBackgroundColor": { - "type": "string" - }, - "overlayBackgroundColor": { - "type": "string" - }, - "customOverlayBackgroundColor": { - "type": "string" - }, - "overlayTextColor": { - "type": "string" - }, - "customOverlayTextColor": { - "type": "string" - }, - "isNavigationChild": { - "type": "boolean" - }, - "showSubmenuIcon": { - "type": "boolean" - }, - "openSubmenusOnClick" : { - "type": "boolean" - } }, "usesContext": [ "textColor", diff --git a/packages/block-library/src/page-list/edit.js b/packages/block-library/src/page-list/edit.js index 5328689edcbbe2..7d44a376ce0429 100644 --- a/packages/block-library/src/page-list/edit.js +++ b/packages/block-library/src/page-list/edit.js @@ -2,6 +2,7 @@ * External dependencies */ import classnames from 'classnames'; +import { sortBy } from 'lodash'; /** * WordPress dependencies @@ -9,14 +10,11 @@ import classnames from 'classnames'; import { BlockControls, useBlockProps, - store as blockEditorStore, getColorClassName, } from '@wordpress/block-editor'; -import ServerSideRender from '@wordpress/server-side-render'; -import { ToolbarButton } from '@wordpress/components'; +import { ToolbarButton, Placeholder, Spinner } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; -import { useEffect, useState } from '@wordpress/element'; -import { useSelect } from '@wordpress/data'; +import { useEffect, useState, memo } from '@wordpress/element'; import apiFetch from '@wordpress/api-fetch'; import { addQueryArgs } from '@wordpress/url'; @@ -24,116 +22,38 @@ import { addQueryArgs } from '@wordpress/url'; * Internal dependencies */ import ConvertToLinksModal from './convert-to-links-modal'; +import { ItemSubmenuIcon } from '../navigation-link/icons'; // 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; -export default function PageListEdit( { - context, - clientId, - attributes, - setAttributes, -} ) { - // Copy context to attributes to make it accessible in the editor's - // ServerSideRender - useEffect( () => { - const { - textColor, - customTextColor, - backgroundColor, - customBackgroundColor, - overlayTextColor, - customOverlayTextColor, - overlayBackgroundColor, - customOverlayBackgroundColor, - } = context; - setAttributes( { - textColor, - customTextColor, - backgroundColor, - customBackgroundColor, - overlayTextColor, - customOverlayTextColor, - overlayBackgroundColor, - customOverlayBackgroundColor, - } ); - }, [ - context.textColor, - context.customTextColor, - context.backgroundColor, - context.customBackgroundColor, - context.overlayTextColor, - context.customOverlayTextColor, - context.overlayBackgroundColor, - context.customOverlayBackgroundColor, - ] ); - - const { textColor, backgroundColor, style } = context || {}; - - const [ allowConvertToLinks, setAllowConvertToLinks ] = useState( false ); - - const blockProps = useBlockProps( { - className: classnames( { - 'has-text-color': !! textColor, - [ getColorClassName( 'color', textColor ) ]: !! textColor, - 'has-background': !! backgroundColor, - [ getColorClassName( - 'background-color', - backgroundColor - ) ]: !! backgroundColor, - } ), - style: { ...style?.color }, - } ); +export default function PageListEdit( { context, clientId } ) { + const { pagesByParentId, totalPages } = usePagesByParentId(); - const isParentNavigation = useSelect( - ( select ) => { - const { getBlockParentsByBlockName } = select( blockEditorStore ); - return ( - getBlockParentsByBlockName( clientId, 'core/navigation' ) - .length > 0 - ); - }, - [ clientId ] - ); - - useEffect( () => { - setAttributes( { - isNavigationChild: isParentNavigation, - openSubmenusOnClick: !! context.openSubmenusOnClick, - showSubmenuIcon: !! context.showSubmenuIcon, - } ); - }, [ context.openSubmenusOnClick, context.showSubmenuIcon ] ); - - useEffect( () => { - if ( isParentNavigation ) { - apiFetch( { - path: addQueryArgs( '/wp/v2/pages', { - per_page: 1, - _fields: [ 'id' ], - } ), - parse: false, - } ).then( ( res ) => { - setAllowConvertToLinks( - res.headers.get( 'X-WP-Total' ) <= MAX_PAGE_COUNT - ); - } ); - } else { - setAllowConvertToLinks( false ); - } - }, [ isParentNavigation ] ); + 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 ); - // Update parent status before component first renders. - const attributesWithParentStatus = { - ...attributes, - isNavigationChild: isParentNavigation, - openSubmenusOnClick: !! context.openSubmenusOnClick, - showSubmenuIcon: !! context.showSubmenuIcon, - }; + const blockProps = useBlockProps( { + className: classnames( 'wp-block-page-list', { + 'has-text-color': !! context.textColor, + [ getColorClassName( + 'color', + context.textColor + ) ]: !! context.textColor, + 'has-background': !! context.backgroundColor, + [ getColorClassName( + 'background-color', + context.backgroundColor + ) ]: !! context.backgroundColor, + } ), + style: { ...context.style?.color }, + } ); return ( <> @@ -150,15 +70,150 @@ export default function PageListEdit( { clientId={ clientId } /> ) } -
- ( - { __( 'Page List: No pages to show.' ) } - ) } - /> -
+ { totalPages === null && ( +
+ + + +
+ ) } + { totalPages === 0 && ( +
+ { __( 'Page List: No pages to show.' ) } +
+ ) } + { totalPages > 0 && ( + + ) } ); } + +function usePagesByParentId() { + const [ pagesByParentId, setPagesByParentId ] = useState( null ); + const [ totalPages, setTotalPages ] = useState( null ); + + useEffect( () => { + async function performFetch() { + setPagesByParentId( null ); + setTotalPages( null ); + + let pages = await apiFetch( { + path: addQueryArgs( '/wp/v2/pages', { + orderby: 'menu_order', + order: 'asc', + _fields: [ 'id', 'link', 'parent', 'title', 'menu_order' ], + per_page: -1, + } ), + } ); + + // TODO: Once the REST API supports passing multiple values to + // 'orderby', this can be removed. + // https://core.trac.wordpress.org/ticket/39037 + pages = sortBy( pages, [ 'menu_order', 'title.rendered' ] ); + + setPagesByParentId( + pages.reduce( ( accumulator, page ) => { + const { parent } = page; + if ( accumulator.has( parent ) ) { + accumulator.get( parent ).push( page ); + } else { + accumulator.set( parent, [ page ] ); + } + return accumulator; + }, new Map() ) + ); + setTotalPages( pages.length ); + } + performFetch(); + }, [] ); + + return { + pagesByParentId, + totalPages, + }; +} + +const PageItems = memo( function PageItems( { + context, + pagesByParentId, + parentId = 0, + depth = 0, +} ) { + const pages = pagesByParentId.get( parentId ); + + if ( ! pages?.length ) { + return []; + } + + return pages.map( ( page ) => { + const hasChildren = pagesByParentId.has( page.id ); + const isNavigationChild = 'showSubmenuIcon' in context; + return ( +
  • + { hasChildren && context.openSubmenusOnClick ? ( + + ) : ( + + { page.title?.rendered } + + ) } + { hasChildren && ( + <> + { ! context.openSubmenusOnClick && + context.showSubmenuIcon && } +
      + +
    + + ) } +
    • + ); + } ); +} ); + +function ItemSubmenuToggle( { title } ) { + return ( + + ); +} diff --git a/packages/block-library/src/page-list/index.php b/packages/block-library/src/page-list/index.php index dc1a654d757a07..37419f9cf70cff 100644 --- a/packages/block-library/src/page-list/index.php +++ b/packages/block-library/src/page-list/index.php @@ -22,8 +22,8 @@ function block_core_page_list_build_css_colors( $attributes, $context ) { ); // Text color. - $has_named_text_color = array_key_exists( 'textColor', $attributes ); - $has_picked_text_color = array_key_exists( 'customTextColor', $attributes ); + $has_named_text_color = array_key_exists( 'textColor', $context ); + $has_picked_text_color = array_key_exists( 'customTextColor', $context ); $has_custom_text_color = isset( $context['style']['color']['text'] ); // If has text color. @@ -34,17 +34,17 @@ function block_core_page_list_build_css_colors( $attributes, $context ) { if ( $has_named_text_color ) { // Add the color class. - $colors['css_classes'][] = sprintf( 'has-%s-color', gutenberg_experimental_to_kebab_case( $attributes['textColor'] ) ); + $colors['css_classes'][] = sprintf( 'has-%s-color', gutenberg_experimental_to_kebab_case( $context['textColor'] ) ); } elseif ( $has_picked_text_color ) { - $colors['inline_styles'] .= sprintf( 'color: %s;', $attributes['customTextColor'] ); + $colors['inline_styles'] .= sprintf( 'color: %s;', $context['customTextColor'] ); } elseif ( $has_custom_text_color ) { // Add the custom color inline style. $colors['inline_styles'] .= sprintf( 'color: %s;', $context['style']['color']['text'] ); } // Background color. - $has_named_background_color = array_key_exists( 'backgroundColor', $attributes ); - $has_picked_background_color = array_key_exists( 'customBackgroundColor', $attributes ); + $has_named_background_color = array_key_exists( 'backgroundColor', $context ); + $has_picked_background_color = array_key_exists( 'customBackgroundColor', $context ); $has_custom_background_color = isset( $context['style']['color']['background'] ); // If has background color. @@ -55,17 +55,17 @@ function block_core_page_list_build_css_colors( $attributes, $context ) { if ( $has_named_background_color ) { // Add the background-color class. - $colors['css_classes'][] = sprintf( 'has-%s-background-color', gutenberg_experimental_to_kebab_case( $attributes['backgroundColor'] ) ); + $colors['css_classes'][] = sprintf( 'has-%s-background-color', gutenberg_experimental_to_kebab_case( $context['backgroundColor'] ) ); } elseif ( $has_picked_background_color ) { - $colors['inline_styles'] .= sprintf( 'background-color: %s;', $attributes['customBackgroundColor'] ); + $colors['inline_styles'] .= sprintf( 'background-color: %s;', $context['customBackgroundColor'] ); } elseif ( $has_custom_background_color ) { // Add the custom background-color inline style. $colors['inline_styles'] .= sprintf( 'background-color: %s;', $context['style']['color']['background'] ); } // Overlay text color. - $has_named_overlay_text_color = array_key_exists( 'overlayTextColor', $attributes ); - $has_picked_overlay_text_color = array_key_exists( 'customOverlayTextColor', $attributes ); + $has_named_overlay_text_color = array_key_exists( 'overlayTextColor', $context ); + $has_picked_overlay_text_color = array_key_exists( 'customOverlayTextColor', $context ); // If it has a text color. if ( $has_named_overlay_text_color || $has_picked_overlay_text_color ) { @@ -74,14 +74,14 @@ function block_core_page_list_build_css_colors( $attributes, $context ) { // Give overlay colors priority, fall back to Navigation block colors, then global styles. if ( $has_named_overlay_text_color ) { - $colors['overlay_css_classes'][] = sprintf( 'has-%s-color', gutenberg_experimental_to_kebab_case( $attributes['overlayTextColor'] ) ); + $colors['overlay_css_classes'][] = sprintf( 'has-%s-color', gutenberg_experimental_to_kebab_case( $context['overlayTextColor'] ) ); } elseif ( $has_picked_overlay_text_color ) { - $colors['overlay_inline_styles'] .= sprintf( 'color: %s;', $attributes['customOverlayTextColor'] ); + $colors['overlay_inline_styles'] .= sprintf( 'color: %s;', $context['customOverlayTextColor'] ); } // Overlay background colors. - $has_named_overlay_background_color = array_key_exists( 'overlayBackgroundColor', $attributes ); - $has_picked_overlay_background_color = array_key_exists( 'customOverlayBackgroundColor', $attributes ); + $has_named_overlay_background_color = array_key_exists( 'overlayBackgroundColor', $context ); + $has_picked_overlay_background_color = array_key_exists( 'customOverlayBackgroundColor', $context ); // If has background color. if ( $has_named_overlay_background_color || $has_picked_overlay_background_color ) { @@ -89,9 +89,9 @@ function block_core_page_list_build_css_colors( $attributes, $context ) { } if ( $has_named_overlay_background_color ) { - $colors['overlay_css_classes'][] = sprintf( 'has-%s-background-color', gutenberg_experimental_to_kebab_case( $attributes['overlayBackgroundColor'] ) ); + $colors['overlay_css_classes'][] = sprintf( 'has-%s-background-color', gutenberg_experimental_to_kebab_case( $context['overlayBackgroundColor'] ) ); } elseif ( $has_picked_overlay_background_color ) { - $colors['overlay_inline_styles'] .= sprintf( 'background-color: %s;', $attributes['customOverlayBackgroundColor'] ); + $colors['overlay_inline_styles'] .= sprintf( 'background-color: %s;', $context['customOverlayBackgroundColor'] ); } return $colors; @@ -238,20 +238,14 @@ function render_block_core_page_list( $attributes, $content, $block ) { static $block_id = 0; $block_id++; - // TODO: When https://core.trac.wordpress.org/ticket/39037 REST API support for multiple orderby values is resolved, - // update 'sort_column' to 'menu_order, post_title'. Sorting by both menu_order and post_title ensures a stable sort. - // Otherwise with pages that have the same menu_order value, we can see different ordering depending on how DB - // queries are constructed internally. For example we might see a different order when a limit is set to <499 - // versus >= 500. $all_pages = get_pages( array( - 'sort_column' => 'menu_order', + 'sort_column' => 'menu_order,post_title', 'order' => 'asc', ) ); // If thare are no pages, there is nothing to show. - // Return early and empty to trigger EmptyResponsePlaceholder. if ( empty( $all_pages ) ) { return; } @@ -298,11 +292,11 @@ function render_block_core_page_list( $attributes, $content, $block ) { $nested_pages = block_core_page_list_nest_pages( $top_level_pages, $pages_with_children ); - $is_navigation_child = array_key_exists( 'isNavigationChild', $attributes ) ? $attributes['isNavigationChild'] : ! empty( $block->context ); + $is_navigation_child = array_key_exists( 'showSubmenuIcon', $block->context ); - $open_submenus_on_click = array_key_exists( 'openSubmenusOnClick', $attributes ) ? $attributes['openSubmenusOnClick'] : false; + $open_submenus_on_click = array_key_exists( 'openSubmenusOnClick', $block->context ) ? $block->context['openSubmenusOnClick'] : false; - $show_submenu_icons = array_key_exists( 'showSubmenuIcon', $attributes ) ? $attributes['showSubmenuIcon'] : false; + $show_submenu_icons = array_key_exists( 'showSubmenuIcon', $block->context ) ? $block->context['showSubmenuIcon'] : false; $wrapper_markup = ''; diff --git a/packages/e2e-tests/specs/experiments/blocks/__snapshots__/navigation.test.js.snap b/packages/e2e-tests/specs/experiments/blocks/__snapshots__/navigation.test.js.snap index 1614ace7f64a4e..b46899aec5aa1b 100644 --- a/packages/e2e-tests/specs/experiments/blocks/__snapshots__/navigation.test.js.snap +++ b/packages/e2e-tests/specs/experiments/blocks/__snapshots__/navigation.test.js.snap @@ -40,7 +40,7 @@ exports[`Navigation Creating from existing Menus creates an empty navigation blo exports[`Navigation Creating from existing Pages allows a navigation block to be created using existing pages 1`] = ` " - + " `;