Skip to content

Commit

Permalink
ToolsPanel: Refine component behaviour (#34530)
Browse files Browse the repository at this point in the history 
* Separate default and optional controls in the menu
* Set min-width on ToolsPanel dropdown menu
* Make reset all a tertiary button as per design
* Add aria-labels to tools panel menu items
* Replace existing check and minus icons
  • Loading branch information
@aaronrobertshaw
aaronrobertshaw authored Sep 14, 2021
1 parent 7c9ecc5 commit 41e1ca8
Show file tree
Hide file tree
Showing 15 changed files with 224 additions and 63 deletions.
2 changes: 1 addition & 1 deletion packages/block-editor/src/components/color-palette/test/__snapshots__/control.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ exports[`ColorPaletteControl matches the snapshot 1`] = `
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M18.3 5.6L9.9 16.9l-4.6-3.4-.9 1.2 5.8 4.3 9.3-12.6z"
d="M16.7 7.1l-6.3 8.5-3.3-2.5-.9 1.2 4.5 3.4L17.9 8z"
/>
</svg>
</div>
Expand Down
6 changes: 3 additions & 3 deletions packages/components/src/color-palette/test/__snapshots__/index.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ exports[`ColorPalette should render a dynamic toolbar of colors 1`] = `
xmlns="http://www.w3.org/2000/svg"
>
<Path
d="M18.3 5.6L9.9 16.9l-4.6-3.4-.9 1.2 5.8 4.3 9.3-12.6z"
d="M16.7 7.1l-6.3 8.5-3.3-2.5-.9 1.2 4.5 3.4L17.9 8z"
/>
</SVG>
}
Expand All @@ -388,10 +388,10 @@ exports[`ColorPalette should render a dynamic toolbar of colors 1`] = `
xmlns="http://www.w3.org/2000/svg"
>
<Path
d="M18.3 5.6L9.9 16.9l-4.6-3.4-.9 1.2 5.8 4.3 9.3-12.6z"
d="M16.7 7.1l-6.3 8.5-3.3-2.5-.9 1.2 4.5 3.4L17.9 8z"
>
<path
d="M18.3 5.6L9.9 16.9l-4.6-3.4-.9 1.2 5.8 4.3 9.3-12.6z"
d="M16.7 7.1l-6.3 8.5-3.3-2.5-.9 1.2 4.5 3.4L17.9 8z"
/>
</Path>
</svg>
Expand Down
4 changes: 2 additions & 2 deletions packages/components/src/mobile/bottom-sheet/stepper-cell/stepper.ios.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { Text, TouchableOpacity, View } from 'react-native';
* WordPress dependencies
*/
import { withPreferredColorScheme } from '@wordpress/compose';
import { Icon, minus, plus } from '@wordpress/icons';
import { Icon, plus, reset } from '@wordpress/icons';

/**
* Internal dependencies
Expand Down Expand Up @@ -46,7 +46,7 @@ function Stepper( {
onPressOut={ onPressOut }
style={ [ buttonStyle, isMinValue ? { opacity: 0.4 } : null ] }
>
<Icon icon={ minus } size={ 24 } color={ buttonStyle.color } />
<Icon icon={ reset } size={ 24 } color={ buttonStyle.color } />
</TouchableOpacity>
<TouchableOpacity
testID={ 'Increment' }
Expand Down
2 changes: 2 additions & 0 deletions packages/components/src/tools-panel/stories/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export const _default = () => {
hasValue={ () => !! width }
label="Width"
onDeselect={ () => setWidth( undefined ) }
isShownByDefault={ true }
>
<UnitControl
label="Width"
Expand All @@ -53,6 +54,7 @@ export const _default = () => {
hasValue={ () => !! height }
label="Height"
onDeselect={ () => setHeight( undefined ) }
isShownByDefault={ true }
>
<UnitControl
label="Height"
Expand Down
4 changes: 4 additions & 0 deletions packages/components/src/tools-panel/styles.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,7 @@ export const ToolsPanelItem = css`
max-width: 100%;
}
`;

export const DropdownMenu = css`
min-width: 200px;
`;
30 changes: 27 additions & 3 deletions packages/components/src/tools-panel/test/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,15 +257,15 @@ describe( 'ToolsPanel', () => {
expect( control ).toBeInTheDocument();
} );

it( 'should prevent panel item rendering when toggled off via menu item', async () => {
it( 'should prevent optional panel item rendering when toggled off via menu item', async () => {
renderPanel();
await selectMenuItem( controlProps.label );
const control = screen.queryByText( 'Example control' );

expect( control ).not.toBeInTheDocument();
} );

it( 'should prevent shown by default item rendering when toggled off via menu item', async () => {
it( 'should continue to render shown by default item after it is toggled off via menu item', async () => {
render(
<ToolsPanel { ...defaultProps }>
<ToolsPanelItem
Expand All @@ -284,7 +284,31 @@ describe( 'ToolsPanel', () => {
await selectMenuItem( controlProps.label );
const resetControl = screen.queryByText( 'Default control' );

expect( resetControl ).not.toBeInTheDocument();
expect( resetControl ).toBeInTheDocument();
} );

it( 'should render appropriate menu groups', async () => {
const { container } = render(
<ToolsPanel { ...defaultProps }>
<ToolsPanelItem
{ ...controlProps }
isShownByDefault={ true }
>
<div>Default control</div>
</ToolsPanelItem>
<ToolsPanelItem { ...altControlProps }>
<div>Optional control</div>
</ToolsPanelItem>
</ToolsPanel>
);
openDropdownMenu();

const menuGroups = container.querySelectorAll(
'.components-menu-group'
);

// Groups should be: default controls, optional controls & reset all.
expect( menuGroups.length ).toEqual( 3 );
} );
} );

Expand Down
123 changes: 100 additions & 23 deletions packages/components/src/tools-panel/tools-panel-header/component.js
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
/**
* WordPress dependencies
*/
import { check, moreVertical } from '@wordpress/icons';
import { __ } from '@wordpress/i18n';
import { check, reset, moreVertical } from '@wordpress/icons';
import { __, sprintf } from '@wordpress/i18n';

/**
* Internal dependencies
Expand All @@ -13,8 +13,87 @@ import MenuItem from '../../menu-item';
import { useToolsPanelHeader } from './hook';
import { contextConnect } from '../../ui/context';

const DefaultControlsGroup = ( { items, onClose, toggleItem } ) => {
if ( ! items.length ) {
return null;
}

return (
<MenuGroup>
{ items.map( ( [ label, hasValue ] ) => {
const icon = hasValue ? reset : check;
const itemLabel = hasValue
? sprintf(
// translators: %s: The name of the control being reset e.g. "Padding".
__( 'Reset %s' ),
label
)
: undefined;

return (
<MenuItem
key={ label }
icon={ icon }
isSelected={ true }
disabled={ ! hasValue }
label={ itemLabel }
onClick={ () => {
toggleItem( label );
onClose();
} }
role="menuitemcheckbox"
>
{ label }
</MenuItem>
);
} ) }
</MenuGroup>
);
};

const OptionalControlsGroup = ( { items, onClose, toggleItem } ) => {
if ( ! items.length ) {
return null;
}

return (
<MenuGroup>
{ items.map( ( [ label, isSelected ] ) => {
const itemLabel = isSelected
? sprintf(
// translators: %s: The name of the control being hidden and reset e.g. "Padding".
__( 'Hide and reset %s' ),
label
)
: sprintf(
// translators: %s: The name of the control to display e.g. "Padding".
__( 'Show %s' ),
label
);

return (
<MenuItem
key={ label }
icon={ isSelected && check }
isSelected={ isSelected }
label={ itemLabel }
onClick={ () => {
toggleItem( label );
onClose();
} }
role="menuitemcheckbox"
>
{ label }
</MenuItem>
);
} ) }
</MenuGroup>
);
};

const ToolsPanelHeader = ( props, forwardedRef ) => {
const {
dropdownMenuClassName,
hasMenuItems,
label: labelText,
menuItems,
Expand All @@ -27,35 +106,33 @@ const ToolsPanelHeader = ( props, forwardedRef ) => {
return null;
}

const defaultItems = Object.entries( menuItems?.default || {} );
const optionalItems = Object.entries( menuItems?.optional || {} );

return (
<h2 { ...headerProps } ref={ forwardedRef }>
{ labelText }
{ hasMenuItems && (
<DropdownMenu icon={ moreVertical } label={ labelText }>
<DropdownMenu
icon={ moreVertical }
label={ labelText }
menuProps={ { className: dropdownMenuClassName } }
>
{ ( { onClose } ) => (
<>
<MenuGroup label={ __( 'Display options' ) }>
{ Object.entries( menuItems ).map(
( [ label, isSelected ] ) => {
return (
<MenuItem
key={ label }
icon={ isSelected && check }
isSelected={ isSelected }
onClick={ () => {
toggleItem( label );
onClose();
} }
role="menuitemcheckbox"
>
{ label }
</MenuItem>
);
}
) }
</MenuGroup>
<DefaultControlsGroup
items={ defaultItems }
onClose={ onClose }
toggleItem={ toggleItem }
/>
<OptionalControlsGroup
items={ optionalItems }
onClose={ onClose }
toggleItem={ toggleItem }
/>
<MenuGroup>
<MenuItem
variant={ 'tertiary' }
onClick={ () => {
resetAll();
onClose();
Expand Down
8 changes: 6 additions & 2 deletions packages/components/src/tools-panel/tools-panel-header/hook.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,15 @@ export function useToolsPanelHeader( props ) {
return cx( styles.ToolsPanelHeader, className );
}, [ className ] );

const { menuItems } = useToolsPanelContext();
const hasMenuItems = !! Object.entries( menuItems ).length;
const dropdownMenuClassName = useMemo( () => {
return cx( styles.DropdownMenu );
}, [] );

const { menuItems, hasMenuItems } = useToolsPanelContext();

return {
...otherProps,
dropdownMenuClassName,
hasMenuItems,
menuItems,
className: classes,
Expand Down
22 changes: 20 additions & 2 deletions packages/components/src/tools-panel/tools-panel-item/hook.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export function useToolsPanelItem( props ) {
menuItems,
registerPanelItem,
deregisterPanelItem,
flagItemCustomization,
isResetting,
} = useToolsPanelContext();

Expand All @@ -55,10 +56,20 @@ export function useToolsPanelItem( props ) {
}, [ panelId ] );

const isValueSet = hasValue();
const wasValueSet = usePrevious( isValueSet );

// If this item represents a default control it will need to notify the
// panel when a custom value has been set.
useEffect( () => {
if ( isShownByDefault && isValueSet && ! wasValueSet ) {
flagItemCustomization( label );
}
}, [ isValueSet, wasValueSet, isShownByDefault, label ] );

// Note: `label` is used as a key when building menu item state in
// `ToolsPanel`.
const isMenuItemChecked = menuItems[ label ];
const menuGroup = isShownByDefault ? 'default' : 'optional';
const isMenuItemChecked = menuItems?.[ menuGroup ]?.[ label ];
const wasMenuItemChecked = usePrevious( isMenuItemChecked );

// Determine if the panel item's corresponding menu is being toggled and
Expand All @@ -77,9 +88,16 @@ export function useToolsPanelItem( props ) {
}
}, [ isMenuItemChecked, wasMenuItemChecked, isValueSet, isResetting ] );

// The item is shown if it is a default control regardless of whether it
// has a value. Optional items are shown when they are checked or have
// a value.
const isShown = isShownByDefault
? menuItems?.[ menuGroup ]?.[ label ] !== undefined
: isMenuItemChecked;

return {
...otherProps,
isShown: isMenuItemChecked,
isShown,
className: classes,
};
}
Loading

0 comments on commit 41e1ca8

Please sign in to comment.