From 795fe998039d8c1e6519a1bdff80d57eef53b428 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Mon, 31 May 2021 19:16:04 +1000 Subject: [PATCH 01/50] Add utils to spacing supports to check or reset values --- packages/block-editor/src/hooks/margin.js | 33 ++++++++++++++++++++++ packages/block-editor/src/hooks/padding.js | 33 ++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/packages/block-editor/src/hooks/margin.js b/packages/block-editor/src/hooks/margin.js index c0cb5c5d4ef5d..db9c6cb7ea5fa 100644 --- a/packages/block-editor/src/hooks/margin.js +++ b/packages/block-editor/src/hooks/margin.js @@ -28,6 +28,38 @@ export function hasMarginSupport( blockType ) { return !! ( true === support || support?.margin ); } +/** + * Checks if there is a current value in the margin block support attributes. + * + * @param {Object} props Block props. + * @return {boolean} Whether or not the block has a margin value set. + */ +export function hasMarginValue( props ) { + return props.attributes.style?.spacing?.margin !== undefined; +} + +/** + * Resets the margin block support attributes. This can be used when disabling + * the margin support controls for a block via a progressive discovery panel. + * + * @param {Object} props Block props. + * @param {Object} props.attributes Block's attributes. + * @param {Object} props.setAttributes Function to set block's attributes. + */ +export function resetMargin( { attributes = {}, setAttributes } ) { + const { style } = attributes; + + setAttributes( { + style: { + ...style, + spacing: { + ...style?.spacing, + margin: undefined, + }, + }, + } ); +} + /** * Custom hook that checks if margin settings have been disabled. * @@ -106,6 +138,7 @@ export function MarginEdit( props ) { label={ __( 'Margin' ) } sides={ sides } units={ units } + allowReset={ false } /> ), diff --git a/packages/block-editor/src/hooks/padding.js b/packages/block-editor/src/hooks/padding.js index 6a29ad7fd7f00..b3fd9bb79d87d 100644 --- a/packages/block-editor/src/hooks/padding.js +++ b/packages/block-editor/src/hooks/padding.js @@ -28,6 +28,38 @@ export function hasPaddingSupport( blockType ) { return !! ( true === support || support?.padding ); } +/** + * Checks if there is a current value in the padding block support attributes. + * + * @param {Object} props Block props. + * @return {boolean} Whether or not the block has a padding value set. + */ +export function hasPaddingValue( props ) { + return props.attributes.style?.spacing?.padding !== undefined; +} + +/** + * Resets the padding block support attributes. This can be used when disabling + * the padding support controls for a block via a progressive discovery panel. + * + * @param {Object} props Block props. + * @param {Object} props.attributes Block's attributes. + * @param {Object} props.setAttributes Function to set block's attributes. + */ +export function resetPadding( { attributes = {}, setAttributes } ) { + const { style } = attributes; + + setAttributes( { + style: { + ...style, + spacing: { + ...style?.spacing, + padding: undefined, + }, + }, + } ); +} + /** * Custom hook that checks if padding settings have been disabled. * @@ -106,6 +138,7 @@ export function PaddingEdit( props ) { label={ __( 'Padding' ) } sides={ sides } units={ units } + allowReset={ false } /> ), From dfb1403e24d6108b7d77e08f720a9ea05fd5581f Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Mon, 31 May 2021 19:20:34 +1000 Subject: [PATCH 02/50] Draft block support panel Adds a new component that handles progressive display of block support controls within an inspector controls panel. It also triggers resetting block support attributes when controls are toggled on/off. --- docs/manifest.json | 6 ++ .../src/block-support-panel/README.md | 1 + .../src/block-support-panel/index.js | 93 +++++++++++++++++++ .../src/block-support-panel/stories/index.js | 1 + .../src/block-support-panel/style.scss | 45 +++++++++ .../src/block-support-panel/test/index.js | 1 + .../src/block-support-panel/title.js | 64 +++++++++++++ packages/components/src/index.js | 1 + packages/components/src/style.scss | 1 + 9 files changed, 213 insertions(+) create mode 100644 packages/components/src/block-support-panel/README.md create mode 100644 packages/components/src/block-support-panel/index.js create mode 100644 packages/components/src/block-support-panel/stories/index.js create mode 100644 packages/components/src/block-support-panel/style.scss create mode 100644 packages/components/src/block-support-panel/test/index.js create mode 100644 packages/components/src/block-support-panel/title.js diff --git a/docs/manifest.json b/docs/manifest.json index 654c6037674e9..ef6392b9be2b2 100644 --- a/docs/manifest.json +++ b/docs/manifest.json @@ -695,6 +695,12 @@ "markdown_source": "../packages/components/src/base-field/README.md", "parent": "components" }, + { + "title": "BlockSupportPanel", + "slug": "block-support-panel", + "markdown_source": "../packages/components/src/block-support-panel/README.md", + "parent": "components" + }, { "title": "BoxControl", "slug": "box-control", diff --git a/packages/components/src/block-support-panel/README.md b/packages/components/src/block-support-panel/README.md new file mode 100644 index 0000000000000..75c21b40a4b37 --- /dev/null +++ b/packages/components/src/block-support-panel/README.md @@ -0,0 +1 @@ + diff --git a/packages/components/src/block-support-panel/index.js b/packages/components/src/block-support-panel/index.js new file mode 100644 index 0000000000000..69e109c0882ad --- /dev/null +++ b/packages/components/src/block-support-panel/index.js @@ -0,0 +1,93 @@ +/** + * External dependencies + */ +import classnames from 'classnames'; + +/** + * WordPress dependencies + */ +import { useEffect, useState } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import BlockSupportPanelTitle from './title'; + +const BlockSupportPanel = ( props ) => { + const { children, className, label: menuLabel, resetAll, title } = props; + const [ menuItems, setMenuItems ] = useState( {} ); + const classes = classnames( 'components-block-support-panel', className ); + + // Collect data to manage control visibility via the panel's dropdown menu. + useEffect( () => { + const items = {}; + + children.forEach( ( child ) => { + items[ child.props.label ] = child.props.hasValue( child.props ); + } ); + + setMenuItems( items ); + }, [] ); + + const getControlByMenuLabel = ( label ) => { + return children.find( ( child ) => child.props.label === label ); + }; + + // Toggles display of a block support control resetting the attributes if + // being turned off. + const toggleControl = ( label ) => { + const isSelected = menuItems[ label ]; + + if ( isSelected ) { + const control = getControlByMenuLabel( label ); + control.props.reset( control.props ); + } + + setMenuItems( { + ...menuItems, + [ label ]: ! isSelected, + } ); + }; + + // Resets all block support attributes for controls represented by the + // menu items. Then turns off their display. + const resetAllControls = () => { + // Reset the block support attributes. + resetAll(); + + // Turn off all the controls in menu. + const resetMenuItems = {}; + + children.forEach( ( child ) => { + resetMenuItems[ child.props.label ] = false; + } ); + + setMenuItems( resetMenuItems ); + }; + + return ( +
+ + { children.map( ( child ) => { + const { label, hasValue } = child.props; + + // Only display the block support controls if the support + // attributes have a value or the controls have be chosen for + // display by the user. + if ( menuItems[ label ] || hasValue( child.props ) ) { + return child; + } + + return null; + } ) } +
+ ); +}; + +export default BlockSupportPanel; diff --git a/packages/components/src/block-support-panel/stories/index.js b/packages/components/src/block-support-panel/stories/index.js new file mode 100644 index 0000000000000..06daf7b01ae57 --- /dev/null +++ b/packages/components/src/block-support-panel/stories/index.js @@ -0,0 +1 @@ +// TODO: Add Storybook entries for block support panel. diff --git a/packages/components/src/block-support-panel/style.scss b/packages/components/src/block-support-panel/style.scss new file mode 100644 index 0000000000000..6644751aecc8e --- /dev/null +++ b/packages/components/src/block-support-panel/style.scss @@ -0,0 +1,45 @@ +.components-block-support-panel { + border: none; + border-top: $border-width solid $gray-200; + padding: $grid-unit-20; + + & + .components-block-support-panel { + margin-top: -1px; + } + + .components-block-support-panel__title { + margin: 0; + display: flex; + align-items: center; + justify-content: space-between; + font-weight: 500; + line-height: normal; + + .components-dropdown-menu { + height: $grid-unit-30; + margin-top: -4px; + margin-bottom: -4px; + } + + .components-dropdown-menu__toggle { + padding: 0; + min-width: $grid-unit-30; + width: $grid-unit-30; + height: $grid-unit-30; + } + + &:not(:last-child) { + padding-bottom: $grid-unit-30; + } + } + + > div { + padding-bottom: 0; + margin-bottom: $grid-unit-30; + max-width: 100%; + + &:last-child { + margin-bottom: $grid-unit-10; + } + } +} diff --git a/packages/components/src/block-support-panel/test/index.js b/packages/components/src/block-support-panel/test/index.js new file mode 100644 index 0000000000000..b3f1c4ff19eb8 --- /dev/null +++ b/packages/components/src/block-support-panel/test/index.js @@ -0,0 +1 @@ +// TODO: Add automatic tests for block support panel. diff --git a/packages/components/src/block-support-panel/title.js b/packages/components/src/block-support-panel/title.js new file mode 100644 index 0000000000000..8383dc10897b7 --- /dev/null +++ b/packages/components/src/block-support-panel/title.js @@ -0,0 +1,64 @@ +/** + * WordPress dependencies + */ +import { check, moreHorizontal } from '@wordpress/icons'; +import { __ } from '@wordpress/i18n'; + +/** + * Internal dependencies + */ +import MenuGroup from '../menu-group'; +import MenuItem from '../menu-item'; +import DropdownMenu from '../dropdown-menu'; + +const BlockSupportPanelTitle = ( props ) => { + const { menuItems = {}, menuLabel, resetAll, title, toggleControl } = props; + + if ( ! title ) { + return null; + } + + return ( +

+ { title } + + { ( { onClose } ) => ( + <> + + { Object.entries( menuItems ).map( + ( [ label, isSelected ] ) => { + return ( + { + toggleControl( label ); + onClose(); + } } + role="menuitemcheckbox" + > + { label } + + ); + } + ) } + + + { + resetAll(); + onClose(); + } } + > + { __( 'Reset all' ) } + + + + ) } + +

+ ); +}; + +export default BlockSupportPanelTitle; diff --git a/packages/components/src/index.js b/packages/components/src/index.js index 86d40d1db38fb..8c2d93f252ec0 100644 --- a/packages/components/src/index.js +++ b/packages/components/src/index.js @@ -22,6 +22,7 @@ export { useAutocompleteProps as __unstableUseAutocompleteProps, } from './autocomplete'; export { default as BaseControl } from './base-control'; +export { default as __experimentalBlockSupportPanel } from './block-support-panel'; export { default as __experimentalBoxControl } from './box-control'; export { default as Button } from './button'; export { default as ButtonGroup } from './button-group'; diff --git a/packages/components/src/style.scss b/packages/components/src/style.scss index 22ae2fb477995..1d1d252ef27bc 100644 --- a/packages/components/src/style.scss +++ b/packages/components/src/style.scss @@ -1,5 +1,6 @@ @import "./animate/style.scss"; @import "./autocomplete/style.scss"; +@import "./block-support-panel/style.scss"; @import "./button-group/style.scss"; @import "./button/style.scss"; @import "./checkbox-control/style.scss"; From 0cb9f82fcdf935b297b080cb83ff197a3405ceec Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Mon, 31 May 2021 19:23:01 +1000 Subject: [PATCH 03/50] Update spacing hook to use new block support panel In addition to using the new block support panel with progressive display of controls, the panel has been renamed Dimensions with the view that future block support for height and width will be added within this panel. --- packages/block-editor/src/hooks/spacing.js | 53 +++++++++++++++++++--- 1 file changed, 46 insertions(+), 7 deletions(-) diff --git a/packages/block-editor/src/hooks/spacing.js b/packages/block-editor/src/hooks/spacing.js index f356a211248f7..2127739ac7587 100644 --- a/packages/block-editor/src/hooks/spacing.js +++ b/packages/block-editor/src/hooks/spacing.js @@ -1,7 +1,7 @@ /** * WordPress dependencies */ -import { PanelBody } from '@wordpress/components'; +import { __experimentalBlockSupportPanel as BlockSupportPanel } from '@wordpress/components'; import { Platform } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; import { getBlockSupport } from '@wordpress/blocks'; @@ -10,10 +10,18 @@ import { getBlockSupport } from '@wordpress/blocks'; * Internal dependencies */ import InspectorControls from '../components/inspector-controls'; -import { MarginEdit, hasMarginSupport, useIsMarginDisabled } from './margin'; +import { + MarginEdit, + hasMarginSupport, + hasMarginValue, + resetMargin, + useIsMarginDisabled, +} from './margin'; import { PaddingEdit, hasPaddingSupport, + hasPaddingValue, + resetPadding, useIsPaddingDisabled, } from './padding'; @@ -34,12 +42,43 @@ export function SpacingPanel( props ) { return null; } + // Callback to reset all block support attributes controlled via this panel. + const resetAll = () => { + const { style } = props.attributes; + + props.setAttributes( { + style: { + ...style, + spacing: { + ...style?.spacing, + margin: undefined, + padding: undefined, + }, + }, + } ); + }; + return ( - - - - - + + + + + ); } From 1b23faddfa85b730db06d35db5263172f42cf09b Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Tue, 1 Jun 2021 11:33:44 +1000 Subject: [PATCH 04/50] Update block support panel to handle false children --- .../components/src/block-support-panel/index.js | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/components/src/block-support-panel/index.js b/packages/components/src/block-support-panel/index.js index 69e109c0882ad..da58dbc07b008 100644 --- a/packages/components/src/block-support-panel/index.js +++ b/packages/components/src/block-support-panel/index.js @@ -18,11 +18,16 @@ const BlockSupportPanel = ( props ) => { const [ menuItems, setMenuItems ] = useState( {} ); const classes = classnames( 'components-block-support-panel', className ); + // If a block support UI has been disabled via theme.json a boolean `false` + // will be passed as a child. This panel is only interested in the children + // to be displayed. + const filteredChildren = children.filter( Boolean ); + // Collect data to manage control visibility via the panel's dropdown menu. useEffect( () => { const items = {}; - children.forEach( ( child ) => { + filteredChildren.forEach( ( child ) => { items[ child.props.label ] = child.props.hasValue( child.props ); } ); @@ -30,7 +35,9 @@ const BlockSupportPanel = ( props ) => { }, [] ); const getControlByMenuLabel = ( label ) => { - return children.find( ( child ) => child.props.label === label ); + return filteredChildren.find( + ( child ) => child.props.label === label + ); }; // Toggles display of a block support control resetting the attributes if @@ -58,7 +65,7 @@ const BlockSupportPanel = ( props ) => { // Turn off all the controls in menu. const resetMenuItems = {}; - children.forEach( ( child ) => { + filteredChildren.forEach( ( child ) => { resetMenuItems[ child.props.label ] = false; } ); @@ -74,8 +81,8 @@ const BlockSupportPanel = ( props ) => { toggleControl={ toggleControl } resetAll={ resetAllControls } /> - { children.map( ( child ) => { - const { label, hasValue } = child.props; + { filteredChildren.map( ( child ) => { + const { label, hasValue } = child?.props || {}; // Only display the block support controls if the support // attributes have a value or the controls have be chosen for From 2487805cf70bfef064489a4d4278611c362493cc Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Tue, 1 Jun 2021 11:35:17 +1000 Subject: [PATCH 05/50] Conditionally display spacing block support controls This is to prevent the block support feature appearing within the new block support panel's menu. --- packages/block-editor/src/hooks/spacing.js | 31 +++++++++++++--------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/packages/block-editor/src/hooks/spacing.js b/packages/block-editor/src/hooks/spacing.js index 2127739ac7587..4d557ab463044 100644 --- a/packages/block-editor/src/hooks/spacing.js +++ b/packages/block-editor/src/hooks/spacing.js @@ -35,6 +35,8 @@ export const SPACING_SUPPORT_KEY = 'spacing'; * @return {WPElement} Inspector controls for spacing support features. */ export function SpacingPanel( props ) { + const isPaddingDisabled = useIsPaddingDisabled( props ); + const isMarginDisabled = useIsMarginDisabled( props ); const isDisabled = useIsSpacingDisabled( props ); const isSupported = hasSpacingSupport( props.name ); @@ -65,19 +67,22 @@ export function SpacingPanel( props ) { title={ __( 'Dimensions' ) } resetAll={ resetAll } > - - + { ! isPaddingDisabled && ( + + ) } + { ! isMarginDisabled && ( + + ) } ); From 696070f6389aa4adfecebc4f6cd877a569562163 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Tue, 1 Jun 2021 13:03:18 +1000 Subject: [PATCH 06/50] Draft README for BlockSupportPanel This will need updating with uploaded screenshots. --- .../src/block-support-panel/README.md | 101 +++++++++++++++++- 1 file changed, 100 insertions(+), 1 deletion(-) diff --git a/packages/components/src/block-support-panel/README.md b/packages/components/src/block-support-panel/README.md index 75c21b40a4b37..63fca61e6d29f 100644 --- a/packages/components/src/block-support-panel/README.md +++ b/packages/components/src/block-support-panel/README.md @@ -1 +1,100 @@ - +# Block Support Panel + +These panels provide progressive discovery options for multiple controls +provided via block supports. + +## Development guidelines + +The `BlockSupportPanel` creates a container with a header including a dropdown +menu. The menu is generated automatically from the panel's children. Each menu +item allows for the display of the corresponding child control to be toggled +on or off. When toggled off the control's reset callback is fired allowing for +the block support provided attribtues to reset. + +Whether a child control is initially displayed or not is dependent upon +whether there has previously been a value set. This is checked via the +`hasValue` function provided through the child's props. + +### Usage + +```jsx +import { __experimentalBlockSupportPanel as BlockSupportPanel } from '@wordpress/components'; +import { + PaddingEdit, + hasPaddingValue, + resetPadding, + useIsPaddingDisabled, +} from './padding'; + + +export function DimensionPanel( props ) { + const isPaddingDisabled = useIsPaddingDisabled( props ); + + const resetAll = () => { + // Reset attributes for all block support features in this panel. + }; + + return ( + + { ! isPaddingDisabled && ( + + ) } + + ); +} +``` + +### Sub-Components + +#### BlockSupportPanelTitle + +This is a simple component to display the panel title and house the dropdown +menu for toggling control display. It is used by the `BlockSupportPanel` +component under the hood, so it does not typically need to be used. + +##### Props +###### resetAll + +A function to call when the `Reset all` menu option is selected. + +- Type: `function` +- Required: Yes + +###### toggleControl + +Callback used to toggle display of an individual block support control and reset +its value if being turned off. + +- Type: `function` +- Required: Yes + +###### menuItems + +This object represents the child controls and their visibility state. It +is built by the parent panel using its children. + +- Type: `Object` +- Required: No + +###### menuLabel + +A label for the dropdown menu. + +- Type: `String` +- Required: No + +###### title + +The panel title to display. + +- Type: `String` +- Required: No From 03c87c49c7f8362746320a18b2a948f1fc6027bd Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Tue, 1 Jun 2021 17:27:06 +1000 Subject: [PATCH 07/50] Make block supports panel handle filtered children --- packages/components/src/block-support-panel/index.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/components/src/block-support-panel/index.js b/packages/components/src/block-support-panel/index.js index da58dbc07b008..a66742503b222 100644 --- a/packages/components/src/block-support-panel/index.js +++ b/packages/components/src/block-support-panel/index.js @@ -16,12 +16,13 @@ import BlockSupportPanelTitle from './title'; const BlockSupportPanel = ( props ) => { const { children, className, label: menuLabel, resetAll, title } = props; const [ menuItems, setMenuItems ] = useState( {} ); - const classes = classnames( 'components-block-support-panel', className ); // If a block support UI has been disabled via theme.json a boolean `false` // will be passed as a child. This panel is only interested in the children // to be displayed. - const filteredChildren = children.filter( Boolean ); + const filteredChildren = Array.isArray( children ) + ? children.filter( Boolean ) + : []; // Collect data to manage control visibility via the panel's dropdown menu. useEffect( () => { @@ -34,6 +35,10 @@ const BlockSupportPanel = ( props ) => { setMenuItems( items ); }, [] ); + if ( filteredChildren.length === 0 ) { + return null; + } + const getControlByMenuLabel = ( label ) => { return filteredChildren.find( ( child ) => child.props.label === label @@ -72,6 +77,8 @@ const BlockSupportPanel = ( props ) => { setMenuItems( resetMenuItems ); }; + const classes = classnames( 'components-block-support-panel', className ); + return (
Date: Tue, 1 Jun 2021 17:53:01 +1000 Subject: [PATCH 08/50] Add initial tests for block supports panel --- .../src/block-support-panel/test/index.js | 204 +++++++++++++++++- 1 file changed, 203 insertions(+), 1 deletion(-) diff --git a/packages/components/src/block-support-panel/test/index.js b/packages/components/src/block-support-panel/test/index.js index b3f1c4ff19eb8..5412591a3c70f 100644 --- a/packages/components/src/block-support-panel/test/index.js +++ b/packages/components/src/block-support-panel/test/index.js @@ -1 +1,203 @@ -// TODO: Add automatic tests for block support panel. +/** + * External dependencies + */ +import { render, screen, fireEvent } from '@testing-library/react'; + +/** + * Internal dependencies + */ +import BlockSupportPanel from '../'; + +// Used to represent the block support provided controls. +const MockControl = ( { children } ) =>
{ children }
; + +const resetAll = jest.fn(); +const hasValue = jest.fn().mockImplementation( ( props ) => { + // Use fudged attribute to determine existence of value for testing. + return props.attributes.value; +} ); + +// Default props for the block supports panel. +const defaultProps = { + label: 'Display options', + title: 'Panel title', + resetAll, +}; + +// Default props for enabled block supports control to be rendered within panel. +const controlProps = { + attributes: { value: true }, + hasValue, + label: 'Example', + reset: jest.fn().mockImplementation( () => { + controlProps.attributes.value = undefined; + } ), +}; + +// Default props without a value for alternate block supports control to be +// rendered within the panel. +const altControlProps = { + attributes: { value: false }, + hasValue, + label: 'Alt', + reset: jest.fn(), +}; + +// Attempts to find the block supports panel via its CSS class. +const getPanel = ( container ) => + container.querySelector( '.components-block-support-panel' ); + +// Renders a default block supports panel including a control that simulates +// block support being disabled for that particular control. +const renderPanel = () => { + return render( + + { false &&
Hidden
} + Example control + Alt control + + ); +}; + +// Helper to find the menu button and simulate a user click. +const openDropdownMenu = () => { + const menuButton = screen.getByLabelText( defaultProps.label ); + fireEvent.click( menuButton ); +}; + +// Opens dropdown then selects the menu item by label before simulating a click. +const selectMenuItem = async ( label ) => { + openDropdownMenu(); + const menuItem = await screen.findByText( label ); + fireEvent.click( menuItem ); +}; + +describe( 'BlockSupportPanel', () => { + describe( 'basic rendering', () => { + it( 'should not render when no children provided', () => { + const { container } = render( + + ); + + expect( getPanel( container ) ).not.toBeInTheDocument(); + } ); + + it( 'should not render when only child has been filtered out', () => { + // This covers case where children prop is not an array. + const { container } = render( + + { false && Should not show } + + ); + + expect( getPanel( container ) ).not.toBeInTheDocument(); + } ); + + it( 'should not render when all children have been filtered out', () => { + const { container } = render( + + { false && Should not show } + { false && Not shown either } + + ); + + expect( getPanel( container ) ).not.toBeInTheDocument(); + } ); + + it( 'should render panel when at least one child', () => { + const { container } = renderPanel(); + + expect( getPanel( container ) ).toBeInTheDocument(); + } ); + + it( 'should render display options menu', () => { + renderPanel(); + + const menuButton = screen.getByLabelText( defaultProps.label ); + expect( menuButton ).toBeInTheDocument(); + } ); + + it( 'should render reset all item in menu', async () => { + renderPanel(); + openDropdownMenu(); + + const resetAllItem = await screen.findByRole( 'menuitem' ); + + expect( resetAllItem ).toBeInTheDocument(); + } ); + + it( 'should render display options menu items correctly', async () => { + renderPanel(); + openDropdownMenu(); + + const menuItems = await screen.findAllByRole( 'menuitemcheckbox' ); + + expect( menuItems.length ).toEqual( 2 ); + expect( menuItems[ 0 ] ).toHaveAttribute( 'aria-checked', 'true' ); + expect( menuItems[ 1 ] ).toHaveAttribute( 'aria-checked', 'false' ); + } ); + + it( 'should render panel title', () => { + renderPanel(); + const title = screen.getByText( defaultProps.title ); + + expect( title ).toBeInTheDocument(); + } ); + } ); + + describe( 'conditional rendering of inner controls', () => { + it( 'should render child control when it has a value', () => { + renderPanel(); + + const exampleControl = screen.getByText( 'Example control' ); + const altControl = screen.queryByText( 'Alt control' ); + + expect( exampleControl ).toBeInTheDocument(); + expect( altControl ).not.toBeInTheDocument(); + } ); + + it( 'should render child control when corresponding menu is selected', async () => { + renderPanel(); + await selectMenuItem( altControlProps.label ); + const control = await screen.findByText( 'Alt control' ); + + expect( control ).toBeInTheDocument(); + } ); + + it( 'should prevent child rendering when toggled off via menu', async () => { + renderPanel(); + await selectMenuItem( controlProps.label ); + const control = screen.queryByText( 'Example control' ); + + expect( control ).not.toBeInTheDocument(); + } ); + } ); + + describe( 'reset callbacks on menu item selection', () => { + beforeEach( () => { + jest.clearAllMocks(); + controlProps.attributes.value = true; + } ); + + it( 'should call reset callback when menu item is toggled off', async () => { + renderPanel(); + await selectMenuItem( controlProps.label ); + + expect( controlProps.reset ).toHaveBeenCalledTimes( 1 ); + } ); + + it( 'should not call reset callback when menu item is toggled on', async () => { + renderPanel(); + await selectMenuItem( altControlProps.label ); + + expect( altControlProps.reset ).not.toHaveBeenCalled(); + } ); + + it( 'should call resetAll callback when its menu item is selected', async () => { + renderPanel(); + await selectMenuItem( 'Reset all' ); + + expect( resetAll ).toHaveBeenCalledTimes( 1 ); + } ); + } ); +} ); From 7fb7e8a6f1b2bf41904c0e5149ef7cba30e995eb Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Tue, 1 Jun 2021 18:36:08 +1000 Subject: [PATCH 09/50] Add story for block support panel --- .../src/block-support-panel/stories/index.js | 70 ++++++++++++++++++- .../src/block-support-panel/style.scss | 5 ++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/packages/components/src/block-support-panel/stories/index.js b/packages/components/src/block-support-panel/stories/index.js index 06daf7b01ae57..0b14422760375 100644 --- a/packages/components/src/block-support-panel/stories/index.js +++ b/packages/components/src/block-support-panel/stories/index.js @@ -1 +1,69 @@ -// TODO: Add Storybook entries for block support panel. +/** + * External dependencies + */ +import styled from '@emotion/styled'; + +/** + * WordPress dependencies + */ +import { useState } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import BlockSupportPanel from '../'; +import Panel from '../../panel'; +import UnitControl from '../../unit-control'; + +export default { + title: 'Components/BlockSupportPanel', + component: BlockSupportPanel, +}; + +export const _default = () => { + const [ height, setHeight ] = useState(); + const [ width, setWidth ] = useState(); + + const resetAll = () => { + setHeight( undefined ); + setWidth( undefined ); + }; + + return ( + + + + !! height } + label="Height" + reset={ () => setHeight( undefined ) } + value={ height } + onChange={ ( next ) => setHeight( next ) } + /> + !! width } + label="Width" + reset={ () => setWidth( undefined ) } + value={ width } + onChange={ ( next ) => setWidth( next ) } + /> + + + + ); +}; + +function PlaceholderControl( { label, value, onChange } ) { + return ( + + ); +} + +const PanelWrapperView = styled.div` + max-width: 232px; + font-size: 13px; +`; diff --git a/packages/components/src/block-support-panel/style.scss b/packages/components/src/block-support-panel/style.scss index 6644751aecc8e..67cbd408a530c 100644 --- a/packages/components/src/block-support-panel/style.scss +++ b/packages/components/src/block-support-panel/style.scss @@ -7,6 +7,10 @@ margin-top: -1px; } + &:first-child { + margin-top: -1px; + } + .components-block-support-panel__title { margin: 0; display: flex; @@ -14,6 +18,7 @@ justify-content: space-between; font-weight: 500; line-height: normal; + font-size: inherit; .components-dropdown-menu { height: $grid-unit-30; From 62d789508e3256a32eaf839eecc7a76a0080deea Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Wed, 2 Jun 2021 16:10:55 +1000 Subject: [PATCH 10/50] Rename spacing support to dimensions This does NOT change the shape of the block attributes style object. Spacing related block support styles such as margin and padding remain under `style.spacing.padding` etc. --- .../{spacing.php => dimensions.php} | 38 +++++++++++++++---- lib/load.php | 2 +- .../src/hooks/{spacing.js => dimensions.js} | 16 ++++---- packages/block-editor/src/hooks/index.js | 2 +- packages/block-editor/src/hooks/margin.js | 2 +- packages/block-editor/src/hooks/padding.js | 2 +- packages/block-editor/src/hooks/style.js | 4 +- 7 files changed, 44 insertions(+), 22 deletions(-) rename lib/block-supports/{spacing.php => dimensions.php} (71%) rename packages/block-editor/src/hooks/{spacing.js => dimensions.js} (87%) diff --git a/lib/block-supports/spacing.php b/lib/block-supports/dimensions.php similarity index 71% rename from lib/block-supports/spacing.php rename to lib/block-supports/dimensions.php index 592949f0473b8..b51682a34e016 100644 --- a/lib/block-supports/spacing.php +++ b/lib/block-supports/dimensions.php @@ -1,6 +1,6 @@ attributes ) { $block_type->attributes = array(); } - if ( $has_spacing_support && ! array_key_exists( 'style', $block_type->attributes ) ) { + // Check for existing style attribute definition e.g. from block.json. + if ( array_key_exists( 'style', $block_type->attributes ) ) { + return; + } + + $has_spacing_support = gutenberg_block_has_support( $block_type, array( 'spacing' ), false ); + // Future block supports such as height & width will be added here. + + if ( $has_spacing_support ) { $block_type->attributes['style'] = array( 'type' => 'object', ); } } +/** + * Add CSS classes for block dimensions to the incoming attributes array. + * This will be applied to the block markup in the front-end. + * + * @param WP_Block_Type $block_type Block Type. + * @param array $block_attributes Block attributes. + * + * @return array Block spacing CSS classes and inline styles. + */ +function gutenberg_apply_dimensions_support( $block_type, $block_attributes ) { + $spacing_styles = gutenberg_apply_spacing_support( $block_type, $block_attributes ); + // Future block supports such as height and width will be added here. + + return $spacing_styles; +} + /** * Add CSS classes for block spacing to the incoming attributes array. * This will be applied to the block markup in the front-end. @@ -88,9 +110,9 @@ function gutenberg_skip_spacing_serialization( $block_type ) { // Register the block support. WP_Block_Supports::get_instance()->register( - 'spacing', + 'dimensions', array( - 'register_attribute' => 'gutenberg_register_spacing_support', - 'apply' => 'gutenberg_apply_spacing_support', + 'register_attribute' => 'gutenberg_register_dimensions_support', + 'apply' => 'gutenberg_apply_dimensions_support', ) ); diff --git a/lib/load.php b/lib/load.php index d6cf4449b2168..3e9292efbf57b 100644 --- a/lib/load.php +++ b/lib/load.php @@ -128,5 +128,5 @@ function gutenberg_is_experiment_enabled( $name ) { require __DIR__ . '/block-supports/custom-classname.php'; require __DIR__ . '/block-supports/border.php'; require __DIR__ . '/block-supports/layout.php'; -require __DIR__ . '/block-supports/spacing.php'; +require __DIR__ . '/block-supports/dimensions.php'; require __DIR__ . '/block-supports/duotone.php'; diff --git a/packages/block-editor/src/hooks/spacing.js b/packages/block-editor/src/hooks/dimensions.js similarity index 87% rename from packages/block-editor/src/hooks/spacing.js rename to packages/block-editor/src/hooks/dimensions.js index 4d557ab463044..14ed1663b51b8 100644 --- a/packages/block-editor/src/hooks/spacing.js +++ b/packages/block-editor/src/hooks/dimensions.js @@ -28,17 +28,17 @@ import { export const SPACING_SUPPORT_KEY = 'spacing'; /** - * Inspector controls for spacing support. + * Inspector controls for dimensions support. * * @param {Object} props Block props. * * @return {WPElement} Inspector controls for spacing support features. */ -export function SpacingPanel( props ) { +export function DimensionsPanel( props ) { const isPaddingDisabled = useIsPaddingDisabled( props ); const isMarginDisabled = useIsMarginDisabled( props ); - const isDisabled = useIsSpacingDisabled( props ); - const isSupported = hasSpacingSupport( props.name ); + const isDisabled = useIsDimensionsDisabled( props ); + const isSupported = hasDimensionsSupport( props.name ); if ( isDisabled || ! isSupported ) { return null; @@ -89,13 +89,13 @@ export function SpacingPanel( props ) { } /** - * Determine whether there is block support for padding or margins. + * Determine whether there is dimensions related block support. * * @param {string} blockName Block name. * * @return {boolean} Whether there is support. */ -export function hasSpacingSupport( blockName ) { +export function hasDimensionsSupport( blockName ) { if ( Platform.OS !== 'web' ) { return false; } @@ -104,13 +104,13 @@ export function hasSpacingSupport( blockName ) { } /** - * Determines whether spacing support has been disabled. + * Determines whether dimensions support has been disabled. * * @param {Object} props Block properties. * * @return {boolean} If spacing support is completely disabled. */ -const useIsSpacingDisabled = ( props = {} ) => { +const useIsDimensionsDisabled = ( props = {} ) => { const paddingDisabled = useIsPaddingDisabled( props ); const marginDisabled = useIsMarginDisabled( props ); diff --git a/packages/block-editor/src/hooks/index.js b/packages/block-editor/src/hooks/index.js index c0e5c1b5f8bb8..e8a976277f970 100644 --- a/packages/block-editor/src/hooks/index.js +++ b/packages/block-editor/src/hooks/index.js @@ -12,7 +12,7 @@ import './font-size'; import './border-color'; import './layout'; -export { useCustomSides } from './spacing'; +export { useCustomSides } from './dimensions'; export { getBorderClassesAndStyles, useBorderProps } from './use-border-props'; export { getColorClassesAndStyles, useColorProps } from './use-color-props'; export { getSpacingClassesAndStyles } from './use-spacing-props'; diff --git a/packages/block-editor/src/hooks/margin.js b/packages/block-editor/src/hooks/margin.js index db9c6cb7ea5fa..b734191a35b12 100644 --- a/packages/block-editor/src/hooks/margin.js +++ b/packages/block-editor/src/hooks/margin.js @@ -13,7 +13,7 @@ import { * Internal dependencies */ import useSetting from '../components/use-setting'; -import { SPACING_SUPPORT_KEY, useCustomSides } from './spacing'; +import { SPACING_SUPPORT_KEY, useCustomSides } from './dimensions'; import { cleanEmptyObject } from './utils'; /** diff --git a/packages/block-editor/src/hooks/padding.js b/packages/block-editor/src/hooks/padding.js index b3fd9bb79d87d..c8392f04ec7b7 100644 --- a/packages/block-editor/src/hooks/padding.js +++ b/packages/block-editor/src/hooks/padding.js @@ -13,7 +13,7 @@ import { * Internal dependencies */ import useSetting from '../components/use-setting'; -import { SPACING_SUPPORT_KEY, useCustomSides } from './spacing'; +import { SPACING_SUPPORT_KEY, useCustomSides } from './dimensions'; import { cleanEmptyObject } from './utils'; /** diff --git a/packages/block-editor/src/hooks/style.js b/packages/block-editor/src/hooks/style.js index 8c5d6ce1872f9..87bf4685d50d4 100644 --- a/packages/block-editor/src/hooks/style.js +++ b/packages/block-editor/src/hooks/style.js @@ -37,7 +37,7 @@ import { TYPOGRAPHY_SUPPORT_KEY, TYPOGRAPHY_SUPPORT_KEYS, } from './typography'; -import { SPACING_SUPPORT_KEY, SpacingPanel } from './spacing'; +import { SPACING_SUPPORT_KEY, DimensionsPanel } from './dimensions'; import useDisplayBlockControls from '../components/use-display-block-controls'; const styleSupportKeys = [ @@ -232,7 +232,7 @@ export const withBlockControls = createHigherOrderComponent( - + ) } From 5f7b39fdd23d1b621628b746e73a5ee37ad6088d Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Wed, 2 Jun 2021 16:17:27 +1000 Subject: [PATCH 11/50] Rename spacing support to dimensions in FSE --- .../{spacing-panel.js => dimensions-panel.js} | 6 +++--- .../src/components/sidebar/global-styles-sidebar.js | 13 ++++++++----- 2 files changed, 11 insertions(+), 8 deletions(-) rename packages/edit-site/src/components/sidebar/{spacing-panel.js => dimensions-panel.js} (94%) diff --git a/packages/edit-site/src/components/sidebar/spacing-panel.js b/packages/edit-site/src/components/sidebar/dimensions-panel.js similarity index 94% rename from packages/edit-site/src/components/sidebar/spacing-panel.js rename to packages/edit-site/src/components/sidebar/dimensions-panel.js index cf224ee63fc25..1eed695fde8a6 100644 --- a/packages/edit-site/src/components/sidebar/spacing-panel.js +++ b/packages/edit-site/src/components/sidebar/dimensions-panel.js @@ -14,7 +14,7 @@ import { __experimentalUseCustomSides as useCustomSides } from '@wordpress/block */ import { useSetting } from '../editor/utils'; -export function useHasSpacingPanel( context ) { +export function useHasDimensionsPanel( context ) { const hasPadding = useHasPadding( context ); const hasMargin = useHasMargin( context ); @@ -61,7 +61,7 @@ function splitStyleValue( value ) { return value; } -export default function SpacingPanel( { context, getStyle, setStyle } ) { +export default function DimensionsPanel( { context, getStyle, setStyle } ) { const { name } = context; const showPaddingControl = useHasPadding( context ); const showMarginControl = useHasMargin( context ); @@ -92,7 +92,7 @@ export default function SpacingPanel( { context, getStyle, setStyle } ) { }; return ( - + { showPaddingControl && ( ) } - { hasSpacingPanel && ( - Date: Wed, 2 Jun 2021 17:29:31 +1000 Subject: [PATCH 12/50] Update GlobalStyles dimensions panel --- .../src/block-support-panel/style.scss | 19 +++++++------ .../components/sidebar/dimensions-panel.js | 27 ++++++++++++++++--- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/packages/components/src/block-support-panel/style.scss b/packages/components/src/block-support-panel/style.scss index 67cbd408a530c..407825312cc65 100644 --- a/packages/components/src/block-support-panel/style.scss +++ b/packages/components/src/block-support-panel/style.scss @@ -2,14 +2,7 @@ border: none; border-top: $border-width solid $gray-200; padding: $grid-unit-20; - - & + .components-block-support-panel { - margin-top: -1px; - } - - &:first-child { - margin-top: -1px; - } + margin-top: -1px; .components-block-support-panel__title { margin: 0; @@ -48,3 +41,13 @@ } } } + +.edit-site { + .components-block-support-panel { + border-bottom: $border-width solid $gray-200; + } + + .components-block-support-panel + .components-panel__body { + margin-top: -1px; + } +} diff --git a/packages/edit-site/src/components/sidebar/dimensions-panel.js b/packages/edit-site/src/components/sidebar/dimensions-panel.js index 1eed695fde8a6..c0c09cde6c5b2 100644 --- a/packages/edit-site/src/components/sidebar/dimensions-panel.js +++ b/packages/edit-site/src/components/sidebar/dimensions-panel.js @@ -3,8 +3,8 @@ */ import { __ } from '@wordpress/i18n'; import { + __experimentalBlockSupportPanel as BlockSupportPanel, __experimentalBoxControl as BoxControl, - PanelBody, __experimentalUseCustomUnits as useCustomUnits, } from '@wordpress/components'; import { __experimentalUseCustomSides as useCustomSides } from '@wordpress/block-editor'; @@ -82,6 +82,9 @@ export default function DimensionsPanel( { context, getStyle, setStyle } ) { const padding = filterValuesBySides( newPaddingValues, paddingSides ); setStyle( name, 'padding', padding ); }; + const resetPaddingValue = () => setPaddingValues( {} ); + const hasPaddingValue = () => + paddingValues && Object.keys( paddingValues ).length; const marginValues = splitStyleValue( getStyle( name, 'margin' ) ); const marginSides = useCustomSides( name, 'margin' ); @@ -90,9 +93,21 @@ export default function DimensionsPanel( { context, getStyle, setStyle } ) { const margin = filterValuesBySides( newMarginValues, marginSides ); setStyle( name, 'margin', margin ); }; + const resetMarginValue = () => setMarginValues( {} ); + const hasMarginValue = () => + marginValues && Object.keys( marginValues ).length; + + const resetAll = () => { + resetPaddingValue(); + resetMarginValue(); + }; return ( - + { showPaddingControl && ( ) } { showMarginControl && ( @@ -109,8 +127,11 @@ export default function DimensionsPanel( { context, getStyle, setStyle } ) { label={ __( 'Margin' ) } sides={ marginSides } units={ units } + hasValue={ hasMarginValue } + reset={ resetMarginValue } + allowReset={ false } /> ) } - + ); } From dd015b00b6817ad5dc52a63ea1db15561d9d54b6 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Fri, 4 Jun 2021 20:51:48 +1000 Subject: [PATCH 13/50] Add means to handle default controls in block support panel --- packages/block-editor/src/hooks/dimensions.js | 7 +++ .../src/block-support-panel/index.js | 54 ++++++++++++++----- .../components/sidebar/dimensions-panel.js | 2 + 3 files changed, 51 insertions(+), 12 deletions(-) diff --git a/packages/block-editor/src/hooks/dimensions.js b/packages/block-editor/src/hooks/dimensions.js index 14ed1663b51b8..dd28feb901959 100644 --- a/packages/block-editor/src/hooks/dimensions.js +++ b/packages/block-editor/src/hooks/dimensions.js @@ -44,6 +44,11 @@ export function DimensionsPanel( props ) { return null; } + const defaultSpacingControls = getBlockSupport( props.name, [ + SPACING_SUPPORT_KEY, + '__experimentalDefaultControls', + ] ); + // Callback to reset all block support attributes controlled via this panel. const resetAll = () => { const { style } = props.attributes; @@ -73,6 +78,7 @@ export function DimensionsPanel( props ) { hasValue={ hasPaddingValue } label={ __( 'Padding' ) } reset={ resetPadding } + isShownByDefault={ defaultSpacingControls?.padding } /> ) } { ! isMarginDisabled && ( @@ -81,6 +87,7 @@ export function DimensionsPanel( props ) { hasValue={ hasMarginValue } label={ __( 'Margin' ) } reset={ resetMargin } + isShownByDefault={ defaultSpacingControls?.margin } /> ) } diff --git a/packages/components/src/block-support-panel/index.js b/packages/components/src/block-support-panel/index.js index a66742503b222..3ed4367c233a1 100644 --- a/packages/components/src/block-support-panel/index.js +++ b/packages/components/src/block-support-panel/index.js @@ -16,6 +16,7 @@ import BlockSupportPanelTitle from './title'; const BlockSupportPanel = ( props ) => { const { children, className, label: menuLabel, resetAll, title } = props; const [ menuItems, setMenuItems ] = useState( {} ); + const [ defaultControls, setDefaultControls ] = useState( {} ); // If a block support UI has been disabled via theme.json a boolean `false` // will be passed as a child. This panel is only interested in the children @@ -24,17 +25,48 @@ const BlockSupportPanel = ( props ) => { ? children.filter( Boolean ) : []; - // Collect data to manage control visibility via the panel's dropdown menu. + // Collect which controls have custom values. Used to update menu state to + // reflect customization for controls that display by default / always show. + const customizedChildren = filteredChildren.map( ( child ) => + child.props.hasValue( child.props ) ? child.props.label : undefined + ); + + // On first render determine initial menu state and which controls should + // always display by default. useEffect( () => { const items = {}; + const defaults = {}; filteredChildren.forEach( ( child ) => { items[ child.props.label ] = child.props.hasValue( child.props ); + defaults[ child.props.label ] = child.props.isShownByDefault; } ); setMenuItems( items ); + setDefaultControls( defaults ); }, [] ); + // As the default controls are visible all the time. Reflect their + // customizations in the menu items' selected state. + useEffect( () => { + const menuLabels = Object.keys( menuItems ); + + // Skip if no children or menu state not initialized. + if ( menuLabels.length === 0 ) { + return; + } + + const updatedItems = { ...menuItems }; + + menuLabels.forEach( ( label ) => { + if ( defaultControls[ label ] ) { + updatedItems[ label ] = customizedChildren.includes( label ); + } + } ); + + setMenuItems( updatedItems ); + }, customizedChildren ); + if ( filteredChildren.length === 0 ) { return null; } @@ -45,8 +77,9 @@ const BlockSupportPanel = ( props ) => { ); }; - // Toggles display of a block support control resetting the attributes if - // being turned off. + // Toggles the customized state of the block support control and its display + // if it isn't to be displayed by default. When toggling off a control its + // associated block attribute is reset. const toggleControl = ( label ) => { const isSelected = menuItems[ label ]; @@ -89,16 +122,13 @@ const BlockSupportPanel = ( props ) => { resetAll={ resetAllControls } /> { filteredChildren.map( ( child ) => { - const { label, hasValue } = child?.props || {}; - - // Only display the block support controls if the support - // attributes have a value or the controls have be chosen for - // display by the user. - if ( menuItems[ label ] || hasValue( child.props ) ) { - return child; - } + // Only display the block support control if it is toggled + // on in the menu or is set to display by default. + const isShown = + menuItems[ child.props.label ] || + defaultControls[ child.props.label ]; - return null; + return isShown ? child : null; } ) }
); diff --git a/packages/edit-site/src/components/sidebar/dimensions-panel.js b/packages/edit-site/src/components/sidebar/dimensions-panel.js index c0c09cde6c5b2..d6df96c0a83ed 100644 --- a/packages/edit-site/src/components/sidebar/dimensions-panel.js +++ b/packages/edit-site/src/components/sidebar/dimensions-panel.js @@ -118,6 +118,7 @@ export default function DimensionsPanel( { context, getStyle, setStyle } ) { hasValue={ hasPaddingValue } reset={ resetPaddingValue } allowReset={ false } + isShownByDefault={ true } /> ) } { showMarginControl && ( @@ -130,6 +131,7 @@ export default function DimensionsPanel( { context, getStyle, setStyle } ) { hasValue={ hasMarginValue } reset={ resetMarginValue } allowReset={ false } + isShownByDefault={ true } /> ) } From 659a79e67e97166bbe3913b20bedb96485a61d27 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Mon, 7 Jun 2021 11:05:26 +1000 Subject: [PATCH 14/50] Change default controls display in block supports panel Default controls now show as checked when panel initially displayed. These controls can also now be toggled off from display. --- .../src/block-support-panel/index.js | 52 +++---------------- 1 file changed, 8 insertions(+), 44 deletions(-) diff --git a/packages/components/src/block-support-panel/index.js b/packages/components/src/block-support-panel/index.js index 3ed4367c233a1..b86b0118f19e3 100644 --- a/packages/components/src/block-support-panel/index.js +++ b/packages/components/src/block-support-panel/index.js @@ -16,7 +16,6 @@ import BlockSupportPanelTitle from './title'; const BlockSupportPanel = ( props ) => { const { children, className, label: menuLabel, resetAll, title } = props; const [ menuItems, setMenuItems ] = useState( {} ); - const [ defaultControls, setDefaultControls ] = useState( {} ); // If a block support UI has been disabled via theme.json a boolean `false` // will be passed as a child. This panel is only interested in the children @@ -25,48 +24,20 @@ const BlockSupportPanel = ( props ) => { ? children.filter( Boolean ) : []; - // Collect which controls have custom values. Used to update menu state to - // reflect customization for controls that display by default / always show. - const customizedChildren = filteredChildren.map( ( child ) => - child.props.hasValue( child.props ) ? child.props.label : undefined - ); - - // On first render determine initial menu state and which controls should - // always display by default. + // On first render determine initial menu state. Default controls will + // initially display and have a check mark beside their menu item regardless + // of whether they have a value. useEffect( () => { const items = {}; - const defaults = {}; filteredChildren.forEach( ( child ) => { - items[ child.props.label ] = child.props.hasValue( child.props ); - defaults[ child.props.label ] = child.props.isShownByDefault; + const { hasValue, isShownByDefault, label } = child.props; + items[ label ] = isShownByDefault || hasValue( child.props ); } ); setMenuItems( items ); - setDefaultControls( defaults ); }, [] ); - // As the default controls are visible all the time. Reflect their - // customizations in the menu items' selected state. - useEffect( () => { - const menuLabels = Object.keys( menuItems ); - - // Skip if no children or menu state not initialized. - if ( menuLabels.length === 0 ) { - return; - } - - const updatedItems = { ...menuItems }; - - menuLabels.forEach( ( label ) => { - if ( defaultControls[ label ] ) { - updatedItems[ label ] = customizedChildren.includes( label ); - } - } ); - - setMenuItems( updatedItems ); - }, customizedChildren ); - if ( filteredChildren.length === 0 ) { return null; } @@ -77,9 +48,8 @@ const BlockSupportPanel = ( props ) => { ); }; - // Toggles the customized state of the block support control and its display - // if it isn't to be displayed by default. When toggling off a control its - // associated block attribute is reset. + // Toggles the display of the block support control and resets its + // associated block attribute via the control's reset callback prop. const toggleControl = ( label ) => { const isSelected = menuItems[ label ]; @@ -122,13 +92,7 @@ const BlockSupportPanel = ( props ) => { resetAll={ resetAllControls } /> { filteredChildren.map( ( child ) => { - // Only display the block support control if it is toggled - // on in the menu or is set to display by default. - const isShown = - menuItems[ child.props.label ] || - defaultControls[ child.props.label ]; - - return isShown ? child : null; + return menuItems[ child.props.label ] ? child : null; } ) } ); From 213d250e88e636ef542c0f103a5df3ab4e8122db Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Tue, 8 Jun 2021 09:32:24 +1000 Subject: [PATCH 15/50] Make default controls still show after reset all --- packages/components/src/block-support-panel/index.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/components/src/block-support-panel/index.js b/packages/components/src/block-support-panel/index.js index b86b0118f19e3..789e28cd64ab5 100644 --- a/packages/components/src/block-support-panel/index.js +++ b/packages/components/src/block-support-panel/index.js @@ -16,6 +16,7 @@ import BlockSupportPanelTitle from './title'; const BlockSupportPanel = ( props ) => { const { children, className, label: menuLabel, resetAll, title } = props; const [ menuItems, setMenuItems ] = useState( {} ); + const [ defaultControls, setDefaultControls ] = useState( {} ); // If a block support UI has been disabled via theme.json a boolean `false` // will be passed as a child. This panel is only interested in the children @@ -29,13 +30,16 @@ const BlockSupportPanel = ( props ) => { // of whether they have a value. useEffect( () => { const items = {}; + const defaults = {}; filteredChildren.forEach( ( child ) => { const { hasValue, isShownByDefault, label } = child.props; items[ label ] = isShownByDefault || hasValue( child.props ); + defaults[ label ] = isShownByDefault; } ); setMenuItems( items ); + setDefaultControls( defaults ); }, [] ); if ( filteredChildren.length === 0 ) { @@ -70,11 +74,11 @@ const BlockSupportPanel = ( props ) => { // Reset the block support attributes. resetAll(); - // Turn off all the controls in menu. + // Turn off menu items unless they are to display by default. const resetMenuItems = {}; - filteredChildren.forEach( ( child ) => { - resetMenuItems[ child.props.label ] = false; + filteredChildren.forEach( ( { props: { label } } ) => { + resetMenuItems[ label ] = defaultControls[ label ]; } ); setMenuItems( resetMenuItems ); From a96dc3563e1119ed74da1303362c9275f7157547 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Wed, 9 Jun 2021 10:39:07 +1000 Subject: [PATCH 16/50] Change back to default controls always displaying --- .../src/block-support-panel/index.js | 53 +++++++++++++++---- 1 file changed, 42 insertions(+), 11 deletions(-) diff --git a/packages/components/src/block-support-panel/index.js b/packages/components/src/block-support-panel/index.js index 789e28cd64ab5..53c4469396d89 100644 --- a/packages/components/src/block-support-panel/index.js +++ b/packages/components/src/block-support-panel/index.js @@ -25,23 +25,47 @@ const BlockSupportPanel = ( props ) => { ? children.filter( Boolean ) : []; - // On first render determine initial menu state. Default controls will - // initially display and have a check mark beside their menu item regardless - // of whether they have a value. + // Collect which controls have custom values. Used to update menu state to + // reflect customization for controls that display by default / always show. + const customizedChildren = filteredChildren.map( ( child ) => + child.props.hasValue( child.props ) ? child.props.label : undefined + ); + + // On first render determine initial menu state and which controls should + // always display by default. useEffect( () => { const items = {}; const defaults = {}; filteredChildren.forEach( ( child ) => { - const { hasValue, isShownByDefault, label } = child.props; - items[ label ] = isShownByDefault || hasValue( child.props ); - defaults[ label ] = isShownByDefault; + items[ child.props.label ] = child.props.hasValue( child.props ); + defaults[ child.props.label ] = child.props.isShownByDefault; } ); setMenuItems( items ); setDefaultControls( defaults ); }, [] ); + // As the default controls are visible all the time. Reflect their + // customizations in the menu items' selected state. + useEffect( () => { + const menuLabels = Object.keys( menuItems ); + + // Skip if no children or menu state not initialized. + if ( menuLabels.length === 0 ) { + return; + } + + const updatedItems = { ...menuItems }; + menuLabels.forEach( ( label ) => { + if ( defaultControls[ label ] ) { + updatedItems[ label ] = customizedChildren.includes( label ); + } + } ); + + setMenuItems( updatedItems ); + }, customizedChildren ); + if ( filteredChildren.length === 0 ) { return null; } @@ -52,8 +76,9 @@ const BlockSupportPanel = ( props ) => { ); }; - // Toggles the display of the block support control and resets its - // associated block attribute via the control's reset callback prop. + // Toggles the customized state of the block support control and its display + // if it isn't to be displayed by default. When toggling off a control its + // associated block attribute is reset via the control's reset callback. const toggleControl = ( label ) => { const isSelected = menuItems[ label ]; @@ -77,8 +102,8 @@ const BlockSupportPanel = ( props ) => { // Turn off menu items unless they are to display by default. const resetMenuItems = {}; - filteredChildren.forEach( ( { props: { label } } ) => { - resetMenuItems[ label ] = defaultControls[ label ]; + filteredChildren.forEach( ( child ) => { + resetMenuItems[ child.props.label ] = false; } ); setMenuItems( resetMenuItems ); @@ -96,7 +121,13 @@ const BlockSupportPanel = ( props ) => { resetAll={ resetAllControls } /> { filteredChildren.map( ( child ) => { - return menuItems[ child.props.label ] ? child : null; + // Only display the block support control if it is toggled on + // in the menu or is set to display by default. + const isShown = + menuItems[ child.props.label ] || + defaultControls[ child.props.label ]; + + return isShown ? child : null; } ) } ); From 481ea39718ba60a319c37621cac72d32366821cb Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Wed, 23 Jun 2021 16:58:02 +1000 Subject: [PATCH 17/50] Tweak panel to make it a little more generic --- docs/manifest.json | 12 +- packages/block-editor/src/hooks/dimensions.js | 10 +- .../src/block-support-panel/README.md | 100 --------------- packages/components/src/index.js | 2 +- .../progressive-disclosure-panel/README.md | 117 ++++++++++++++++++ .../index.js | 74 +++++------ .../stories/index.js | 16 +-- .../style.scss | 8 +- .../test/index.js | 62 +++++----- .../title.js | 10 +- packages/components/src/style.scss | 2 +- 11 files changed, 219 insertions(+), 194 deletions(-) delete mode 100644 packages/components/src/block-support-panel/README.md create mode 100644 packages/components/src/progressive-disclosure-panel/README.md rename packages/components/src/{block-support-panel => progressive-disclosure-panel}/index.js (54%) rename packages/components/src/{block-support-panel => progressive-disclosure-panel}/stories/index.js (77%) rename packages/components/src/{block-support-panel => progressive-disclosure-panel}/style.scss (79%) rename packages/components/src/{block-support-panel => progressive-disclosure-panel}/test/index.js (70%) rename packages/components/src/{block-support-panel => progressive-disclosure-panel}/title.js (80%) diff --git a/docs/manifest.json b/docs/manifest.json index ef6392b9be2b2..a1e4afa816823 100644 --- a/docs/manifest.json +++ b/docs/manifest.json @@ -695,12 +695,6 @@ "markdown_source": "../packages/components/src/base-field/README.md", "parent": "components" }, - { - "title": "BlockSupportPanel", - "slug": "block-support-panel", - "markdown_source": "../packages/components/src/block-support-panel/README.md", - "parent": "components" - }, { "title": "BoxControl", "slug": "box-control", @@ -1097,6 +1091,12 @@ "markdown_source": "../packages/components/src/popover/README.md", "parent": "components" }, + { + "title": "ProgressiveDisclosurePanel", + "slug": "progressive-disclosure-panel", + "markdown_source": "../packages/components/src/progressive-disclosure-panel/README.md", + "parent": "components" + }, { "title": "QueryControls", "slug": "query-controls", diff --git a/packages/block-editor/src/hooks/dimensions.js b/packages/block-editor/src/hooks/dimensions.js index dd28feb901959..290cb5ce227c8 100644 --- a/packages/block-editor/src/hooks/dimensions.js +++ b/packages/block-editor/src/hooks/dimensions.js @@ -1,7 +1,7 @@ /** * WordPress dependencies */ -import { __experimentalBlockSupportPanel as BlockSupportPanel } from '@wordpress/components'; +import { __experimentalProgressiveDisclosurePanel as ProgressiveDisclosurePanel } from '@wordpress/components'; import { Platform } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; import { getBlockSupport } from '@wordpress/blocks'; @@ -67,7 +67,7 @@ export function DimensionsPanel( props ) { return ( - ) } @@ -86,11 +86,11 @@ export function DimensionsPanel( props ) { { ...props } hasValue={ hasMarginValue } label={ __( 'Margin' ) } - reset={ resetMargin } + onDeselect={ resetMargin } isShownByDefault={ defaultSpacingControls?.margin } /> ) } - + ); } diff --git a/packages/components/src/block-support-panel/README.md b/packages/components/src/block-support-panel/README.md deleted file mode 100644 index 63fca61e6d29f..0000000000000 --- a/packages/components/src/block-support-panel/README.md +++ /dev/null @@ -1,100 +0,0 @@ -# Block Support Panel - -These panels provide progressive discovery options for multiple controls -provided via block supports. - -## Development guidelines - -The `BlockSupportPanel` creates a container with a header including a dropdown -menu. The menu is generated automatically from the panel's children. Each menu -item allows for the display of the corresponding child control to be toggled -on or off. When toggled off the control's reset callback is fired allowing for -the block support provided attribtues to reset. - -Whether a child control is initially displayed or not is dependent upon -whether there has previously been a value set. This is checked via the -`hasValue` function provided through the child's props. - -### Usage - -```jsx -import { __experimentalBlockSupportPanel as BlockSupportPanel } from '@wordpress/components'; -import { - PaddingEdit, - hasPaddingValue, - resetPadding, - useIsPaddingDisabled, -} from './padding'; - - -export function DimensionPanel( props ) { - const isPaddingDisabled = useIsPaddingDisabled( props ); - - const resetAll = () => { - // Reset attributes for all block support features in this panel. - }; - - return ( - - { ! isPaddingDisabled && ( - - ) } - - ); -} -``` - -### Sub-Components - -#### BlockSupportPanelTitle - -This is a simple component to display the panel title and house the dropdown -menu for toggling control display. It is used by the `BlockSupportPanel` -component under the hood, so it does not typically need to be used. - -##### Props -###### resetAll - -A function to call when the `Reset all` menu option is selected. - -- Type: `function` -- Required: Yes - -###### toggleControl - -Callback used to toggle display of an individual block support control and reset -its value if being turned off. - -- Type: `function` -- Required: Yes - -###### menuItems - -This object represents the child controls and their visibility state. It -is built by the parent panel using its children. - -- Type: `Object` -- Required: No - -###### menuLabel - -A label for the dropdown menu. - -- Type: `String` -- Required: No - -###### title - -The panel title to display. - -- Type: `String` -- Required: No diff --git a/packages/components/src/index.js b/packages/components/src/index.js index 8c2d93f252ec0..540fbf2759518 100644 --- a/packages/components/src/index.js +++ b/packages/components/src/index.js @@ -22,7 +22,6 @@ export { useAutocompleteProps as __unstableUseAutocompleteProps, } from './autocomplete'; export { default as BaseControl } from './base-control'; -export { default as __experimentalBlockSupportPanel } from './block-support-panel'; export { default as __experimentalBoxControl } from './box-control'; export { default as Button } from './button'; export { default as ButtonGroup } from './button-group'; @@ -101,6 +100,7 @@ export { default as PanelHeader } from './panel/header'; export { default as PanelRow } from './panel/row'; export { default as Placeholder } from './placeholder'; export { default as Popover } from './popover'; +export { default as __experimentalProgressiveDisclosurePanel } from './progressive-disclosure-panel'; export { default as QueryControls } from './query-controls'; export { default as __experimentalRadio } from './radio'; export { default as __experimentalRadioGroup } from './radio-group'; diff --git a/packages/components/src/progressive-disclosure-panel/README.md b/packages/components/src/progressive-disclosure-panel/README.md new file mode 100644 index 0000000000000..f56fd865ee285 --- /dev/null +++ b/packages/components/src/progressive-disclosure-panel/README.md @@ -0,0 +1,117 @@ +# Progressive Disclosure Panel + +These panels provide progressive discovery options for their children. For +example the controls provided via block supports. + +## Development guidelines + +The `ProgressiveDisclosurePanel` creates a container with a header including a +dropdown menu. The menu is generated automatically from the panel's children. +Each menu item allows for the display of the corresponding child to be +toggled on or off. The control's `onSelect` and `onDeselect` callbacks are fired +allowing for greater control over the child e.g. resetting block attributes when +a block support control is toggled off. + +Whether a child control is initially displayed or not is dependent upon +if there has previously been a value set or the child has been flagged as +displaying by default through the `isShownByDefault` prop. Determining whether a +child has a value is done via the `hasValue` function provided through the +child's props. + +### Usage + +```jsx +import { __experimentalProgressiveDisclosurePanel as ProgressiveDisclosurePanel } from '@wordpress/components'; +import { + PaddingEdit, + hasPaddingValue, + resetPadding, + useIsPaddingDisabled, +} from './padding'; + + +export function DimensionPanel( props ) { + const isPaddingDisabled = useIsPaddingDisabled( props ); + + const resetAll = () => { + // Reset attributes for all block support features in this panel. + }; + + return ( + + { ! isPaddingDisabled && ( + + ) } + + ); +} +``` + +### Props + +#### label + +The label for the panel's dropdown menu. + +#### resetAll + +A function to call when the `Reset all` menu option is selected. This is passed +through to the panel's title component. + +#### title + +Title to be displayed within the panel's title. + +### Sub-Components + +#### ProgressiveDisclosurePanelTitle + +This is a simple component to display the panel title and house the dropdown +menu for toggling child display. It is used by the `ProgressiveDisclosurePanel` +component under the hood, so it does not typically need to be used. + +##### Props +###### resetAll + +A function to call when the `Reset all` menu option is selected. + +- Type: `function` +- Required: Yes + +###### toggleChild + +Callback used to toggle display of an individual child component. + +- Type: `function` +- Required: Yes + +###### menuItems + +This object represents the panel's children and their visibility state. It +is built by the parent panel from its children prop. + +- Type: `Object` +- Required: No + +###### menuLabel + +A label for the dropdown menu. + +- Type: `String` +- Required: No + +###### title + +The panel title to display. + +- Type: `String` +- Required: No diff --git a/packages/components/src/block-support-panel/index.js b/packages/components/src/progressive-disclosure-panel/index.js similarity index 54% rename from packages/components/src/block-support-panel/index.js rename to packages/components/src/progressive-disclosure-panel/index.js index 53c4469396d89..371f8bcf58bab 100644 --- a/packages/components/src/block-support-panel/index.js +++ b/packages/components/src/progressive-disclosure-panel/index.js @@ -2,6 +2,7 @@ * External dependencies */ import classnames from 'classnames'; +import noop from 'lodash'; /** * WordPress dependencies @@ -11,27 +12,27 @@ import { useEffect, useState } from '@wordpress/element'; /** * Internal dependencies */ -import BlockSupportPanelTitle from './title'; +import ProgressiveDisclosurePanelTitle from './title'; -const BlockSupportPanel = ( props ) => { +const ProgressiveDisclosurePanel = ( props ) => { const { children, className, label: menuLabel, resetAll, title } = props; const [ menuItems, setMenuItems ] = useState( {} ); - const [ defaultControls, setDefaultControls ] = useState( {} ); + const [ defaultChildren, setDefaultChildren ] = useState( {} ); - // If a block support UI has been disabled via theme.json a boolean `false` - // will be passed as a child. This panel is only interested in the children - // to be displayed. + // When conditionally including components e.g. { isShown && } + // a boolean `false` will be passed as a child if component is excluded. + // This panel is only interested in the children to be displayed. const filteredChildren = Array.isArray( children ) ? children.filter( Boolean ) : []; - // Collect which controls have custom values. Used to update menu state to - // reflect customization for controls that display by default / always show. + // Collect which children have custom values. Used to update menu state to + // reflect customization for children that display by default / always show. const customizedChildren = filteredChildren.map( ( child ) => child.props.hasValue( child.props ) ? child.props.label : undefined ); - // On first render determine initial menu state and which controls should + // On first render determine initial menu state and which children should // always display by default. useEffect( () => { const items = {}; @@ -43,10 +44,10 @@ const BlockSupportPanel = ( props ) => { } ); setMenuItems( items ); - setDefaultControls( defaults ); + setDefaultChildren( defaults ); }, [] ); - // As the default controls are visible all the time. Reflect their + // As the default children are visible all the time. Reflect their // customizations in the menu items' selected state. useEffect( () => { const menuLabels = Object.keys( menuItems ); @@ -58,7 +59,7 @@ const BlockSupportPanel = ( props ) => { const updatedItems = { ...menuItems }; menuLabels.forEach( ( label ) => { - if ( defaultControls[ label ] ) { + if ( defaultChildren[ label ] ) { updatedItems[ label ] = customizedChildren.includes( label ); } } ); @@ -70,33 +71,33 @@ const BlockSupportPanel = ( props ) => { return null; } - const getControlByMenuLabel = ( label ) => { + const getChildByMenuLabel = ( label ) => { return filteredChildren.find( ( child ) => child.props.label === label ); }; - // Toggles the customized state of the block support control and its display - // if it isn't to be displayed by default. When toggling off a control its - // associated block attribute is reset via the control's reset callback. - const toggleControl = ( label ) => { - const isSelected = menuItems[ label ]; - - if ( isSelected ) { - const control = getControlByMenuLabel( label ); - control.props.reset( control.props ); + // Toggles the customized state of the child and its display if it isn't to + // be displayed by default. When toggling a child it's callback is executed. + const toggleChild = ( label ) => { + const wasSelected = menuItems[ label ]; + const child = getChildByMenuLabel( label ); + const { onDeselect = noop, onSelect = noop } = child.props; + + if ( wasSelected ) { + onDeselect( child.props ); + } else { + onSelect( child.props ); } setMenuItems( { ...menuItems, - [ label ]: ! isSelected, + [ label ]: ! wasSelected, } ); }; - // Resets all block support attributes for controls represented by the - // menu items. Then turns off their display. - const resetAllControls = () => { - // Reset the block support attributes. + // Resets display of children and executes resetAll callback if available. + const resetAllChildren = () => { resetAll(); // Turn off menu items unless they are to display by default. @@ -109,23 +110,26 @@ const BlockSupportPanel = ( props ) => { setMenuItems( resetMenuItems ); }; - const classes = classnames( 'components-block-support-panel', className ); + const classes = classnames( + 'components-progressive-disclosure-panel', + className + ); return (
- { filteredChildren.map( ( child ) => { - // Only display the block support control if it is toggled on - // in the menu or is set to display by default. + // Only display the child if it is toggled on in the menu or is + // set to display by default. const isShown = menuItems[ child.props.label ] || - defaultControls[ child.props.label ]; + defaultChildren[ child.props.label ]; return isShown ? child : null; } ) } @@ -133,4 +137,4 @@ const BlockSupportPanel = ( props ) => { ); }; -export default BlockSupportPanel; +export default ProgressiveDisclosurePanel; diff --git a/packages/components/src/block-support-panel/stories/index.js b/packages/components/src/progressive-disclosure-panel/stories/index.js similarity index 77% rename from packages/components/src/block-support-panel/stories/index.js rename to packages/components/src/progressive-disclosure-panel/stories/index.js index 0b14422760375..a8525aa25caf5 100644 --- a/packages/components/src/block-support-panel/stories/index.js +++ b/packages/components/src/progressive-disclosure-panel/stories/index.js @@ -11,13 +11,13 @@ import { useState } from '@wordpress/element'; /** * Internal dependencies */ -import BlockSupportPanel from '../'; +import ProgressiveDisclosurePanel from '../'; import Panel from '../../panel'; import UnitControl from '../../unit-control'; export default { - title: 'Components/BlockSupportPanel', - component: BlockSupportPanel, + title: 'Components/ProgressiveDisclosurePanel', + component: ProgressiveDisclosurePanel, }; export const _default = () => { @@ -32,26 +32,26 @@ export const _default = () => { return ( - !! height } label="Height" - reset={ () => setHeight( undefined ) } + onDeselect={ () => setHeight( undefined ) } value={ height } onChange={ ( next ) => setHeight( next ) } /> !! width } label="Width" - reset={ () => setWidth( undefined ) } + onDeselect={ () => setWidth( undefined ) } value={ width } onChange={ ( next ) => setWidth( next ) } /> - + ); diff --git a/packages/components/src/block-support-panel/style.scss b/packages/components/src/progressive-disclosure-panel/style.scss similarity index 79% rename from packages/components/src/block-support-panel/style.scss rename to packages/components/src/progressive-disclosure-panel/style.scss index 407825312cc65..d332fcd14c07d 100644 --- a/packages/components/src/block-support-panel/style.scss +++ b/packages/components/src/progressive-disclosure-panel/style.scss @@ -1,10 +1,10 @@ -.components-block-support-panel { +.components-progressive-disclosure-panel { border: none; border-top: $border-width solid $gray-200; padding: $grid-unit-20; margin-top: -1px; - .components-block-support-panel__title { + .components-progressive-disclosure-panel__title { margin: 0; display: flex; align-items: center; @@ -43,11 +43,11 @@ } .edit-site { - .components-block-support-panel { + .components-progressive-disclosure-panel { border-bottom: $border-width solid $gray-200; } - .components-block-support-panel + .components-panel__body { + .components-progressive-disclosure-panel + .components-panel__body { margin-top: -1px; } } diff --git a/packages/components/src/block-support-panel/test/index.js b/packages/components/src/progressive-disclosure-panel/test/index.js similarity index 70% rename from packages/components/src/block-support-panel/test/index.js rename to packages/components/src/progressive-disclosure-panel/test/index.js index 5412591a3c70f..3fff561a758a8 100644 --- a/packages/components/src/block-support-panel/test/index.js +++ b/packages/components/src/progressive-disclosure-panel/test/index.js @@ -6,9 +6,9 @@ import { render, screen, fireEvent } from '@testing-library/react'; /** * Internal dependencies */ -import BlockSupportPanel from '../'; +import ProgressiveDisclosurePanel from '../'; -// Used to represent the block support provided controls. +// Represents a child provided to the panel such as a block support control. const MockControl = ( { children } ) =>
{ children }
; const resetAll = jest.fn(); @@ -17,45 +17,47 @@ const hasValue = jest.fn().mockImplementation( ( props ) => { return props.attributes.value; } ); -// Default props for the block supports panel. +// Default props for the progressive disclosure panel. const defaultProps = { label: 'Display options', title: 'Panel title', resetAll, }; -// Default props for enabled block supports control to be rendered within panel. +// Default props for an enabled control to be rendered within panel. const controlProps = { attributes: { value: true }, hasValue, label: 'Example', - reset: jest.fn().mockImplementation( () => { + onDeselect: jest.fn().mockImplementation( () => { controlProps.attributes.value = undefined; } ), + onSelect: jest.fn(), }; -// Default props without a value for alternate block supports control to be -// rendered within the panel. +// Default props without a value for an alternate control to be rendered within +// the panel. const altControlProps = { attributes: { value: false }, hasValue, label: 'Alt', - reset: jest.fn(), + onDeselect: jest.fn(), + onSelect: jest.fn(), }; -// Attempts to find the block supports panel via its CSS class. +// Attempts to find the progressive disclosure panel via its CSS class. const getPanel = ( container ) => - container.querySelector( '.components-block-support-panel' ); + container.querySelector( '.components-progressive-disclosure-panel' ); -// Renders a default block supports panel including a control that simulates -// block support being disabled for that particular control. +// Renders a default progressive disclosure panel including a child that +// is being conditionally disabled. const renderPanel = () => { return render( - + { false &&
Hidden
} Example control Alt control - + ); }; @@ -72,11 +74,11 @@ const selectMenuItem = async ( label ) => { fireEvent.click( menuItem ); }; -describe( 'BlockSupportPanel', () => { +describe( 'ProgressiveDisclosurePanel', () => { describe( 'basic rendering', () => { it( 'should not render when no children provided', () => { const { container } = render( - + ); expect( getPanel( container ) ).not.toBeInTheDocument(); @@ -85,9 +87,9 @@ describe( 'BlockSupportPanel', () => { it( 'should not render when only child has been filtered out', () => { // This covers case where children prop is not an array. const { container } = render( - + { false && Should not show } - + ); expect( getPanel( container ) ).not.toBeInTheDocument(); @@ -95,10 +97,10 @@ describe( 'BlockSupportPanel', () => { it( 'should not render when all children have been filtered out', () => { const { container } = render( - + { false && Should not show } { false && Not shown either } - + ); expect( getPanel( container ) ).not.toBeInTheDocument(); @@ -145,8 +147,8 @@ describe( 'BlockSupportPanel', () => { } ); } ); - describe( 'conditional rendering of inner controls', () => { - it( 'should render child control when it has a value', () => { + describe( 'conditional rendering of children', () => { + it( 'should render child when it has a value', () => { renderPanel(); const exampleControl = screen.getByText( 'Example control' ); @@ -156,7 +158,7 @@ describe( 'BlockSupportPanel', () => { expect( altControl ).not.toBeInTheDocument(); } ); - it( 'should render child control when corresponding menu is selected', async () => { + it( 'should render child when corresponding menu item is selected', async () => { renderPanel(); await selectMenuItem( altControlProps.label ); const control = await screen.findByText( 'Alt control' ); @@ -164,7 +166,7 @@ describe( 'BlockSupportPanel', () => { expect( control ).toBeInTheDocument(); } ); - it( 'should prevent child rendering when toggled off via menu', async () => { + it( 'should prevent child rendering when toggled off via menu item', async () => { renderPanel(); await selectMenuItem( controlProps.label ); const control = screen.queryByText( 'Example control' ); @@ -173,24 +175,26 @@ describe( 'BlockSupportPanel', () => { } ); } ); - describe( 'reset callbacks on menu item selection', () => { + describe( 'callbacks on menu item selection', () => { beforeEach( () => { jest.clearAllMocks(); controlProps.attributes.value = true; } ); - it( 'should call reset callback when menu item is toggled off', async () => { + it( 'should call onDeselect callback when menu item is toggled off', async () => { renderPanel(); await selectMenuItem( controlProps.label ); - expect( controlProps.reset ).toHaveBeenCalledTimes( 1 ); + expect( controlProps.onSelect ).not.toHaveBeenCalled(); + expect( controlProps.onDeselect ).toHaveBeenCalledTimes( 1 ); } ); - it( 'should not call reset callback when menu item is toggled on', async () => { + it( 'should call onSelect callback when menu item is toggled on', async () => { renderPanel(); await selectMenuItem( altControlProps.label ); - expect( altControlProps.reset ).not.toHaveBeenCalled(); + expect( altControlProps.onSelect ).toHaveBeenCalledTimes( 1 ); + expect( altControlProps.onDeselect ).not.toHaveBeenCalled(); } ); it( 'should call resetAll callback when its menu item is selected', async () => { diff --git a/packages/components/src/block-support-panel/title.js b/packages/components/src/progressive-disclosure-panel/title.js similarity index 80% rename from packages/components/src/block-support-panel/title.js rename to packages/components/src/progressive-disclosure-panel/title.js index 8383dc10897b7..f0cb6273c773a 100644 --- a/packages/components/src/block-support-panel/title.js +++ b/packages/components/src/progressive-disclosure-panel/title.js @@ -11,15 +11,15 @@ import MenuGroup from '../menu-group'; import MenuItem from '../menu-item'; import DropdownMenu from '../dropdown-menu'; -const BlockSupportPanelTitle = ( props ) => { - const { menuItems = {}, menuLabel, resetAll, title, toggleControl } = props; +const ProgressiveDisclosurePanelTitle = ( props ) => { + const { menuItems = {}, menuLabel, resetAll, title, toggleChild } = props; if ( ! title ) { return null; } return ( -

+

{ title } { ( { onClose } ) => ( @@ -33,7 +33,7 @@ const BlockSupportPanelTitle = ( props ) => { icon={ isSelected && check } isSelected={ isSelected } onClick={ () => { - toggleControl( label ); + toggleChild( label ); onClose(); } } role="menuitemcheckbox" @@ -61,4 +61,4 @@ const BlockSupportPanelTitle = ( props ) => { ); }; -export default BlockSupportPanelTitle; +export default ProgressiveDisclosurePanelTitle; diff --git a/packages/components/src/style.scss b/packages/components/src/style.scss index 1d1d252ef27bc..60d3e98e4583e 100644 --- a/packages/components/src/style.scss +++ b/packages/components/src/style.scss @@ -1,6 +1,5 @@ @import "./animate/style.scss"; @import "./autocomplete/style.scss"; -@import "./block-support-panel/style.scss"; @import "./button-group/style.scss"; @import "./button/style.scss"; @import "./checkbox-control/style.scss"; @@ -31,6 +30,7 @@ @import "./panel/style.scss"; @import "./placeholder/style.scss"; @import "./popover/style.scss"; +@import "./progressive-disclosure-panel/style.scss"; @import "./radio-control/style.scss"; @import "./resizable-box/style.scss"; @import "./responsive-wrapper/style.scss"; From 0d75ba7681486edcd4a6c1458d567f305de28315 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Thu, 8 Jul 2021 14:48:22 +1000 Subject: [PATCH 18/50] Simplify progressive disclosure panel state --- .../src/progressive-disclosure-panel/index.js | 60 ++++++------------- 1 file changed, 19 insertions(+), 41 deletions(-) diff --git a/packages/components/src/progressive-disclosure-panel/index.js b/packages/components/src/progressive-disclosure-panel/index.js index 371f8bcf58bab..3f785d59061de 100644 --- a/packages/components/src/progressive-disclosure-panel/index.js +++ b/packages/components/src/progressive-disclosure-panel/index.js @@ -7,7 +7,7 @@ import noop from 'lodash'; /** * WordPress dependencies */ -import { useEffect, useState } from '@wordpress/element'; +import { useEffect, useMemo, useState } from '@wordpress/element'; /** * Internal dependencies @@ -17,55 +17,30 @@ import ProgressiveDisclosurePanelTitle from './title'; const ProgressiveDisclosurePanel = ( props ) => { const { children, className, label: menuLabel, resetAll, title } = props; const [ menuItems, setMenuItems ] = useState( {} ); - const [ defaultChildren, setDefaultChildren ] = useState( {} ); // When conditionally including components e.g. { isShown && } // a boolean `false` will be passed as a child if component is excluded. // This panel is only interested in the children to be displayed. - const filteredChildren = Array.isArray( children ) - ? children.filter( Boolean ) - : []; - - // Collect which children have custom values. Used to update menu state to - // reflect customization for children that display by default / always show. - const customizedChildren = filteredChildren.map( ( child ) => - child.props.hasValue( child.props ) ? child.props.label : undefined - ); + const filteredChildren = useMemo( () => { + return Array.isArray( children ) ? children.filter( Boolean ) : []; + }, [ children ] ); - // On first render determine initial menu state and which children should - // always display by default. + // Refresh which children should be reflected in the menu and what their + // associated menu item's state is; checked or not. useEffect( () => { const items = {}; - const defaults = {}; filteredChildren.forEach( ( child ) => { - items[ child.props.label ] = child.props.hasValue( child.props ); - defaults[ child.props.label ] = child.props.isShownByDefault; + // New item is checked if: + // - it currently has a value + // - or it was checked in previous menuItems state. + items[ child.props.label ] = + child.props.hasValue( child.props ) || + menuItems[ child.props.label ]; } ); setMenuItems( items ); - setDefaultChildren( defaults ); - }, [] ); - - // As the default children are visible all the time. Reflect their - // customizations in the menu items' selected state. - useEffect( () => { - const menuLabels = Object.keys( menuItems ); - - // Skip if no children or menu state not initialized. - if ( menuLabels.length === 0 ) { - return; - } - - const updatedItems = { ...menuItems }; - menuLabels.forEach( ( label ) => { - if ( defaultChildren[ label ] ) { - updatedItems[ label ] = customizedChildren.includes( label ); - } - } ); - - setMenuItems( updatedItems ); - }, customizedChildren ); + }, [ filteredChildren ] ); if ( filteredChildren.length === 0 ) { return null; @@ -98,9 +73,12 @@ const ProgressiveDisclosurePanel = ( props ) => { // Resets display of children and executes resetAll callback if available. const resetAllChildren = () => { - resetAll(); + if ( typeof resetAll === 'function' ) { + resetAll(); + } - // Turn off menu items unless they are to display by default. + // Turn off all menu items. Default controls will continue to display + // by virtue of their `isShownByDefault` prop. const resetMenuItems = {}; filteredChildren.forEach( ( child ) => { @@ -129,7 +107,7 @@ const ProgressiveDisclosurePanel = ( props ) => { // set to display by default. const isShown = menuItems[ child.props.label ] || - defaultChildren[ child.props.label ]; + child.props.isShownByDefault; return isShown ? child : null; } ) } From ffedb612123b4262e101d9370ed94e74a4d7a6af Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Thu, 8 Jul 2021 16:36:05 +1000 Subject: [PATCH 19/50] Add progressive disclosure panel item component This utilises a new context for the panel's menu items. The menu in the panel title is generated from this and the individual items control their display based of if the item is selected in that menu item context. --- packages/block-editor/src/hooks/dimensions.js | 27 ++++---- packages/components/src/index.js | 1 + .../src/progressive-disclosure-panel/index.js | 61 ++++++++++--------- .../src/progressive-disclosure-panel/item.js | 23 +++++++ .../src/progressive-disclosure-panel/title.js | 4 +- 5 files changed, 76 insertions(+), 40 deletions(-) create mode 100644 packages/components/src/progressive-disclosure-panel/item.js diff --git a/packages/block-editor/src/hooks/dimensions.js b/packages/block-editor/src/hooks/dimensions.js index 290cb5ce227c8..57ce3a764b31e 100644 --- a/packages/block-editor/src/hooks/dimensions.js +++ b/packages/block-editor/src/hooks/dimensions.js @@ -1,7 +1,10 @@ /** * WordPress dependencies */ -import { __experimentalProgressiveDisclosurePanel as ProgressiveDisclosurePanel } from '@wordpress/components'; +import { + __experimentalProgressiveDisclosurePanel as ProgressiveDisclosurePanel, + __experimentalProgressiveDisclosurePanelItem as ProgressiveDisclosurePanelItem, +} from '@wordpress/components'; import { Platform } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; import { getBlockSupport } from '@wordpress/blocks'; @@ -73,22 +76,24 @@ export function DimensionsPanel( props ) { resetAll={ resetAll } > { ! isPaddingDisabled && ( - hasPaddingValue( props ) } label={ __( 'Padding' ) } - onDeselect={ resetPadding } + onDeselect={ () => resetPadding( props ) } isShownByDefault={ defaultSpacingControls?.padding } - /> + > + + ) } { ! isMarginDisabled && ( - hasMarginValue( props ) } label={ __( 'Margin' ) } - onDeselect={ resetMargin } + onDeselect={ () => resetMargin( props ) } isShownByDefault={ defaultSpacingControls?.margin } - /> + > + + ) } diff --git a/packages/components/src/index.js b/packages/components/src/index.js index 540fbf2759518..7dc6c9f5e64fa 100644 --- a/packages/components/src/index.js +++ b/packages/components/src/index.js @@ -101,6 +101,7 @@ export { default as PanelRow } from './panel/row'; export { default as Placeholder } from './placeholder'; export { default as Popover } from './popover'; export { default as __experimentalProgressiveDisclosurePanel } from './progressive-disclosure-panel'; +export { default as __experimentalProgressiveDisclosurePanelItem } from './progressive-disclosure-panel/item'; export { default as QueryControls } from './query-controls'; export { default as __experimentalRadio } from './radio'; export { default as __experimentalRadioGroup } from './radio-group'; diff --git a/packages/components/src/progressive-disclosure-panel/index.js b/packages/components/src/progressive-disclosure-panel/index.js index 3f785d59061de..eba21a8b56c95 100644 --- a/packages/components/src/progressive-disclosure-panel/index.js +++ b/packages/components/src/progressive-disclosure-panel/index.js @@ -2,18 +2,30 @@ * External dependencies */ import classnames from 'classnames'; -import noop from 'lodash'; /** * WordPress dependencies */ -import { useEffect, useMemo, useState } from '@wordpress/element'; +import { + createContext, + useContext, + useEffect, + useMemo, + useState, +} from '@wordpress/element'; /** * Internal dependencies */ +import ProgressiveDisclosurePanelItem from './item'; import ProgressiveDisclosurePanelTitle from './title'; +const PanelContext = createContext( {} ); + +export const usePanelContext = () => useContext( PanelContext ); + +const isMenuItem = ( item ) => item.type === ProgressiveDisclosurePanelItem; + const ProgressiveDisclosurePanel = ( props ) => { const { children, className, label: menuLabel, resetAll, title } = props; const [ menuItems, setMenuItems ] = useState( {} ); @@ -22,7 +34,7 @@ const ProgressiveDisclosurePanel = ( props ) => { // a boolean `false` will be passed as a child if component is excluded. // This panel is only interested in the children to be displayed. const filteredChildren = useMemo( () => { - return Array.isArray( children ) ? children.filter( Boolean ) : []; + return Array.isArray( children ) ? children.filter( isMenuItem ) : []; }, [ children ] ); // Refresh which children should be reflected in the menu and what their @@ -30,13 +42,11 @@ const ProgressiveDisclosurePanel = ( props ) => { useEffect( () => { const items = {}; - filteredChildren.forEach( ( child ) => { + filteredChildren.forEach( ( { props: { hasValue, label } } ) => { // New item is checked if: // - it currently has a value // - or it was checked in previous menuItems state. - items[ child.props.label ] = - child.props.hasValue( child.props ) || - menuItems[ child.props.label ]; + items[ label ] = hasValue() || menuItems[ label ]; } ); setMenuItems( items ); @@ -57,12 +67,14 @@ const ProgressiveDisclosurePanel = ( props ) => { const toggleChild = ( label ) => { const wasSelected = menuItems[ label ]; const child = getChildByMenuLabel( label ); - const { onDeselect = noop, onSelect = noop } = child.props; + const { onDeselect, onSelect } = child.props; + + if ( wasSelected && onDeselect ) { + onDeselect(); + } - if ( wasSelected ) { - onDeselect( child.props ); - } else { - onSelect( child.props ); + if ( ! wasSelected && onSelect ) { + onSelect(); } setMenuItems( { @@ -95,22 +107,15 @@ const ProgressiveDisclosurePanel = ( props ) => { return (
- - { filteredChildren.map( ( child ) => { - // Only display the child if it is toggled on in the menu or is - // set to display by default. - const isShown = - menuItems[ child.props.label ] || - child.props.isShownByDefault; - - return isShown ? child : null; - } ) } + + + { children } +
); }; diff --git a/packages/components/src/progressive-disclosure-panel/item.js b/packages/components/src/progressive-disclosure-panel/item.js new file mode 100644 index 0000000000000..5cba0f2f5f7c9 --- /dev/null +++ b/packages/components/src/progressive-disclosure-panel/item.js @@ -0,0 +1,23 @@ +/** + * Internal dependencies + */ +import { usePanelContext } from './'; + +// This wraps controls to be conditionally displayed within a progressive +// disclosure panel. It helps prevent props being applied to HTML elements that +// would otherwise be invalid. +const ProgressiveDisclosurePanelItem = ( props ) => { + const { children, isShownByDefault, label } = props; + const menuItems = usePanelContext(); + + // Do not show if menu item not selected and not shown by default. + // If the item has a value that will be reflected in the menu item's + // selected status provided by context. + if ( ! menuItems[ label ] && ! isShownByDefault ) { + return null; + } + + return children; +}; + +export default ProgressiveDisclosurePanelItem; diff --git a/packages/components/src/progressive-disclosure-panel/title.js b/packages/components/src/progressive-disclosure-panel/title.js index f0cb6273c773a..7f29f80d002e5 100644 --- a/packages/components/src/progressive-disclosure-panel/title.js +++ b/packages/components/src/progressive-disclosure-panel/title.js @@ -7,12 +7,14 @@ import { __ } from '@wordpress/i18n'; /** * Internal dependencies */ +import { usePanelContext } from './'; import MenuGroup from '../menu-group'; import MenuItem from '../menu-item'; import DropdownMenu from '../dropdown-menu'; const ProgressiveDisclosurePanelTitle = ( props ) => { - const { menuItems = {}, menuLabel, resetAll, title, toggleChild } = props; + const { menuLabel, resetAll, title, toggleChild } = props; + const menuItems = usePanelContext(); if ( ! title ) { return null; From ae4a3b23c60c3d71a09ce883350a61a758d306dc Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Thu, 8 Jul 2021 17:06:59 +1000 Subject: [PATCH 20/50] Update dimensions global style sidebar panel --- .../components/sidebar/dimensions-panel.js | 51 +++++++++++-------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/packages/edit-site/src/components/sidebar/dimensions-panel.js b/packages/edit-site/src/components/sidebar/dimensions-panel.js index d6df96c0a83ed..abea542bacd08 100644 --- a/packages/edit-site/src/components/sidebar/dimensions-panel.js +++ b/packages/edit-site/src/components/sidebar/dimensions-panel.js @@ -3,7 +3,8 @@ */ import { __ } from '@wordpress/i18n'; import { - __experimentalBlockSupportPanel as BlockSupportPanel, + __experimentalProgressiveDisclosurePanel as ProgressiveDisclosurePanel, + __experimentalProgressiveDisclosurePanelItem as ProgressiveDisclosurePanelItem, __experimentalBoxControl as BoxControl, __experimentalUseCustomUnits as useCustomUnits, } from '@wordpress/components'; @@ -103,37 +104,45 @@ export default function DimensionsPanel( { context, getStyle, setStyle } ) { }; return ( - { showPaddingControl && ( - + > + + ) } { showMarginControl && ( - + > + + ) } - + ); } From e7a89ea533b93bbd385a67391d3dd9664e5bf19a Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Mon, 12 Jul 2021 18:47:17 +1000 Subject: [PATCH 21/50] Update docs and comments after restructure --- .../progressive-disclosure-panel/README.md | 68 ++++++++----------- .../src/progressive-disclosure-panel/index.js | 5 +- 2 files changed, 31 insertions(+), 42 deletions(-) diff --git a/packages/components/src/progressive-disclosure-panel/README.md b/packages/components/src/progressive-disclosure-panel/README.md index f56fd865ee285..350f49db58da4 100644 --- a/packages/components/src/progressive-disclosure-panel/README.md +++ b/packages/components/src/progressive-disclosure-panel/README.md @@ -6,7 +6,9 @@ example the controls provided via block supports. ## Development guidelines The `ProgressiveDisclosurePanel` creates a container with a header including a -dropdown menu. The menu is generated automatically from the panel's children. +dropdown menu. The menu is generated automatically from the panel's children +matching the `ProgressiveDisclosurePanelItem` component type. + Each menu item allows for the display of the corresponding child to be toggled on or off. The control's `onSelect` and `onDeselect` callbacks are fired allowing for greater control over the child e.g. resetting block attributes when @@ -21,7 +23,12 @@ child's props. ### Usage ```jsx -import { __experimentalProgressiveDisclosurePanel as ProgressiveDisclosurePanel } from '@wordpress/components'; +import { + __experimentalProgressiveDisclosurePanel as ProgressiveDisclosurePanel, + __experimentalProgressiveDisclosurePanelItem as ProgressiveDisclosurePanelItem, +} from '@wordpress/components'; +import { __ } from '@wordpress/i18n'; + import { PaddingEdit, hasPaddingValue, @@ -44,12 +51,13 @@ export function DimensionPanel( props ) { resetAll={ resetAll } > { ! isPaddingDisabled && ( - hasPaddingValue( props ) } label={ __( 'Padding' ) } - onDeselect={ resetPadding } - /> + onDeselect={ () => resetPadding( props ) } + > + + ) } ); @@ -73,45 +81,27 @@ Title to be displayed within the panel's title. ### Sub-Components -#### ProgressiveDisclosurePanelTitle +#### ProgressiveDisclosurePanelItem -This is a simple component to display the panel title and house the dropdown -menu for toggling child display. It is used by the `ProgressiveDisclosurePanel` -component under the hood, so it does not typically need to be used. +This component acts a wrapper and controls the display of items to contained +within a ProgressiveDisclosurePanel. An item is displayed if it is +flagged as a default control or the corresponding panel menu item, provided via +context, is toggled on for this item. ##### Props -###### resetAll +###### isShownByDefault -A function to call when the `Reset all` menu option is selected. +This prop identifies the current item as being displayed by default. This means +it will show regardless of whether it has a value set or is toggled on in the +panel's menu. -- Type: `function` +- Type: `boolean` - Required: Yes -###### toggleChild +###### label -Callback used to toggle display of an individual child component. +The label acts as a key to locate the corresponding item in the panel's menu +context. This is used when checking if the panel item should be displayed. -- Type: `function` +- Type: `string` - Required: Yes - -###### menuItems - -This object represents the panel's children and their visibility state. It -is built by the parent panel from its children prop. - -- Type: `Object` -- Required: No - -###### menuLabel - -A label for the dropdown menu. - -- Type: `String` -- Required: No - -###### title - -The panel title to display. - -- Type: `String` -- Required: No diff --git a/packages/components/src/progressive-disclosure-panel/index.js b/packages/components/src/progressive-disclosure-panel/index.js index eba21a8b56c95..45c9c410ec4b6 100644 --- a/packages/components/src/progressive-disclosure-panel/index.js +++ b/packages/components/src/progressive-disclosure-panel/index.js @@ -30,9 +30,8 @@ const ProgressiveDisclosurePanel = ( props ) => { const { children, className, label: menuLabel, resetAll, title } = props; const [ menuItems, setMenuItems ] = useState( {} ); - // When conditionally including components e.g. { isShown && } - // a boolean `false` will be passed as a child if component is excluded. - // This panel is only interested in the children to be displayed. + // This panel only needs to concern itself with the + // ProgressiveDisclosurePanelItem components to be displayed in the menu. const filteredChildren = useMemo( () => { return Array.isArray( children ) ? children.filter( isMenuItem ) : []; }, [ children ] ); From 07420bc5c142c74c37c291fc463d7b70a87bbcff Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Mon, 12 Jul 2021 18:56:17 +1000 Subject: [PATCH 22/50] Update panel story with new item component --- .../stories/index.js | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/packages/components/src/progressive-disclosure-panel/stories/index.js b/packages/components/src/progressive-disclosure-panel/stories/index.js index a8525aa25caf5..f081cb68e97d4 100644 --- a/packages/components/src/progressive-disclosure-panel/stories/index.js +++ b/packages/components/src/progressive-disclosure-panel/stories/index.js @@ -12,6 +12,7 @@ import { useState } from '@wordpress/element'; * Internal dependencies */ import ProgressiveDisclosurePanel from '../'; +import ProgressiveDisclosurePanelItem from '../item'; import Panel from '../../panel'; import UnitControl from '../../unit-control'; @@ -37,20 +38,28 @@ export const _default = () => { title="Progressive Disclosure Panel" resetAll={ resetAll } > - !! height } label="Height" onDeselect={ () => setHeight( undefined ) } - value={ height } - onChange={ ( next ) => setHeight( next ) } - /> - + setHeight( next ) } + /> + + !! width } label="Width" onDeselect={ () => setWidth( undefined ) } - value={ width } - onChange={ ( next ) => setWidth( next ) } - /> + > + setWidth( next ) } + /> + @@ -64,6 +73,6 @@ function PlaceholderControl( { label, value, onChange } ) { } const PanelWrapperView = styled.div` - max-width: 232px; + max-width: 250px; font-size: 13px; `; From 9ec400faf5feabeddfea81510c2d55b759900730 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Mon, 12 Jul 2021 20:44:08 +1000 Subject: [PATCH 23/50] Update tests to handle new panel item component --- .../test/index.js | 65 ++++++++++++++----- .../src/progressive-disclosure-panel/title.js | 2 +- 2 files changed, 48 insertions(+), 19 deletions(-) diff --git a/packages/components/src/progressive-disclosure-panel/test/index.js b/packages/components/src/progressive-disclosure-panel/test/index.js index 3fff561a758a8..866aa1305e3fa 100644 --- a/packages/components/src/progressive-disclosure-panel/test/index.js +++ b/packages/components/src/progressive-disclosure-panel/test/index.js @@ -7,15 +7,9 @@ import { render, screen, fireEvent } from '@testing-library/react'; * Internal dependencies */ import ProgressiveDisclosurePanel from '../'; - -// Represents a child provided to the panel such as a block support control. -const MockControl = ( { children } ) =>
{ children }
; +import PanelItem from '../item'; const resetAll = jest.fn(); -const hasValue = jest.fn().mockImplementation( ( props ) => { - // Use fudged attribute to determine existence of value for testing. - return props.attributes.value; -} ); // Default props for the progressive disclosure panel. const defaultProps = { @@ -27,7 +21,9 @@ const defaultProps = { // Default props for an enabled control to be rendered within panel. const controlProps = { attributes: { value: true }, - hasValue, + hasValue: jest.fn().mockImplementation( () => { + return !! controlProps.attributes.value; + } ), label: 'Example', onDeselect: jest.fn().mockImplementation( () => { controlProps.attributes.value = undefined; @@ -39,7 +35,9 @@ const controlProps = { // the panel. const altControlProps = { attributes: { value: false }, - hasValue, + hasValue: jest.fn().mockImplementation( () => { + return !! altControlProps.attributes.value; + } ), label: 'Alt', onDeselect: jest.fn(), onSelect: jest.fn(), @@ -49,14 +47,19 @@ const altControlProps = { const getPanel = ( container ) => container.querySelector( '.components-progressive-disclosure-panel' ); -// Renders a default progressive disclosure panel including a child that -// is being conditionally disabled. +// Renders a default progressive disclosure panel including children that are +// not to be represented within the panel's menu. const renderPanel = () => { return render( { false &&
Hidden
} - Example control - Alt control + +
Example control
+ + +
Alt control
+ + Visible ); }; @@ -88,30 +91,56 @@ describe( 'ProgressiveDisclosurePanel', () => { // This covers case where children prop is not an array. const { container } = render( - { false && Should not show } + { false && Should not show } ); expect( getPanel( container ) ).not.toBeInTheDocument(); } ); - it( 'should not render when all children have been filtered out', () => { + it( 'should not render when there are no progressive panel items', () => { const { container } = render( - { false && Should not show } - { false && Not shown either } + { false && Should not show } + { false && Not shown either } + Visible but insignificant ); expect( getPanel( container ) ).not.toBeInTheDocument(); } ); - it( 'should render panel when at least one child', () => { + it( 'should render panel when at least one panel item as child', () => { const { container } = renderPanel(); expect( getPanel( container ) ).toBeInTheDocument(); } ); + it( 'should render non panel item child', () => { + renderPanel(); + + const nonPanelItem = screen.queryByText( 'Visible' ); + + expect( nonPanelItem ).toBeInTheDocument(); + } ); + + it( 'should render child flagged as default control even without value', () => { + render( + + +
Example control
+ + +
Alt control
+ + + ); + + const altControl = screen.getByText( 'Alt control' ); + + expect( altControl ).toBeInTheDocument(); + } ); + it( 'should render display options menu', () => { renderPanel(); diff --git a/packages/components/src/progressive-disclosure-panel/title.js b/packages/components/src/progressive-disclosure-panel/title.js index 7f29f80d002e5..636ed67a872e2 100644 --- a/packages/components/src/progressive-disclosure-panel/title.js +++ b/packages/components/src/progressive-disclosure-panel/title.js @@ -33,7 +33,7 @@ const ProgressiveDisclosurePanelTitle = ( props ) => { { toggleChild( label ); onClose(); From a4d96b04258e4136510436dfb24a57c9190809e3 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Wed, 21 Jul 2021 17:25:34 +1000 Subject: [PATCH 24/50] Clean new style objects after resetting spacing values --- packages/block-editor/src/hooks/margin.js | 6 +++--- packages/block-editor/src/hooks/padding.js | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/block-editor/src/hooks/margin.js b/packages/block-editor/src/hooks/margin.js index b734191a35b12..6a264d0d4d0e3 100644 --- a/packages/block-editor/src/hooks/margin.js +++ b/packages/block-editor/src/hooks/margin.js @@ -31,7 +31,7 @@ export function hasMarginSupport( blockType ) { /** * Checks if there is a current value in the margin block support attributes. * - * @param {Object} props Block props. + * @param {Object} props Block props. * @return {boolean} Whether or not the block has a margin value set. */ export function hasMarginValue( props ) { @@ -50,13 +50,13 @@ export function resetMargin( { attributes = {}, setAttributes } ) { const { style } = attributes; setAttributes( { - style: { + style: cleanEmptyObject( { ...style, spacing: { ...style?.spacing, margin: undefined, }, - }, + } ), } ); } diff --git a/packages/block-editor/src/hooks/padding.js b/packages/block-editor/src/hooks/padding.js index c8392f04ec7b7..9e881f4423300 100644 --- a/packages/block-editor/src/hooks/padding.js +++ b/packages/block-editor/src/hooks/padding.js @@ -31,7 +31,7 @@ export function hasPaddingSupport( blockType ) { /** * Checks if there is a current value in the padding block support attributes. * - * @param {Object} props Block props. + * @param {Object} props Block props. * @return {boolean} Whether or not the block has a padding value set. */ export function hasPaddingValue( props ) { @@ -50,13 +50,13 @@ export function resetPadding( { attributes = {}, setAttributes } ) { const { style } = attributes; setAttributes( { - style: { + style: cleanEmptyObject( { ...style, spacing: { ...style?.spacing, padding: undefined, }, - }, + } ), } ); } From 17bd1b6576164aefb1b2f804be6058127ad24b7b Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Thu, 22 Jul 2021 14:55:21 +1000 Subject: [PATCH 25/50] Disable default control menu items when they have no value --- .../src/progressive-disclosure-panel/index.js | 31 ++++++++++++++----- .../src/progressive-disclosure-panel/item.js | 5 ++- .../src/progressive-disclosure-panel/title.js | 8 +++-- 3 files changed, 31 insertions(+), 13 deletions(-) diff --git a/packages/components/src/progressive-disclosure-panel/index.js b/packages/components/src/progressive-disclosure-panel/index.js index 45c9c410ec4b6..41c892b80ace3 100644 --- a/packages/components/src/progressive-disclosure-panel/index.js +++ b/packages/components/src/progressive-disclosure-panel/index.js @@ -41,11 +41,20 @@ const ProgressiveDisclosurePanel = ( props ) => { useEffect( () => { const items = {}; - filteredChildren.forEach( ( { props: { hasValue, label } } ) => { - // New item is checked if: + filteredChildren.forEach( ( { props: childProps } ) => { + const { hasValue, isShownByDefault, label } = childProps; + + // Menu item is checked if: // - it currently has a value // - or it was checked in previous menuItems state. - items[ label ] = hasValue() || menuItems[ label ]; + const isChecked = hasValue() || menuItems[ label ] === true; + + // Menu item will be `disabled` if: + // - it is not checked + // - and is shown by default. + const isDisabled = ! isChecked && isShownByDefault; + + items[ label ] = isDisabled ? 'disabled' : isChecked; } ); setMenuItems( items ); @@ -66,7 +75,7 @@ const ProgressiveDisclosurePanel = ( props ) => { const toggleChild = ( label ) => { const wasSelected = menuItems[ label ]; const child = getChildByMenuLabel( label ); - const { onDeselect, onSelect } = child.props; + const { onDeselect, onSelect, isShownByDefault } = child.props; if ( wasSelected && onDeselect ) { onDeselect(); @@ -76,9 +85,13 @@ const ProgressiveDisclosurePanel = ( props ) => { onSelect(); } + // If child is was checked but is no longer and also shown by default + // disable the child's menu item. + const isDisabled = wasSelected && isShownByDefault; + setMenuItems( { ...menuItems, - [ label ]: ! wasSelected, + [ label ]: isDisabled ? 'disabled' : ! wasSelected, } ); }; @@ -89,11 +102,13 @@ const ProgressiveDisclosurePanel = ( props ) => { } // Turn off all menu items. Default controls will continue to display - // by virtue of their `isShownByDefault` prop. + // by virtue of their `isShownByDefault` prop however their menu item + // will be disabled to prevent behaviour where toggling has no effect. const resetMenuItems = {}; - filteredChildren.forEach( ( child ) => { - resetMenuItems[ child.props.label ] = false; + filteredChildren.forEach( ( { props: childProps } ) => { + const { label, isShownByDefault } = childProps; + resetMenuItems[ label ] = isShownByDefault ? 'disabled' : false; } ); setMenuItems( resetMenuItems ); diff --git a/packages/components/src/progressive-disclosure-panel/item.js b/packages/components/src/progressive-disclosure-panel/item.js index 5cba0f2f5f7c9..f1f46ddff5d3d 100644 --- a/packages/components/src/progressive-disclosure-panel/item.js +++ b/packages/components/src/progressive-disclosure-panel/item.js @@ -6,14 +6,13 @@ import { usePanelContext } from './'; // This wraps controls to be conditionally displayed within a progressive // disclosure panel. It helps prevent props being applied to HTML elements that // would otherwise be invalid. -const ProgressiveDisclosurePanelItem = ( props ) => { - const { children, isShownByDefault, label } = props; +const ProgressiveDisclosurePanelItem = ( { children, label } ) => { const menuItems = usePanelContext(); // Do not show if menu item not selected and not shown by default. // If the item has a value that will be reflected in the menu item's // selected status provided by context. - if ( ! menuItems[ label ] && ! isShownByDefault ) { + if ( menuItems[ label ] === false ) { return null; } diff --git a/packages/components/src/progressive-disclosure-panel/title.js b/packages/components/src/progressive-disclosure-panel/title.js index 636ed67a872e2..a166db4174a26 100644 --- a/packages/components/src/progressive-disclosure-panel/title.js +++ b/packages/components/src/progressive-disclosure-panel/title.js @@ -28,12 +28,16 @@ const ProgressiveDisclosurePanelTitle = ( props ) => { <> { Object.entries( menuItems ).map( - ( [ label, isSelected ] ) => { + ( [ label, itemState ] ) => { + const isSelected = itemState === true; + const isDisabled = itemState === 'disabled'; + return ( { toggleChild( label ); onClose(); From 4e33e6c727f5068a97ece3b46bdf76b39d0d2096 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Thu, 22 Jul 2021 16:49:48 +1000 Subject: [PATCH 26/50] Add test for disabling default control menu items --- .../test/index.js | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/components/src/progressive-disclosure-panel/test/index.js b/packages/components/src/progressive-disclosure-panel/test/index.js index 866aa1305e3fa..ba5e3ad2b4773 100644 --- a/packages/components/src/progressive-disclosure-panel/test/index.js +++ b/packages/components/src/progressive-disclosure-panel/test/index.js @@ -168,6 +168,26 @@ describe( 'ProgressiveDisclosurePanel', () => { expect( menuItems[ 1 ] ).toHaveAttribute( 'aria-checked', 'false' ); } ); + it( 'should disable default control menu items when the control has no value', async () => { + render( + + +
Example control
+ + +
Alt control
+ + + ); + openDropdownMenu(); + + const menuItems = await screen.findAllByRole( 'menuitemcheckbox' ); + + expect( menuItems.length ).toEqual( 2 ); + expect( menuItems[ 0 ] ).not.toHaveAttribute( 'disabled' ); + expect( menuItems[ 1 ] ).toHaveAttribute( 'disabled' ); + } ); + it( 'should render panel title', () => { renderPanel(); const title = screen.getByText( defaultProps.title ); From 3609b5a21b266465e576eefa8bf9cc6f4e851844 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Tue, 27 Jul 2021 16:14:55 +1000 Subject: [PATCH 27/50] Add cleanEmptyObject to dimensions resetAll --- packages/block-editor/src/hooks/dimensions.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/hooks/dimensions.js b/packages/block-editor/src/hooks/dimensions.js index 57ce3a764b31e..ea732e4903151 100644 --- a/packages/block-editor/src/hooks/dimensions.js +++ b/packages/block-editor/src/hooks/dimensions.js @@ -27,6 +27,7 @@ import { resetPadding, useIsPaddingDisabled, } from './padding'; +import { cleanEmptyObject } from './utils'; export const SPACING_SUPPORT_KEY = 'spacing'; @@ -57,14 +58,14 @@ export function DimensionsPanel( props ) { const { style } = props.attributes; props.setAttributes( { - style: { + style: cleanEmptyObject( { ...style, spacing: { ...style?.spacing, margin: undefined, padding: undefined, }, - }, + } ), } ); }; From 5746e25338f5114c7076f404021da1c8daf43d7c Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Tue, 27 Jul 2021 16:48:00 +1000 Subject: [PATCH 28/50] Switch panel to grid layout for future multi-column layout This will default the panel to a 2 column CSS grid layout. Immediate controls wrapped in divs or fieldsets will default to spanning both columns given that is the most common case. For controls that can be collapsed onto a single row they only need set `grid-column: 1`. This will assist displaying height/width on a single row etc. --- .../src/progressive-disclosure-panel/style.scss | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/components/src/progressive-disclosure-panel/style.scss b/packages/components/src/progressive-disclosure-panel/style.scss index d332fcd14c07d..c57853d4180ca 100644 --- a/packages/components/src/progressive-disclosure-panel/style.scss +++ b/packages/components/src/progressive-disclosure-panel/style.scss @@ -3,6 +3,10 @@ border-top: $border-width solid $gray-200; padding: $grid-unit-20; margin-top: -1px; + display: grid; + grid-template-columns: 1fr 1fr; + column-gap: $grid-unit-20; + row-gap: $grid-unit-30; .components-progressive-disclosure-panel__title { margin: 0; @@ -12,6 +16,7 @@ font-weight: 500; line-height: normal; font-size: inherit; + grid-column: span 2; .components-dropdown-menu { height: $grid-unit-30; @@ -25,16 +30,14 @@ width: $grid-unit-30; height: $grid-unit-30; } - - &:not(:last-child) { - padding-bottom: $grid-unit-30; - } } - > div { + > div, + > fieldset { padding-bottom: 0; - margin-bottom: $grid-unit-30; + margin-bottom: 0; max-width: 100%; + grid-column: span 2; &:last-child { margin-bottom: $grid-unit-10; From c8d96ee28c453e43b4c34ca30e4ff119089d28ed Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Wed, 28 Jul 2021 11:48:07 +1000 Subject: [PATCH 29/50] Tweak defaults for controls spacing in progressive disclosure panel --- .../progressive-disclosure-panel/style.scss | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/components/src/progressive-disclosure-panel/style.scss b/packages/components/src/progressive-disclosure-panel/style.scss index c57853d4180ca..d4413a393ee93 100644 --- a/packages/components/src/progressive-disclosure-panel/style.scss +++ b/packages/components/src/progressive-disclosure-panel/style.scss @@ -31,16 +31,16 @@ height: $grid-unit-30; } } +} - > div, - > fieldset { - padding-bottom: 0; - margin-bottom: 0; - max-width: 100%; - grid-column: span 2; - - &:last-child { - margin-bottom: $grid-unit-10; +.block-editor-block-inspector { + .components-progressive-disclosure-panel { + > div, + > fieldset { + padding-bottom: 0; + margin-bottom: 0; + max-width: 100%; + grid-column: span 2; } } } From 7c22312a211d0f5ee131c524bdb89ad2a929616c Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Wed, 28 Jul 2021 15:19:33 +1000 Subject: [PATCH 30/50] Fix Global Styles sidebar panel styling --- .../components/src/progressive-disclosure-panel/style.scss | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/components/src/progressive-disclosure-panel/style.scss b/packages/components/src/progressive-disclosure-panel/style.scss index d4413a393ee93..bc5b3f6efdf44 100644 --- a/packages/components/src/progressive-disclosure-panel/style.scss +++ b/packages/components/src/progressive-disclosure-panel/style.scss @@ -33,7 +33,8 @@ } } -.block-editor-block-inspector { +.block-editor-block-inspector, +.edit-site { .components-progressive-disclosure-panel { > div, > fieldset { From dbaa31c8fc583dcc77ac0da8a70d4a8cda2f8fec Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Thu, 29 Jul 2021 15:48:40 +1000 Subject: [PATCH 31/50] Handle cases where panel item's inner element returns null --- packages/components/src/progressive-disclosure-panel/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/progressive-disclosure-panel/index.js b/packages/components/src/progressive-disclosure-panel/index.js index 41c892b80ace3..e315fe917b2c7 100644 --- a/packages/components/src/progressive-disclosure-panel/index.js +++ b/packages/components/src/progressive-disclosure-panel/index.js @@ -24,7 +24,7 @@ const PanelContext = createContext( {} ); export const usePanelContext = () => useContext( PanelContext ); -const isMenuItem = ( item ) => item.type === ProgressiveDisclosurePanelItem; +const isMenuItem = ( item ) => item?.type === ProgressiveDisclosurePanelItem; const ProgressiveDisclosurePanel = ( props ) => { const { children, className, label: menuLabel, resetAll, title } = props; From 8f196bc94ce59f073d380c61b93fcf50065b47db Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Fri, 30 Jul 2021 14:41:54 +1000 Subject: [PATCH 32/50] Correct component name in readme Co-authored-by: Marco Ciampini --- packages/components/src/progressive-disclosure-panel/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/progressive-disclosure-panel/README.md b/packages/components/src/progressive-disclosure-panel/README.md index 350f49db58da4..3803eb4482c8a 100644 --- a/packages/components/src/progressive-disclosure-panel/README.md +++ b/packages/components/src/progressive-disclosure-panel/README.md @@ -1,4 +1,4 @@ -# Progressive Disclosure Panel +# ProgressiveDisclosurePanel These panels provide progressive discovery options for their children. For example the controls provided via block supports. From 76a2b8df3072159924e13058963c4889a39ec1a0 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Fri, 30 Jul 2021 14:52:10 +1000 Subject: [PATCH 33/50] Update readme with experimental alert and improved prop format --- .../src/progressive-disclosure-panel/README.md | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/components/src/progressive-disclosure-panel/README.md b/packages/components/src/progressive-disclosure-panel/README.md index 3803eb4482c8a..da21f55709cb3 100644 --- a/packages/components/src/progressive-disclosure-panel/README.md +++ b/packages/components/src/progressive-disclosure-panel/README.md @@ -1,5 +1,9 @@ # ProgressiveDisclosurePanel +
+This feature is still experimental. “Experimental” means this is an early implementation subject to drastic and breaking changes. +
+ These panels provide progressive discovery options for their children. For example the controls provided via block supports. @@ -66,18 +70,24 @@ export function DimensionPanel( props ) { ### Props -#### label +#### `label`: `string` The label for the panel's dropdown menu. -#### resetAll +- Required: Yes + +#### `resetAll`: `function` A function to call when the `Reset all` menu option is selected. This is passed through to the panel's title component. -#### title +- Required: Yes + +#### `title`: `string` + +Text to be displayed within the panel's title. -Title to be displayed within the panel's title. +- Required: Yes ### Sub-Components From 224c0eaffde8bb2e40cc9780136c0420c60c69b0 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Fri, 30 Jul 2021 15:42:14 +1000 Subject: [PATCH 34/50] Fix menuItem child filtering using Children util. --- packages/components/src/progressive-disclosure-panel/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/components/src/progressive-disclosure-panel/index.js b/packages/components/src/progressive-disclosure-panel/index.js index e315fe917b2c7..11cd28dae3bb5 100644 --- a/packages/components/src/progressive-disclosure-panel/index.js +++ b/packages/components/src/progressive-disclosure-panel/index.js @@ -7,6 +7,7 @@ import classnames from 'classnames'; * WordPress dependencies */ import { + Children, createContext, useContext, useEffect, @@ -33,7 +34,7 @@ const ProgressiveDisclosurePanel = ( props ) => { // This panel only needs to concern itself with the // ProgressiveDisclosurePanelItem components to be displayed in the menu. const filteredChildren = useMemo( () => { - return Array.isArray( children ) ? children.filter( isMenuItem ) : []; + return Children.toArray( children ).filter( isMenuItem ); }, [ children ] ); // Refresh which children should be reflected in the menu and what their From 8343c9b7c9f96723d14432c9192dc32b9d150e83 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Fri, 30 Jul 2021 15:50:02 +1000 Subject: [PATCH 35/50] Accurately name the ProgressiveDisclosurePanelItem in tests --- .../test/index.js | 53 +++++++++++++------ 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/packages/components/src/progressive-disclosure-panel/test/index.js b/packages/components/src/progressive-disclosure-panel/test/index.js index ba5e3ad2b4773..3a23263d06565 100644 --- a/packages/components/src/progressive-disclosure-panel/test/index.js +++ b/packages/components/src/progressive-disclosure-panel/test/index.js @@ -7,7 +7,7 @@ import { render, screen, fireEvent } from '@testing-library/react'; * Internal dependencies */ import ProgressiveDisclosurePanel from '../'; -import PanelItem from '../item'; +import ProgressiveDisclosurePanelItem from '../item'; const resetAll = jest.fn(); @@ -53,12 +53,12 @@ const renderPanel = () => { return render( { false &&
Hidden
} - +
Example control
- - + +
Alt control
- + Visible ); @@ -91,7 +91,11 @@ describe( 'ProgressiveDisclosurePanel', () => { // This covers case where children prop is not an array. const { container } = render( - { false && Should not show } + { false && ( + + Should not show + + ) } ); @@ -101,8 +105,16 @@ describe( 'ProgressiveDisclosurePanel', () => { it( 'should not render when there are no progressive panel items', () => { const { container } = render( - { false && Should not show } - { false && Not shown either } + { false && ( + + Should not show + + ) } + { false && ( + + Not shown either + + ) } Visible but insignificant ); @@ -127,12 +139,15 @@ describe( 'ProgressiveDisclosurePanel', () => { it( 'should render child flagged as default control even without value', () => { render( - +
Example control
- - + +
Alt control
- + ); @@ -171,12 +186,18 @@ describe( 'ProgressiveDisclosurePanel', () => { it( 'should disable default control menu items when the control has no value', async () => { render( - +
Example control
- - + +
Alt control
- + ); openDropdownMenu(); From 6068bec5b812f8aad493af038206d4b30db889ff Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Fri, 30 Jul 2021 16:51:35 +1000 Subject: [PATCH 36/50] Initial pass at restructuring component This relocates the various progressive disclosure panel components into different subdirectories as an initial step to bringing this component into inline with newer components being added to Gutenberg. --- docs/manifest.json | 14 +- packages/components/src/index.js | 6 +- .../src/progressive-disclosure-panel/index.js | 140 +----------------- .../README.md | 33 +++++ .../component.js} | 2 +- .../index.js | 1 + .../README.md | 35 +++++ .../component.js} | 8 +- .../index.js | 1 + .../README.md | 39 +---- .../progressive-disclosure-panel/component.js | 138 +++++++++++++++++ .../progressive-disclosure-panel/index.js | 1 + .../stories/index.js | 6 +- .../test/index.js | 6 +- 14 files changed, 247 insertions(+), 183 deletions(-) create mode 100644 packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/README.md rename packages/components/src/progressive-disclosure-panel/{item.js => progressive-disclosure-panel-item/component.js} (90%) create mode 100644 packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/index.js create mode 100644 packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-title/README.md rename packages/components/src/progressive-disclosure-panel/{title.js => progressive-disclosure-panel-title/component.js} (88%) create mode 100644 packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-title/index.js rename packages/components/src/progressive-disclosure-panel/{ => progressive-disclosure-panel}/README.md (73%) create mode 100644 packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/component.js create mode 100644 packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/index.js diff --git a/docs/manifest.json b/docs/manifest.json index a1e4afa816823..fafda77de8e7d 100644 --- a/docs/manifest.json +++ b/docs/manifest.json @@ -1091,10 +1091,22 @@ "markdown_source": "../packages/components/src/popover/README.md", "parent": "components" }, + { + "title": "ProgressiveDisclosurePanelItem", + "slug": "progressive-disclosure-panel-item", + "markdown_source": "../packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/README.md", + "parent": "components" + }, + { + "title": "ProgressiveDisclosurePanelTitle", + "slug": "progressive-disclosure-panel-title", + "markdown_source": "../packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-title/README.md", + "parent": "components" + }, { "title": "ProgressiveDisclosurePanel", "slug": "progressive-disclosure-panel", - "markdown_source": "../packages/components/src/progressive-disclosure-panel/README.md", + "markdown_source": "../packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/README.md", "parent": "components" }, { diff --git a/packages/components/src/index.js b/packages/components/src/index.js index 7dc6c9f5e64fa..c8d2c5409ab28 100644 --- a/packages/components/src/index.js +++ b/packages/components/src/index.js @@ -100,8 +100,10 @@ export { default as PanelHeader } from './panel/header'; export { default as PanelRow } from './panel/row'; export { default as Placeholder } from './placeholder'; export { default as Popover } from './popover'; -export { default as __experimentalProgressiveDisclosurePanel } from './progressive-disclosure-panel'; -export { default as __experimentalProgressiveDisclosurePanelItem } from './progressive-disclosure-panel/item'; +export { + ProgressiveDisclosurePanel as __experimentalProgressiveDisclosurePanel, + ProgressiveDisclosurePanelItem as __experimentalProgressiveDisclosurePanelItem, +} from './progressive-disclosure-panel'; export { default as QueryControls } from './query-controls'; export { default as __experimentalRadio } from './radio'; export { default as __experimentalRadioGroup } from './radio-group'; diff --git a/packages/components/src/progressive-disclosure-panel/index.js b/packages/components/src/progressive-disclosure-panel/index.js index 11cd28dae3bb5..c4549adfd9c28 100644 --- a/packages/components/src/progressive-disclosure-panel/index.js +++ b/packages/components/src/progressive-disclosure-panel/index.js @@ -1,138 +1,2 @@ -/** - * External dependencies - */ -import classnames from 'classnames'; - -/** - * WordPress dependencies - */ -import { - Children, - createContext, - useContext, - useEffect, - useMemo, - useState, -} from '@wordpress/element'; - -/** - * Internal dependencies - */ -import ProgressiveDisclosurePanelItem from './item'; -import ProgressiveDisclosurePanelTitle from './title'; - -const PanelContext = createContext( {} ); - -export const usePanelContext = () => useContext( PanelContext ); - -const isMenuItem = ( item ) => item?.type === ProgressiveDisclosurePanelItem; - -const ProgressiveDisclosurePanel = ( props ) => { - const { children, className, label: menuLabel, resetAll, title } = props; - const [ menuItems, setMenuItems ] = useState( {} ); - - // This panel only needs to concern itself with the - // ProgressiveDisclosurePanelItem components to be displayed in the menu. - const filteredChildren = useMemo( () => { - return Children.toArray( children ).filter( isMenuItem ); - }, [ children ] ); - - // Refresh which children should be reflected in the menu and what their - // associated menu item's state is; checked or not. - useEffect( () => { - const items = {}; - - filteredChildren.forEach( ( { props: childProps } ) => { - const { hasValue, isShownByDefault, label } = childProps; - - // Menu item is checked if: - // - it currently has a value - // - or it was checked in previous menuItems state. - const isChecked = hasValue() || menuItems[ label ] === true; - - // Menu item will be `disabled` if: - // - it is not checked - // - and is shown by default. - const isDisabled = ! isChecked && isShownByDefault; - - items[ label ] = isDisabled ? 'disabled' : isChecked; - } ); - - setMenuItems( items ); - }, [ filteredChildren ] ); - - if ( filteredChildren.length === 0 ) { - return null; - } - - const getChildByMenuLabel = ( label ) => { - return filteredChildren.find( - ( child ) => child.props.label === label - ); - }; - - // Toggles the customized state of the child and its display if it isn't to - // be displayed by default. When toggling a child it's callback is executed. - const toggleChild = ( label ) => { - const wasSelected = menuItems[ label ]; - const child = getChildByMenuLabel( label ); - const { onDeselect, onSelect, isShownByDefault } = child.props; - - if ( wasSelected && onDeselect ) { - onDeselect(); - } - - if ( ! wasSelected && onSelect ) { - onSelect(); - } - - // If child is was checked but is no longer and also shown by default - // disable the child's menu item. - const isDisabled = wasSelected && isShownByDefault; - - setMenuItems( { - ...menuItems, - [ label ]: isDisabled ? 'disabled' : ! wasSelected, - } ); - }; - - // Resets display of children and executes resetAll callback if available. - const resetAllChildren = () => { - if ( typeof resetAll === 'function' ) { - resetAll(); - } - - // Turn off all menu items. Default controls will continue to display - // by virtue of their `isShownByDefault` prop however their menu item - // will be disabled to prevent behaviour where toggling has no effect. - const resetMenuItems = {}; - - filteredChildren.forEach( ( { props: childProps } ) => { - const { label, isShownByDefault } = childProps; - resetMenuItems[ label ] = isShownByDefault ? 'disabled' : false; - } ); - - setMenuItems( resetMenuItems ); - }; - - const classes = classnames( - 'components-progressive-disclosure-panel', - className - ); - - return ( -
- - - { children } - -
- ); -}; - -export default ProgressiveDisclosurePanel; +export { default as ProgressiveDisclosurePanel } from './progressive-disclosure-panel'; +export { default as ProgressiveDisclosurePanelItem } from './progressive-disclosure-panel-item'; diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/README.md b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/README.md new file mode 100644 index 0000000000000..e897b0259a85e --- /dev/null +++ b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/README.md @@ -0,0 +1,33 @@ +# ProgressiveDisclosurePanelItem + +
+This feature is still experimental. “Experimental” means this is an early implementation subject to drastic and breaking changes. +
+
+ +This component acts a wrapper and controls the display of items to be contained +within a ProgressiveDisclosurePanel. An item is displayed if it is +flagged as a default control or the corresponding panel menu item, provided via +context, is toggled on for this item. + +## Usage + +See [`progressive-disclosure-panel/README.md#usage`](/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/) for how to use +`ProgressiveDisclosurePanelItem`. + +## Props + +### `isShownByDefault`: `boolean` + +This prop identifies the current item as being displayed by default. This means +it will show regardless of whether it has a value set or is toggled on in the +panel's menu. + +- Required: Yes + +### `label`: `string` + +The label acts as a key to locate the corresponding item in the panel's menu +context. This is used when checking if the panel item should be displayed. + +- Required: Yes diff --git a/packages/components/src/progressive-disclosure-panel/item.js b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/component.js similarity index 90% rename from packages/components/src/progressive-disclosure-panel/item.js rename to packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/component.js index f1f46ddff5d3d..454697de295a2 100644 --- a/packages/components/src/progressive-disclosure-panel/item.js +++ b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/component.js @@ -1,7 +1,7 @@ /** * Internal dependencies */ -import { usePanelContext } from './'; +import { usePanelContext } from '../progressive-disclosure-panel'; // This wraps controls to be conditionally displayed within a progressive // disclosure panel. It helps prevent props being applied to HTML elements that diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/index.js b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/index.js new file mode 100644 index 0000000000000..b404d7fd44a81 --- /dev/null +++ b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/index.js @@ -0,0 +1 @@ +export { default } from './component'; diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-title/README.md b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-title/README.md new file mode 100644 index 0000000000000..e000b1f5273db --- /dev/null +++ b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-title/README.md @@ -0,0 +1,35 @@ +# ProgressiveDisclosurePanelTitle + +
+This feature is still experimental. “Experimental” means this is an early implementation subject to drastic and breaking changes. +
+
+ +This component renders a progressive disclosure panel's title and menu. + + +## Usage + +This component is generated automatically by its parent +`ProgressiveDisclosurePanel`. + +
+In general, this should not be used directly. +
+ +## Props + +### `isShownByDefault`: `boolean` + +This prop identifies the current item as being displayed by default. This means +it will show regardless of whether it has a value set or is toggled on in the +panel's menu. + +- Required: Yes + +### `label`: `string` + +The label acts as a key to locate the corresponding item in the panel's menu +context. This is used when checking if the panel item should be displayed. + +- Required: Yes diff --git a/packages/components/src/progressive-disclosure-panel/title.js b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-title/component.js similarity index 88% rename from packages/components/src/progressive-disclosure-panel/title.js rename to packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-title/component.js index a166db4174a26..d27336324281d 100644 --- a/packages/components/src/progressive-disclosure-panel/title.js +++ b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-title/component.js @@ -7,10 +7,10 @@ import { __ } from '@wordpress/i18n'; /** * Internal dependencies */ -import { usePanelContext } from './'; -import MenuGroup from '../menu-group'; -import MenuItem from '../menu-item'; -import DropdownMenu from '../dropdown-menu'; +import { usePanelContext } from '../progressive-disclosure-panel'; +import MenuGroup from '../../menu-group'; +import MenuItem from '../../menu-item'; +import DropdownMenu from '../../dropdown-menu'; const ProgressiveDisclosurePanelTitle = ( props ) => { const { menuLabel, resetAll, title, toggleChild } = props; diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-title/index.js b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-title/index.js new file mode 100644 index 0000000000000..b404d7fd44a81 --- /dev/null +++ b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-title/index.js @@ -0,0 +1 @@ +export { default } from './component'; diff --git a/packages/components/src/progressive-disclosure-panel/README.md b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/README.md similarity index 73% rename from packages/components/src/progressive-disclosure-panel/README.md rename to packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/README.md index da21f55709cb3..e4791ce25d2f6 100644 --- a/packages/components/src/progressive-disclosure-panel/README.md +++ b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/README.md @@ -3,7 +3,7 @@
This feature is still experimental. “Experimental” means this is an early implementation subject to drastic and breaking changes.
- +
These panels provide progressive discovery options for their children. For example the controls provided via block supports. @@ -24,7 +24,7 @@ displaying by default through the `isShownByDefault` prop. Determining whether a child has a value is done via the `hasValue` function provided through the child's props. -### Usage +## Usage ```jsx import { @@ -68,50 +68,23 @@ export function DimensionPanel( props ) { } ``` -### Props +## Props -#### `label`: `string` +### `label`: `string` The label for the panel's dropdown menu. - Required: Yes -#### `resetAll`: `function` +### `resetAll`: `function` A function to call when the `Reset all` menu option is selected. This is passed through to the panel's title component. - Required: Yes -#### `title`: `string` +### `title`: `string` Text to be displayed within the panel's title. - Required: Yes - -### Sub-Components - -#### ProgressiveDisclosurePanelItem - -This component acts a wrapper and controls the display of items to contained -within a ProgressiveDisclosurePanel. An item is displayed if it is -flagged as a default control or the corresponding panel menu item, provided via -context, is toggled on for this item. - -##### Props -###### isShownByDefault - -This prop identifies the current item as being displayed by default. This means -it will show regardless of whether it has a value set or is toggled on in the -panel's menu. - -- Type: `boolean` -- Required: Yes - -###### label - -The label acts as a key to locate the corresponding item in the panel's menu -context. This is used when checking if the panel item should be displayed. - -- Type: `string` -- Required: Yes diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/component.js b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/component.js new file mode 100644 index 0000000000000..40aac13c95a29 --- /dev/null +++ b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/component.js @@ -0,0 +1,138 @@ +/** + * External dependencies + */ +import classnames from 'classnames'; + +/** + * WordPress dependencies + */ +import { + Children, + createContext, + useContext, + useEffect, + useMemo, + useState, +} from '@wordpress/element'; + +/** + * Internal dependencies + */ +import ProgressiveDisclosurePanelItem from '../progressive-disclosure-panel-item'; +import ProgressiveDisclosurePanelTitle from '../progressive-disclosure-panel-title'; + +const PanelContext = createContext( {} ); + +export const usePanelContext = () => useContext( PanelContext ); + +const isMenuItem = ( item ) => item?.type === ProgressiveDisclosurePanelItem; + +const ProgressiveDisclosurePanel = ( props ) => { + const { children, className, label: menuLabel, resetAll, title } = props; + const [ menuItems, setMenuItems ] = useState( {} ); + + // This panel only needs to concern itself with the + // ProgressiveDisclosurePanelItem components to be displayed in the menu. + const filteredChildren = useMemo( () => { + return Children.toArray( children ).filter( isMenuItem ); + }, [ children ] ); + + // Refresh which children should be reflected in the menu and what their + // associated menu item's state is; checked or not. + useEffect( () => { + const items = {}; + + filteredChildren.forEach( ( { props: childProps } ) => { + const { hasValue, isShownByDefault, label } = childProps; + + // Menu item is checked if: + // - it currently has a value + // - or it was checked in previous menuItems state. + const isChecked = hasValue() || menuItems[ label ] === true; + + // Menu item will be `disabled` if: + // - it is not checked + // - and is shown by default. + const isDisabled = ! isChecked && isShownByDefault; + + items[ label ] = isDisabled ? 'disabled' : isChecked; + } ); + + setMenuItems( items ); + }, [ filteredChildren ] ); + + if ( filteredChildren.length === 0 ) { + return null; + } + + const getChildByMenuLabel = ( label ) => { + return filteredChildren.find( + ( child ) => child.props.label === label + ); + }; + + // Toggles the customized state of the child and its display if it isn't to + // be displayed by default. When toggling a child it's callback is executed. + const toggleChild = ( label ) => { + const wasSelected = menuItems[ label ]; + const child = getChildByMenuLabel( label ); + const { onDeselect, onSelect, isShownByDefault } = child.props; + + if ( wasSelected && onDeselect ) { + onDeselect(); + } + + if ( ! wasSelected && onSelect ) { + onSelect(); + } + + // If child is was checked but is no longer and also shown by default + // disable the child's menu item. + const isDisabled = wasSelected && isShownByDefault; + + setMenuItems( { + ...menuItems, + [ label ]: isDisabled ? 'disabled' : ! wasSelected, + } ); + }; + + // Resets display of children and executes resetAll callback if available. + const resetAllChildren = () => { + if ( typeof resetAll === 'function' ) { + resetAll(); + } + + // Turn off all menu items. Default controls will continue to display + // by virtue of their `isShownByDefault` prop however their menu item + // will be disabled to prevent behaviour where toggling has no effect. + const resetMenuItems = {}; + + filteredChildren.forEach( ( { props: childProps } ) => { + const { label, isShownByDefault } = childProps; + resetMenuItems[ label ] = isShownByDefault ? 'disabled' : false; + } ); + + setMenuItems( resetMenuItems ); + }; + + const classes = classnames( + 'components-progressive-disclosure-panel', + className + ); + + return ( +
+ + + { children } + +
+ ); +}; + +export default ProgressiveDisclosurePanel; diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/index.js b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/index.js new file mode 100644 index 0000000000000..728a3a15e1aae --- /dev/null +++ b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/index.js @@ -0,0 +1 @@ +export { default, usePanelContext } from './component'; diff --git a/packages/components/src/progressive-disclosure-panel/stories/index.js b/packages/components/src/progressive-disclosure-panel/stories/index.js index f081cb68e97d4..78ce5921ba343 100644 --- a/packages/components/src/progressive-disclosure-panel/stories/index.js +++ b/packages/components/src/progressive-disclosure-panel/stories/index.js @@ -11,8 +11,10 @@ import { useState } from '@wordpress/element'; /** * Internal dependencies */ -import ProgressiveDisclosurePanel from '../'; -import ProgressiveDisclosurePanelItem from '../item'; +import { + ProgressiveDisclosurePanel, + ProgressiveDisclosurePanelItem, +} from '../'; import Panel from '../../panel'; import UnitControl from '../../unit-control'; diff --git a/packages/components/src/progressive-disclosure-panel/test/index.js b/packages/components/src/progressive-disclosure-panel/test/index.js index 3a23263d06565..fb5ba52e86919 100644 --- a/packages/components/src/progressive-disclosure-panel/test/index.js +++ b/packages/components/src/progressive-disclosure-panel/test/index.js @@ -6,8 +6,10 @@ import { render, screen, fireEvent } from '@testing-library/react'; /** * Internal dependencies */ -import ProgressiveDisclosurePanel from '../'; -import ProgressiveDisclosurePanelItem from '../item'; +import { + ProgressiveDisclosurePanel, + ProgressiveDisclosurePanelItem, +} from '../'; const resetAll = jest.fn(); From 48388f59105b03f7c5e2c01f852be143a7f621c6 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Fri, 30 Jul 2021 17:20:55 +1000 Subject: [PATCH 37/50] Rename title component and props to header --- docs/manifest.json | 12 ++++++------ packages/block-editor/src/hooks/dimensions.js | 2 +- .../README.md | 5 ++--- .../component.js | 12 ++++++------ .../index.js | 0 .../progressive-disclosure-panel/README.md | 8 ++++---- .../progressive-disclosure-panel/component.js | 10 +++++----- .../progressive-disclosure-panel/stories/index.js | 2 +- .../src/progressive-disclosure-panel/style.scss | 2 +- .../src/progressive-disclosure-panel/test/index.js | 8 ++++---- 10 files changed, 30 insertions(+), 31 deletions(-) rename packages/components/src/progressive-disclosure-panel/{progressive-disclosure-panel-title => progressive-disclosure-panel-header}/README.md (87%) rename packages/components/src/progressive-disclosure-panel/{progressive-disclosure-panel-title => progressive-disclosure-panel-header}/component.js (84%) rename packages/components/src/progressive-disclosure-panel/{progressive-disclosure-panel-title => progressive-disclosure-panel-header}/index.js (100%) diff --git a/docs/manifest.json b/docs/manifest.json index fafda77de8e7d..b94064d753d8c 100644 --- a/docs/manifest.json +++ b/docs/manifest.json @@ -1092,15 +1092,15 @@ "parent": "components" }, { - "title": "ProgressiveDisclosurePanelItem", - "slug": "progressive-disclosure-panel-item", - "markdown_source": "../packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/README.md", + "title": "ProgressiveDisclosurePanelHeader", + "slug": "progressive-disclosure-panel-header", + "markdown_source": "../packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/README.md", "parent": "components" }, { - "title": "ProgressiveDisclosurePanelTitle", - "slug": "progressive-disclosure-panel-title", - "markdown_source": "../packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-title/README.md", + "title": "ProgressiveDisclosurePanelItem", + "slug": "progressive-disclosure-panel-item", + "markdown_source": "../packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/README.md", "parent": "components" }, { diff --git a/packages/block-editor/src/hooks/dimensions.js b/packages/block-editor/src/hooks/dimensions.js index ea732e4903151..dac93610a2f48 100644 --- a/packages/block-editor/src/hooks/dimensions.js +++ b/packages/block-editor/src/hooks/dimensions.js @@ -73,7 +73,7 @@ export function DimensionsPanel( props ) { { ! isPaddingDisabled && ( diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-title/README.md b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/README.md similarity index 87% rename from packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-title/README.md rename to packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/README.md index e000b1f5273db..75ff37180c975 100644 --- a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-title/README.md +++ b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/README.md @@ -1,12 +1,11 @@ -# ProgressiveDisclosurePanelTitle +# ProgressiveDisclosurePanelHeader
This feature is still experimental. “Experimental” means this is an early implementation subject to drastic and breaking changes.

-This component renders a progressive disclosure panel's title and menu. - +This component renders a progressive disclosure panel's header including a menu. ## Usage diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-title/component.js b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/component.js similarity index 84% rename from packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-title/component.js rename to packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/component.js index d27336324281d..9a4e19eba295e 100644 --- a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-title/component.js +++ b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/component.js @@ -12,17 +12,17 @@ import MenuGroup from '../../menu-group'; import MenuItem from '../../menu-item'; import DropdownMenu from '../../dropdown-menu'; -const ProgressiveDisclosurePanelTitle = ( props ) => { - const { menuLabel, resetAll, title, toggleChild } = props; +const ProgressiveDisclosurePanelHeader = ( props ) => { + const { menuLabel, resetAll, header, toggleChild } = props; const menuItems = usePanelContext(); - if ( ! title ) { + if ( ! header ) { return null; } return ( -

- { title } +

+ { header } { ( { onClose } ) => ( <> @@ -67,4 +67,4 @@ const ProgressiveDisclosurePanelTitle = ( props ) => { ); }; -export default ProgressiveDisclosurePanelTitle; +export default ProgressiveDisclosurePanelHeader; diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-title/index.js b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/index.js similarity index 100% rename from packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-title/index.js rename to packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/index.js diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/README.md b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/README.md index e4791ce25d2f6..951aa22070b4e 100644 --- a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/README.md +++ b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/README.md @@ -50,8 +50,8 @@ export function DimensionPanel( props ) { return ( { ! isPaddingDisabled && ( @@ -79,12 +79,12 @@ The label for the panel's dropdown menu. ### `resetAll`: `function` A function to call when the `Reset all` menu option is selected. This is passed -through to the panel's title component. +through to the panel's header component. - Required: Yes -### `title`: `string` +### `header`: `string` -Text to be displayed within the panel's title. +Text to be displayed within the panel's header. - Required: Yes diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/component.js b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/component.js index 40aac13c95a29..c261cc490c3b1 100644 --- a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/component.js +++ b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/component.js @@ -19,7 +19,7 @@ import { * Internal dependencies */ import ProgressiveDisclosurePanelItem from '../progressive-disclosure-panel-item'; -import ProgressiveDisclosurePanelTitle from '../progressive-disclosure-panel-title'; +import ProgressiveDisclosurePanelHeader from '../progressive-disclosure-panel-header'; const PanelContext = createContext( {} ); @@ -28,7 +28,7 @@ export const usePanelContext = () => useContext( PanelContext ); const isMenuItem = ( item ) => item?.type === ProgressiveDisclosurePanelItem; const ProgressiveDisclosurePanel = ( props ) => { - const { children, className, label: menuLabel, resetAll, title } = props; + const { children, className, header, label: menuLabel, resetAll } = props; const [ menuItems, setMenuItems ] = useState( {} ); // This panel only needs to concern itself with the @@ -123,11 +123,11 @@ const ProgressiveDisclosurePanel = ( props ) => { return (
- { children } diff --git a/packages/components/src/progressive-disclosure-panel/stories/index.js b/packages/components/src/progressive-disclosure-panel/stories/index.js index 78ce5921ba343..6fd602c9251e3 100644 --- a/packages/components/src/progressive-disclosure-panel/stories/index.js +++ b/packages/components/src/progressive-disclosure-panel/stories/index.js @@ -36,8 +36,8 @@ export const _default = () => { { expect( menuItems[ 1 ] ).toHaveAttribute( 'disabled' ); } ); - it( 'should render panel title', () => { + it( 'should render panel header', () => { renderPanel(); - const title = screen.getByText( defaultProps.title ); + const header = screen.getByText( defaultProps.header ); - expect( title ).toBeInTheDocument(); + expect( header ).toBeInTheDocument(); } ); } ); From 92bcf52433711d36f2904287de4654177a3a12d9 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Mon, 2 Aug 2021 15:47:32 +1000 Subject: [PATCH 38/50] Change progressive panel items to register themselves --- .../README.md | 27 ++++-- .../component.js | 88 ++++++++++--------- .../README.md | 9 +- .../component.js | 38 +++++++- .../progressive-disclosure-panel/component.js | 82 ++++++++--------- .../test/index.js | 80 ++++++----------- 6 files changed, 177 insertions(+), 147 deletions(-) diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/README.md b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/README.md index 75ff37180c975..179fdb5f0c33d 100644 --- a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/README.md +++ b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/README.md @@ -18,17 +18,30 @@ This component is generated automatically by its parent ## Props -### `isShownByDefault`: `boolean` +### `header`: `string` -This prop identifies the current item as being displayed by default. This means -it will show regardless of whether it has a value set or is toggled on in the -panel's menu. +Text to be displayed within the panel header. - Required: Yes -### `label`: `string` +### `menuLabel`: `string` -The label acts as a key to locate the corresponding item in the panel's menu -context. This is used when checking if the panel item should be displayed. +This is passed along as the `label` for the panel header's `DropdownMenu`. + +- Required: No + +### `resetAll`: `function` + +The `resetAll` prop provides the callback to execute when the "Reset all" menu +item is selected. It's purpose is to facilitate resetting any control values +for items contained within this header's panel. + +- Required: Yes + +### `toggleItem`: `function` + +This is executed when an individual control's menu item is toggled. It +will update the panel's menu item state and call the panel item's `onSelect` or +`onDeselect` callbacks as appropriate. - Required: Yes diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/component.js b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/component.js index 9a4e19eba295e..d3e37add4183d 100644 --- a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/component.js +++ b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/component.js @@ -13,56 +13,62 @@ import MenuItem from '../../menu-item'; import DropdownMenu from '../../dropdown-menu'; const ProgressiveDisclosurePanelHeader = ( props ) => { - const { menuLabel, resetAll, header, toggleChild } = props; - const menuItems = usePanelContext(); + const { menuLabel, resetAll, header, toggleItem } = props; + const { menuItems } = usePanelContext(); if ( ! header ) { return null; } + const menuItemEntries = Object.entries( menuItems ); + const hasMenuItems = !! menuItemEntries.length; + return (

{ header } - - { ( { onClose } ) => ( - <> - - { Object.entries( menuItems ).map( - ( [ label, itemState ] ) => { - const isSelected = itemState === true; - const isDisabled = itemState === 'disabled'; + { hasMenuItems && ( + + { ( { onClose } ) => ( + <> + + { Object.entries( menuItems ).map( + ( [ label, itemState ] ) => { + const isSelected = itemState === true; + const isDisabled = + itemState === 'disabled'; - return ( - { - toggleChild( label ); - onClose(); - } } - role="menuitemcheckbox" - > - { label } - - ); - } - ) } - - - { - resetAll(); - onClose(); - } } - > - { __( 'Reset all' ) } - - - - ) } - + return ( + { + toggleItem( label ); + onClose(); + } } + role="menuitemcheckbox" + > + { label } + + ); + } + ) } + + + { + resetAll(); + onClose(); + } } + > + { __( 'Reset all' ) } + + + + ) } + + ) }

); }; diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/README.md b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/README.md index e897b0259a85e..bb07b739a1250 100644 --- a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/README.md +++ b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/README.md @@ -27,7 +27,12 @@ panel's menu. ### `label`: `string` -The label acts as a key to locate the corresponding item in the panel's menu -context. This is used when checking if the panel item should be displayed. +The supplied label is dual purpose. +It is used as: +1. the human readable label for the panel's dropdown menu +2. a key to locate the corresponding item in the panel's menu context to determine +if the panel item should be displayed. + +A panel item's `label` should be unique among all items within a single panel. - Required: Yes diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/component.js b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/component.js index 454697de295a2..aa2c8438cfaac 100644 --- a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/component.js +++ b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/component.js @@ -1,3 +1,8 @@ +/** + * WordPress dependencies + */ +import { useEffect } from '@wordpress/element'; + /** * Internal dependencies */ @@ -6,8 +11,37 @@ import { usePanelContext } from '../progressive-disclosure-panel'; // This wraps controls to be conditionally displayed within a progressive // disclosure panel. It helps prevent props being applied to HTML elements that // would otherwise be invalid. -const ProgressiveDisclosurePanelItem = ( { children, label } ) => { - const menuItems = usePanelContext(); +const ProgressiveDisclosurePanelItem = ( { + children, + hasValue, + isShownByDefault, + label, + onDeselect, + onSelect, +} ) => { + const { checkMenuItem, menuItems, registerPanelItem } = usePanelContext(); + const isValueSet = hasValue(); + + useEffect( () => { + registerPanelItem( { + hasValue, + isShownByDefault, + label, + onDeselect, + onSelect, + } ); + }, [] ); + + // When the user sets a value on the panel item's control, tell the panel + // to check its corresponding menu item. + useEffect( () => { + if ( isValueSet ) { + checkMenuItem( label ); + } + }, [ isValueSet ] ); + + // Note: `label` is used as a key when building menu item state in + // `ProgressiveDisclosurePanel`. // Do not show if menu item not selected and not shown by default. // If the item has a value that will be reflected in the menu item's diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/component.js b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/component.js index c261cc490c3b1..fa53ad5de9829 100644 --- a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/component.js +++ b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/component.js @@ -7,44 +7,39 @@ import classnames from 'classnames'; * WordPress dependencies */ import { - Children, createContext, useContext, useEffect, - useMemo, useState, } from '@wordpress/element'; /** * Internal dependencies */ -import ProgressiveDisclosurePanelItem from '../progressive-disclosure-panel-item'; import ProgressiveDisclosurePanelHeader from '../progressive-disclosure-panel-header'; const PanelContext = createContext( {} ); export const usePanelContext = () => useContext( PanelContext ); -const isMenuItem = ( item ) => item?.type === ProgressiveDisclosurePanelItem; - const ProgressiveDisclosurePanel = ( props ) => { const { children, className, header, label: menuLabel, resetAll } = props; - const [ menuItems, setMenuItems ] = useState( {} ); - // This panel only needs to concern itself with the - // ProgressiveDisclosurePanelItem components to be displayed in the menu. - const filteredChildren = useMemo( () => { - return Children.toArray( children ).filter( isMenuItem ); - }, [ children ] ); + // Allow panel items to register themselves. + const [ panelItems, setPanelItems ] = useState( [] ); + + const registerPanelItem = ( item ) => { + setPanelItems( ( items ) => [ ...items, item ] ); + }; + + // Manage and share display state of menu items representing child controls. + const [ menuItems, setMenuItems ] = useState( {} ); - // Refresh which children should be reflected in the menu and what their - // associated menu item's state is; checked or not. + // Setup menuItems state as panel items register themselves. useEffect( () => { const items = {}; - filteredChildren.forEach( ( { props: childProps } ) => { - const { hasValue, isShownByDefault, label } = childProps; - + panelItems.forEach( ( { hasValue, isShownByDefault, label } ) => { // Menu item is checked if: // - it currently has a value // - or it was checked in previous menuItems state. @@ -59,36 +54,34 @@ const ProgressiveDisclosurePanel = ( props ) => { } ); setMenuItems( items ); - }, [ filteredChildren ] ); - - if ( filteredChildren.length === 0 ) { - return null; - } - - const getChildByMenuLabel = ( label ) => { - return filteredChildren.find( - ( child ) => child.props.label === label - ); + }, [ panelItems ] ); + + // When a panel item gets a value set, update its menu item. + const checkMenuItem = ( label ) => { + setMenuItems( ( items ) => ( { + ...items, + [ label ]: true, + } ) ); }; - // Toggles the customized state of the child and its display if it isn't to - // be displayed by default. When toggling a child it's callback is executed. - const toggleChild = ( label ) => { + // Toggles the customized state of the panel item and its display if it + // isn't to be displayed by default. When toggling a panel item its + // onSelect or onDeselect callbacks are called as appropriate. + const toggleItem = ( label ) => { const wasSelected = menuItems[ label ]; - const child = getChildByMenuLabel( label ); - const { onDeselect, onSelect, isShownByDefault } = child.props; + const panelItem = panelItems.find( ( item ) => item.label === label ); - if ( wasSelected && onDeselect ) { - onDeselect(); + if ( wasSelected && panelItem?.onDeselect ) { + panelItem.onDeselect(); } - if ( ! wasSelected && onSelect ) { - onSelect(); + if ( ! wasSelected && panelItem?.onSelect ) { + panelItem.onSelect(); } - // If child is was checked but is no longer and also shown by default - // disable the child's menu item. - const isDisabled = wasSelected && isShownByDefault; + // If item was checked but is no longer and also shown by default + // disable the item's menu item. + const isDisabled = wasSelected && panelItem.isShownByDefault; setMenuItems( { ...menuItems, @@ -97,7 +90,7 @@ const ProgressiveDisclosurePanel = ( props ) => { }; // Resets display of children and executes resetAll callback if available. - const resetAllChildren = () => { + const resetAllItems = () => { if ( typeof resetAll === 'function' ) { resetAll(); } @@ -107,8 +100,7 @@ const ProgressiveDisclosurePanel = ( props ) => { // will be disabled to prevent behaviour where toggling has no effect. const resetMenuItems = {}; - filteredChildren.forEach( ( { props: childProps } ) => { - const { label, isShownByDefault } = childProps; + panelItems.forEach( ( { label, isShownByDefault } ) => { resetMenuItems[ label ] = isShownByDefault ? 'disabled' : false; } ); @@ -120,14 +112,16 @@ const ProgressiveDisclosurePanel = ( props ) => { className ); + const panelContext = { checkMenuItem, menuItems, registerPanelItem }; + return (
- + { children } diff --git a/packages/components/src/progressive-disclosure-panel/test/index.js b/packages/components/src/progressive-disclosure-panel/test/index.js index 672f827aa8d6d..eb96f94fa9027 100644 --- a/packages/components/src/progressive-disclosure-panel/test/index.js +++ b/packages/components/src/progressive-disclosure-panel/test/index.js @@ -81,50 +81,7 @@ const selectMenuItem = async ( label ) => { describe( 'ProgressiveDisclosurePanel', () => { describe( 'basic rendering', () => { - it( 'should not render when no children provided', () => { - const { container } = render( - - ); - - expect( getPanel( container ) ).not.toBeInTheDocument(); - } ); - - it( 'should not render when only child has been filtered out', () => { - // This covers case where children prop is not an array. - const { container } = render( - - { false && ( - - Should not show - - ) } - - ); - - expect( getPanel( container ) ).not.toBeInTheDocument(); - } ); - - it( 'should not render when there are no progressive panel items', () => { - const { container } = render( - - { false && ( - - Should not show - - ) } - { false && ( - - Not shown either - - ) } - Visible but insignificant - - ); - - expect( getPanel( container ) ).not.toBeInTheDocument(); - } ); - - it( 'should render panel when at least one panel item as child', () => { + it( 'should render panel', () => { const { container } = renderPanel(); expect( getPanel( container ) ).toBeInTheDocument(); @@ -138,7 +95,7 @@ describe( 'ProgressiveDisclosurePanel', () => { expect( nonPanelItem ).toBeInTheDocument(); } ); - it( 'should render child flagged as default control even without value', () => { + it( 'should render panel item flagged as default control even without value', () => { render( @@ -158,7 +115,28 @@ describe( 'ProgressiveDisclosurePanel', () => { expect( altControl ).toBeInTheDocument(); } ); - it( 'should render display options menu', () => { + it( 'should not render panel menu when there are no progressive panel items', () => { + render( + + { false && ( + + Should not show + + ) } + { false && ( + + Not shown either + + ) } + Visible but insignificant + + ); + + const menu = screen.queryByLabelText( defaultProps.label ); + expect( menu ).not.toBeInTheDocument(); + } ); + + it( 'should render panel menu when at least one panel item', () => { renderPanel(); const menuButton = screen.getByLabelText( defaultProps.label ); @@ -174,7 +152,7 @@ describe( 'ProgressiveDisclosurePanel', () => { expect( resetAllItem ).toBeInTheDocument(); } ); - it( 'should render display options menu items correctly', async () => { + it( 'should render panel menu items correctly', async () => { renderPanel(); openDropdownMenu(); @@ -219,8 +197,8 @@ describe( 'ProgressiveDisclosurePanel', () => { } ); } ); - describe( 'conditional rendering of children', () => { - it( 'should render child when it has a value', () => { + describe( 'conditional rendering of panel items', () => { + it( 'should render panel item when it has a value', () => { renderPanel(); const exampleControl = screen.getByText( 'Example control' ); @@ -230,7 +208,7 @@ describe( 'ProgressiveDisclosurePanel', () => { expect( altControl ).not.toBeInTheDocument(); } ); - it( 'should render child when corresponding menu item is selected', async () => { + it( 'should render panel item when corresponding menu item is selected', async () => { renderPanel(); await selectMenuItem( altControlProps.label ); const control = await screen.findByText( 'Alt control' ); @@ -238,7 +216,7 @@ describe( 'ProgressiveDisclosurePanel', () => { expect( control ).toBeInTheDocument(); } ); - it( 'should prevent child rendering when toggled off via menu item', async () => { + it( 'should prevent panel item rendering when toggled off via menu item', async () => { renderPanel(); await selectMenuItem( controlProps.label ); const control = screen.queryByText( 'Example control' ); From a6242026d406df47a11508e457274350805aaa84 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Mon, 2 Aug 2021 17:54:42 +1000 Subject: [PATCH 39/50] Change menu item selection state to enum --- .../component.js | 7 +-- .../component.js | 6 +-- .../progressive-disclosure-panel/component.js | 51 ++++++++++++------- .../progressive-disclosure-panel/index.js | 2 +- 4 files changed, 39 insertions(+), 27 deletions(-) diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/component.js b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/component.js index d3e37add4183d..b8e5fd808d57e 100644 --- a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/component.js +++ b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/component.js @@ -7,7 +7,7 @@ import { __ } from '@wordpress/i18n'; /** * Internal dependencies */ -import { usePanelContext } from '../progressive-disclosure-panel'; +import { MENU_STATES, usePanelContext } from '../progressive-disclosure-panel'; import MenuGroup from '../../menu-group'; import MenuItem from '../../menu-item'; import DropdownMenu from '../../dropdown-menu'; @@ -33,9 +33,10 @@ const ProgressiveDisclosurePanelHeader = ( props ) => { { Object.entries( menuItems ).map( ( [ label, itemState ] ) => { - const isSelected = itemState === true; + const isSelected = + itemState === MENU_STATES.CHECKED; const isDisabled = - itemState === 'disabled'; + itemState === MENU_STATES.DISABLED; return ( useContext( PanelContext ); +export const MENU_STATES = { + CHECKED: 'checked', + UNCHECKED: 'unchecked', + DISABLED: 'disabled', +}; + const ProgressiveDisclosurePanel = ( props ) => { const { children, className, header, label: menuLabel, resetAll } = props; @@ -40,17 +45,16 @@ const ProgressiveDisclosurePanel = ( props ) => { const items = {}; panelItems.forEach( ( { hasValue, isShownByDefault, label } ) => { - // Menu item is checked if: - // - it currently has a value - // - or it was checked in previous menuItems state. - const isChecked = hasValue() || menuItems[ label ] === true; + let menuItemState = hasValue() + ? MENU_STATES.CHECKED + : MENU_STATES.UNCHECKED; - // Menu item will be `disabled` if: - // - it is not checked - // - and is shown by default. - const isDisabled = ! isChecked && isShownByDefault; + // Disable the menu item if its unchecked and a default control. + if ( menuItemState === MENU_STATES.UNCHECKED && isShownByDefault ) { + menuItemState = MENU_STATES.DISABLED; + } - items[ label ] = isDisabled ? 'disabled' : isChecked; + items[ label ] = menuItemState; } ); setMenuItems( items ); @@ -60,7 +64,7 @@ const ProgressiveDisclosurePanel = ( props ) => { const checkMenuItem = ( label ) => { setMenuItems( ( items ) => ( { ...items, - [ label ]: true, + [ label ]: MENU_STATES.CHECKED, } ) ); }; @@ -68,24 +72,31 @@ const ProgressiveDisclosurePanel = ( props ) => { // isn't to be displayed by default. When toggling a panel item its // onSelect or onDeselect callbacks are called as appropriate. const toggleItem = ( label ) => { - const wasSelected = menuItems[ label ]; + const wasChecked = menuItems[ label ] === MENU_STATES.CHECKED; const panelItem = panelItems.find( ( item ) => item.label === label ); - if ( wasSelected && panelItem?.onDeselect ) { + if ( wasChecked && panelItem?.onDeselect ) { panelItem.onDeselect(); } - if ( ! wasSelected && panelItem?.onSelect ) { + if ( ! wasChecked && panelItem?.onSelect ) { panelItem.onSelect(); } - // If item was checked but is no longer and also shown by default - // disable the item's menu item. - const isDisabled = wasSelected && panelItem.isShownByDefault; + let menuItemState = wasChecked + ? MENU_STATES.UNCHECKED + : MENU_STATES.CHECKED; + + if ( + menuItemState === MENU_STATES.UNCHECKED && + panelItem.isShownByDefault + ) { + menuItemState = MENU_STATES.DISABLED; + } setMenuItems( { ...menuItems, - [ label ]: isDisabled ? 'disabled' : ! wasSelected, + [ label ]: menuItemState, } ); }; @@ -101,7 +112,9 @@ const ProgressiveDisclosurePanel = ( props ) => { const resetMenuItems = {}; panelItems.forEach( ( { label, isShownByDefault } ) => { - resetMenuItems[ label ] = isShownByDefault ? 'disabled' : false; + resetMenuItems[ label ] = isShownByDefault + ? MENU_STATES.DISABLED + : MENU_STATES.UNCHECKED; } ); setMenuItems( resetMenuItems ); diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/index.js b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/index.js index 728a3a15e1aae..56349ec46dd1d 100644 --- a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/index.js +++ b/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/index.js @@ -1 +1 @@ -export { default, usePanelContext } from './component'; +export { default, usePanelContext, MENU_STATES } from './component'; From eaad38e98c7751883ecaefdca3e4fdb6be116a21 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Tue, 3 Aug 2021 15:50:09 +1000 Subject: [PATCH 40/50] Improve tests and rename to ToolsPanel --- docs/manifest.json | 36 +- packages/block-editor/src/hooks/dimensions.js | 16 +- packages/block-editor/src/hooks/margin.js | 2 +- packages/block-editor/src/hooks/padding.js | 2 +- packages/components/src/index.js | 8 +- .../src/progressive-disclosure-panel/index.js | 2 - .../test/index.js | 257 ---------- packages/components/src/style.scss | 2 +- packages/components/src/tools-panel/index.js | 2 + .../stories/index.js | 23 +- .../style.scss | 10 +- .../components/src/tools-panel/test/index.js | 457 ++++++++++++++++++ .../tools-panel-header}/README.md | 6 +- .../tools-panel-header}/component.js | 8 +- .../tools-panel-header}/index.js | 0 .../tools-panel-item}/README.md | 19 +- .../tools-panel-item}/component.js | 13 +- .../tools-panel-item}/index.js | 0 .../tools-panel}/README.md | 21 +- .../tools-panel}/component.js | 14 +- .../tools-panel}/index.js | 0 .../components/sidebar/dimensions-panel.js | 16 +- 22 files changed, 554 insertions(+), 360 deletions(-) delete mode 100644 packages/components/src/progressive-disclosure-panel/index.js delete mode 100644 packages/components/src/progressive-disclosure-panel/test/index.js create mode 100644 packages/components/src/tools-panel/index.js rename packages/components/src/{progressive-disclosure-panel => tools-panel}/stories/index.js (75%) rename packages/components/src/{progressive-disclosure-panel => tools-panel}/style.scss (77%) create mode 100644 packages/components/src/tools-panel/test/index.js rename packages/components/src/{progressive-disclosure-panel/progressive-disclosure-panel-header => tools-panel/tools-panel-header}/README.md (88%) rename packages/components/src/{progressive-disclosure-panel/progressive-disclosure-panel-header => tools-panel/tools-panel-header}/component.js (86%) rename packages/components/src/{progressive-disclosure-panel/progressive-disclosure-panel-header => tools-panel/tools-panel-header}/index.js (100%) rename packages/components/src/{progressive-disclosure-panel/progressive-disclosure-panel-item => tools-panel/tools-panel-item}/README.md (58%) rename packages/components/src/{progressive-disclosure-panel/progressive-disclosure-panel-item => tools-panel/tools-panel-item}/component.js (69%) rename packages/components/src/{progressive-disclosure-panel/progressive-disclosure-panel-item => tools-panel/tools-panel-item}/index.js (100%) rename packages/components/src/{progressive-disclosure-panel/progressive-disclosure-panel => tools-panel/tools-panel}/README.md (79%) rename packages/components/src/{progressive-disclosure-panel/progressive-disclosure-panel => tools-panel/tools-panel}/component.js (91%) rename packages/components/src/{progressive-disclosure-panel/progressive-disclosure-panel => tools-panel/tools-panel}/index.js (100%) diff --git a/docs/manifest.json b/docs/manifest.json index b94064d753d8c..5099d81ab31f6 100644 --- a/docs/manifest.json +++ b/docs/manifest.json @@ -1091,24 +1091,6 @@ "markdown_source": "../packages/components/src/popover/README.md", "parent": "components" }, - { - "title": "ProgressiveDisclosurePanelHeader", - "slug": "progressive-disclosure-panel-header", - "markdown_source": "../packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/README.md", - "parent": "components" - }, - { - "title": "ProgressiveDisclosurePanelItem", - "slug": "progressive-disclosure-panel-item", - "markdown_source": "../packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/README.md", - "parent": "components" - }, - { - "title": "ProgressiveDisclosurePanel", - "slug": "progressive-disclosure-panel", - "markdown_source": "../packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/README.md", - "parent": "components" - }, { "title": "QueryControls", "slug": "query-controls", @@ -1283,6 +1265,24 @@ "markdown_source": "../packages/components/src/toolbar/README.md", "parent": "components" }, + { + "title": "ToolsPanelHeader", + "slug": "tools-panel-header", + "markdown_source": "../packages/components/src/tools-panel/tools-panel-header/README.md", + "parent": "components" + }, + { + "title": "ToolsPanelItem", + "slug": "tools-panel-item", + "markdown_source": "../packages/components/src/tools-panel/tools-panel-item/README.md", + "parent": "components" + }, + { + "title": "ToolsPanel", + "slug": "tools-panel", + "markdown_source": "../packages/components/src/tools-panel/tools-panel/README.md", + "parent": "components" + }, { "title": "Tooltip", "slug": "tooltip", diff --git a/packages/block-editor/src/hooks/dimensions.js b/packages/block-editor/src/hooks/dimensions.js index dac93610a2f48..8351676640b17 100644 --- a/packages/block-editor/src/hooks/dimensions.js +++ b/packages/block-editor/src/hooks/dimensions.js @@ -2,8 +2,8 @@ * WordPress dependencies */ import { - __experimentalProgressiveDisclosurePanel as ProgressiveDisclosurePanel, - __experimentalProgressiveDisclosurePanelItem as ProgressiveDisclosurePanelItem, + __experimentalToolsPanel as ToolsPanel, + __experimentalToolsPanelItem as ToolsPanelItem, } from '@wordpress/components'; import { Platform } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; @@ -71,32 +71,32 @@ export function DimensionsPanel( props ) { return ( - { ! isPaddingDisabled && ( - hasPaddingValue( props ) } label={ __( 'Padding' ) } onDeselect={ () => resetPadding( props ) } isShownByDefault={ defaultSpacingControls?.padding } > - + ) } { ! isMarginDisabled && ( - hasMarginValue( props ) } label={ __( 'Margin' ) } onDeselect={ () => resetMargin( props ) } isShownByDefault={ defaultSpacingControls?.margin } > - + ) } - + ); } diff --git a/packages/block-editor/src/hooks/margin.js b/packages/block-editor/src/hooks/margin.js index 6a264d0d4d0e3..7e89049b40846 100644 --- a/packages/block-editor/src/hooks/margin.js +++ b/packages/block-editor/src/hooks/margin.js @@ -40,7 +40,7 @@ export function hasMarginValue( props ) { /** * Resets the margin block support attributes. This can be used when disabling - * the margin support controls for a block via a progressive discovery panel. + * the margin support controls for a block via a `ToolsPanel`. * * @param {Object} props Block props. * @param {Object} props.attributes Block's attributes. diff --git a/packages/block-editor/src/hooks/padding.js b/packages/block-editor/src/hooks/padding.js index 9e881f4423300..e481946e3defc 100644 --- a/packages/block-editor/src/hooks/padding.js +++ b/packages/block-editor/src/hooks/padding.js @@ -40,7 +40,7 @@ export function hasPaddingValue( props ) { /** * Resets the padding block support attributes. This can be used when disabling - * the padding support controls for a block via a progressive discovery panel. + * the padding support controls for a block via a `ToolsPanel`. * * @param {Object} props Block props. * @param {Object} props.attributes Block's attributes. diff --git a/packages/components/src/index.js b/packages/components/src/index.js index c8d2c5409ab28..d155278d85470 100644 --- a/packages/components/src/index.js +++ b/packages/components/src/index.js @@ -100,10 +100,6 @@ export { default as PanelHeader } from './panel/header'; export { default as PanelRow } from './panel/row'; export { default as Placeholder } from './placeholder'; export { default as Popover } from './popover'; -export { - ProgressiveDisclosurePanel as __experimentalProgressiveDisclosurePanel, - ProgressiveDisclosurePanelItem as __experimentalProgressiveDisclosurePanelItem, -} from './progressive-disclosure-panel'; export { default as QueryControls } from './query-controls'; export { default as __experimentalRadio } from './radio'; export { default as __experimentalRadioGroup } from './radio-group'; @@ -137,6 +133,10 @@ export { default as ToolbarDropdownMenu } from './toolbar-dropdown-menu'; export { default as __experimentalToolbarContext } from './toolbar-context'; export { default as ToolbarGroup } from './toolbar-group'; export { default as ToolbarItem } from './toolbar-item'; +export { + ToolsPanel as __experimentalToolsPanel, + ToolsPanelItem as __experimentalToolsPanelItem, +} from './tools-panel'; export { default as Tooltip } from './tooltip'; export { default as __experimentalTreeGrid, diff --git a/packages/components/src/progressive-disclosure-panel/index.js b/packages/components/src/progressive-disclosure-panel/index.js deleted file mode 100644 index c4549adfd9c28..0000000000000 --- a/packages/components/src/progressive-disclosure-panel/index.js +++ /dev/null @@ -1,2 +0,0 @@ -export { default as ProgressiveDisclosurePanel } from './progressive-disclosure-panel'; -export { default as ProgressiveDisclosurePanelItem } from './progressive-disclosure-panel-item'; diff --git a/packages/components/src/progressive-disclosure-panel/test/index.js b/packages/components/src/progressive-disclosure-panel/test/index.js deleted file mode 100644 index eb96f94fa9027..0000000000000 --- a/packages/components/src/progressive-disclosure-panel/test/index.js +++ /dev/null @@ -1,257 +0,0 @@ -/** - * External dependencies - */ -import { render, screen, fireEvent } from '@testing-library/react'; - -/** - * Internal dependencies - */ -import { - ProgressiveDisclosurePanel, - ProgressiveDisclosurePanelItem, -} from '../'; - -const resetAll = jest.fn(); - -// Default props for the progressive disclosure panel. -const defaultProps = { - header: 'Panel header', - label: 'Display options', - resetAll, -}; - -// Default props for an enabled control to be rendered within panel. -const controlProps = { - attributes: { value: true }, - hasValue: jest.fn().mockImplementation( () => { - return !! controlProps.attributes.value; - } ), - label: 'Example', - onDeselect: jest.fn().mockImplementation( () => { - controlProps.attributes.value = undefined; - } ), - onSelect: jest.fn(), -}; - -// Default props without a value for an alternate control to be rendered within -// the panel. -const altControlProps = { - attributes: { value: false }, - hasValue: jest.fn().mockImplementation( () => { - return !! altControlProps.attributes.value; - } ), - label: 'Alt', - onDeselect: jest.fn(), - onSelect: jest.fn(), -}; - -// Attempts to find the progressive disclosure panel via its CSS class. -const getPanel = ( container ) => - container.querySelector( '.components-progressive-disclosure-panel' ); - -// Renders a default progressive disclosure panel including children that are -// not to be represented within the panel's menu. -const renderPanel = () => { - return render( - - { false &&
Hidden
} - -
Example control
- - -
Alt control
- - Visible - - ); -}; - -// Helper to find the menu button and simulate a user click. -const openDropdownMenu = () => { - const menuButton = screen.getByLabelText( defaultProps.label ); - fireEvent.click( menuButton ); -}; - -// Opens dropdown then selects the menu item by label before simulating a click. -const selectMenuItem = async ( label ) => { - openDropdownMenu(); - const menuItem = await screen.findByText( label ); - fireEvent.click( menuItem ); -}; - -describe( 'ProgressiveDisclosurePanel', () => { - describe( 'basic rendering', () => { - it( 'should render panel', () => { - const { container } = renderPanel(); - - expect( getPanel( container ) ).toBeInTheDocument(); - } ); - - it( 'should render non panel item child', () => { - renderPanel(); - - const nonPanelItem = screen.queryByText( 'Visible' ); - - expect( nonPanelItem ).toBeInTheDocument(); - } ); - - it( 'should render panel item flagged as default control even without value', () => { - render( - - -
Example control
- - -
Alt control
- - - ); - - const altControl = screen.getByText( 'Alt control' ); - - expect( altControl ).toBeInTheDocument(); - } ); - - it( 'should not render panel menu when there are no progressive panel items', () => { - render( - - { false && ( - - Should not show - - ) } - { false && ( - - Not shown either - - ) } - Visible but insignificant - - ); - - const menu = screen.queryByLabelText( defaultProps.label ); - expect( menu ).not.toBeInTheDocument(); - } ); - - it( 'should render panel menu when at least one panel item', () => { - renderPanel(); - - const menuButton = screen.getByLabelText( defaultProps.label ); - expect( menuButton ).toBeInTheDocument(); - } ); - - it( 'should render reset all item in menu', async () => { - renderPanel(); - openDropdownMenu(); - - const resetAllItem = await screen.findByRole( 'menuitem' ); - - expect( resetAllItem ).toBeInTheDocument(); - } ); - - it( 'should render panel menu items correctly', async () => { - renderPanel(); - openDropdownMenu(); - - const menuItems = await screen.findAllByRole( 'menuitemcheckbox' ); - - expect( menuItems.length ).toEqual( 2 ); - expect( menuItems[ 0 ] ).toHaveAttribute( 'aria-checked', 'true' ); - expect( menuItems[ 1 ] ).toHaveAttribute( 'aria-checked', 'false' ); - } ); - - it( 'should disable default control menu items when the control has no value', async () => { - render( - - -
Example control
- - -
Alt control
- - - ); - openDropdownMenu(); - - const menuItems = await screen.findAllByRole( 'menuitemcheckbox' ); - - expect( menuItems.length ).toEqual( 2 ); - expect( menuItems[ 0 ] ).not.toHaveAttribute( 'disabled' ); - expect( menuItems[ 1 ] ).toHaveAttribute( 'disabled' ); - } ); - - it( 'should render panel header', () => { - renderPanel(); - const header = screen.getByText( defaultProps.header ); - - expect( header ).toBeInTheDocument(); - } ); - } ); - - describe( 'conditional rendering of panel items', () => { - it( 'should render panel item when it has a value', () => { - renderPanel(); - - const exampleControl = screen.getByText( 'Example control' ); - const altControl = screen.queryByText( 'Alt control' ); - - expect( exampleControl ).toBeInTheDocument(); - expect( altControl ).not.toBeInTheDocument(); - } ); - - it( 'should render panel item when corresponding menu item is selected', async () => { - renderPanel(); - await selectMenuItem( altControlProps.label ); - const control = await screen.findByText( 'Alt control' ); - - expect( control ).toBeInTheDocument(); - } ); - - it( 'should prevent 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(); - } ); - } ); - - describe( 'callbacks on menu item selection', () => { - beforeEach( () => { - jest.clearAllMocks(); - controlProps.attributes.value = true; - } ); - - it( 'should call onDeselect callback when menu item is toggled off', async () => { - renderPanel(); - await selectMenuItem( controlProps.label ); - - expect( controlProps.onSelect ).not.toHaveBeenCalled(); - expect( controlProps.onDeselect ).toHaveBeenCalledTimes( 1 ); - } ); - - it( 'should call onSelect callback when menu item is toggled on', async () => { - renderPanel(); - await selectMenuItem( altControlProps.label ); - - expect( altControlProps.onSelect ).toHaveBeenCalledTimes( 1 ); - expect( altControlProps.onDeselect ).not.toHaveBeenCalled(); - } ); - - it( 'should call resetAll callback when its menu item is selected', async () => { - renderPanel(); - await selectMenuItem( 'Reset all' ); - - expect( resetAll ).toHaveBeenCalledTimes( 1 ); - } ); - } ); -} ); diff --git a/packages/components/src/style.scss b/packages/components/src/style.scss index 60d3e98e4583e..89a2a31e03025 100644 --- a/packages/components/src/style.scss +++ b/packages/components/src/style.scss @@ -30,7 +30,7 @@ @import "./panel/style.scss"; @import "./placeholder/style.scss"; @import "./popover/style.scss"; -@import "./progressive-disclosure-panel/style.scss"; +@import "./tools-panel/style.scss"; @import "./radio-control/style.scss"; @import "./resizable-box/style.scss"; @import "./responsive-wrapper/style.scss"; diff --git a/packages/components/src/tools-panel/index.js b/packages/components/src/tools-panel/index.js new file mode 100644 index 0000000000000..e557e232330b5 --- /dev/null +++ b/packages/components/src/tools-panel/index.js @@ -0,0 +1,2 @@ +export { default as ToolsPanel } from './tools-panel'; +export { default as ToolsPanelItem } from './tools-panel-item'; diff --git a/packages/components/src/progressive-disclosure-panel/stories/index.js b/packages/components/src/tools-panel/stories/index.js similarity index 75% rename from packages/components/src/progressive-disclosure-panel/stories/index.js rename to packages/components/src/tools-panel/stories/index.js index 6fd602c9251e3..4dfe68c61d5bf 100644 --- a/packages/components/src/progressive-disclosure-panel/stories/index.js +++ b/packages/components/src/tools-panel/stories/index.js @@ -11,16 +11,13 @@ import { useState } from '@wordpress/element'; /** * Internal dependencies */ -import { - ProgressiveDisclosurePanel, - ProgressiveDisclosurePanelItem, -} from '../'; +import { ToolsPanel, ToolsPanelItem } from '../'; import Panel from '../../panel'; import UnitControl from '../../unit-control'; export default { - title: 'Components/ProgressiveDisclosurePanel', - component: ProgressiveDisclosurePanel, + title: 'Components/ToolsPanel', + component: ToolsPanel, }; export const _default = () => { @@ -35,12 +32,12 @@ export const _default = () => { return ( - - !! height } label="Height" onDeselect={ () => setHeight( undefined ) } @@ -50,8 +47,8 @@ export const _default = () => { value={ height } onChange={ ( next ) => setHeight( next ) } /> - - + !! width } label="Width" onDeselect={ () => setWidth( undefined ) } @@ -61,8 +58,8 @@ export const _default = () => { value={ width } onChange={ ( next ) => setWidth( next ) } /> - - + + ); diff --git a/packages/components/src/progressive-disclosure-panel/style.scss b/packages/components/src/tools-panel/style.scss similarity index 77% rename from packages/components/src/progressive-disclosure-panel/style.scss rename to packages/components/src/tools-panel/style.scss index b76768ac29094..16481cba187db 100644 --- a/packages/components/src/progressive-disclosure-panel/style.scss +++ b/packages/components/src/tools-panel/style.scss @@ -1,4 +1,4 @@ -.components-progressive-disclosure-panel { +.components-tools-panel { border: none; border-top: $border-width solid $gray-200; padding: $grid-unit-20; @@ -8,7 +8,7 @@ column-gap: $grid-unit-20; row-gap: $grid-unit-30; - .components-progressive-disclosure-panel__header { + .components-tools-panel__header { margin: 0; display: flex; align-items: center; @@ -35,7 +35,7 @@ .block-editor-block-inspector, .edit-site { - .components-progressive-disclosure-panel { + .components-tools-panel { > div, > fieldset { padding-bottom: 0; @@ -47,11 +47,11 @@ } .edit-site { - .components-progressive-disclosure-panel { + .components-tools-panel { border-bottom: $border-width solid $gray-200; } - .components-progressive-disclosure-panel + .components-panel__body { + .components-tools-panel + .components-panel__body { margin-top: -1px; } } diff --git a/packages/components/src/tools-panel/test/index.js b/packages/components/src/tools-panel/test/index.js new file mode 100644 index 0000000000000..41b451e00c212 --- /dev/null +++ b/packages/components/src/tools-panel/test/index.js @@ -0,0 +1,457 @@ +/** + * External dependencies + */ +import { render, screen, fireEvent } from '@testing-library/react'; + +/** + * Internal dependencies + */ +import { ToolsPanel, ToolsPanelItem } from '../'; + +const resetAll = jest.fn(); + +// Default props for the tools panel. +const defaultProps = { + header: 'Panel header', + label: 'Display options', + resetAll, +}; + +// Default props for an enabled control to be rendered within panel. +const controlProps = { + attributes: { value: true }, + hasValue: jest.fn().mockImplementation( () => { + return !! controlProps.attributes.value; + } ), + label: 'Example', + onDeselect: jest.fn().mockImplementation( () => { + controlProps.attributes.value = undefined; + } ), + onSelect: jest.fn(), +}; + +// Default props without a value for an alternate control to be rendered within +// the panel. +const altControlProps = { + attributes: { value: false }, + hasValue: jest.fn().mockImplementation( () => { + return !! altControlProps.attributes.value; + } ), + label: 'Alt', + onDeselect: jest.fn(), + onSelect: jest.fn(), +}; + +// Default props for wrapped or grouped panel items. +const nestedControlProps = { + attributes: { value: true }, + hasValue: jest.fn().mockImplementation( () => { + return !! nestedControlProps.attributes.value; + } ), + label: 'Nested Control 1', + onDeselect: jest.fn().mockImplementation( () => { + nestedControlProps.attributes.value = undefined; + } ), + onSelect: jest.fn(), + isShownByDefault: true, +}; + +// Alternative props for wrapped or grouped panel items. +const altNestedControlProps = { + attributes: { value: false }, + hasValue: jest.fn().mockImplementation( () => { + return !! altNestedControlProps.attributes.value; + } ), + label: 'Nested Control 2', + onDeselect: jest.fn(), + onSelect: jest.fn(), +}; + +// Simple custom component grouping panel items. Used to test panel item +// registration and display when not an immediate child of `ToolsPanel`. +const GroupedItems = ( { + defaultGroupedProps = nestedControlProps, + altGroupedProps = altNestedControlProps, +} ) => { + return ( + <> + +
Grouped control
+ + +
Alt grouped control
+ + + ); +}; + +// Renders a tools panel including panel items that have been grouped within +// a custom component. +const renderGroupedItemsInPanel = () => { + return render( + + + + ); +}; + +// Custom component rendering a panel item within a wrapping element. Also used +// to test panel item registration and rendering. +const WrappedItem = ( { text, ...props } ) => { + return ( +
+ +
{ text }
+ +
+ ); +}; + +// Renders a `ToolsPanel` with single wrapped panel item via a custom component. +const renderWrappedItemInPanel = () => { + return render( + + + + + ); +}; + +// Attempts to find the tools panel via its CSS class. +const getPanel = ( container ) => + container.querySelector( '.components-tools-panel' ); + +// Renders a default tools panel including children that are +// not to be represented within the panel's menu. +const renderPanel = () => { + return render( + + { false &&
Hidden
} + +
Example control
+ + +
Alt control
+ + Visible + + ); +}; + +// Helper to find the menu button and simulate a user click. +const openDropdownMenu = () => { + const menuButton = screen.getByLabelText( defaultProps.label ); + fireEvent.click( menuButton ); +}; + +// Opens dropdown then selects the menu item by label before simulating a click. +const selectMenuItem = async ( label ) => { + openDropdownMenu(); + const menuItem = await screen.findByText( label ); + fireEvent.click( menuItem ); +}; + +describe( 'ToolsPanel', () => { + describe( 'basic rendering', () => { + it( 'should render panel', () => { + const { container } = renderPanel(); + + expect( getPanel( container ) ).toBeInTheDocument(); + } ); + + it( 'should render non panel item child', () => { + renderPanel(); + + const nonPanelItem = screen.queryByText( 'Visible' ); + + expect( nonPanelItem ).toBeInTheDocument(); + } ); + + it( 'should render panel item flagged as default control even without value', () => { + render( + + +
Example control
+ + +
Alt control
+ + + ); + + const altControl = screen.getByText( 'Alt control' ); + + expect( altControl ).toBeInTheDocument(); + } ); + + it( 'should not render panel menu when there are no panel items', () => { + render( + + { false && ( + Should not show + ) } + { false && ( + Not shown either + ) } + Visible but insignificant + + ); + + const menu = screen.queryByLabelText( defaultProps.label ); + expect( menu ).not.toBeInTheDocument(); + } ); + + it( 'should render panel menu when at least one panel item', () => { + renderPanel(); + + const menuButton = screen.getByLabelText( defaultProps.label ); + expect( menuButton ).toBeInTheDocument(); + } ); + + it( 'should render reset all item in menu', async () => { + renderPanel(); + openDropdownMenu(); + + const resetAllItem = await screen.findByRole( 'menuitem' ); + + expect( resetAllItem ).toBeInTheDocument(); + } ); + + it( 'should render panel menu items correctly', async () => { + renderPanel(); + openDropdownMenu(); + + const menuItems = await screen.findAllByRole( 'menuitemcheckbox' ); + + expect( menuItems.length ).toEqual( 2 ); + expect( menuItems[ 0 ] ).toHaveAttribute( 'aria-checked', 'true' ); + expect( menuItems[ 1 ] ).toHaveAttribute( 'aria-checked', 'false' ); + } ); + + it( 'should disable default control menu items when the control has no value', async () => { + render( + + +
Example control
+ + +
Alt control
+ + + ); + openDropdownMenu(); + + const menuItems = await screen.findAllByRole( 'menuitemcheckbox' ); + + expect( menuItems.length ).toEqual( 2 ); + expect( menuItems[ 0 ] ).not.toHaveAttribute( 'disabled' ); + expect( menuItems[ 1 ] ).toHaveAttribute( 'disabled' ); + } ); + + it( 'should render panel header', () => { + renderPanel(); + const header = screen.getByText( defaultProps.header ); + + expect( header ).toBeInTheDocument(); + } ); + } ); + + describe( 'conditional rendering of panel items', () => { + it( 'should render panel item when it has a value', () => { + renderPanel(); + + const exampleControl = screen.getByText( 'Example control' ); + const altControl = screen.queryByText( 'Alt control' ); + + expect( exampleControl ).toBeInTheDocument(); + expect( altControl ).not.toBeInTheDocument(); + } ); + + it( 'should render panel item when corresponding menu item is selected', async () => { + renderPanel(); + await selectMenuItem( altControlProps.label ); + const control = await screen.findByText( 'Alt control' ); + + expect( control ).toBeInTheDocument(); + } ); + + it( 'should prevent 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(); + } ); + } ); + + describe( 'callbacks on menu item selection', () => { + beforeEach( () => { + jest.clearAllMocks(); + controlProps.attributes.value = true; + } ); + + it( 'should call onDeselect callback when menu item is toggled off', async () => { + renderPanel(); + await selectMenuItem( controlProps.label ); + + expect( controlProps.onSelect ).not.toHaveBeenCalled(); + expect( controlProps.onDeselect ).toHaveBeenCalledTimes( 1 ); + } ); + + it( 'should call onSelect callback when menu item is toggled on', async () => { + renderPanel(); + await selectMenuItem( altControlProps.label ); + + expect( altControlProps.onSelect ).toHaveBeenCalledTimes( 1 ); + expect( altControlProps.onDeselect ).not.toHaveBeenCalled(); + } ); + + it( 'should call resetAll callback when its menu item is selected', async () => { + renderPanel(); + await selectMenuItem( 'Reset all' ); + + expect( resetAll ).toHaveBeenCalledTimes( 1 ); + } ); + } ); + + describe( 'grouped panel items within custom components', () => { + it( 'should render grouped items correctly', () => { + renderGroupedItemsInPanel(); + + const defaultItem = screen.getByText( 'Grouped control' ); + const altItem = screen.queryByText( 'Alt grouped control' ); + + expect( defaultItem ).toBeInTheDocument(); + expect( altItem ).not.toBeInTheDocument(); + } ); + + it( 'should render grouped items within the menu', async () => { + renderGroupedItemsInPanel(); + openDropdownMenu(); + + const defaultItem = screen.getByText( 'Nested Control 1' ); + const defaultMenuItem = defaultItem.parentNode; + + const altItem = screen.getByText( 'Nested Control 2' ); + const altMenuItem = altItem.parentNode; + + expect( defaultItem ).toBeInTheDocument(); + expect( defaultMenuItem ).toHaveAttribute( 'aria-checked', 'true' ); + + expect( altItem ).toBeInTheDocument(); + expect( altMenuItem ).toHaveAttribute( 'aria-checked', 'false' ); + } ); + + it( 'should check menu item when updating grouped item control value', async () => { + const { rerender } = render( + + + + ); + + openDropdownMenu(); + const menuItem = screen.getByText( 'Nested Control 2' ).parentNode; + + expect( menuItem ).toHaveAttribute( 'aria-checked', 'false' ); + altNestedControlProps.attributes = { value: true }; + + rerender( + + + + ); + + expect( menuItem ).toHaveAttribute( 'aria-checked', 'true' ); + altNestedControlProps.attributes = { value: false }; + } ); + } ); + + describe( 'wrapped panel items within custom components', () => { + it( 'should render wrapped items correctly', () => { + const { container } = renderWrappedItemInPanel(); + + const wrappers = container.querySelectorAll( + '.wrapped-panel-item-container' + ); + const defaultItem = screen.getByText( 'Wrapped 1' ); + const altItem = screen.queryByText( 'Wrapped 2' ); + + // Both wrappers should be rendered but only the panel item + // displayed by default should be within the document. + expect( wrappers.length ).toEqual( 2 ); + expect( defaultItem ).toBeInTheDocument(); + expect( altItem ).not.toBeInTheDocument(); + } ); + + it( 'should render wrapped items within the menu', () => { + renderWrappedItemInPanel(); + openDropdownMenu(); + + const defaultItem = screen.getByText( 'Nested Control 1' ); + const defaultMenuItem = defaultItem.parentNode; + + const altItem = screen.getByText( 'Nested Control 2' ); + const altMenuItem = altItem.parentNode; + + expect( defaultItem ).toBeInTheDocument(); + expect( defaultMenuItem ).toHaveAttribute( 'aria-checked', 'true' ); + + expect( altItem ).toBeInTheDocument(); + expect( altMenuItem ).toHaveAttribute( 'aria-checked', 'false' ); + } ); + + it( 'should check menu item when updating wrapped item control value', () => { + const { rerender } = render( + + + + ); + + openDropdownMenu(); + const menuItem = screen.getByText( 'Nested Control 2' ).parentNode; + + expect( menuItem ).toHaveAttribute( 'aria-checked', 'false' ); + altNestedControlProps.attributes = { value: true }; + + rerender( + + + + ); + + expect( menuItem ).toHaveAttribute( 'aria-checked', 'true' ); + altNestedControlProps.attributes = { value: false }; + } ); + } ); +} ); diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/README.md b/packages/components/src/tools-panel/tools-panel-header/README.md similarity index 88% rename from packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/README.md rename to packages/components/src/tools-panel/tools-panel-header/README.md index 179fdb5f0c33d..da6840b8a2a1f 100644 --- a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/README.md +++ b/packages/components/src/tools-panel/tools-panel-header/README.md @@ -1,16 +1,16 @@ -# ProgressiveDisclosurePanelHeader +# ToolsPanelHeader
This feature is still experimental. “Experimental” means this is an early implementation subject to drastic and breaking changes.

-This component renders a progressive disclosure panel's header including a menu. +This component renders a tools panel's header including a menu. ## Usage This component is generated automatically by its parent -`ProgressiveDisclosurePanel`. +`ToolsPanel`.
In general, this should not be used directly. diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/component.js b/packages/components/src/tools-panel/tools-panel-header/component.js similarity index 86% rename from packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/component.js rename to packages/components/src/tools-panel/tools-panel-header/component.js index b8e5fd808d57e..6fce559d37918 100644 --- a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/component.js +++ b/packages/components/src/tools-panel/tools-panel-header/component.js @@ -7,12 +7,12 @@ import { __ } from '@wordpress/i18n'; /** * Internal dependencies */ -import { MENU_STATES, usePanelContext } from '../progressive-disclosure-panel'; +import { MENU_STATES, usePanelContext } from '../tools-panel'; import MenuGroup from '../../menu-group'; import MenuItem from '../../menu-item'; import DropdownMenu from '../../dropdown-menu'; -const ProgressiveDisclosurePanelHeader = ( props ) => { +const ToolsPanelHeader = ( props ) => { const { menuLabel, resetAll, header, toggleItem } = props; const { menuItems } = usePanelContext(); @@ -24,7 +24,7 @@ const ProgressiveDisclosurePanelHeader = ( props ) => { const hasMenuItems = !! menuItemEntries.length; return ( -

+

{ header } { hasMenuItems && ( @@ -74,4 +74,4 @@ const ProgressiveDisclosurePanelHeader = ( props ) => { ); }; -export default ProgressiveDisclosurePanelHeader; +export default ToolsPanelHeader; diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/index.js b/packages/components/src/tools-panel/tools-panel-header/index.js similarity index 100% rename from packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-header/index.js rename to packages/components/src/tools-panel/tools-panel-header/index.js diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/README.md b/packages/components/src/tools-panel/tools-panel-item/README.md similarity index 58% rename from packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/README.md rename to packages/components/src/tools-panel/tools-panel-item/README.md index bb07b739a1250..2384b32f7be41 100644 --- a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/README.md +++ b/packages/components/src/tools-panel/tools-panel-item/README.md @@ -1,19 +1,20 @@ -# ProgressiveDisclosurePanelItem +# ToolsPanelItem
-This feature is still experimental. “Experimental” means this is an early implementation subject to drastic and breaking changes. +This feature is still experimental. “Experimental” means this is an early +implementation subject to drastic and breaking changes.

This component acts a wrapper and controls the display of items to be contained -within a ProgressiveDisclosurePanel. An item is displayed if it is -flagged as a default control or the corresponding panel menu item, provided via -context, is toggled on for this item. +within a ToolsPanel. An item is displayed if it is flagged as a default control +or the corresponding panel menu item, provided via context, is toggled on for +this item. ## Usage -See [`progressive-disclosure-panel/README.md#usage`](/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/) for how to use -`ProgressiveDisclosurePanelItem`. +See [`tools-panel/README.md#usage`](/packages/components/src/tools-panel/tools-panel/) +for how to use `ToolsPanelItem`. ## Props @@ -30,8 +31,8 @@ panel's menu. The supplied label is dual purpose. It is used as: 1. the human readable label for the panel's dropdown menu -2. a key to locate the corresponding item in the panel's menu context to determine -if the panel item should be displayed. +2. a key to locate the corresponding item in the panel's menu context to +determine if the panel item should be displayed. A panel item's `label` should be unique among all items within a single panel. diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/component.js b/packages/components/src/tools-panel/tools-panel-item/component.js similarity index 69% rename from packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/component.js rename to packages/components/src/tools-panel/tools-panel-item/component.js index e2a214105d22d..08313e8f03aaf 100644 --- a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/component.js +++ b/packages/components/src/tools-panel/tools-panel-item/component.js @@ -6,12 +6,11 @@ import { useEffect } from '@wordpress/element'; /** * Internal dependencies */ -import { MENU_STATES, usePanelContext } from '../progressive-disclosure-panel'; +import { MENU_STATES, usePanelContext } from '../tools-panel'; -// This wraps controls to be conditionally displayed within a progressive -// disclosure panel. It helps prevent props being applied to HTML elements that -// would otherwise be invalid. -const ProgressiveDisclosurePanelItem = ( { +// This wraps controls to be conditionally displayed within a tools panel. It +// prevents props being applied to HTML elements that would make them invalid. +const ToolsPanelItem = ( { children, hasValue, isShownByDefault, @@ -41,7 +40,7 @@ const ProgressiveDisclosurePanelItem = ( { }, [ isValueSet ] ); // Note: `label` is used as a key when building menu item state in - // `ProgressiveDisclosurePanel`. + // `ToolsPanel`. // Do not show if menu item not selected and not shown by default. if ( menuItems[ label ] === MENU_STATES.UNCHECKED ) { @@ -51,4 +50,4 @@ const ProgressiveDisclosurePanelItem = ( { return children; }; -export default ProgressiveDisclosurePanelItem; +export default ToolsPanelItem; diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/index.js b/packages/components/src/tools-panel/tools-panel-item/index.js similarity index 100% rename from packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel-item/index.js rename to packages/components/src/tools-panel/tools-panel-item/index.js diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/README.md b/packages/components/src/tools-panel/tools-panel/README.md similarity index 79% rename from packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/README.md rename to packages/components/src/tools-panel/tools-panel/README.md index 951aa22070b4e..4424ef880af9b 100644 --- a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/README.md +++ b/packages/components/src/tools-panel/tools-panel/README.md @@ -1,7 +1,8 @@ -# ProgressiveDisclosurePanel +# ToolsPanel
-This feature is still experimental. “Experimental” means this is an early implementation subject to drastic and breaking changes. +This feature is still experimental. “Experimental” means this is an early +implementation subject to drastic and breaking changes.

These panels provide progressive discovery options for their children. For @@ -9,9 +10,9 @@ example the controls provided via block supports. ## Development guidelines -The `ProgressiveDisclosurePanel` creates a container with a header including a +The `ToolsPanel` creates a container with a header including a dropdown menu. The menu is generated automatically from the panel's children -matching the `ProgressiveDisclosurePanelItem` component type. +matching the `ToolsPanelItem` component type. Each menu item allows for the display of the corresponding child to be toggled on or off. The control's `onSelect` and `onDeselect` callbacks are fired @@ -28,8 +29,8 @@ child's props. ```jsx import { - __experimentalProgressiveDisclosurePanel as ProgressiveDisclosurePanel, - __experimentalProgressiveDisclosurePanelItem as ProgressiveDisclosurePanelItem, + __experimentalToolsPanel as ToolsPanel, + __experimentalToolsPanelItem as ToolsPanelItem, } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; @@ -49,21 +50,21 @@ export function DimensionPanel( props ) { }; return ( - { ! isPaddingDisabled && ( - hasPaddingValue( props ) } label={ __( 'Padding' ) } onDeselect={ () => resetPadding( props ) } > - + ) } - + ); } ``` diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/component.js b/packages/components/src/tools-panel/tools-panel/component.js similarity index 91% rename from packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/component.js rename to packages/components/src/tools-panel/tools-panel/component.js index 9ac7193c34397..250de1955e544 100644 --- a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/component.js +++ b/packages/components/src/tools-panel/tools-panel/component.js @@ -16,7 +16,7 @@ import { /** * Internal dependencies */ -import ProgressiveDisclosurePanelHeader from '../progressive-disclosure-panel-header'; +import ToolsPanelHeader from '../tools-panel-header'; const PanelContext = createContext( {} ); export const usePanelContext = () => useContext( PanelContext ); @@ -27,7 +27,7 @@ export const MENU_STATES = { DISABLED: 'disabled', }; -const ProgressiveDisclosurePanel = ( props ) => { +const ToolsPanel = ( props ) => { const { children, className, header, label: menuLabel, resetAll } = props; // Allow panel items to register themselves. @@ -120,17 +120,13 @@ const ProgressiveDisclosurePanel = ( props ) => { setMenuItems( resetMenuItems ); }; - const classes = classnames( - 'components-progressive-disclosure-panel', - className - ); - + const classes = classnames( 'components-tools-panel', className ); const panelContext = { checkMenuItem, menuItems, registerPanelItem }; return (
- { ); }; -export default ProgressiveDisclosurePanel; +export default ToolsPanel; diff --git a/packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/index.js b/packages/components/src/tools-panel/tools-panel/index.js similarity index 100% rename from packages/components/src/progressive-disclosure-panel/progressive-disclosure-panel/index.js rename to packages/components/src/tools-panel/tools-panel/index.js diff --git a/packages/edit-site/src/components/sidebar/dimensions-panel.js b/packages/edit-site/src/components/sidebar/dimensions-panel.js index abea542bacd08..9eb87a0afaeef 100644 --- a/packages/edit-site/src/components/sidebar/dimensions-panel.js +++ b/packages/edit-site/src/components/sidebar/dimensions-panel.js @@ -3,8 +3,8 @@ */ import { __ } from '@wordpress/i18n'; import { - __experimentalProgressiveDisclosurePanel as ProgressiveDisclosurePanel, - __experimentalProgressiveDisclosurePanelItem as ProgressiveDisclosurePanelItem, + __experimentalToolsPanel as ToolsPanel, + __experimentalToolsPanelItem as ToolsPanelItem, __experimentalBoxControl as BoxControl, __experimentalUseCustomUnits as useCustomUnits, } from '@wordpress/components'; @@ -104,13 +104,13 @@ export default function DimensionsPanel( { context, getStyle, setStyle } ) { }; return ( - { showPaddingControl && ( - - + ) } { showMarginControl && ( - - + ) } - + ); } From 8a7df11218a025f83575507ce82e6ce48e613df8 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Tue, 3 Aug 2021 17:11:53 +1000 Subject: [PATCH 41/50] Refactor panel context to be consistent with other components --- packages/components/src/tools-panel/context.js | 7 +++++++ .../tools-panel/tools-panel-header/component.js | 5 +++-- .../src/tools-panel/tools-panel-item/component.js | 10 ++++++++-- .../src/tools-panel/tools-panel/component.js | 15 ++++----------- .../src/tools-panel/tools-panel/index.js | 2 +- 5 files changed, 23 insertions(+), 16 deletions(-) create mode 100644 packages/components/src/tools-panel/context.js diff --git a/packages/components/src/tools-panel/context.js b/packages/components/src/tools-panel/context.js new file mode 100644 index 0000000000000..e74e08708db9f --- /dev/null +++ b/packages/components/src/tools-panel/context.js @@ -0,0 +1,7 @@ +/** + * WordPress dependencies + */ +import { createContext, useContext } from '@wordpress/element'; + +export const ToolsPanelContext = createContext( {} ); +export const useToolsPanelContext = () => useContext( ToolsPanelContext ); diff --git a/packages/components/src/tools-panel/tools-panel-header/component.js b/packages/components/src/tools-panel/tools-panel-header/component.js index 6fce559d37918..7dc35b4b47d10 100644 --- a/packages/components/src/tools-panel/tools-panel-header/component.js +++ b/packages/components/src/tools-panel/tools-panel-header/component.js @@ -7,14 +7,15 @@ import { __ } from '@wordpress/i18n'; /** * Internal dependencies */ -import { MENU_STATES, usePanelContext } from '../tools-panel'; +import { useToolsPanelContext } from '../context'; +import { MENU_STATES } from '../tools-panel'; import MenuGroup from '../../menu-group'; import MenuItem from '../../menu-item'; import DropdownMenu from '../../dropdown-menu'; const ToolsPanelHeader = ( props ) => { const { menuLabel, resetAll, header, toggleItem } = props; - const { menuItems } = usePanelContext(); + const { menuItems } = useToolsPanelContext(); if ( ! header ) { return null; diff --git a/packages/components/src/tools-panel/tools-panel-item/component.js b/packages/components/src/tools-panel/tools-panel-item/component.js index 08313e8f03aaf..f351042e6e80c 100644 --- a/packages/components/src/tools-panel/tools-panel-item/component.js +++ b/packages/components/src/tools-panel/tools-panel-item/component.js @@ -6,7 +6,8 @@ import { useEffect } from '@wordpress/element'; /** * Internal dependencies */ -import { MENU_STATES, usePanelContext } from '../tools-panel'; +import { useToolsPanelContext } from '../context'; +import { MENU_STATES } from '../tools-panel'; // This wraps controls to be conditionally displayed within a tools panel. It // prevents props being applied to HTML elements that would make them invalid. @@ -18,7 +19,12 @@ const ToolsPanelItem = ( { onDeselect, onSelect, } ) => { - const { checkMenuItem, menuItems, registerPanelItem } = usePanelContext(); + const { + checkMenuItem, + menuItems, + registerPanelItem, + } = useToolsPanelContext(); + const isValueSet = hasValue(); useEffect( () => { diff --git a/packages/components/src/tools-panel/tools-panel/component.js b/packages/components/src/tools-panel/tools-panel/component.js index 250de1955e544..e423d0c4a2bd2 100644 --- a/packages/components/src/tools-panel/tools-panel/component.js +++ b/packages/components/src/tools-panel/tools-panel/component.js @@ -6,20 +6,13 @@ import classnames from 'classnames'; /** * WordPress dependencies */ -import { - createContext, - useContext, - useEffect, - useState, -} from '@wordpress/element'; +import { useEffect, useState } from '@wordpress/element'; /** * Internal dependencies */ import ToolsPanelHeader from '../tools-panel-header'; - -const PanelContext = createContext( {} ); -export const usePanelContext = () => useContext( PanelContext ); +import { ToolsPanelContext } from '../context'; export const MENU_STATES = { CHECKED: 'checked', @@ -125,7 +118,7 @@ const ToolsPanel = ( props ) => { return (
- + { toggleItem={ toggleItem } /> { children } - +
); }; diff --git a/packages/components/src/tools-panel/tools-panel/index.js b/packages/components/src/tools-panel/tools-panel/index.js index 56349ec46dd1d..8823cb1bb6cd9 100644 --- a/packages/components/src/tools-panel/tools-panel/index.js +++ b/packages/components/src/tools-panel/tools-panel/index.js @@ -1 +1 @@ -export { default, usePanelContext, MENU_STATES } from './component'; +export { default, MENU_STATES } from './component'; From ccee3acc13b86a285a423c88f8726db3e617747d Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Tue, 3 Aug 2021 17:22:29 +1000 Subject: [PATCH 42/50] Extract menu states to utils --- .../src/tools-panel/tools-panel-header/component.js | 2 +- .../src/tools-panel/tools-panel-item/component.js | 2 +- .../components/src/tools-panel/tools-panel/component.js | 7 +------ packages/components/src/tools-panel/tools-panel/index.js | 2 +- packages/components/src/tools-panel/utils.js | 5 +++++ 5 files changed, 9 insertions(+), 9 deletions(-) create mode 100644 packages/components/src/tools-panel/utils.js diff --git a/packages/components/src/tools-panel/tools-panel-header/component.js b/packages/components/src/tools-panel/tools-panel-header/component.js index 7dc35b4b47d10..1abba91384bb8 100644 --- a/packages/components/src/tools-panel/tools-panel-header/component.js +++ b/packages/components/src/tools-panel/tools-panel-header/component.js @@ -8,7 +8,7 @@ import { __ } from '@wordpress/i18n'; * Internal dependencies */ import { useToolsPanelContext } from '../context'; -import { MENU_STATES } from '../tools-panel'; +import { MENU_STATES } from '../utils'; import MenuGroup from '../../menu-group'; import MenuItem from '../../menu-item'; import DropdownMenu from '../../dropdown-menu'; diff --git a/packages/components/src/tools-panel/tools-panel-item/component.js b/packages/components/src/tools-panel/tools-panel-item/component.js index f351042e6e80c..19a064450f2eb 100644 --- a/packages/components/src/tools-panel/tools-panel-item/component.js +++ b/packages/components/src/tools-panel/tools-panel-item/component.js @@ -7,7 +7,7 @@ import { useEffect } from '@wordpress/element'; * Internal dependencies */ import { useToolsPanelContext } from '../context'; -import { MENU_STATES } from '../tools-panel'; +import { MENU_STATES } from '../utils'; // This wraps controls to be conditionally displayed within a tools panel. It // prevents props being applied to HTML elements that would make them invalid. diff --git a/packages/components/src/tools-panel/tools-panel/component.js b/packages/components/src/tools-panel/tools-panel/component.js index e423d0c4a2bd2..edc893f322f8e 100644 --- a/packages/components/src/tools-panel/tools-panel/component.js +++ b/packages/components/src/tools-panel/tools-panel/component.js @@ -13,12 +13,7 @@ import { useEffect, useState } from '@wordpress/element'; */ import ToolsPanelHeader from '../tools-panel-header'; import { ToolsPanelContext } from '../context'; - -export const MENU_STATES = { - CHECKED: 'checked', - UNCHECKED: 'unchecked', - DISABLED: 'disabled', -}; +import { MENU_STATES } from '../utils'; const ToolsPanel = ( props ) => { const { children, className, header, label: menuLabel, resetAll } = props; diff --git a/packages/components/src/tools-panel/tools-panel/index.js b/packages/components/src/tools-panel/tools-panel/index.js index 8823cb1bb6cd9..b404d7fd44a81 100644 --- a/packages/components/src/tools-panel/tools-panel/index.js +++ b/packages/components/src/tools-panel/tools-panel/index.js @@ -1 +1 @@ -export { default, MENU_STATES } from './component'; +export { default } from './component'; diff --git a/packages/components/src/tools-panel/utils.js b/packages/components/src/tools-panel/utils.js new file mode 100644 index 0000000000000..20a111062e421 --- /dev/null +++ b/packages/components/src/tools-panel/utils.js @@ -0,0 +1,5 @@ +export const MENU_STATES = { + CHECKED: 'checked', + UNCHECKED: 'unchecked', + DISABLED: 'disabled', +}; From f2851b64af8f73cc33c415b9ac005bec06eb5e20 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Wed, 4 Aug 2021 15:44:37 +1000 Subject: [PATCH 43/50] Fix caching of block attributes via callbacks stored in ToolsPanel state --- packages/block-editor/src/hooks/dimensions.js | 33 ++++++++++++++++--- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/packages/block-editor/src/hooks/dimensions.js b/packages/block-editor/src/hooks/dimensions.js index 8351676640b17..d705ed92348b6 100644 --- a/packages/block-editor/src/hooks/dimensions.js +++ b/packages/block-editor/src/hooks/dimensions.js @@ -8,6 +8,7 @@ import { import { Platform } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; import { getBlockSupport } from '@wordpress/blocks'; +import { useSelect } from '@wordpress/data'; /** * Internal dependencies @@ -27,6 +28,7 @@ import { resetPadding, useIsPaddingDisabled, } from './padding'; +import { store as blockEditorStore } from '../store'; import { cleanEmptyObject } from './utils'; export const SPACING_SUPPORT_KEY = 'spacing'; @@ -39,11 +41,17 @@ export const SPACING_SUPPORT_KEY = 'spacing'; * @return {WPElement} Inspector controls for spacing support features. */ export function DimensionsPanel( props ) { + const { clientId, setAttributes } = props; const isPaddingDisabled = useIsPaddingDisabled( props ); const isMarginDisabled = useIsMarginDisabled( props ); const isDisabled = useIsDimensionsDisabled( props ); const isSupported = hasDimensionsSupport( props.name ); + const getBlock = useSelect( + ( select ) => select( blockEditorStore ).getBlock, + [ clientId ] + ); + if ( isDisabled || ! isSupported ) { return null; } @@ -53,15 +61,20 @@ export function DimensionsPanel( props ) { '__experimentalDefaultControls', ] ); + // Attributes updated via the reset functions need to be freshly retrieved + // to avoid attribute values being cached within the callbacks passed + // through the from ToolsPanelItems into the parent ToolsPanel state when + // they register. + // Callback to reset all block support attributes controlled via this panel. const resetAll = () => { - const { style } = props.attributes; + const { attributes } = getBlock( props.clientId ); props.setAttributes( { style: cleanEmptyObject( { - ...style, + ...attributes.style, spacing: { - ...style?.spacing, + ...attributes.style?.spacing, margin: undefined, padding: undefined, }, @@ -69,6 +82,16 @@ export function DimensionsPanel( props ) { } ); }; + const resetPaddingValue = () => { + const { attributes } = getBlock( props.clientId ); + resetPadding( { attributes, setAttributes } ); + }; + + const resetMarginValue = () => { + const { attributes } = getBlock( props.clientId ); + resetMargin( { attributes, setAttributes } ); + }; + return ( hasPaddingValue( props ) } label={ __( 'Padding' ) } - onDeselect={ () => resetPadding( props ) } + onDeselect={ resetPaddingValue } isShownByDefault={ defaultSpacingControls?.padding } > @@ -90,7 +113,7 @@ export function DimensionsPanel( props ) { hasMarginValue( props ) } label={ __( 'Margin' ) } - onDeselect={ () => resetMargin( props ) } + onDeselect={ resetMarginValue } isShownByDefault={ defaultSpacingControls?.margin } > From 3d7e73c4a3dd44f6fa65137dce4e9904bd80be9c Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Wed, 4 Aug 2021 22:45:43 +1000 Subject: [PATCH 44/50] Move execution of panel item callbacks into the item on menu toggle --- packages/block-editor/src/hooks/dimensions.js | 33 +++---------------- .../tools-panel/tools-panel-item/component.js | 26 ++++++++++++--- .../src/tools-panel/tools-panel/component.js | 8 ----- .../components/sidebar/dimensions-panel.js | 2 +- 4 files changed, 28 insertions(+), 41 deletions(-) diff --git a/packages/block-editor/src/hooks/dimensions.js b/packages/block-editor/src/hooks/dimensions.js index d705ed92348b6..8351676640b17 100644 --- a/packages/block-editor/src/hooks/dimensions.js +++ b/packages/block-editor/src/hooks/dimensions.js @@ -8,7 +8,6 @@ import { import { Platform } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; import { getBlockSupport } from '@wordpress/blocks'; -import { useSelect } from '@wordpress/data'; /** * Internal dependencies @@ -28,7 +27,6 @@ import { resetPadding, useIsPaddingDisabled, } from './padding'; -import { store as blockEditorStore } from '../store'; import { cleanEmptyObject } from './utils'; export const SPACING_SUPPORT_KEY = 'spacing'; @@ -41,17 +39,11 @@ export const SPACING_SUPPORT_KEY = 'spacing'; * @return {WPElement} Inspector controls for spacing support features. */ export function DimensionsPanel( props ) { - const { clientId, setAttributes } = props; const isPaddingDisabled = useIsPaddingDisabled( props ); const isMarginDisabled = useIsMarginDisabled( props ); const isDisabled = useIsDimensionsDisabled( props ); const isSupported = hasDimensionsSupport( props.name ); - const getBlock = useSelect( - ( select ) => select( blockEditorStore ).getBlock, - [ clientId ] - ); - if ( isDisabled || ! isSupported ) { return null; } @@ -61,20 +53,15 @@ export function DimensionsPanel( props ) { '__experimentalDefaultControls', ] ); - // Attributes updated via the reset functions need to be freshly retrieved - // to avoid attribute values being cached within the callbacks passed - // through the from ToolsPanelItems into the parent ToolsPanel state when - // they register. - // Callback to reset all block support attributes controlled via this panel. const resetAll = () => { - const { attributes } = getBlock( props.clientId ); + const { style } = props.attributes; props.setAttributes( { style: cleanEmptyObject( { - ...attributes.style, + ...style, spacing: { - ...attributes.style?.spacing, + ...style?.spacing, margin: undefined, padding: undefined, }, @@ -82,16 +69,6 @@ export function DimensionsPanel( props ) { } ); }; - const resetPaddingValue = () => { - const { attributes } = getBlock( props.clientId ); - resetPadding( { attributes, setAttributes } ); - }; - - const resetMarginValue = () => { - const { attributes } = getBlock( props.clientId ); - resetMargin( { attributes, setAttributes } ); - }; - return ( hasPaddingValue( props ) } label={ __( 'Padding' ) } - onDeselect={ resetPaddingValue } + onDeselect={ () => resetPadding( props ) } isShownByDefault={ defaultSpacingControls?.padding } > @@ -113,7 +90,7 @@ export function DimensionsPanel( props ) { hasMarginValue( props ) } label={ __( 'Margin' ) } - onDeselect={ resetMarginValue } + onDeselect={ () => resetMargin( props ) } isShownByDefault={ defaultSpacingControls?.margin } > diff --git a/packages/components/src/tools-panel/tools-panel-item/component.js b/packages/components/src/tools-panel/tools-panel-item/component.js index 19a064450f2eb..09a3f7ca22c35 100644 --- a/packages/components/src/tools-panel/tools-panel-item/component.js +++ b/packages/components/src/tools-panel/tools-panel-item/component.js @@ -1,6 +1,7 @@ /** * WordPress dependencies */ +import { usePrevious } from '@wordpress/compose'; import { useEffect } from '@wordpress/element'; /** @@ -16,8 +17,8 @@ const ToolsPanelItem = ( { hasValue, isShownByDefault, label, - onDeselect, - onSelect, + onDeselect = () => undefined, + onSelect = () => undefined, } ) => { const { checkMenuItem, @@ -32,8 +33,6 @@ const ToolsPanelItem = ( { hasValue, isShownByDefault, label, - onDeselect, - onSelect, } ); }, [] ); @@ -47,6 +46,25 @@ const ToolsPanelItem = ( { // Note: `label` is used as a key when building menu item state in // `ToolsPanel`. + const isMenuItemChecked = menuItems[ label ] === MENU_STATES.CHECKED; + const wasMenuItemChecked = usePrevious( isMenuItemChecked ); + + useEffect( () => { + // If the panel's menu item is now checked but wasn't previously and + // we don't have a current value, consider the menu item has just been + // selected. + if ( + isMenuItemChecked && + ! isValueSet && + wasMenuItemChecked === false + ) { + onSelect(); + } + + if ( ! isMenuItemChecked && wasMenuItemChecked ) { + onDeselect(); + } + }, [ isMenuItemChecked, wasMenuItemChecked, isValueSet ] ); // Do not show if menu item not selected and not shown by default. if ( menuItems[ label ] === MENU_STATES.UNCHECKED ) { diff --git a/packages/components/src/tools-panel/tools-panel/component.js b/packages/components/src/tools-panel/tools-panel/component.js index edc893f322f8e..e0a1d0ac1e878 100644 --- a/packages/components/src/tools-panel/tools-panel/component.js +++ b/packages/components/src/tools-panel/tools-panel/component.js @@ -63,14 +63,6 @@ const ToolsPanel = ( props ) => { const wasChecked = menuItems[ label ] === MENU_STATES.CHECKED; const panelItem = panelItems.find( ( item ) => item.label === label ); - if ( wasChecked && panelItem?.onDeselect ) { - panelItem.onDeselect(); - } - - if ( ! wasChecked && panelItem?.onSelect ) { - panelItem.onSelect(); - } - let menuItemState = wasChecked ? MENU_STATES.UNCHECKED : MENU_STATES.CHECKED; diff --git a/packages/edit-site/src/components/sidebar/dimensions-panel.js b/packages/edit-site/src/components/sidebar/dimensions-panel.js index 9eb87a0afaeef..4f19f7e19cae6 100644 --- a/packages/edit-site/src/components/sidebar/dimensions-panel.js +++ b/packages/edit-site/src/components/sidebar/dimensions-panel.js @@ -106,7 +106,7 @@ export default function DimensionsPanel( { context, getStyle, setStyle } ) { return ( { showPaddingControl && ( From ed518fdc698872d7aea47ec9589780c3451d7d84 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Thu, 5 Aug 2021 14:25:37 +1000 Subject: [PATCH 45/50] Convert to styled components and connect to context system --- packages/components/src/style.scss | 1 - .../components/src/tools-panel/style.scss | 57 ----------------- packages/components/src/tools-panel/styles.js | 64 +++++++++++++++++++ .../tools-panel-header/component.js | 28 ++++++-- .../tools-panel/tools-panel-header/hook.js | 29 +++++++++ .../tools-panel/tools-panel-item/component.js | 35 +++++++--- .../src/tools-panel/tools-panel-item/hook.js | 29 +++++++++ .../src/tools-panel/tools-panel-item/index.js | 1 + .../src/tools-panel/tools-panel/component.js | 29 +++++---- .../src/tools-panel/tools-panel/hook.js | 29 +++++++++ .../src/tools-panel/tools-panel/index.js | 1 + 11 files changed, 216 insertions(+), 87 deletions(-) delete mode 100644 packages/components/src/tools-panel/style.scss create mode 100644 packages/components/src/tools-panel/styles.js create mode 100644 packages/components/src/tools-panel/tools-panel-header/hook.js create mode 100644 packages/components/src/tools-panel/tools-panel-item/hook.js create mode 100644 packages/components/src/tools-panel/tools-panel/hook.js diff --git a/packages/components/src/style.scss b/packages/components/src/style.scss index 89a2a31e03025..22ae2fb477995 100644 --- a/packages/components/src/style.scss +++ b/packages/components/src/style.scss @@ -30,7 +30,6 @@ @import "./panel/style.scss"; @import "./placeholder/style.scss"; @import "./popover/style.scss"; -@import "./tools-panel/style.scss"; @import "./radio-control/style.scss"; @import "./resizable-box/style.scss"; @import "./responsive-wrapper/style.scss"; diff --git a/packages/components/src/tools-panel/style.scss b/packages/components/src/tools-panel/style.scss deleted file mode 100644 index 16481cba187db..0000000000000 --- a/packages/components/src/tools-panel/style.scss +++ /dev/null @@ -1,57 +0,0 @@ -.components-tools-panel { - border: none; - border-top: $border-width solid $gray-200; - padding: $grid-unit-20; - margin-top: -1px; - display: grid; - grid-template-columns: 1fr 1fr; - column-gap: $grid-unit-20; - row-gap: $grid-unit-30; - - .components-tools-panel__header { - margin: 0; - display: flex; - align-items: center; - justify-content: space-between; - font-weight: 500; - line-height: normal; - font-size: inherit; - grid-column: span 2; - - .components-dropdown-menu { - height: $grid-unit-30; - margin-top: -4px; - margin-bottom: -4px; - } - - .components-dropdown-menu__toggle { - padding: 0; - min-width: $grid-unit-30; - width: $grid-unit-30; - height: $grid-unit-30; - } - } -} - -.block-editor-block-inspector, -.edit-site { - .components-tools-panel { - > div, - > fieldset { - padding-bottom: 0; - margin-bottom: 0; - max-width: 100%; - grid-column: span 2; - } - } -} - -.edit-site { - .components-tools-panel { - border-bottom: $border-width solid $gray-200; - } - - .components-tools-panel + .components-panel__body { - margin-top: -1px; - } -} diff --git a/packages/components/src/tools-panel/styles.js b/packages/components/src/tools-panel/styles.js new file mode 100644 index 0000000000000..d9c7a985c011f --- /dev/null +++ b/packages/components/src/tools-panel/styles.js @@ -0,0 +1,64 @@ +/** + * External dependencies + */ +import { css } from '@emotion/react'; + +/** + * Internal dependencies + */ +import { COLORS, CONFIG } from '../utils'; +import { space } from '../ui/utils/space'; + +export const ToolsPanel = css` + border-top: ${ CONFIG.borderWidth } solid ${ COLORS.gray[ 200 ] }; + column-gap: ${ space( 4 ) }; + display: grid; + grid-template-columns: 1fr 1fr; + margin-top: -1px; + padding: ${ space( 4 ) }; + row-gap: ${ space( 6 ) }; +`; + +export const ToolsPanelHeader = css` + align-items: center; + display: flex; + font-size: inherit; + font-weight: 500; + grid-column: span 2; + justify-content: space-between; + line-height: normal; + + .components-tools-panel & { + margin: 0; + } + + .components-dropdown-menu { + margin-top: ${ space( -1 ) }; + margin-bottom: ${ space( -1 ) }; + height: ${ space( 6 ) }; + } + + .components-dropdown-menu__toggle { + padding: 0; + height: ${ space( 6 ) }; + min-width: ${ space( 6 ) }; + width: ${ space( 6 ) }; + } +`; + +export const ToolsPanelItem = css` + grid-column: span 2; + + &.single-column { + grid-column: span 1; + } + + /* Clear spacing in and around controls added as panel items. */ + /* Remove when they can be addressed via context system. */ + & > div, + & > fieldset { + padding-bottom: 0; + margin-bottom: 0; + max-width: 100%; + } +`; diff --git a/packages/components/src/tools-panel/tools-panel-header/component.js b/packages/components/src/tools-panel/tools-panel-header/component.js index 1abba91384bb8..8751d62dd69e9 100644 --- a/packages/components/src/tools-panel/tools-panel-header/component.js +++ b/packages/components/src/tools-panel/tools-panel-header/component.js @@ -7,14 +7,23 @@ import { __ } from '@wordpress/i18n'; /** * Internal dependencies */ -import { useToolsPanelContext } from '../context'; -import { MENU_STATES } from '../utils'; +import DropdownMenu from '../../dropdown-menu'; import MenuGroup from '../../menu-group'; import MenuItem from '../../menu-item'; -import DropdownMenu from '../../dropdown-menu'; +import { useToolsPanelContext } from '../context'; +import { useToolsPanelHeader } from './hook'; +import { MENU_STATES } from '../utils'; +import { contextConnect } from '../../ui/context'; + +const ToolsPanelHeader = ( props, forwardedRef ) => { + const { + menuLabel, + resetAll, + header, + toggleItem, + ...headerProps + } = useToolsPanelHeader( props ); -const ToolsPanelHeader = ( props ) => { - const { menuLabel, resetAll, header, toggleItem } = props; const { menuItems } = useToolsPanelContext(); if ( ! header ) { @@ -25,7 +34,7 @@ const ToolsPanelHeader = ( props ) => { const hasMenuItems = !! menuItemEntries.length; return ( -

+

{ header } { hasMenuItems && ( @@ -75,4 +84,9 @@ const ToolsPanelHeader = ( props ) => { ); }; -export default ToolsPanelHeader; +const ConnectedToolsPanelHeader = contextConnect( + ToolsPanelHeader, + 'ToolsPanelHeader' +); + +export default ConnectedToolsPanelHeader; diff --git a/packages/components/src/tools-panel/tools-panel-header/hook.js b/packages/components/src/tools-panel/tools-panel-header/hook.js new file mode 100644 index 0000000000000..8ae082b174608 --- /dev/null +++ b/packages/components/src/tools-panel/tools-panel-header/hook.js @@ -0,0 +1,29 @@ +/** + * WordPress dependencies + */ +import { useMemo } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import * as styles from '../styles'; +import { useContextSystem } from '../../ui/context'; +import { useCx } from '../../utils/hooks/use-cx'; + +export function useToolsPanelHeader( props ) { + const { className, ...otherProps } = useContextSystem( + props, + 'ToolsPanelHeader' + ); + + const cx = useCx(); + + const classes = useMemo( () => { + return cx( styles.ToolsPanelHeader, className ); + }, [ className ] ); + + return { + ...otherProps, + className: classes, + }; +} diff --git a/packages/components/src/tools-panel/tools-panel-item/component.js b/packages/components/src/tools-panel/tools-panel-item/component.js index 09a3f7ca22c35..9002bb08ecce9 100644 --- a/packages/components/src/tools-panel/tools-panel-item/component.js +++ b/packages/components/src/tools-panel/tools-panel-item/component.js @@ -8,18 +8,24 @@ import { useEffect } from '@wordpress/element'; * Internal dependencies */ import { useToolsPanelContext } from '../context'; +import { useToolsPanelItem } from './hook'; import { MENU_STATES } from '../utils'; +import { View } from '../../view'; +import { contextConnect } from '../../ui/context'; // This wraps controls to be conditionally displayed within a tools panel. It // prevents props being applied to HTML elements that would make them invalid. -const ToolsPanelItem = ( { - children, - hasValue, - isShownByDefault, - label, - onDeselect = () => undefined, - onSelect = () => undefined, -} ) => { +const ToolsPanelItem = ( props, forwardedRef ) => { + const { + children, + hasValue, + isShownByDefault, + label, + onDeselect = () => undefined, + onSelect = () => undefined, + ...toolsPanelItemProps + } = useToolsPanelItem( props ); + const { checkMenuItem, menuItems, @@ -71,7 +77,16 @@ const ToolsPanelItem = ( { return null; } - return children; + return ( + + { children } + + ); }; -export default ToolsPanelItem; +const ConnectedToolsPanelItem = contextConnect( + ToolsPanelItem, + 'ToolsPanelItem' +); + +export default ConnectedToolsPanelItem; diff --git a/packages/components/src/tools-panel/tools-panel-item/hook.js b/packages/components/src/tools-panel/tools-panel-item/hook.js new file mode 100644 index 0000000000000..d64d7cedcc777 --- /dev/null +++ b/packages/components/src/tools-panel/tools-panel-item/hook.js @@ -0,0 +1,29 @@ +/** + * WordPress dependencies + */ +import { useMemo } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import * as styles from '../styles'; +import { useContextSystem } from '../../ui/context'; +import { useCx } from '../../utils/hooks/use-cx'; + +export function useToolsPanelItem( props ) { + const { className, ...otherProps } = useContextSystem( + props, + 'ToolsPanel' + ); + + const cx = useCx(); + + const classes = useMemo( () => { + return cx( styles.ToolsPanelItem, className ); + } ); + + return { + ...otherProps, + className: classes, + }; +} diff --git a/packages/components/src/tools-panel/tools-panel-item/index.js b/packages/components/src/tools-panel/tools-panel-item/index.js index b404d7fd44a81..8b4b163fe0f22 100644 --- a/packages/components/src/tools-panel/tools-panel-item/index.js +++ b/packages/components/src/tools-panel/tools-panel-item/index.js @@ -1 +1,2 @@ export { default } from './component'; +export { useToolsPanelItem } from './hook'; diff --git a/packages/components/src/tools-panel/tools-panel/component.js b/packages/components/src/tools-panel/tools-panel/component.js index e0a1d0ac1e878..79bc80d22467d 100644 --- a/packages/components/src/tools-panel/tools-panel/component.js +++ b/packages/components/src/tools-panel/tools-panel/component.js @@ -1,8 +1,3 @@ -/** - * External dependencies - */ -import classnames from 'classnames'; - /** * WordPress dependencies */ @@ -13,10 +8,19 @@ import { useEffect, useState } from '@wordpress/element'; */ import ToolsPanelHeader from '../tools-panel-header'; import { ToolsPanelContext } from '../context'; +import { useToolsPanel } from './hook'; import { MENU_STATES } from '../utils'; - -const ToolsPanel = ( props ) => { - const { children, className, header, label: menuLabel, resetAll } = props; +import { View } from '../../view'; +import { contextConnect } from '../../ui/context'; + +const ToolsPanel = ( props, forwardedRef ) => { + const { + children, + header, + label: menuLabel, + resetAll, + ...toolsPanelProps + } = useToolsPanel( props ); // Allow panel items to register themselves. const [ panelItems, setPanelItems ] = useState( [] ); @@ -100,11 +104,10 @@ const ToolsPanel = ( props ) => { setMenuItems( resetMenuItems ); }; - const classes = classnames( 'components-tools-panel', className ); const panelContext = { checkMenuItem, menuItems, registerPanelItem }; return ( -
+ { /> { children } -
+ ); }; -export default ToolsPanel; +const ConnectedToolsPanel = contextConnect( ToolsPanel, 'ToolsPanel' ); + +export default ConnectedToolsPanel; diff --git a/packages/components/src/tools-panel/tools-panel/hook.js b/packages/components/src/tools-panel/tools-panel/hook.js new file mode 100644 index 0000000000000..d8957e9217598 --- /dev/null +++ b/packages/components/src/tools-panel/tools-panel/hook.js @@ -0,0 +1,29 @@ +/** + * WordPress dependencies + */ +import { useMemo } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import * as styles from '../styles'; +import { useContextSystem } from '../../ui/context'; +import { useCx } from '../../utils/hooks/use-cx'; + +export function useToolsPanel( props ) { + const { className, ...otherProps } = useContextSystem( + props, + 'ToolsPanel' + ); + + const cx = useCx(); + + const classes = useMemo( () => { + return cx( styles.ToolsPanel, className ); + }, [ className ] ); + + return { + ...otherProps, + className: classes, + }; +} diff --git a/packages/components/src/tools-panel/tools-panel/index.js b/packages/components/src/tools-panel/tools-panel/index.js index b404d7fd44a81..a0c01e54d0691 100644 --- a/packages/components/src/tools-panel/tools-panel/index.js +++ b/packages/components/src/tools-panel/tools-panel/index.js @@ -1 +1,2 @@ export { default } from './component'; +export { useToolsPanel } from './hook'; From f9411d14a1d3542ec1987460fdef40f8e51f6ef7 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Thu, 5 Aug 2021 15:40:37 +1000 Subject: [PATCH 46/50] Limit component.js files to presentation --- .../tools-panel-header/component.js | 10 +- .../tools-panel/tools-panel-header/hook.js | 7 +- .../tools-panel/tools-panel-item/component.js | 69 +------------ .../src/tools-panel/tools-panel-item/hook.js | 71 ++++++++++++-- .../src/tools-panel/tools-panel/component.js | 98 +------------------ .../src/tools-panel/tools-panel/hook.js | 93 +++++++++++++++++- 6 files changed, 173 insertions(+), 175 deletions(-) diff --git a/packages/components/src/tools-panel/tools-panel-header/component.js b/packages/components/src/tools-panel/tools-panel-header/component.js index 8751d62dd69e9..0de574250d714 100644 --- a/packages/components/src/tools-panel/tools-panel-header/component.js +++ b/packages/components/src/tools-panel/tools-panel-header/component.js @@ -10,29 +10,25 @@ import { __ } from '@wordpress/i18n'; import DropdownMenu from '../../dropdown-menu'; import MenuGroup from '../../menu-group'; import MenuItem from '../../menu-item'; -import { useToolsPanelContext } from '../context'; import { useToolsPanelHeader } from './hook'; import { MENU_STATES } from '../utils'; import { contextConnect } from '../../ui/context'; const ToolsPanelHeader = ( props, forwardedRef ) => { const { + hasMenuItems, + header, + menuItems, menuLabel, resetAll, - header, toggleItem, ...headerProps } = useToolsPanelHeader( props ); - const { menuItems } = useToolsPanelContext(); - if ( ! header ) { return null; } - const menuItemEntries = Object.entries( menuItems ); - const hasMenuItems = !! menuItemEntries.length; - return (

{ header } diff --git a/packages/components/src/tools-panel/tools-panel-header/hook.js b/packages/components/src/tools-panel/tools-panel-header/hook.js index 8ae082b174608..a3c2644ba7475 100644 --- a/packages/components/src/tools-panel/tools-panel-header/hook.js +++ b/packages/components/src/tools-panel/tools-panel-header/hook.js @@ -7,6 +7,7 @@ import { useMemo } from '@wordpress/element'; * Internal dependencies */ import * as styles from '../styles'; +import { useToolsPanelContext } from '../context'; import { useContextSystem } from '../../ui/context'; import { useCx } from '../../utils/hooks/use-cx'; @@ -17,13 +18,17 @@ export function useToolsPanelHeader( props ) { ); const cx = useCx(); - const classes = useMemo( () => { return cx( styles.ToolsPanelHeader, className ); }, [ className ] ); + const { menuItems } = useToolsPanelContext(); + const hasMenuItems = !! Object.entries( menuItems ).length; + return { ...otherProps, + hasMenuItems, + menuItems, className: classes, }; } diff --git a/packages/components/src/tools-panel/tools-panel-item/component.js b/packages/components/src/tools-panel/tools-panel-item/component.js index 9002bb08ecce9..92cca8a626022 100644 --- a/packages/components/src/tools-panel/tools-panel-item/component.js +++ b/packages/components/src/tools-panel/tools-panel-item/component.js @@ -1,79 +1,18 @@ -/** - * WordPress dependencies - */ -import { usePrevious } from '@wordpress/compose'; -import { useEffect } from '@wordpress/element'; - /** * Internal dependencies */ -import { useToolsPanelContext } from '../context'; import { useToolsPanelItem } from './hook'; -import { MENU_STATES } from '../utils'; import { View } from '../../view'; import { contextConnect } from '../../ui/context'; // This wraps controls to be conditionally displayed within a tools panel. It // prevents props being applied to HTML elements that would make them invalid. const ToolsPanelItem = ( props, forwardedRef ) => { - const { - children, - hasValue, - isShownByDefault, - label, - onDeselect = () => undefined, - onSelect = () => undefined, - ...toolsPanelItemProps - } = useToolsPanelItem( props ); - - const { - checkMenuItem, - menuItems, - registerPanelItem, - } = useToolsPanelContext(); - - const isValueSet = hasValue(); - - useEffect( () => { - registerPanelItem( { - hasValue, - isShownByDefault, - label, - } ); - }, [] ); - - // When the user sets a value on the panel item's control, tell the panel - // to check its corresponding menu item. - useEffect( () => { - if ( isValueSet ) { - checkMenuItem( label ); - } - }, [ isValueSet ] ); - - // Note: `label` is used as a key when building menu item state in - // `ToolsPanel`. - const isMenuItemChecked = menuItems[ label ] === MENU_STATES.CHECKED; - const wasMenuItemChecked = usePrevious( isMenuItemChecked ); - - useEffect( () => { - // If the panel's menu item is now checked but wasn't previously and - // we don't have a current value, consider the menu item has just been - // selected. - if ( - isMenuItemChecked && - ! isValueSet && - wasMenuItemChecked === false - ) { - onSelect(); - } - - if ( ! isMenuItemChecked && wasMenuItemChecked ) { - onDeselect(); - } - }, [ isMenuItemChecked, wasMenuItemChecked, isValueSet ] ); + const { children, isShown, ...toolsPanelItemProps } = useToolsPanelItem( + props + ); - // Do not show if menu item not selected and not shown by default. - if ( menuItems[ label ] === MENU_STATES.UNCHECKED ) { + if ( ! isShown ) { return null; } diff --git a/packages/components/src/tools-panel/tools-panel-item/hook.js b/packages/components/src/tools-panel/tools-panel-item/hook.js index d64d7cedcc777..c8b1d374e11c0 100644 --- a/packages/components/src/tools-panel/tools-panel-item/hook.js +++ b/packages/components/src/tools-panel/tools-panel-item/hook.js @@ -1,29 +1,88 @@ /** * WordPress dependencies */ -import { useMemo } from '@wordpress/element'; +import { usePrevious } from '@wordpress/compose'; +import { useEffect, useMemo } from '@wordpress/element'; /** * Internal dependencies */ import * as styles from '../styles'; +import { useToolsPanelContext } from '../context'; +import { MENU_STATES } from '../utils'; import { useContextSystem } from '../../ui/context'; import { useCx } from '../../utils/hooks/use-cx'; export function useToolsPanelItem( props ) { - const { className, ...otherProps } = useContextSystem( - props, - 'ToolsPanel' - ); + const { + className, + hasValue, + isShownByDefault, + label, + onDeselect = () => undefined, + onSelect = () => undefined, + ...otherProps + } = useContextSystem( props, 'ToolsPanelItem' ); const cx = useCx(); - const classes = useMemo( () => { return cx( styles.ToolsPanelItem, className ); } ); + const { + checkMenuItem, + menuItems, + registerPanelItem, + } = useToolsPanelContext(); + + // Registering the panel item allows the panel to include it in its + // automatically generated menu and determine its initial checked status. + useEffect( () => { + registerPanelItem( { + hasValue, + isShownByDefault, + label, + } ); + }, [] ); + + const isValueSet = hasValue(); + + // When the user sets a value on the panel item's control, tell the panel + // to check its corresponding menu item. + useEffect( () => { + if ( isValueSet ) { + checkMenuItem( label ); + } + }, [ isValueSet ] ); + + // Note: `label` is used as a key when building menu item state in + // `ToolsPanel`. + const isShown = menuItems[ label ] !== MENU_STATES.UNCHECKED; + const isMenuItemChecked = menuItems[ label ] === MENU_STATES.CHECKED; + const wasMenuItemChecked = usePrevious( isMenuItemChecked ); + + // Determine if the panel item's corresponding menu it is being toggled and + // trigger appropriate callback if it is. + useEffect( () => { + // If the panel's menu item is now checked but wasn't previously and + // we don't have a current value, consider the menu item as having just + // been selected. + if ( + isMenuItemChecked && + ! isValueSet && + wasMenuItemChecked === false + ) { + onSelect(); + } + + if ( ! isMenuItemChecked && wasMenuItemChecked ) { + onDeselect(); + } + }, [ isMenuItemChecked, wasMenuItemChecked, isValueSet ] ); + return { ...otherProps, + isShown, className: classes, }; } diff --git a/packages/components/src/tools-panel/tools-panel/component.js b/packages/components/src/tools-panel/tools-panel/component.js index 79bc80d22467d..99ac9baebec1f 100644 --- a/packages/components/src/tools-panel/tools-panel/component.js +++ b/packages/components/src/tools-panel/tools-panel/component.js @@ -1,15 +1,9 @@ -/** - * WordPress dependencies - */ -import { useEffect, useState } from '@wordpress/element'; - /** * Internal dependencies */ import ToolsPanelHeader from '../tools-panel-header'; import { ToolsPanelContext } from '../context'; import { useToolsPanel } from './hook'; -import { MENU_STATES } from '../utils'; import { View } from '../../view'; import { contextConnect } from '../../ui/context'; @@ -17,101 +11,19 @@ const ToolsPanel = ( props, forwardedRef ) => { const { children, header, - label: menuLabel, - resetAll, + label, + panelContext, + resetAllItems, + toggleItem, ...toolsPanelProps } = useToolsPanel( props ); - // Allow panel items to register themselves. - const [ panelItems, setPanelItems ] = useState( [] ); - - const registerPanelItem = ( item ) => { - setPanelItems( ( items ) => [ ...items, item ] ); - }; - - // Manage and share display state of menu items representing child controls. - const [ menuItems, setMenuItems ] = useState( {} ); - - // Setup menuItems state as panel items register themselves. - useEffect( () => { - const items = {}; - - panelItems.forEach( ( { hasValue, isShownByDefault, label } ) => { - let menuItemState = hasValue() - ? MENU_STATES.CHECKED - : MENU_STATES.UNCHECKED; - - // Disable the menu item if its unchecked and a default control. - if ( menuItemState === MENU_STATES.UNCHECKED && isShownByDefault ) { - menuItemState = MENU_STATES.DISABLED; - } - - items[ label ] = menuItemState; - } ); - - setMenuItems( items ); - }, [ panelItems ] ); - - // When a panel item gets a value set, update its menu item. - const checkMenuItem = ( label ) => { - setMenuItems( ( items ) => ( { - ...items, - [ label ]: MENU_STATES.CHECKED, - } ) ); - }; - - // Toggles the customized state of the panel item and its display if it - // isn't to be displayed by default. When toggling a panel item its - // onSelect or onDeselect callbacks are called as appropriate. - const toggleItem = ( label ) => { - const wasChecked = menuItems[ label ] === MENU_STATES.CHECKED; - const panelItem = panelItems.find( ( item ) => item.label === label ); - - let menuItemState = wasChecked - ? MENU_STATES.UNCHECKED - : MENU_STATES.CHECKED; - - if ( - menuItemState === MENU_STATES.UNCHECKED && - panelItem.isShownByDefault - ) { - menuItemState = MENU_STATES.DISABLED; - } - - setMenuItems( { - ...menuItems, - [ label ]: menuItemState, - } ); - }; - - // Resets display of children and executes resetAll callback if available. - const resetAllItems = () => { - if ( typeof resetAll === 'function' ) { - resetAll(); - } - - // Turn off all menu items. Default controls will continue to display - // by virtue of their `isShownByDefault` prop however their menu item - // will be disabled to prevent behaviour where toggling has no effect. - const resetMenuItems = {}; - - panelItems.forEach( ( { label, isShownByDefault } ) => { - resetMenuItems[ label ] = isShownByDefault - ? MENU_STATES.DISABLED - : MENU_STATES.UNCHECKED; - } ); - - setMenuItems( resetMenuItems ); - }; - - const panelContext = { checkMenuItem, menuItems, registerPanelItem }; - return ( diff --git a/packages/components/src/tools-panel/tools-panel/hook.js b/packages/components/src/tools-panel/tools-panel/hook.js index d8957e9217598..21dd90682784c 100644 --- a/packages/components/src/tools-panel/tools-panel/hook.js +++ b/packages/components/src/tools-panel/tools-panel/hook.js @@ -1,29 +1,116 @@ /** * WordPress dependencies */ -import { useMemo } from '@wordpress/element'; +import { useEffect, useMemo, useState } from '@wordpress/element'; /** * Internal dependencies */ import * as styles from '../styles'; +import { MENU_STATES } from '../utils'; import { useContextSystem } from '../../ui/context'; import { useCx } from '../../utils/hooks/use-cx'; export function useToolsPanel( props ) { - const { className, ...otherProps } = useContextSystem( + const { className, resetAll, ...otherProps } = useContextSystem( props, 'ToolsPanel' ); const cx = useCx(); - const classes = useMemo( () => { return cx( styles.ToolsPanel, className ); }, [ className ] ); + // Allow panel items to register themselves. + const [ panelItems, setPanelItems ] = useState( [] ); + + const registerPanelItem = ( item ) => { + setPanelItems( ( items ) => [ ...items, item ] ); + }; + + // Manage and share display state of menu items representing child controls. + const [ menuItems, setMenuItems ] = useState( {} ); + + // Setup menuItems state as panel items register themselves. + useEffect( () => { + const items = {}; + + panelItems.forEach( ( { hasValue, isShownByDefault, label } ) => { + let menuItemState = hasValue() + ? MENU_STATES.CHECKED + : MENU_STATES.UNCHECKED; + + // Disable the menu item if its unchecked and a default control. + if ( menuItemState === MENU_STATES.UNCHECKED && isShownByDefault ) { + menuItemState = MENU_STATES.DISABLED; + } + + items[ label ] = menuItemState; + } ); + + setMenuItems( items ); + }, [ panelItems ] ); + + // When a panel item gets a value set, update its menu item. + const checkMenuItem = ( label ) => { + setMenuItems( ( items ) => ( { + ...items, + [ label ]: MENU_STATES.CHECKED, + } ) ); + }; + + // Toggles the customized state of the panel item and its display if it + // isn't to be displayed by default. When toggling a panel item its + // onSelect or onDeselect callbacks are called as appropriate. + const toggleItem = ( label ) => { + const wasChecked = menuItems[ label ] === MENU_STATES.CHECKED; + const panelItem = panelItems.find( ( item ) => item.label === label ); + + let menuItemState = wasChecked + ? MENU_STATES.UNCHECKED + : MENU_STATES.CHECKED; + + if ( + menuItemState === MENU_STATES.UNCHECKED && + panelItem.isShownByDefault + ) { + menuItemState = MENU_STATES.DISABLED; + } + + setMenuItems( { + ...menuItems, + [ label ]: menuItemState, + } ); + }; + + // Resets display of children and executes resetAll callback if available. + const resetAllItems = () => { + if ( typeof resetAll === 'function' ) { + resetAll(); + } + + // Turn off all menu items. Default controls will continue to display + // by virtue of their `isShownByDefault` prop however their menu item + // will be disabled to prevent behaviour where toggling has no effect. + const resetMenuItems = {}; + + panelItems.forEach( ( { label, isShownByDefault } ) => { + resetMenuItems[ label ] = isShownByDefault + ? MENU_STATES.DISABLED + : MENU_STATES.UNCHECKED; + } ); + + setMenuItems( resetMenuItems ); + }; + + const panelContext = { checkMenuItem, menuItems, registerPanelItem }; + return { ...otherProps, + panelContext, + resetAllItems, + toggleItem, className: classes, }; } From 8218467fbbfe15644916d6a94f3b9d98204e6067 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Fri, 6 Aug 2021 11:50:25 +1000 Subject: [PATCH 47/50] Switch approach back to being able to hide default controls --- .../components/src/tools-panel/test/index.js | 116 ++++-------------- .../tools-panel-header/component.js | 9 +- .../src/tools-panel/tools-panel-item/hook.js | 29 +---- .../src/tools-panel/tools-panel/hook.js | 51 ++------ packages/components/src/tools-panel/utils.js | 5 - 5 files changed, 34 insertions(+), 176 deletions(-) delete mode 100644 packages/components/src/tools-panel/utils.js diff --git a/packages/components/src/tools-panel/test/index.js b/packages/components/src/tools-panel/test/index.js index 41b451e00c212..d371edabc8be4 100644 --- a/packages/components/src/tools-panel/test/index.js +++ b/packages/components/src/tools-panel/test/index.js @@ -231,32 +231,6 @@ describe( 'ToolsPanel', () => { expect( menuItems[ 1 ] ).toHaveAttribute( 'aria-checked', 'false' ); } ); - it( 'should disable default control menu items when the control has no value', async () => { - render( - - -
Example control
- - -
Alt control
- - - ); - openDropdownMenu(); - - const menuItems = await screen.findAllByRole( 'menuitemcheckbox' ); - - expect( menuItems.length ).toEqual( 2 ); - expect( menuItems[ 0 ] ).not.toHaveAttribute( 'disabled' ); - expect( menuItems[ 1 ] ).toHaveAttribute( 'disabled' ); - } ); - it( 'should render panel header', () => { renderPanel(); const header = screen.getByText( defaultProps.header ); @@ -291,6 +265,28 @@ describe( 'ToolsPanel', () => { expect( control ).not.toBeInTheDocument(); } ); + + it( 'should prevent shown by default item rendering when toggled off via menu item', async () => { + render( + + +
Default control
+ + + ); + + const control = screen.getByText( 'Default control' ); + + expect( control ).toBeInTheDocument(); + + await selectMenuItem( controlProps.label ); + const resetControl = screen.queryByText( 'Default control' ); + + expect( resetControl ).not.toBeInTheDocument(); + } ); } ); describe( 'callbacks on menu item selection', () => { @@ -350,41 +346,6 @@ describe( 'ToolsPanel', () => { expect( altItem ).toBeInTheDocument(); expect( altMenuItem ).toHaveAttribute( 'aria-checked', 'false' ); } ); - - it( 'should check menu item when updating grouped item control value', async () => { - const { rerender } = render( - - - - ); - - openDropdownMenu(); - const menuItem = screen.getByText( 'Nested Control 2' ).parentNode; - - expect( menuItem ).toHaveAttribute( 'aria-checked', 'false' ); - altNestedControlProps.attributes = { value: true }; - - rerender( - - - - ); - - expect( menuItem ).toHaveAttribute( 'aria-checked', 'true' ); - altNestedControlProps.attributes = { value: false }; - } ); } ); describe( 'wrapped panel items within custom components', () => { @@ -420,38 +381,5 @@ describe( 'ToolsPanel', () => { expect( altItem ).toBeInTheDocument(); expect( altMenuItem ).toHaveAttribute( 'aria-checked', 'false' ); } ); - - it( 'should check menu item when updating wrapped item control value', () => { - const { rerender } = render( - - - - ); - - openDropdownMenu(); - const menuItem = screen.getByText( 'Nested Control 2' ).parentNode; - - expect( menuItem ).toHaveAttribute( 'aria-checked', 'false' ); - altNestedControlProps.attributes = { value: true }; - - rerender( - - - - ); - - expect( menuItem ).toHaveAttribute( 'aria-checked', 'true' ); - altNestedControlProps.attributes = { value: false }; - } ); } ); } ); diff --git a/packages/components/src/tools-panel/tools-panel-header/component.js b/packages/components/src/tools-panel/tools-panel-header/component.js index 0de574250d714..6fa9391e20e0e 100644 --- a/packages/components/src/tools-panel/tools-panel-header/component.js +++ b/packages/components/src/tools-panel/tools-panel-header/component.js @@ -11,7 +11,6 @@ import DropdownMenu from '../../dropdown-menu'; import MenuGroup from '../../menu-group'; import MenuItem from '../../menu-item'; import { useToolsPanelHeader } from './hook'; -import { MENU_STATES } from '../utils'; import { contextConnect } from '../../ui/context'; const ToolsPanelHeader = ( props, forwardedRef ) => { @@ -38,18 +37,12 @@ const ToolsPanelHeader = ( props, forwardedRef ) => { <> { Object.entries( menuItems ).map( - ( [ label, itemState ] ) => { - const isSelected = - itemState === MENU_STATES.CHECKED; - const isDisabled = - itemState === MENU_STATES.DISABLED; - + ( [ label, isSelected ] ) => { return ( { toggleItem( label ); onClose(); diff --git a/packages/components/src/tools-panel/tools-panel-item/hook.js b/packages/components/src/tools-panel/tools-panel-item/hook.js index c8b1d374e11c0..ba756db743255 100644 --- a/packages/components/src/tools-panel/tools-panel-item/hook.js +++ b/packages/components/src/tools-panel/tools-panel-item/hook.js @@ -9,7 +9,6 @@ import { useEffect, useMemo } from '@wordpress/element'; */ import * as styles from '../styles'; import { useToolsPanelContext } from '../context'; -import { MENU_STATES } from '../utils'; import { useContextSystem } from '../../ui/context'; import { useCx } from '../../utils/hooks/use-cx'; @@ -29,11 +28,7 @@ export function useToolsPanelItem( props ) { return cx( styles.ToolsPanelItem, className ); } ); - const { - checkMenuItem, - menuItems, - registerPanelItem, - } = useToolsPanelContext(); + const { menuItems, registerPanelItem } = useToolsPanelContext(); // Registering the panel item allows the panel to include it in its // automatically generated menu and determine its initial checked status. @@ -47,31 +42,15 @@ export function useToolsPanelItem( props ) { const isValueSet = hasValue(); - // When the user sets a value on the panel item's control, tell the panel - // to check its corresponding menu item. - useEffect( () => { - if ( isValueSet ) { - checkMenuItem( label ); - } - }, [ isValueSet ] ); - // Note: `label` is used as a key when building menu item state in // `ToolsPanel`. - const isShown = menuItems[ label ] !== MENU_STATES.UNCHECKED; - const isMenuItemChecked = menuItems[ label ] === MENU_STATES.CHECKED; + const isMenuItemChecked = menuItems[ label ]; const wasMenuItemChecked = usePrevious( isMenuItemChecked ); // Determine if the panel item's corresponding menu it is being toggled and // trigger appropriate callback if it is. useEffect( () => { - // If the panel's menu item is now checked but wasn't previously and - // we don't have a current value, consider the menu item as having just - // been selected. - if ( - isMenuItemChecked && - ! isValueSet && - wasMenuItemChecked === false - ) { + if ( isMenuItemChecked && ! isValueSet && ! wasMenuItemChecked ) { onSelect(); } @@ -82,7 +61,7 @@ export function useToolsPanelItem( props ) { return { ...otherProps, - isShown, + isShown: isMenuItemChecked, className: classes, }; } diff --git a/packages/components/src/tools-panel/tools-panel/hook.js b/packages/components/src/tools-panel/tools-panel/hook.js index 21dd90682784c..898d1c5e04333 100644 --- a/packages/components/src/tools-panel/tools-panel/hook.js +++ b/packages/components/src/tools-panel/tools-panel/hook.js @@ -7,7 +7,6 @@ import { useEffect, useMemo, useState } from '@wordpress/element'; * Internal dependencies */ import * as styles from '../styles'; -import { MENU_STATES } from '../utils'; import { useContextSystem } from '../../ui/context'; import { useCx } from '../../utils/hooks/use-cx'; @@ -37,50 +36,18 @@ export function useToolsPanel( props ) { const items = {}; panelItems.forEach( ( { hasValue, isShownByDefault, label } ) => { - let menuItemState = hasValue() - ? MENU_STATES.CHECKED - : MENU_STATES.UNCHECKED; - - // Disable the menu item if its unchecked and a default control. - if ( menuItemState === MENU_STATES.UNCHECKED && isShownByDefault ) { - menuItemState = MENU_STATES.DISABLED; - } - - items[ label ] = menuItemState; + items[ label ] = isShownByDefault || hasValue(); } ); setMenuItems( items ); }, [ panelItems ] ); - // When a panel item gets a value set, update its menu item. - const checkMenuItem = ( label ) => { - setMenuItems( ( items ) => ( { - ...items, - [ label ]: MENU_STATES.CHECKED, - } ) ); - }; - - // Toggles the customized state of the panel item and its display if it - // isn't to be displayed by default. When toggling a panel item its - // onSelect or onDeselect callbacks are called as appropriate. + // Toggle the checked state of a menu item which is then used to determine + // display of the item within the panel. const toggleItem = ( label ) => { - const wasChecked = menuItems[ label ] === MENU_STATES.CHECKED; - const panelItem = panelItems.find( ( item ) => item.label === label ); - - let menuItemState = wasChecked - ? MENU_STATES.UNCHECKED - : MENU_STATES.CHECKED; - - if ( - menuItemState === MENU_STATES.UNCHECKED && - panelItem.isShownByDefault - ) { - menuItemState = MENU_STATES.DISABLED; - } - setMenuItems( { ...menuItems, - [ label ]: menuItemState, + [ label ]: ! menuItems[ label ], } ); }; @@ -90,21 +57,17 @@ export function useToolsPanel( props ) { resetAll(); } - // Turn off all menu items. Default controls will continue to display - // by virtue of their `isShownByDefault` prop however their menu item - // will be disabled to prevent behaviour where toggling has no effect. + // Turn off display of all non-default items. const resetMenuItems = {}; panelItems.forEach( ( { label, isShownByDefault } ) => { - resetMenuItems[ label ] = isShownByDefault - ? MENU_STATES.DISABLED - : MENU_STATES.UNCHECKED; + resetMenuItems[ label ] = !! isShownByDefault; } ); setMenuItems( resetMenuItems ); }; - const panelContext = { checkMenuItem, menuItems, registerPanelItem }; + const panelContext = { menuItems, registerPanelItem }; return { ...otherProps, diff --git a/packages/components/src/tools-panel/utils.js b/packages/components/src/tools-panel/utils.js deleted file mode 100644 index 20a111062e421..0000000000000 --- a/packages/components/src/tools-panel/utils.js +++ /dev/null @@ -1,5 +0,0 @@ -export const MENU_STATES = { - CHECKED: 'checked', - UNCHECKED: 'unchecked', - DISABLED: 'disabled', -}; From 70d7275cb382e0760995f10f3198cfd4c99a0a5e Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Fri, 6 Aug 2021 15:38:11 +1000 Subject: [PATCH 48/50] Tweak storybook entry for ToolsPanel --- packages/components/src/tools-panel/stories/index.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/components/src/tools-panel/stories/index.js b/packages/components/src/tools-panel/stories/index.js index 4dfe68c61d5bf..050e9173c9704 100644 --- a/packages/components/src/tools-panel/stories/index.js +++ b/packages/components/src/tools-panel/stories/index.js @@ -38,22 +38,24 @@ export const _default = () => { resetAll={ resetAll } > !! height } label="Height" onDeselect={ () => setHeight( undefined ) } > - setHeight( next ) } /> !! width } label="Width" onDeselect={ () => setWidth( undefined ) } > - setWidth( next ) } @@ -65,12 +67,6 @@ export const _default = () => { ); }; -function PlaceholderControl( { label, value, onChange } ) { - return ( - - ); -} - const PanelWrapperView = styled.div` max-width: 250px; font-size: 13px; From d7bb353d6baa1b839390f2ec1ef8d5d23f8deb05 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Tue, 10 Aug 2021 15:37:34 +1000 Subject: [PATCH 49/50] Highlight experimental nature of component in story title Co-authored-by: Marco Ciampini --- 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 050e9173c9704..3f40051135857 100644 --- a/packages/components/src/tools-panel/stories/index.js +++ b/packages/components/src/tools-panel/stories/index.js @@ -16,7 +16,7 @@ import Panel from '../../panel'; import UnitControl from '../../unit-control'; export default { - title: 'Components/ToolsPanel', + title: 'Components (Experimental)/ToolsPanel', component: ToolsPanel, }; From c18dcdaae7cd04bf90b2b9d25940a018c1773453 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Tue, 10 Aug 2021 15:38:06 +1000 Subject: [PATCH 50/50] Fix typo Co-authored-by: Marco Ciampini --- packages/components/src/tools-panel/tools-panel-item/hook.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/tools-panel/tools-panel-item/hook.js b/packages/components/src/tools-panel/tools-panel-item/hook.js index ba756db743255..fe9050ab91db9 100644 --- a/packages/components/src/tools-panel/tools-panel-item/hook.js +++ b/packages/components/src/tools-panel/tools-panel-item/hook.js @@ -47,7 +47,7 @@ export function useToolsPanelItem( props ) { const isMenuItemChecked = menuItems[ label ]; const wasMenuItemChecked = usePrevious( isMenuItemChecked ); - // Determine if the panel item's corresponding menu it is being toggled and + // Determine if the panel item's corresponding menu is being toggled and // trigger appropriate callback if it is. useEffect( () => { if ( isMenuItemChecked && ! isValueSet && ! wasMenuItemChecked ) {