From c6c73ebea54b45989fa889416ad7760f0368a9cb Mon Sep 17 00:00:00 2001 From: ramonjd Date: Tue, 17 Aug 2021 19:56:12 +1000 Subject: [PATCH 01/12] When there are no selected controls within a ToolsPanel, change the icon to a plus symbol. This is to indicate to the user that they can add controls to the panel. --- .../src/tools-panel/tools-panel-header/component.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/components/src/tools-panel/tools-panel-header/component.tsx b/packages/components/src/tools-panel/tools-panel-header/component.tsx index 3204615434b4b..1e585b39cfcd5 100644 --- a/packages/components/src/tools-panel/tools-panel-header/component.tsx +++ b/packages/components/src/tools-panel/tools-panel-header/component.tsx @@ -7,7 +7,7 @@ import type { Ref } from 'react'; /** * WordPress dependencies */ -import { check, reset, moreVertical } from '@wordpress/icons'; +import { check, reset, moreVertical, plus } from '@wordpress/icons'; import { __, sprintf } from '@wordpress/i18n'; /** @@ -118,6 +118,7 @@ const ToolsPanelHeader = ( const { dropdownMenuClassName, hasMenuItems, + hasSelectedMenuItems, label: labelText, menuItems, resetAll, @@ -137,7 +138,7 @@ const ToolsPanelHeader = ( { labelText } { hasMenuItems && ( From 1186edf37bc5b910789e0cac4247fd18f6381ed3 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Wed, 18 Aug 2021 13:41:20 +1000 Subject: [PATCH 02/12] Moving business logic into the hook --- .../components/src/tools-panel/tools-panel-header/hook.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/components/src/tools-panel/tools-panel-header/hook.ts b/packages/components/src/tools-panel/tools-panel-header/hook.ts index f31e8cb6372df..25dfea44729c7 100644 --- a/packages/components/src/tools-panel/tools-panel-header/hook.ts +++ b/packages/components/src/tools-panel/tools-panel-header/hook.ts @@ -30,11 +30,15 @@ export function useToolsPanelHeader( }, [] ); const { menuItems, hasMenuItems } = useToolsPanelContext(); - + const menuItemsArray = Object.entries( menuItems ); + const hasSelectedMenuItems = menuItemsArray.some( + ( [ , isSelected ] ) => isSelected + ); return { ...otherProps, dropdownMenuClassName, hasMenuItems, + hasSelectedMenuItems, menuItems, className: classes, }; From 6b2b974daeaa0c15062221d5526d81e15b8b3766 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Wed, 15 Sep 2021 22:36:22 +1000 Subject: [PATCH 03/12] Ensure that optional controls, when hidden and available, trigger the plus icon in the ToolsPanel header. Updated stories. --- .../src/tools-panel/stories/index.js | 46 +++++++++++++++++++ .../tools-panel-header/component.tsx | 4 +- .../tools-panel/tools-panel-header/hook.ts | 13 +++--- .../src/tools-panel/tools-panel/hook.ts | 21 +++++++++ 4 files changed, 76 insertions(+), 8 deletions(-) diff --git a/packages/components/src/tools-panel/stories/index.js b/packages/components/src/tools-panel/stories/index.js index 230a407bff6fc..99a842d088026 100644 --- a/packages/components/src/tools-panel/stories/index.js +++ b/packages/components/src/tools-panel/stories/index.js @@ -66,6 +66,7 @@ export const _default = () => { hasValue={ () => !! minHeight } label="Minimum height" onDeselect={ () => setMinHeight( undefined ) } + isShownByDefault={ true } > { ); }; +export const WithOptionalItemsPlusIcon = () => { + const [ height, setHeight ] = useState(); + const [ width, setWidth ] = useState(); + + const resetAll = () => { + setHeight( undefined ); + setWidth( undefined ); + }; + + return ( + + + + !! width } + label="Width" + onDeselect={ () => setWidth( undefined ) } + isShownByDefault={ false } + > + setWidth( next ) } + /> + + !! height } + label="Height" + onDeselect={ () => setHeight( undefined ) } + isShownByDefault={ false } + > + setHeight( next ) } + /> + + + + + ); +}; + const { Fill: ToolsPanelItems, Slot } = createSlotFill( 'ToolsPanelSlot' ); const panelId = 'unique-tools-panel-id'; diff --git a/packages/components/src/tools-panel/tools-panel-header/component.tsx b/packages/components/src/tools-panel/tools-panel-header/component.tsx index 1e585b39cfcd5..73fc1ea2b822a 100644 --- a/packages/components/src/tools-panel/tools-panel-header/component.tsx +++ b/packages/components/src/tools-panel/tools-panel-header/component.tsx @@ -118,7 +118,7 @@ const ToolsPanelHeader = ( const { dropdownMenuClassName, hasMenuItems, - hasSelectedMenuItems, + areOptionalControlsHidden, label: labelText, menuItems, resetAll, @@ -138,7 +138,7 @@ const ToolsPanelHeader = ( { labelText } { hasMenuItems && ( diff --git a/packages/components/src/tools-panel/tools-panel-header/hook.ts b/packages/components/src/tools-panel/tools-panel-header/hook.ts index 25dfea44729c7..06199cf22f255 100644 --- a/packages/components/src/tools-panel/tools-panel-header/hook.ts +++ b/packages/components/src/tools-panel/tools-panel-header/hook.ts @@ -29,16 +29,17 @@ export function useToolsPanelHeader( return cx( styles.DropdownMenu ); }, [] ); - const { menuItems, hasMenuItems } = useToolsPanelContext(); - const menuItemsArray = Object.entries( menuItems ); - const hasSelectedMenuItems = menuItemsArray.some( - ( [ , isSelected ] ) => isSelected - ); + const { + menuItems, + hasMenuItems, + areOptionalControlsHidden, + } = useToolsPanelContext(); + return { ...otherProps, dropdownMenuClassName, hasMenuItems, - hasSelectedMenuItems, + areOptionalControlsHidden, menuItems, className: classes, }; diff --git a/packages/components/src/tools-panel/tools-panel/hook.ts b/packages/components/src/tools-panel/tools-panel/hook.ts index afbf3174fae90..94190ab34358d 100644 --- a/packages/components/src/tools-panel/tools-panel/hook.ts +++ b/packages/components/src/tools-panel/tools-panel/hook.ts @@ -92,6 +92,26 @@ export function useToolsPanel( setMenuItems( items ); }, [ panelItems ] ); + // Track whether optional controls, if any, are displayed or not. + const [ + areOptionalControlsHidden, + setAreOptionalControlsHidden, + ] = useState( false ); + + // Where no optional menu items are active, we display a plus icon + // to indicate the presence of further menu items. + useEffect( () => { + if ( menuItems.optional ) { + const optionalMenuItemsArray = Object.entries( menuItems.optional ); + const newValue = + optionalMenuItemsArray.length > 0 && + ! Object.entries( menuItems.optional ).some( + ( [ , isSelected ] ) => isSelected + ); + setAreOptionalControlsHidden( newValue ); + } + }, [ menuItems.optional ] ); + // Force a menu item to be checked. // This is intended for use with default panel items. They are displayed // separately to optional items and have different display states, @@ -203,6 +223,7 @@ export function useToolsPanel( registerPanelItem, deregisterPanelItem, flagItemCustomization, + areOptionalControlsHidden, hasMenuItems: !! panelItems.length, isResetting: isResetting.current, shouldRenderPlaceholderItems, From c1b84b2219658bd0a8930a4d6bde2a84d452c13e Mon Sep 17 00:00:00 2001 From: ramonjd Date: Thu, 16 Sep 2021 08:14:18 +1000 Subject: [PATCH 04/12] updating variable name to make it clearer that we want to track if optional items are available at all *and* if so, are they hidden --- packages/components/src/tools-panel/stories/index.js | 10 ++++++++-- .../src/tools-panel/tools-panel-header/component.tsx | 8 ++++++-- .../src/tools-panel/tools-panel-header/hook.ts | 4 ++-- .../components/src/tools-panel/tools-panel/hook.ts | 10 +++++----- 4 files changed, 21 insertions(+), 11 deletions(-) diff --git a/packages/components/src/tools-panel/stories/index.js b/packages/components/src/tools-panel/stories/index.js index 99a842d088026..3c8cc693695fb 100644 --- a/packages/components/src/tools-panel/stories/index.js +++ b/packages/components/src/tools-panel/stories/index.js @@ -35,7 +35,10 @@ export const _default = () => { return ( - + !! width } @@ -92,7 +95,10 @@ export const WithOptionalItemsPlusIcon = () => { return ( - + !! width } diff --git a/packages/components/src/tools-panel/tools-panel-header/component.tsx b/packages/components/src/tools-panel/tools-panel-header/component.tsx index 73fc1ea2b822a..b9f25c06eea83 100644 --- a/packages/components/src/tools-panel/tools-panel-header/component.tsx +++ b/packages/components/src/tools-panel/tools-panel-header/component.tsx @@ -118,7 +118,7 @@ const ToolsPanelHeader = ( const { dropdownMenuClassName, hasMenuItems, - areOptionalControlsHidden, + areOptionalControlsAvailableAndHidden, label: labelText, menuItems, resetAll, @@ -138,7 +138,11 @@ const ToolsPanelHeader = ( { labelText } { hasMenuItems && ( diff --git a/packages/components/src/tools-panel/tools-panel-header/hook.ts b/packages/components/src/tools-panel/tools-panel-header/hook.ts index 06199cf22f255..4535180878a34 100644 --- a/packages/components/src/tools-panel/tools-panel-header/hook.ts +++ b/packages/components/src/tools-panel/tools-panel-header/hook.ts @@ -32,14 +32,14 @@ export function useToolsPanelHeader( const { menuItems, hasMenuItems, - areOptionalControlsHidden, + areOptionalControlsAvailableAndHidden, } = useToolsPanelContext(); return { ...otherProps, dropdownMenuClassName, hasMenuItems, - areOptionalControlsHidden, + areOptionalControlsAvailableAndHidden, menuItems, className: classes, }; diff --git a/packages/components/src/tools-panel/tools-panel/hook.ts b/packages/components/src/tools-panel/tools-panel/hook.ts index 94190ab34358d..6e7425ffdff32 100644 --- a/packages/components/src/tools-panel/tools-panel/hook.ts +++ b/packages/components/src/tools-panel/tools-panel/hook.ts @@ -94,8 +94,8 @@ export function useToolsPanel( // Track whether optional controls, if any, are displayed or not. const [ - areOptionalControlsHidden, - setAreOptionalControlsHidden, + areOptionalControlsAvailableAndHidden, + setAreOptionalControlsAvailableAndHidden, ] = useState( false ); // Where no optional menu items are active, we display a plus icon @@ -105,10 +105,10 @@ export function useToolsPanel( const optionalMenuItemsArray = Object.entries( menuItems.optional ); const newValue = optionalMenuItemsArray.length > 0 && - ! Object.entries( menuItems.optional ).some( + ! optionalMenuItemsArray.some( ( [ , isSelected ] ) => isSelected ); - setAreOptionalControlsHidden( newValue ); + setAreOptionalControlsAvailableAndHidden( newValue ); } }, [ menuItems.optional ] ); @@ -223,7 +223,7 @@ export function useToolsPanel( registerPanelItem, deregisterPanelItem, flagItemCustomization, - areOptionalControlsHidden, + areOptionalControlsAvailableAndHidden, hasMenuItems: !! panelItems.length, isResetting: isResetting.current, shouldRenderPlaceholderItems, From c46a415ef076e6b21c6f29a214fd458ff4b833e4 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Thu, 16 Sep 2021 12:37:14 +1000 Subject: [PATCH 05/12] This commit adds a variable aria-label to the tools panel header menu button to describe the element and its intended effects. Added component tests. --- .../components/src/tools-panel/test/index.js | 45 +++++++++++++++++-- .../tools-panel-header/component.tsx | 19 +++++--- 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/packages/components/src/tools-panel/test/index.js b/packages/components/src/tools-panel/test/index.js index b8aaba1914972..90481397f34e3 100644 --- a/packages/components/src/tools-panel/test/index.js +++ b/packages/components/src/tools-panel/test/index.js @@ -139,10 +139,17 @@ const renderPanel = () => { ); }; -// Helper to find the menu button and simulate a user click. +/** + * Helper to find the menu button and simulate a user click. + * + * @return {HTMLElement} The menuButton. + */ const openDropdownMenu = () => { - const menuButton = screen.getByLabelText( defaultProps.label ); + const menuButton = screen.getByRole( 'button', { + name: /show([\w\s]+)options/i, + } ); fireEvent.click( menuButton ); + return menuButton; }; // Opens dropdown then selects the menu item by label before simulating a click. @@ -212,7 +219,7 @@ describe( 'ToolsPanel', () => { it( 'should render panel menu when at least one panel item', () => { renderPanel(); - const menuButton = screen.getByLabelText( defaultProps.label ); + const menuButton = openDropdownMenu(); expect( menuButton ).toBeInTheDocument(); } ); @@ -509,4 +516,36 @@ describe( 'ToolsPanel', () => { expect( items[ 1 ] ).toHaveTextContent( 'Item 2' ); } ); } ); + + describe( 'panel header icon toggle', () => { + it( 'should continue to render shown by default item after it is toggled off via menu item', async () => { + render( + + +
Default control
+ + + ); + + // There are unactivated, optional menu items in the Tools Panel dropdown. + const optionsHiddenIcon = screen.getByRole( 'button', { + name: 'Show and add options', + } ); + + expect( optionsHiddenIcon ).toBeInTheDocument(); + + await selectMenuItem( controlProps.label ); + + // There are now NO unactivated, optional menu items in the Tools Panel dropdown. + expect( + screen.queryByRole( 'button', { name: 'Show and add options' } ) + ).toBeNull(); + + const optionsDisplayedIcon = screen.getByRole( 'button', { + name: 'Show options', + } ); + + expect( optionsDisplayedIcon ).toBeInTheDocument(); + } ); + } ); } ); diff --git a/packages/components/src/tools-panel/tools-panel-header/component.tsx b/packages/components/src/tools-panel/tools-panel-header/component.tsx index b9f25c06eea83..428f2313683da 100644 --- a/packages/components/src/tools-panel/tools-panel-header/component.tsx +++ b/packages/components/src/tools-panel/tools-panel-header/component.tsx @@ -8,7 +8,7 @@ import type { Ref } from 'react'; * WordPress dependencies */ import { check, reset, moreVertical, plus } from '@wordpress/icons'; -import { __, sprintf } from '@wordpress/i18n'; +import { __, _x, sprintf } from '@wordpress/i18n'; /** * Internal dependencies @@ -132,18 +132,23 @@ const ToolsPanelHeader = ( const defaultItems = Object.entries( menuItems?.default || {} ); const optionalItems = Object.entries( menuItems?.optional || {} ); + const dropDownMenuIcon = areOptionalControlsAvailableAndHidden + ? plus + : moreVertical; + const dropDownMenuLabelText = areOptionalControlsAvailableAndHidden + ? _x( + 'Show and add options', + 'Button label to reveal tool panel options' + ) + : _x( 'Show options', 'Button label to reveal tool panel options' ); return (

{ labelText } { hasMenuItems && ( { ( { onClose = noop } ) => ( From 6045abc5d301f24bd0ae3c0e284540d7047b97d7 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Fri, 17 Sep 2021 09:57:20 +1000 Subject: [PATCH 06/12] Changing copy from "Show" to "View" Updating tests --- packages/components/src/tools-panel/test/index.js | 10 +++++----- .../src/tools-panel/tools-panel-header/component.tsx | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/components/src/tools-panel/test/index.js b/packages/components/src/tools-panel/test/index.js index 90481397f34e3..40f31ccc35e0a 100644 --- a/packages/components/src/tools-panel/test/index.js +++ b/packages/components/src/tools-panel/test/index.js @@ -146,7 +146,7 @@ const renderPanel = () => { */ const openDropdownMenu = () => { const menuButton = screen.getByRole( 'button', { - name: /show([\w\s]+)options/i, + name: /view([\w\s]+)options/i, } ); fireEvent.click( menuButton ); return menuButton; @@ -529,7 +529,7 @@ describe( 'ToolsPanel', () => { // There are unactivated, optional menu items in the Tools Panel dropdown. const optionsHiddenIcon = screen.getByRole( 'button', { - name: 'Show and add options', + name: 'View and add options', } ); expect( optionsHiddenIcon ).toBeInTheDocument(); @@ -538,11 +538,11 @@ describe( 'ToolsPanel', () => { // There are now NO unactivated, optional menu items in the Tools Panel dropdown. expect( - screen.queryByRole( 'button', { name: 'Show and add options' } ) - ).toBeNull(); + screen.queryByRole( 'button', { name: 'View and add options' } ) + ).not.toBeInTheDocument(); const optionsDisplayedIcon = screen.getByRole( 'button', { - name: 'Show options', + name: 'View options', } ); expect( optionsDisplayedIcon ).toBeInTheDocument(); diff --git a/packages/components/src/tools-panel/tools-panel-header/component.tsx b/packages/components/src/tools-panel/tools-panel-header/component.tsx index 428f2313683da..81291d8a5f8f0 100644 --- a/packages/components/src/tools-panel/tools-panel-header/component.tsx +++ b/packages/components/src/tools-panel/tools-panel-header/component.tsx @@ -137,10 +137,10 @@ const ToolsPanelHeader = ( : moreVertical; const dropDownMenuLabelText = areOptionalControlsAvailableAndHidden ? _x( - 'Show and add options', + 'View and add options', 'Button label to reveal tool panel options' ) - : _x( 'Show options', 'Button label to reveal tool panel options' ); + : _x( 'View options', 'Button label to reveal tool panel options' ); return (

From 33b0054c7697bb6f3f99449c2548c4073969b48e Mon Sep 17 00:00:00 2001 From: ramonjd Date: Fri, 24 Sep 2021 14:43:13 +1000 Subject: [PATCH 07/12] Renaming `areAllOptionalControlsHidden` variable and adding it as a property to the ToolsPanelContext type Modified default value of areAllOptionalControlsHidden and updated comments --- .../src/tools-panel/tools-panel-header/component.tsx | 8 +++----- .../src/tools-panel/tools-panel-header/hook.ts | 4 ++-- .../components/src/tools-panel/tools-panel/hook.ts | 11 ++++++----- packages/components/src/tools-panel/types.ts | 1 + 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/components/src/tools-panel/tools-panel-header/component.tsx b/packages/components/src/tools-panel/tools-panel-header/component.tsx index 81291d8a5f8f0..537578c93da0c 100644 --- a/packages/components/src/tools-panel/tools-panel-header/component.tsx +++ b/packages/components/src/tools-panel/tools-panel-header/component.tsx @@ -118,7 +118,7 @@ const ToolsPanelHeader = ( const { dropdownMenuClassName, hasMenuItems, - areOptionalControlsAvailableAndHidden, + areAllOptionalControlsHidden, label: labelText, menuItems, resetAll, @@ -132,10 +132,8 @@ const ToolsPanelHeader = ( const defaultItems = Object.entries( menuItems?.default || {} ); const optionalItems = Object.entries( menuItems?.optional || {} ); - const dropDownMenuIcon = areOptionalControlsAvailableAndHidden - ? plus - : moreVertical; - const dropDownMenuLabelText = areOptionalControlsAvailableAndHidden + const dropDownMenuIcon = areAllOptionalControlsHidden ? plus : moreVertical; + const dropDownMenuLabelText = areAllOptionalControlsHidden ? _x( 'View and add options', 'Button label to reveal tool panel options' diff --git a/packages/components/src/tools-panel/tools-panel-header/hook.ts b/packages/components/src/tools-panel/tools-panel-header/hook.ts index 4535180878a34..064f324c163be 100644 --- a/packages/components/src/tools-panel/tools-panel-header/hook.ts +++ b/packages/components/src/tools-panel/tools-panel-header/hook.ts @@ -32,14 +32,14 @@ export function useToolsPanelHeader( const { menuItems, hasMenuItems, - areOptionalControlsAvailableAndHidden, + areAllOptionalControlsHidden, } = useToolsPanelContext(); return { ...otherProps, dropdownMenuClassName, hasMenuItems, - areOptionalControlsAvailableAndHidden, + areAllOptionalControlsHidden, menuItems, className: classes, }; diff --git a/packages/components/src/tools-panel/tools-panel/hook.ts b/packages/components/src/tools-panel/tools-panel/hook.ts index 6e7425ffdff32..fc6b1b828a232 100644 --- a/packages/components/src/tools-panel/tools-panel/hook.ts +++ b/packages/components/src/tools-panel/tools-panel/hook.ts @@ -93,10 +93,11 @@ export function useToolsPanel( }, [ panelItems ] ); // Track whether optional controls, if any, are displayed or not. + // Default state is that all optional controls, if any, are hidden. const [ - areOptionalControlsAvailableAndHidden, - setAreOptionalControlsAvailableAndHidden, - ] = useState( false ); + areAllOptionalControlsHidden, + setAreAllOptionalControlsHidden, + ] = useState( true ); // Where no optional menu items are active, we display a plus icon // to indicate the presence of further menu items. @@ -108,7 +109,7 @@ export function useToolsPanel( ! optionalMenuItemsArray.some( ( [ , isSelected ] ) => isSelected ); - setAreOptionalControlsAvailableAndHidden( newValue ); + setAreAllOptionalControlsHidden( newValue ); } }, [ menuItems.optional ] ); @@ -223,7 +224,7 @@ export function useToolsPanel( registerPanelItem, deregisterPanelItem, flagItemCustomization, - areOptionalControlsAvailableAndHidden, + areAllOptionalControlsHidden, hasMenuItems: !! panelItems.length, isResetting: isResetting.current, shouldRenderPlaceholderItems, diff --git a/packages/components/src/tools-panel/types.ts b/packages/components/src/tools-panel/types.ts index 1617f32f313bc..82d815e25eec8 100644 --- a/packages/components/src/tools-panel/types.ts +++ b/packages/components/src/tools-panel/types.ts @@ -127,6 +127,7 @@ export type ToolsPanelContext = { flagItemCustomization: ( label: string ) => void; isResetting: boolean; shouldRenderPlaceholderItems: boolean; + areAllOptionalControlsHidden?: boolean; }; export type ToolsPanelControlsGroupProps = { From 33d50ccd7f6e5dc162e83cb5ed99f68aae210b1d Mon Sep 17 00:00:00 2001 From: ramonjd Date: Fri, 24 Sep 2021 15:29:56 +1000 Subject: [PATCH 08/12] Make `areAllOptionalControlsHidden` required on the ToolsPanelContent object. --- packages/components/src/tools-panel/context.ts | 1 + packages/components/src/tools-panel/types.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/components/src/tools-panel/context.ts b/packages/components/src/tools-panel/context.ts index d0104db26e6b7..7b13e59a92acd 100644 --- a/packages/components/src/tools-panel/context.ts +++ b/packages/components/src/tools-panel/context.ts @@ -18,6 +18,7 @@ export const ToolsPanelContext = createContext< ToolsPanelContextType >( { registerPanelItem: noop, deregisterPanelItem: noop, flagItemCustomization: noop, + areAllOptionalControlsHidden: true, } ); export const useToolsPanelContext = () => diff --git a/packages/components/src/tools-panel/types.ts b/packages/components/src/tools-panel/types.ts index 82d815e25eec8..374c98495c8c7 100644 --- a/packages/components/src/tools-panel/types.ts +++ b/packages/components/src/tools-panel/types.ts @@ -127,7 +127,7 @@ export type ToolsPanelContext = { flagItemCustomization: ( label: string ) => void; isResetting: boolean; shouldRenderPlaceholderItems: boolean; - areAllOptionalControlsHidden?: boolean; + areAllOptionalControlsHidden: boolean; }; export type ToolsPanelControlsGroupProps = { From 08a6863fcf1d6fa2a0c2ade31c30f9d0f3371b38 Mon Sep 17 00:00:00 2001 From: Ramon Date: Mon, 27 Sep 2021 11:02:53 +1000 Subject: [PATCH 09/12] Update packages/components/src/tools-panel/test/index.js Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> --- packages/components/src/tools-panel/test/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/tools-panel/test/index.js b/packages/components/src/tools-panel/test/index.js index 40f31ccc35e0a..c84958d306753 100644 --- a/packages/components/src/tools-panel/test/index.js +++ b/packages/components/src/tools-panel/test/index.js @@ -518,7 +518,7 @@ describe( 'ToolsPanel', () => { } ); describe( 'panel header icon toggle', () => { - it( 'should continue to render shown by default item after it is toggled off via menu item', async () => { + it( 'should render appropriate icons for the dropdown menu', async () => { render( From 5b4dd79f2f8c1b09ba2ec8126b6038e31c0d3aa8 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Mon, 27 Sep 2021 11:38:55 +1000 Subject: [PATCH 10/12] Add custom fixtures for optional control props in order to test the panel header icon toggle in isolation from the other tests. --- .../components/src/tools-panel/test/index.js | 17 ++++++++++++--- .../src/tools-panel/tools-panel/hook.ts | 21 ------------------- 2 files changed, 14 insertions(+), 24 deletions(-) diff --git a/packages/components/src/tools-panel/test/index.js b/packages/components/src/tools-panel/test/index.js index c84958d306753..f8379fe4d551d 100644 --- a/packages/components/src/tools-panel/test/index.js +++ b/packages/components/src/tools-panel/test/index.js @@ -518,11 +518,22 @@ describe( 'ToolsPanel', () => { } ); describe( 'panel header icon toggle', () => { + const optionalControls = { + attributes: { value: false }, + hasValue: jest.fn().mockImplementation( () => { + return !! optionalControls.attributes.value; + } ), + label: 'Optional', + onDeselect: jest.fn(), + onSelect: jest.fn(), + isShownByDefault: false, + }; + it( 'should render appropriate icons for the dropdown menu', async () => { render( - -
Default control
+ +
Optional control
 ); @@ -534,7 +545,7 @@ describe( 'ToolsPanel', () => { expect( optionsHiddenIcon ).toBeInTheDocument(); - await selectMenuItem( controlProps.label ); + await selectMenuItem( optionalControls.label ); // There are now NO unactivated, optional menu items in the Tools Panel dropdown. expect( diff --git a/packages/components/src/tools-panel/tools-panel/hook.ts b/packages/components/src/tools-panel/tools-panel/hook.ts index fc6b1b828a232..6039fb321a324 100644 --- a/packages/components/src/tools-panel/tools-panel/hook.ts +++ b/packages/components/src/tools-panel/tools-panel/hook.ts @@ -130,27 +130,6 @@ export function useToolsPanel( } ); }; - // Track whether all optional controls are displayed or not. - // If no optional controls are present, then none are hidden and this will - // be `false`. - const [ - areAllOptionalControlsHidden, - setAreAllOptionalControlsHidden, - ] = useState( false ); - - // We need to track whether any optional menu items are active to later - // determine whether the panel is currently empty and any inner wrapper - // should be hidden. - useEffect( () => { - if ( menuItems.optional ) { - const optionalItems = Object.entries( menuItems.optional ); - const allControlsHidden = - optionalItems.length > 0 && - ! optionalItems.some( ( [ , isSelected ] ) => isSelected ); - setAreAllOptionalControlsHidden( allControlsHidden ); - } - }, [ menuItems.optional ] ); - const cx = useCx(); const classes = useMemo( () => { const hasDefaultMenuItems = From cdc17029e90bd9d9f052c4b7058b0aa7aedef7f6 Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 13 Oct 2021 20:37:16 +1100 Subject: [PATCH 11/12] Update packages/components/src/tools-panel/stories/index.js Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> --- packages/components/src/tools-panel/stories/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/tools-panel/stories/index.js b/packages/components/src/tools-panel/stories/index.js index 3c8cc693695fb..95a33301da56c 100644 --- a/packages/components/src/tools-panel/stories/index.js +++ b/packages/components/src/tools-panel/stories/index.js @@ -96,7 +96,7 @@ export const WithOptionalItemsPlusIcon = () => { Date: Wed, 13 Oct 2021 21:01:56 +1100 Subject: [PATCH 12/12] Consolidating comment and reinstating `false` as the default value for the state var `areAllOptionalControlsHidden` --- .../src/tools-panel/tools-panel/hook.ts | 40 +++++++++---------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/packages/components/src/tools-panel/tools-panel/hook.ts b/packages/components/src/tools-panel/tools-panel/hook.ts index 6039fb321a324..ab0f29c034830 100644 --- a/packages/components/src/tools-panel/tools-panel/hook.ts +++ b/packages/components/src/tools-panel/tools-panel/hook.ts @@ -92,27 +92,6 @@ export function useToolsPanel( setMenuItems( items ); }, [ panelItems ] ); - // Track whether optional controls, if any, are displayed or not. - // Default state is that all optional controls, if any, are hidden. - const [ - areAllOptionalControlsHidden, - setAreAllOptionalControlsHidden, - ] = useState( true ); - - // Where no optional menu items are active, we display a plus icon - // to indicate the presence of further menu items. - useEffect( () => { - if ( menuItems.optional ) { - const optionalMenuItemsArray = Object.entries( menuItems.optional ); - const newValue = - optionalMenuItemsArray.length > 0 && - ! optionalMenuItemsArray.some( - ( [ , isSelected ] ) => isSelected - ); - setAreAllOptionalControlsHidden( newValue ); - } - }, [ menuItems.optional ] ); - // Force a menu item to be checked. // This is intended for use with default panel items. They are displayed // separately to optional items and have different display states, @@ -130,6 +109,25 @@ export function useToolsPanel( } ); }; + // Whether all optional menu items are hidden or not must be tracked + // in order to later determine if the panel display is empty and handle + // conditional display of a plus icon to indicate the presence of further + // menu items. + const [ + areAllOptionalControlsHidden, + setAreAllOptionalControlsHidden, + ] = useState( false ); + + useEffect( () => { + if ( menuItems.optional ) { + const optionalItems = Object.entries( menuItems.optional ); + const allControlsHidden = + optionalItems.length > 0 && + ! optionalItems.some( ( [ , isSelected ] ) => isSelected ); + setAreAllOptionalControlsHidden( allControlsHidden ); + } + }, [ menuItems.optional ] ); + const cx = useCx(); const classes = useMemo( () => { const hasDefaultMenuItems =