Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Reset All button in the typography panel of the block inspector #48123

Merged
merged 6 commits into from
Feb 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
__experimentalToolsPanelItem as ToolsPanelItem,
} from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { useCallback } from '@wordpress/element';

/**
* Internal dependencies
Expand Down Expand Up @@ -92,8 +93,27 @@ function useHasTextDecorationControl( settings ) {
return settings?.typography?.textDecoration;
}

function TypographyToolsPanel( { ...props } ) {
return <ToolsPanel label={ __( 'Typography' ) } { ...props } />;
function TypographyToolsPanel( {
resetAllFilter,
onChange,
value,
panelId,
children,
} ) {
const resetAll = () => {
const updatedValue = resetAllFilter( value );
onChange( updatedValue );
};

return (
<ToolsPanel
label={ __( 'Typography' ) }
resetAll={ resetAll }
panelId={ panelId }
>
{ children }
</ToolsPanel>
);
}

const DEFAULT_CONTROLS = {
Expand Down Expand Up @@ -260,15 +280,20 @@ 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 (
<Wrapper resetAll={ resetAll }>
<Wrapper
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
resetAllFilter={ resetAllFilter }
value={ value }
onChange={ onChange }
panelId={ panelId }
>
{ hasFontFamilyEnabled && (
<ToolsPanelItem
label={ __( 'Font family' ) }
Expand Down
40 changes: 32 additions & 8 deletions packages/block-editor/src/components/inspector-controls/fill.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
} from '@wordpress/components';
import warning from '@wordpress/warning';
import deprecated from '@wordpress/deprecated';
import { useEffect } from '@wordpress/element';

/**
* Internal dependencies
Expand All @@ -23,6 +24,7 @@ export default function InspectorControlsFill( {
children,
group = 'default',
__experimentalGroup,
resetAllFilter,
} ) {
if ( __experimentalGroup ) {
deprecated(
Expand Down Expand Up @@ -50,18 +52,40 @@ export default function InspectorControlsFill( {
<StyleProvider document={ document }>
<Fill>
{ ( 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 (
<ToolsPanelContext.Provider value={ value }>
{ children }
</ToolsPanelContext.Provider>
<ToolsPanelInspectorControl
fillProps={ fillProps }
children={ children }
resetAllFilter={ resetAllFilter }
/>
);
} }
</Fill>
</StyleProvider>
);
}

function ToolsPanelInspectorControl( { 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 (
<ToolsPanelContext.Provider value={ value }>
{ children }
</ToolsPanelContext.Provider>
);
}
101 changes: 64 additions & 37 deletions packages/block-editor/src/hooks/typography.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 (
<InspectorControls group="typography">{ children }</InspectorControls>
<InspectorControls
group="typography"
resetAllFilter={ attributesResetAllFilter }
>
{ children }
</InspectorControls>
);
}

Expand Down Expand Up @@ -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 ) {
Expand Down
4 changes: 4 additions & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions packages/components/src/tools-panel/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ export const ToolsPanelContext = createContext< ToolsPanelContextType >( {
registerPanelItem: noop,
deregisterPanelItem: noop,
flagItemCustomization: noop,
registerResetAllFilter: noop,
deregisterResetAllFilter: noop,
areAllOptionalControlsHidden: true,
} );

Expand Down
72 changes: 71 additions & 1 deletion packages/components/src/tools-panel/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -104,6 +107,8 @@ const panelContext: ToolsPanelContextType = {
shouldRenderPlaceholderItems: false,
registerPanelItem: jest.fn(),
deregisterPanelItem: jest.fn(),
registerResetAllFilter: jest.fn(),
deregisterResetAllFilter: jest.fn(),
flagItemCustomization: noop,
areAllOptionalControlsHidden: true,
};
Expand Down Expand Up @@ -971,6 +976,8 @@ describe( 'ToolsPanel', () => {
shouldRenderPlaceholderItems: false,
registerPanelItem: noop,
deregisterPanelItem: noop,
registerResetAllFilter: noop,
deregisterResetAllFilter: noop,
flagItemCustomization: noop,
areAllOptionalControlsHidden: true,
};
Expand Down Expand Up @@ -1142,6 +1149,69 @@ 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 = (
<>
<ToolsPanelItem
label="a"
hasValue={ () => true }
panelId="a"
resetAllFilter={ resetItem }
isShownByDefault
>
<div>Example control</div>
</ToolsPanelItem>
<ToolsPanelItem
label="b"
hasValue={ () => true }
panelId="b"
resetAllFilter={ resetItemB }
isShownByDefault
>
<div>Alt control</div>
</ToolsPanelItem>
</>
);

const resetAllCallback = (
filters: ResetAllFilter[] | undefined
) => filters?.forEach( ( f ) => f() );

const { rerender } = render(
<ToolsPanel
{ ...defaultProps }
panelId="a"
resetAll={ resetAllCallback }
>
{ children }
</ToolsPanel>
);

await openDropdownMenu();
await selectMenuItem( 'Reset all' );
expect( resetItem ).toHaveBeenCalled();
expect( resetItemB ).not.toHaveBeenCalled();

resetItem.mockClear();

rerender(
<ToolsPanel
{ ...defaultProps }
panelId="b"
resetAll={ resetAllCallback }
>
{ children }
</ToolsPanel>
);

await selectMenuItem( 'Reset all' );
expect( resetItem ).not.toHaveBeenCalled();
expect( resetItemB ).toHaveBeenCalled();
} );
} );

describe( 'reset all button', () => {
Expand Down
Loading