From dd93b47cf5155550baa0c05175c2a61fafb950b9 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Thu, 16 Feb 2023 10:53:18 +0100 Subject: [PATCH 1/6] Fix Reset All button in the typography panel of the block inspector --- .../global-styles/typography-panel.js | 30 ++++-- .../src/components/inspector-controls/fill.js | 40 +++++-- packages/block-editor/src/hooks/typography.js | 101 +++++++++++------- .../components/src/tools-panel/context.ts | 2 + .../components/src/tools-panel/test/index.tsx | 4 + .../src/tools-panel/tools-panel-item/hook.ts | 13 ++- .../src/tools-panel/tools-panel/hook.ts | 43 ++++++-- packages/components/src/tools-panel/types.ts | 21 ++-- 8 files changed, 179 insertions(+), 75 deletions(-) diff --git a/packages/block-editor/src/components/global-styles/typography-panel.js b/packages/block-editor/src/components/global-styles/typography-panel.js index 15ebf70da93a8..9f8d6a371be0f 100644 --- a/packages/block-editor/src/components/global-styles/typography-panel.js +++ b/packages/block-editor/src/components/global-styles/typography-panel.js @@ -7,6 +7,7 @@ import { __experimentalToolsPanelItem as ToolsPanelItem, } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; +import { useCallback } from '@wordpress/element'; /** * Internal dependencies @@ -92,8 +93,17 @@ function useHasTextDecorationControl( settings ) { return settings?.typography?.textDecoration; } -function TypographyToolsPanel( { ...props } ) { - return ; +function TypographyToolsPanel( { resetAllFilter, onChange, value, children } ) { + const resetAll = () => { + const updatedValue = resetAllFilter( value ); + onChange( updatedValue ); + }; + + return ( + + { children } + + ); } const DEFAULT_CONTROLS = { @@ -260,15 +270,19 @@ export default function TypographyPanel( { const hasTextDecoration = () => !! value?.typography?.textDecoration; const resetTextDecoration = () => setTextDecoration( undefined ); - const resetAll = () => { - onChange( { - ...value, + const resetAllFilter = useCallback( ( previousValue ) => { + return { + ...previousValue, typography: {}, - } ); - }; + }; + }, [] ); return ( - + { hasFontFamilyEnabled && ( { ( fillProps ) => { - // Children passed to InspectorControlsFill will not have - // access to any React Context whose Provider is part of - // the InspectorControlsSlot tree. So we re-create the - // Provider in this subtree. - const value = ! isEmpty( fillProps ) ? fillProps : null; return ( - - { children } - + ); } } ); } + +function ToolPanelInspectorControl( { children, resetAllFilter, fillProps } ) { + const { registerResetAllFilter, deregisterResetAllFilter } = fillProps; + useEffect( () => { + if ( resetAllFilter && registerResetAllFilter ) { + registerResetAllFilter( resetAllFilter ); + } + return () => { + if ( resetAllFilter && deregisterResetAllFilter ) { + deregisterResetAllFilter( resetAllFilter ); + } + }; + }, [ resetAllFilter, registerResetAllFilter, deregisterResetAllFilter ] ); + + // Children passed to InspectorControlsFill will not have + // access to any React Context whose Provider is part of + // the InspectorControlsSlot tree. So we re-create the + // Provider in this subtree. + const value = ! isEmpty( fillProps ) ? fillProps : null; + return ( + + { children } + + ); +} diff --git a/packages/block-editor/src/hooks/typography.js b/packages/block-editor/src/hooks/typography.js index a5aac0cb2d312..3cb949b647115 100644 --- a/packages/block-editor/src/hooks/typography.js +++ b/packages/block-editor/src/hooks/typography.js @@ -2,7 +2,7 @@ * WordPress dependencies */ import { getBlockSupport, hasBlockSupport } from '@wordpress/blocks'; -import { useMemo } from '@wordpress/element'; +import { useMemo, useCallback } from '@wordpress/element'; /** * Internal dependencies @@ -46,9 +46,64 @@ export const TYPOGRAPHY_SUPPORT_KEYS = [ LETTER_SPACING_SUPPORT_KEY, ]; -function TypographyInspectorControl( { children } ) { +function styleToAttributes( style ) { + const updatedStyle = { ...omit( style, [ 'fontFamily' ] ) }; + const fontSizeValue = style?.typography?.fontSize; + const fontFamilyValue = style?.typography?.fontFamily; + const fontSizeSlug = fontSizeValue?.startsWith( 'var:preset|font-size|' ) + ? fontSizeValue.substring( 'var:preset|font-size|'.length ) + : undefined; + const fontFamilySlug = fontFamilyValue?.startsWith( + 'var:preset|font-family|' + ) + ? fontFamilyValue.substring( 'var:preset|font-family|'.length ) + : undefined; + updatedStyle.typography = { + ...omit( updatedStyle.typography, [ 'fontFamily' ] ), + fontSize: fontSizeSlug ? undefined : fontSizeValue, + }; + return { + style: cleanEmptyObject( updatedStyle ), + fontFamily: fontFamilySlug, + fontSize: fontSizeSlug, + }; +} + +function attributesToStyle( attributes ) { + return { + ...attributes.style, + typography: { + ...attributes.style?.typography, + fontFamily: attributes.fontFamily + ? 'var:preset|font-family|' + attributes.fontFamily + : undefined, + fontSize: attributes.fontSize + ? 'var:preset|font-size|' + attributes.fontSize + : attributes.style?.typography?.fontSize, + }, + }; +} + +function TypographyInspectorControl( { children, resetAllFilter } ) { + const attributesResetAllFilter = useCallback( + ( attributes ) => { + const existingStyle = attributesToStyle( attributes ); + const updatedStyle = resetAllFilter( existingStyle ); + return { + ...attributes, + ...styleToAttributes( updatedStyle ), + }; + }, + [ resetAllFilter ] + ); + return ( - { children } + + { children } + ); } @@ -106,43 +161,15 @@ export function TypographyPanel( { const settings = useBlockSettings( name ); const isEnabled = useHasTypographyPanel( settings ); const value = useMemo( () => { - return { - ...attributes.style, - typography: { - ...attributes.style?.typography, - fontFamily: attributes.fontFamily - ? 'var:preset|font-family|' + attributes.fontFamily - : undefined, - fontSize: attributes.fontSize - ? 'var:preset|font-size|' + attributes.fontSize - : attributes.style?.typography?.fontSize, - }, - }; + return attributesToStyle( { + style: attributes.style, + fontFamily: attributes.fontFamily, + fontSize: attributes.fontSize, + } ); }, [ attributes.style, attributes.fontSize, attributes.fontFamily ] ); const onChange = ( newStyle ) => { - const updatedStyle = { ...omit( newStyle, [ 'fontFamily' ] ) }; - const fontSizeValue = newStyle?.typography?.fontSize; - const fontFamilyValue = newStyle?.typography?.fontFamily; - const fontSizeSlug = fontSizeValue?.startsWith( - 'var:preset|font-size|' - ) - ? fontSizeValue.substring( 'var:preset|font-size|'.length ) - : undefined; - const fontFamilySlug = fontFamilyValue?.startsWith( - 'var:preset|font-family|' - ) - ? fontFamilyValue.substring( 'var:preset|font-family|'.length ) - : undefined; - updatedStyle.typography = { - ...omit( updatedStyle.typography, [ 'fontFamily' ] ), - fontSize: fontSizeSlug ? undefined : fontSizeValue, - }; - setAttributes( { - style: cleanEmptyObject( updatedStyle ), - fontFamily: fontFamilySlug, - fontSize: fontSizeSlug, - } ); + setAttributes( styleToAttributes( newStyle ) ); }; if ( ! isEnabled ) { diff --git a/packages/components/src/tools-panel/context.ts b/packages/components/src/tools-panel/context.ts index 7b13e59a92acd..d39c49c04ef20 100644 --- a/packages/components/src/tools-panel/context.ts +++ b/packages/components/src/tools-panel/context.ts @@ -18,6 +18,8 @@ export const ToolsPanelContext = createContext< ToolsPanelContextType >( { registerPanelItem: noop, deregisterPanelItem: noop, flagItemCustomization: noop, + registerResetAllFilter: noop, + deregisterResetAllFilter: noop, areAllOptionalControlsHidden: true, } ); diff --git a/packages/components/src/tools-panel/test/index.tsx b/packages/components/src/tools-panel/test/index.tsx index bc91691f9c7d8..218124562a184 100644 --- a/packages/components/src/tools-panel/test/index.tsx +++ b/packages/components/src/tools-panel/test/index.tsx @@ -104,6 +104,8 @@ const panelContext: ToolsPanelContextType = { shouldRenderPlaceholderItems: false, registerPanelItem: jest.fn(), deregisterPanelItem: jest.fn(), + registerResetAllFilter: jest.fn(), + deregisterResetAllFilter: jest.fn(), flagItemCustomization: noop, areAllOptionalControlsHidden: true, }; @@ -971,6 +973,8 @@ describe( 'ToolsPanel', () => { shouldRenderPlaceholderItems: false, registerPanelItem: noop, deregisterPanelItem: noop, + registerResetAllFilter: noop, + deregisterResetAllFilter: noop, flagItemCustomization: noop, areAllOptionalControlsHidden: true, }; diff --git a/packages/components/src/tools-panel/tools-panel-item/hook.ts b/packages/components/src/tools-panel/tools-panel-item/hook.ts index 84572fba451a5..6a7f5ba2bdf60 100644 --- a/packages/components/src/tools-panel/tools-panel-item/hook.ts +++ b/packages/components/src/tools-panel/tools-panel-item/hook.ts @@ -33,6 +33,8 @@ export function useToolsPanelItem( const { panelId: currentPanelId, menuItems, + registerResetAllFilter, + deregisterResetAllFilter, registerPanelItem, deregisterPanelItem, flagItemCustomization, @@ -62,7 +64,6 @@ export function useToolsPanelItem( hasValue: hasValueCallback, isShownByDefault, label, - resetAllFilter: resetAllFilterCallback, panelId, } ); } @@ -83,11 +84,19 @@ export function useToolsPanelItem( hasValueCallback, panelId, previousPanelId, - resetAllFilterCallback, registerPanelItem, deregisterPanelItem, ] ); + useEffect( () => { + registerResetAllFilter( resetAllFilterCallback ); + return () => deregisterResetAllFilter( resetAllFilterCallback ); + }, [ + registerResetAllFilter, + deregisterResetAllFilter, + resetAllFilterCallback, + ] ); + // Note: `label` is used as a key when building menu item state in // `ToolsPanel`. const menuGroup = isShownByDefault ? 'default' : 'optional'; diff --git a/packages/components/src/tools-panel/tools-panel/hook.ts b/packages/components/src/tools-panel/tools-panel/hook.ts index 66b91f0ee671f..70bb8fa3a45bd 100644 --- a/packages/components/src/tools-panel/tools-panel/hook.ts +++ b/packages/components/src/tools-panel/tools-panel/hook.ts @@ -21,6 +21,7 @@ import type { ToolsPanelMenuItems, ToolsPanelMenuItemsConfig, ToolsPanelProps, + ResetAllFilter, } from '../types'; const DEFAULT_COLUMNS = 2; @@ -81,6 +82,9 @@ export function useToolsPanel( // Allow panel items to register themselves. const [ panelItems, setPanelItems ] = useState< ToolsPanelItem[] >( [] ); + const [ resetAllFilters, setResetAllFilters ] = useState< + ResetAllFilter[] + >( [] ); const registerPanelItem = useCallback( ( item: ToolsPanelItem ) => { @@ -123,6 +127,26 @@ export function useToolsPanel( [ setPanelItems ] ); + const registerResetAllFilter = useCallback( + ( newFilter: ResetAllFilter ) => { + setResetAllFilters( ( filters ) => { + return [ ...filters, newFilter ]; + } ); + }, + [ setResetAllFilters ] + ); + + const deregisterResetAllFilter = useCallback( + ( filterToRemove: ResetAllFilter ) => { + setResetAllFilters( ( filters ) => { + return filters.filter( + ( filter ) => filter !== filterToRemove + ); + } ); + }, + [ setResetAllFilters ] + ); + // Manage and share display state of menu items representing child controls. const [ menuItems, setMenuItems ] = useState< ToolsPanelMenuItems >( { default: {}, @@ -237,16 +261,7 @@ export function useToolsPanel( const resetAllItems = useCallback( () => { if ( typeof resetAll === 'function' ) { isResetting.current = true; - - // Collect available reset filters from panel items. - const filters: Array< () => void > = []; - panelItems.forEach( ( item ) => { - if ( item.resetAllFilter ) { - filters.push( item.resetAllFilter ); - } - } ); - - resetAll( filters ); + resetAll( resetAllFilters ); } // Turn off display of all non-default items. @@ -255,7 +270,7 @@ export function useToolsPanel( shouldReset: true, } ); setMenuItems( resetMenuItems ); - }, [ panelItems, resetAll, setMenuItems ] ); + }, [ panelItems, resetAllFilters, resetAll, setMenuItems ] ); // Assist ItemGroup styling when there are potentially hidden placeholder // items by identifying first & last items that are toggled on for display. @@ -277,6 +292,7 @@ export function useToolsPanel( () => ( { areAllOptionalControlsHidden, deregisterPanelItem, + deregisterResetAllFilter, firstDisplayedItem, flagItemCustomization, hasMenuItems: !! panelItems.length, @@ -285,6 +301,8 @@ export function useToolsPanel( menuItems, panelId, registerPanelItem, + registerResetAllFilter, + resetAllFilters, shouldRenderPlaceholderItems, __experimentalFirstVisibleItemClass, __experimentalLastVisibleItemClass, @@ -292,13 +310,16 @@ export function useToolsPanel( [ areAllOptionalControlsHidden, deregisterPanelItem, + deregisterResetAllFilter, firstDisplayedItem, flagItemCustomization, lastDisplayedItem, menuItems, panelId, panelItems, + registerResetAllFilter, registerPanelItem, + resetAllFilters, shouldRenderPlaceholderItems, __experimentalFirstVisibleItemClass, __experimentalLastVisibleItemClass, diff --git a/packages/components/src/tools-panel/types.ts b/packages/components/src/tools-panel/types.ts index 86a8c1579b9b5..2657175caad7a 100644 --- a/packages/components/src/tools-panel/types.ts +++ b/packages/components/src/tools-panel/types.ts @@ -8,7 +8,7 @@ import type { ReactNode } from 'react'; */ import type { HeadingSize } from '../heading/types'; -type ResetAllFilter = ( attributes?: any ) => any; +export type ResetAllFilter = ( attributes?: any ) => any; type ResetAll = ( filters?: ResetAllFilter[] ) => void; export type ToolsPanelProps = { @@ -122,14 +122,6 @@ export type ToolsPanelItem = { * from a shared source. */ panelId?: string | null; - /** - * A `ToolsPanel` will collect each item's `resetAllFilter` and pass an - * array of these functions through to the panel's `resetAll` callback. They - * can then be iterated over to perform additional tasks. - * - * @default noop - */ - resetAllFilter?: ResetAllFilter; }; export type ToolsPanelItemProps = ToolsPanelItem & { @@ -147,6 +139,15 @@ export type ToolsPanelItemProps = ToolsPanelItem & { * menu. */ onSelect?: () => void; + + /** + * A `ToolsPanel` will collect each item's `resetAllFilter` and pass an + * array of these functions through to the panel's `resetAll` callback. They + * can then be iterated over to perform additional tasks. + * + * @default noop + */ + resetAllFilter?: ResetAllFilter; }; export type ToolsPanelMenuItemKey = 'default' | 'optional'; @@ -161,6 +162,8 @@ export type ToolsPanelContext = { hasMenuItems: boolean; registerPanelItem: ( item: ToolsPanelItem ) => void; deregisterPanelItem: ( label: string ) => void; + registerResetAllFilter: ( filter: ResetAllFilter ) => void; + deregisterResetAllFilter: ( filter: ResetAllFilter ) => void; flagItemCustomization: ( label: string, group?: ToolsPanelMenuItemKey From 5b4d2223df265ff16f3de9e246d084e0fb2242d8 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 21 Feb 2023 08:53:28 +0100 Subject: [PATCH 2/6] Remove useless context value --- packages/components/src/tools-panel/tools-panel/hook.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/components/src/tools-panel/tools-panel/hook.ts b/packages/components/src/tools-panel/tools-panel/hook.ts index 70bb8fa3a45bd..65e1e7bd761b4 100644 --- a/packages/components/src/tools-panel/tools-panel/hook.ts +++ b/packages/components/src/tools-panel/tools-panel/hook.ts @@ -302,7 +302,6 @@ export function useToolsPanel( panelId, registerPanelItem, registerResetAllFilter, - resetAllFilters, shouldRenderPlaceholderItems, __experimentalFirstVisibleItemClass, __experimentalLastVisibleItemClass, @@ -319,7 +318,6 @@ export function useToolsPanel( panelItems, registerResetAllFilter, registerPanelItem, - resetAllFilters, shouldRenderPlaceholderItems, __experimentalFirstVisibleItemClass, __experimentalLastVisibleItemClass, From b9c89416c29ba8d97b37f3fcf091076722eef6dd Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 21 Feb 2023 09:03:30 +0100 Subject: [PATCH 3/6] Changes per review --- .../components/global-styles/typography-panel.js | 15 +++++++++++++-- .../src/components/inspector-controls/fill.js | 4 ++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/components/global-styles/typography-panel.js b/packages/block-editor/src/components/global-styles/typography-panel.js index 9f8d6a371be0f..9a928e30227f8 100644 --- a/packages/block-editor/src/components/global-styles/typography-panel.js +++ b/packages/block-editor/src/components/global-styles/typography-panel.js @@ -93,14 +93,24 @@ function useHasTextDecorationControl( settings ) { return settings?.typography?.textDecoration; } -function TypographyToolsPanel( { resetAllFilter, onChange, value, children } ) { +function TypographyToolsPanel( { + resetAllFilter, + onChange, + value, + panelId, + children, +} ) { const resetAll = () => { const updatedValue = resetAllFilter( value ); onChange( updatedValue ); }; return ( - + { children } ); @@ -282,6 +292,7 @@ export default function TypographyPanel( { resetAllFilter={ resetAllFilter } value={ value } onChange={ onChange } + panelId={ panelId } > { hasFontFamilyEnabled && ( { ( fillProps ) => { return ( - { if ( resetAllFilter && registerResetAllFilter ) { From 0eb9c0ce89555cd06c49f96ad955c1a67f525b4f Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 21 Feb 2023 09:57:53 +0100 Subject: [PATCH 4/6] Add unit test --- .../components/src/tools-panel/test/index.tsx | 62 +++++++++++++++++++ .../src/tools-panel/tools-panel-item/hook.ts | 11 +++- 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/packages/components/src/tools-panel/test/index.tsx b/packages/components/src/tools-panel/test/index.tsx index 218124562a184..24f556e72a153 100644 --- a/packages/components/src/tools-panel/test/index.tsx +++ b/packages/components/src/tools-panel/test/index.tsx @@ -1146,6 +1146,68 @@ describe( 'ToolsPanel', () => { // The dropdown toggle no longer has a description. expect( optionsDisplayedIcon ).not.toHaveAccessibleDescription(); } ); + + it( 'should not call reset all for different panelIds', async () => { + const resetItem = jest.fn(); + const resetItemB = jest.fn(); + + const children = ( + <> + true } + panelId="a" + resetAllFilter={ resetItem } + isShownByDefault + > +
Example control
+ + true } + panelId="b" + resetAllFilter={ resetItemB } + isShownByDefault + > +
Alt control
+ + + ); + + const resetAllCallback = ( filters ) => + filters.forEach( ( f ) => f() ); + + const { rerender } = render( + + { children } + + ); + + await openDropdownMenu(); + await selectMenuItem( 'Reset all' ); + expect( resetItem ).toHaveBeenCalled(); + expect( resetItemB ).not.toHaveBeenCalled(); + + resetItem.mockClear(); + + rerender( + + { children } + + ); + + await selectMenuItem( 'Reset all' ); + expect( resetItem ).not.toHaveBeenCalled(); + expect( resetItemB ).toHaveBeenCalled(); + } ); } ); describe( 'reset all button', () => { diff --git a/packages/components/src/tools-panel/tools-panel-item/hook.ts b/packages/components/src/tools-panel/tools-panel-item/hook.ts index 6a7f5ba2bdf60..9acee8ee1d52c 100644 --- a/packages/components/src/tools-panel/tools-panel-item/hook.ts +++ b/packages/components/src/tools-panel/tools-panel-item/hook.ts @@ -89,12 +89,19 @@ export function useToolsPanelItem( ] ); useEffect( () => { - registerResetAllFilter( resetAllFilterCallback ); - return () => deregisterResetAllFilter( resetAllFilterCallback ); + if ( hasMatchingPanel ) { + registerResetAllFilter( resetAllFilterCallback ); + } + return () => { + if ( hasMatchingPanel ) { + deregisterResetAllFilter( resetAllFilterCallback ); + } + }; }, [ registerResetAllFilter, deregisterResetAllFilter, resetAllFilterCallback, + hasMatchingPanel, ] ); // Note: `label` is used as a key when building menu item state in From 6a2f82acf8ee9bb626bb70d786c2add17c6fb826 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Wed, 22 Feb 2023 08:05:59 +0100 Subject: [PATCH 5/6] Fix test post rebase --- packages/components/src/tools-panel/test/index.tsx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/components/src/tools-panel/test/index.tsx b/packages/components/src/tools-panel/test/index.tsx index 24f556e72a153..2a4fdbdf0aa96 100644 --- a/packages/components/src/tools-panel/test/index.tsx +++ b/packages/components/src/tools-panel/test/index.tsx @@ -9,7 +9,10 @@ import userEvent from '@testing-library/user-event'; */ import { ToolsPanel, ToolsPanelContext, ToolsPanelItem } from '../'; import { createSlotFill, Provider as SlotFillProvider } from '../../slot-fill'; -import type { ToolsPanelContext as ToolsPanelContextType } from '../types'; +import type { + ToolsPanelContext as ToolsPanelContextType, + ResetAllFilter, +} from '../types'; const { Fill: ToolsPanelItems, Slot } = createSlotFill( 'ToolsPanelSlot' ); const resetAll = jest.fn(); @@ -1174,8 +1177,9 @@ describe( 'ToolsPanel', () => { ); - const resetAllCallback = ( filters ) => - filters.forEach( ( f ) => f() ); + const resetAllCallback = ( + filters: ResetAllFilter[] | undefined + ) => filters?.forEach( ( f ) => f() ); const { rerender } = render( Date: Wed, 22 Feb 2023 08:07:41 +0100 Subject: [PATCH 6/6] Add changelog entry --- packages/components/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index a180b283a78ff..450c381a83c52 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Enhancements + +- `ToolsPanel`: Separate reset all filter registration from items registration and support global resets ([#48123](https://github.com/WordPress/gutenberg/pull/48123#pullrequestreview-1308386926)). + ## 23.4.0 (2023-02-15) ### Bug Fix