From 850e6c52f5628076017ff082fdcf969af0b1abef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=28Greg=29=20Zi=C3=B3=C5=82kowski?= Date: Mon, 12 Aug 2019 14:44:26 +0200 Subject: [PATCH] Components: Refactor DropdownMenu to stop using unstable props (#15968) * Remove __unstableLabelPosition by reusing position provided for the dropdown menu * Replaces unstable class names with the documented contentClassName prop * Refactor DropdownMenu component to allow passing props to nested components * Add CHANGELOG entries and deprecations messages for update props * Swap params for mergeProps helper --- .../components/block-settings-menu/index.js | 10 +-- .../components/block-settings-menu/style.scss | 8 +-- .../rich-text/format-toolbar/index.js | 6 +- packages/components/CHANGELOG.md | 10 ++- .../components/src/dropdown-menu/README.md | 41 +++++++----- .../components/src/dropdown-menu/index.js | 63 +++++++++++++++---- .../e2e-tests/specs/block-grouping.test.js | 2 +- .../src/components/header/more-menu/index.js | 13 +++- .../test/__snapshots__/index.js.snap | 22 +++++-- 9 files changed, 127 insertions(+), 48 deletions(-) diff --git a/packages/block-editor/src/components/block-settings-menu/index.js b/packages/block-editor/src/components/block-settings-menu/index.js index 527f84ab0ca984..a0133182548025 100644 --- a/packages/block-editor/src/components/block-settings-menu/index.js +++ b/packages/block-editor/src/components/block-settings-menu/index.js @@ -25,6 +25,11 @@ import BlockUnknownConvertButton from './block-unknown-convert-button'; import __experimentalBlockSettingsMenuFirstItem from './block-settings-menu-first-item'; import __experimentalBlockSettingsMenuPluginsExtension from './block-settings-menu-plugins-extension'; +const POPOVER_PROPS = { + className: 'block-editor-block-settings-menu__popover editor-block-settings-menu__popover', + position: 'bottom right', +}; + export function BlockSettingsMenu( { clientIds } ) { const blockClientIds = castArray( clientIds ); const count = blockClientIds.length; @@ -45,11 +50,8 @@ export function BlockSettingsMenu( { clientIds } ) { { ( { onClose } ) => ( <> diff --git a/packages/block-editor/src/components/block-settings-menu/style.scss b/packages/block-editor/src/components/block-settings-menu/style.scss index 0b690745012edd..d7b1ae4f5d7db3 100644 --- a/packages/block-editor/src/components/block-settings-menu/style.scss +++ b/packages/block-editor/src/components/block-settings-menu/style.scss @@ -1,7 +1,7 @@ -.block-editor-block-settings-menu__content { - padding: 0; +.block-editor-block-settings-menu .components-dropdown-menu__toggle .dashicon { + transform: rotate(90deg); } -.block-editor-block-settings-menu__toggle .dashicon { - transform: rotate(90deg); +.block-editor-block-settings-menu__popover .components-dropdown-menu__menu { + padding: 0; } diff --git a/packages/block-editor/src/components/rich-text/format-toolbar/index.js b/packages/block-editor/src/components/rich-text/format-toolbar/index.js index e596de9d6ea28f..5e6fb5cf9846d6 100644 --- a/packages/block-editor/src/components/rich-text/format-toolbar/index.js +++ b/packages/block-editor/src/components/rich-text/format-toolbar/index.js @@ -11,6 +11,10 @@ import { orderBy } from 'lodash'; import { __ } from '@wordpress/i18n'; import { Toolbar, Slot, DropdownMenu } from '@wordpress/components'; +const POPOVER_PROPS = { + position: 'bottom left', +}; + const FormatToolbar = () => { return (
@@ -22,9 +26,9 @@ const FormatToolbar = () => { { ( fills ) => fills.length !== 0 && props ), 'title' ) } + popoverProps={ POPOVER_PROPS } /> } diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index afe7a14e81421a..77e05d4bbe59dd 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -2,8 +2,16 @@ ### New Features -- Added a new `popoverProps` prop to the `Dropdown` component which allows users of the `Dropdown` component to pass props directly to the `PopOver` component. +- Added a new `popoverProps` prop to the `Dropdown` component which allows users of the `Dropdown` component to pass props directly to the `Popover` component. - Added and documented `hideLabelFromVision` prop to `BaseControl` used by `SelectControl`, `TextControl`, and `TextareaControl`. +- Added a new `popoverProps` prop to the `DropdownMenu` component which allows to pass props directly to the nested `Popover` component. +- Added a new `toggleProps` prop to the `DropdownMenu` component which allows to pass props directly to the nested `IconButton` component. +- Added a new `menuProps` prop to the `DropdownMenu` component which allows to pass props directly to the nested `NavigableMenu` component. + +### Deprecations + +- `menuLabel` prop in `DropdownComponent` has been deprecated. Consider using `menuProps` object and its `aria-label` property instead. +- `position` prop in `DropdownComponent` has been deprecated. Consider using `popoverProps` object and its `position` property instead. ### Bug Fixes diff --git a/packages/components/src/dropdown-menu/README.md b/packages/components/src/dropdown-menu/README.md index f118a1bc4c241f..3cab663adeb143 100644 --- a/packages/components/src/dropdown-menu/README.md +++ b/packages/components/src/dropdown-menu/README.md @@ -163,21 +163,6 @@ A human-readable label to present as accessibility text on the focused collapsed - Type: `String` - Required: Yes -#### menuLabel - -A human-readable label to present as accessibility text on the expanded menu container. - -- Type: `String` -- Required: No - -#### position - -The direction in which the menu should open. Specify y- and x-axis as a space-separated string. Supports `"top"`, `"middle"`, `"bottom"` y axis, and `"left"`, `"center"`, `"right"` x axis. - -- Type: `String` -- Required: No -- Default: `"top center"` - #### controls An array of objects describing the options to be shown in the expanded menu. @@ -202,7 +187,31 @@ See also: [https://developer.wordpress.org/resource/dashicons/](https://develope #### className -A class name to apply to the dropdown wrapper element. +A class name to apply to the dropdown menu's toggle element wrapper. - Type: `String` - Required: No + +#### popoverProps + +Properties of `popoverProps` object will be passed as props to the nested `Popover` component. +Use this object to modify props available for the `Popover` component that are not already exposed in the `DropdownMenu` component, e.g.: the direction in which the popover should open relative to its parent node set with `position` prop. + + - Type: `Object` + - Required: No + +#### toggleProps + +Properties of `toggleProps` object will be passed as props to the nested `IconButton` component in the `renderToggle` implementation of the `Dropdown` component used internally. +Use this object to modify props available for the `IconButton` component that are not already exposed in the `DropdownMenu` component, e.g.: the tooltip text displayed on hover set with `tooltip` prop. + + - Type: `Object` + - Required: No + +#### menuProps + +Properties of `menuProps` object will be passed as props to the nested `NavigableMenu` component in the `renderContent` implementation of the `Dropdown` component used internally. +Use this object to modify props available for the `NavigableMenu` component that are not already exposed in the `DropdownMenu` component, e.g.: the orientation of the menu set with `orientation` prop. + + - Type: `Object` + - Required: No diff --git a/packages/components/src/dropdown-menu/index.js b/packages/components/src/dropdown-menu/index.js index 8212deaeda5956..e94b246231ad1b 100644 --- a/packages/components/src/dropdown-menu/index.js +++ b/packages/components/src/dropdown-menu/index.js @@ -8,6 +8,7 @@ import { flatMap, isEmpty, isFunction } from 'lodash'; * WordPress dependencies */ import { DOWN } from '@wordpress/keycodes'; +import deprecated from '@wordpress/deprecated'; /** * Internal dependencies @@ -16,6 +17,19 @@ import IconButton from '../icon-button'; import Dropdown from '../dropdown'; import { NavigableMenu } from '../navigable-container'; +function mergeProps( defaultProps = {}, props = {} ) { + const mergedProps = { + ...defaultProps, + ...props, + }; + + if ( props.className && defaultProps.className ) { + mergedProps.className = classnames( props.className, defaultProps.className ); + } + + return mergedProps; +} + function DropdownMenu( { children, className, @@ -23,13 +37,27 @@ function DropdownMenu( { hasArrowIndicator = false, icon = 'menu', label, + popoverProps, + toggleProps, + menuProps, + // The following props exist for backward compatibility. menuLabel, position, - __unstableLabelPosition, - __unstableMenuClassName, - __unstablePopoverClassName, - __unstableToggleClassName, } ) { + if ( menuLabel ) { + deprecated( '`menuLabel` prop in `DropdownComponent`', { + alternative: '`menuProps` object and its `aria-label` property', + plugin: 'Gutenberg', + } ); + } + + if ( position ) { + deprecated( '`position` prop in `DropdownComponent`', { + alternative: '`popoverProps` object and its `position` property', + plugin: 'Gutenberg', + } ); + } + if ( isEmpty( controls ) && ! isFunction( children ) ) { return null; } @@ -42,12 +70,15 @@ function DropdownMenu( { controlSets = [ controlSets ]; } } + const mergedPopoverProps = mergeProps( { + className: 'components-dropdown-menu__popover', + position, + }, popoverProps ); return ( { const openOnArrowDown = ( event ) => { if ( ! isOpen && event.keyCode === DOWN ) { @@ -56,31 +87,37 @@ function DropdownMenu( { onToggle(); } }; + const mergedToggleProps = mergeProps( { + className: classnames( 'components-dropdown-menu__toggle', { + 'is-opened': isOpen, + } ), + tooltip: label, + }, toggleProps ); return ( { ( ! icon || hasArrowIndicator ) && } ); } } renderContent={ ( props ) => { + const mergedMenuProps = mergeProps( { + 'aria-label': menuLabel || label, + className: 'components-dropdown-menu__menu', + }, menuProps ); + return ( { isFunction( children ) ? diff --git a/packages/e2e-tests/specs/block-grouping.test.js b/packages/e2e-tests/specs/block-grouping.test.js index 64bba812f4f278..8d12894a6b3111 100644 --- a/packages/e2e-tests/specs/block-grouping.test.js +++ b/packages/e2e-tests/specs/block-grouping.test.js @@ -146,7 +146,7 @@ describe( 'Block Grouping', () => { it( 'does not show group option in the options toolbar if Grouping block is disabled ', async () => { await clickBlockToolbarButton( 'More options' ); - const blockOptionsDropdownHTML = await page.evaluate( () => document.querySelector( '.block-editor-block-settings-menu__content' ).innerHTML ); + const blockOptionsDropdownHTML = await page.evaluate( () => document.querySelector( '.block-editor-block-settings-menu__popover' ).innerHTML ); expect( blockOptionsDropdownHTML ).not.toContain( 'Group' ); } ); diff --git a/packages/edit-post/src/components/header/more-menu/index.js b/packages/edit-post/src/components/header/more-menu/index.js index 16d9b16afb77a1..c04be0527baafc 100644 --- a/packages/edit-post/src/components/header/more-menu/index.js +++ b/packages/edit-post/src/components/header/more-menu/index.js @@ -13,14 +13,21 @@ import ToolsMoreMenuGroup from '../tools-more-menu-group'; import OptionsMenuItem from '../options-menu-item'; import WritingMenu from '../writing-menu'; +const POPOVER_PROPS = { + className: 'edit-post-more-menu__content', + position: 'bottom left', +}; +const TOGGLE_PROPS = { + labelPosition: 'bottom', +}; + const MoreMenu = () => ( { ( { onClose } ) => ( <> diff --git a/packages/edit-post/src/components/header/more-menu/test/__snapshots__/index.js.snap b/packages/edit-post/src/components/header/more-menu/test/__snapshots__/index.js.snap index 889803c75d9107..3f01ba28572d48 100644 --- a/packages/edit-post/src/components/header/more-menu/test/__snapshots__/index.js.snap +++ b/packages/edit-post/src/components/header/more-menu/test/__snapshots__/index.js.snap @@ -3,17 +3,29 @@ exports[`MoreMenu should match snapshot 1`] = `