From a56aafa749c440621928085b32b175a50a5b5af2 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 19 Oct 2018 15:08:15 -0400 Subject: [PATCH] Components: Add `info` prop support to MenuItem, FeatureToggle (#10802) * Components: Allow aria-label override on IconButton * Components: Add `info` prop support to MenuItem * Edit Post: Add `info` prop support to FeatureToggle * Edit Post: Add feature toggle info texts * Components: Menu Item: Break text beyond mobile toolbar * Components: Icon Button: Indent text by SVG margin Avoids offset indent on wrapped text * Tweak menu item styling * Avoid italic text --- packages/components/CHANGELOG.md | 13 +++- packages/components/src/icon-button/index.js | 2 +- .../components/src/icon-button/style.scss | 5 +- .../components/src/icon-button/test/index.js | 5 ++ packages/components/src/menu-item/README.md | 47 ++++++++++++ packages/components/src/menu-item/index.js | 76 +++++++++++++------ packages/components/src/menu-item/style.scss | 18 +++-- .../test/__snapshots__/index.js.snap | 27 +++++++ .../components/src/menu-item/test/index.js | 27 ++++++- .../components/header/feature-toggle/index.js | 4 +- .../components/header/writing-menu/index.js | 18 ++++- .../reusable-block-delete-button.js.snap | 4 +- .../test/block-mode-toggle.js | 20 +++-- .../test/reusable-block-convert-button.js | 30 +++++--- .../test/reusable-block-delete-button.js | 14 +++- .../__snapshots__/plugins-api.test.js.snap | 2 +- test/e2e/specs/links.test.js | 19 ++--- 17 files changed, 249 insertions(+), 82 deletions(-) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 106fd58c417462..59ae8b21f3865d 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -3,10 +3,15 @@ ### New Feature - Added a new `ColorPicker` component ([#10564](https://github.com/WordPress/gutenberg/pull/10564)). +- `MenuItem` now accepts an `info` prop for including an extended description. + +### Bug Fix + +- `IconButton` correctly respects a passed `aria-label` prop. ### Deprecation -- `wp.components.PanelColor` has been deprecated in favor of `wp.editor.PanelColorSettings`. +- `PanelColor` has been deprecated in favor of `wp.editor.PanelColorSettings`. ## 4.1.0 (2018-10-10) @@ -18,7 +23,7 @@ ### Breaking Change -- `wp.components.Draggable` as a DOM node drag handler has been removed. Please, use `wp.components.Draggable` as a wrap component for your DOM node drag handler. +- `Draggable` as a DOM node drag handler has been removed. Please, use `Draggable` as a wrap component for your DOM node drag handler. ### Deprecation @@ -29,9 +34,9 @@ ### Breaking Change - `withAPIData` has been removed. Please use the Core Data module or `@wordpress/api-fetch` directly instead. -- `wp.components.Draggable` as a DOM node drag handler has been deprecated. Please, use `wp.components.Draggable` as a wrap component for your DOM node drag handler. +- `Draggable` as a DOM node drag handler has been deprecated. Please, use `Draggable` as a wrap component for your DOM node drag handler. - Change how required built-ins are polyfilled with Babel 7 ([#9171](https://github.com/WordPress/gutenberg/pull/9171)). If you're using an environment that has limited or no support for ES2015+ such as lower versions of IE then using [core-js](https://github.com/zloirock/core-js) or [@babel/polyfill](https://babeljs.io/docs/en/next/babel-polyfill) will add support for these methods. -- `wp.components.withContext` has been removed. Please use `wp.element.createContext` instead. See: https://reactjs.org/docs/context.html. +- `withContext` has been removed. Please use `wp.element.createContext` instead. See: https://reactjs.org/docs/context.html. ### New Feature diff --git a/packages/components/src/icon-button/index.js b/packages/components/src/icon-button/index.js index b6521f790fbfa8..745152409ccd46 100644 --- a/packages/components/src/icon-button/index.js +++ b/packages/components/src/icon-button/index.js @@ -41,7 +41,7 @@ class IconButton extends Component { ); let element = ( - diff --git a/packages/components/src/icon-button/style.scss b/packages/components/src/icon-button/style.scss index c25c9c820345c5..cc3ee2f89564ba 100644 --- a/packages/components/src/icon-button/style.scss +++ b/packages/components/src/icon-button/style.scss @@ -8,7 +8,6 @@ color: $dark-gray-500; position: relative; overflow: hidden; - text-indent: 4px; border-radius: $radius-round-rectangle; .dashicon { @@ -20,6 +19,10 @@ svg { fill: currentColor; outline: none; + + &:not:only-child { + margin-right: 4px; + } } &:not(:disabled):not([aria-disabled="true"]):not(.is-default):hover { diff --git a/packages/components/src/icon-button/test/index.js b/packages/components/src/icon-button/test/index.js index b8f8276afcf62b..d5b67eec7930f6 100644 --- a/packages/components/src/icon-button/test/index.js +++ b/packages/components/src/icon-button/test/index.js @@ -41,6 +41,11 @@ describe( 'IconButton', () => { expect( iconButton.find( 'Button' ).prop( 'aria-label' ) ).toBe( 'WordPress' ); } ); + it( 'should support explicit aria-label override', () => { + const iconButton = shallow( ); + expect( iconButton.prop( 'aria-label' ) ).toBe( 'Custom' ); + } ); + it( 'should add an additional className', () => { const iconButton = shallow( ); expect( iconButton.hasClass( 'test' ) ).toBe( true ); diff --git a/packages/components/src/menu-item/README.md b/packages/components/src/menu-item/README.md index 974cd07b7dd256..c7f4777bd43b0d 100644 --- a/packages/components/src/menu-item/README.md +++ b/packages/components/src/menu-item/README.md @@ -1,5 +1,7 @@ # MenuItem +MenuItem is a component which renders a button intended to be used in combination with the [DropdownMenu component](../dropdown-menu). + ## Usage ```jsx @@ -18,3 +20,48 @@ const MyMenuItem = withState( { ) ); ``` + +## Props + +MenuItem supports the following props. Any additional props are passed through to the underlying [Button](../button) or [IconButton](../icon-button) component. + +### `children` + +- Type: `WPElement` +- Required: No + +Element to render as child of button. + +Element + +### `label` + +- Type: `string` +- Required: No + +String to use as primary button label text, applied as `aria-label`. Useful in cases where an `info` prop is passed, where `label` should be the minimal text of the button, described in further detail by `info`. + +Defaults to the value of `children`, if `children` is passed as a string. + +### `info` + +- Type: `string` +- Required: No + +Text to use as description for button text. + +Refer to documentation for [`label`](#label). + +### `icon` + +- Type: `string` +- Required: No + +Refer to documentation for [IconButton's `icon` prop](../icon-button/README.md#icon). + +### `shortcut` + +- Type: `string` +- Required: No + +Refer to documentation for [Shortcut's `shortcut` prop](../shortcut/README.md#shortcut). diff --git a/packages/components/src/menu-item/index.js b/packages/components/src/menu-item/index.js index 0904eb2463e852..e281c6bf0ea657 100644 --- a/packages/components/src/menu-item/index.js +++ b/packages/components/src/menu-item/index.js @@ -7,7 +7,8 @@ import { isString } from 'lodash'; /** * WordPress dependencies */ -import { cloneElement } from '@wordpress/element'; +import { createElement, cloneElement } from '@wordpress/element'; +import { withInstanceId } from '@wordpress/compose'; /** * Internal dependencies @@ -21,11 +22,45 @@ import IconButton from '../icon-button'; * * @return {WPElement} More menu item. */ -function MenuItem( { children, className, icon, shortcut, isSelected, role = 'menuitem', ...props } ) { +export function MenuItem( { + children, + label = children, + info, + className, + icon, + shortcut, + isSelected, + role = 'menuitem', + instanceId, + ...props +} ) { className = classnames( 'components-menu-item__button', className, { 'has-icon': icon, } ); + // Avoid using label if it is passed as non-string children. + label = isString( label ) ? label : undefined; + + if ( info ) { + const infoId = 'edit-post-feature-toggle__info-' + instanceId; + + // Deconstructed props is scoped to the function; mutation is fine. + props[ 'aria-describedby' ] = infoId; + + children = ( + + { children } + + { info } + + + ); + } + + let tagName = Button; + if ( icon ) { if ( ! isString( icon ) ) { icon = cloneElement( icon, { @@ -35,31 +70,22 @@ function MenuItem( { children, className, icon, shortcut, isSelected, role = 'me } ); } - return ( - - { children } - - - ); + tagName = IconButton; + props.icon = icon; } - return ( - + return createElement( + tagName, + { + 'aria-label': label, + 'aria-checked': isSelected, + role, + className, + ...props, + }, + children, + ); } -export default MenuItem; +export default withInstanceId( MenuItem ); diff --git a/packages/components/src/menu-item/style.scss b/packages/components/src/menu-item/style.scss index d36b174ec893a0..55c7137a3c672f 100644 --- a/packages/components/src/menu-item/style.scss +++ b/packages/components/src/menu-item/style.scss @@ -3,7 +3,7 @@ width: 100%; padding: 8px; text-align: left; - color: $dark-gray-500; + color: $dark-gray-600; // Target plugin icons that can have arbitrary classes by using an aggressive selector. .dashicon, @@ -24,13 +24,17 @@ &:focus:not(:disabled):not([aria-disabled="true"]) { @include menu-style__focus; } +} - // Don't wrap text until viewport is beyond the mobile breakpoint. - @include break-mobile() { - .components-popover:not(.is-mobile) & { - white-space: nowrap; - } - } +.components-menu-item__info-wrapper { + display: flex; + flex-direction: column; +} + +.components-menu-item__info { + margin-top: $grid-size-small; + font-size: $default-font-size - 1px; + opacity: 0.82; } .components-menu-item__shortcut { diff --git a/packages/components/src/menu-item/test/__snapshots__/index.js.snap b/packages/components/src/menu-item/test/__snapshots__/index.js.snap index 1427a4d1d20837..74eea47847f4ab 100644 --- a/packages/components/src/menu-item/test/__snapshots__/index.js.snap +++ b/packages/components/src/menu-item/test/__snapshots__/index.js.snap @@ -3,6 +3,7 @@ exports[`MenuItem should match snapshot when all props provided 1`] = ` `; +exports[`MenuItem should match snapshot when info is provided 1`] = ` + +`; + exports[`MenuItem should match snapshot when isSelected and role are optionally provided 1`] = ` diff --git a/packages/components/src/menu-item/test/index.js b/packages/components/src/menu-item/test/index.js index 1e3f89fbc05c79..9fbd8b869ff470 100644 --- a/packages/components/src/menu-item/test/index.js +++ b/packages/components/src/menu-item/test/index.js @@ -7,12 +7,12 @@ import { noop } from 'lodash'; /** * Internal dependencies */ -import MenuItem from '../'; +import { MenuItem } from '../'; jest.mock( '../../button' ); describe( 'MenuItem', () => { - test( 'should match snapshot when only label provided', () => { + it( 'should match snapshot when only label provided', () => { const wrapper = shallow( My item @@ -22,7 +22,7 @@ describe( 'MenuItem', () => { expect( wrapper ).toMatchSnapshot(); } ); - test( 'should match snapshot when all props provided', () => { + it( 'should match snapshot when all props provided', () => { const wrapper = shallow( { expect( wrapper ).toMatchSnapshot(); } ); - test( 'should match snapshot when isSelected and role are optionally provided', () => { + it( 'should match snapshot when isSelected and role are optionally provided', () => { const wrapper = shallow( { expect( wrapper ).toMatchSnapshot(); } ); + + it( 'should match snapshot when info is provided', () => { + const wrapper = shallow( + + My item + + ); + + expect( wrapper.prop( 'aria-label' ) ).not.toBeUndefined(); + expect( wrapper ).toMatchSnapshot(); + } ); + + it( 'should avoid using aria-label if only has non-string children', () => { + const wrapper = shallow( +
+ ); + + expect( wrapper.prop( 'aria-label' ) ).toBeUndefined(); + } ); } ); diff --git a/packages/edit-post/src/components/header/feature-toggle/index.js b/packages/edit-post/src/components/header/feature-toggle/index.js index afa2763916b0d5..3b811e653fac41 100644 --- a/packages/edit-post/src/components/header/feature-toggle/index.js +++ b/packages/edit-post/src/components/header/feature-toggle/index.js @@ -5,13 +5,15 @@ import { withSelect, withDispatch } from '@wordpress/data'; import { compose } from '@wordpress/compose'; import { MenuItem } from '@wordpress/components'; -function FeatureToggle( { onToggle, isActive, label } ) { +function FeatureToggle( { onToggle, isActive, label, info } ) { return ( { label } diff --git a/packages/edit-post/src/components/header/writing-menu/index.js b/packages/edit-post/src/components/header/writing-menu/index.js index 64fb516ef3518e..f99d0be3c67d3f 100644 --- a/packages/edit-post/src/components/header/writing-menu/index.js +++ b/packages/edit-post/src/components/header/writing-menu/index.js @@ -15,9 +15,21 @@ function WritingMenu( { onClose } ) { - - - + + + ); } diff --git a/packages/editor/src/components/block-settings-menu/test/__snapshots__/reusable-block-delete-button.js.snap b/packages/editor/src/components/block-settings-menu/test/__snapshots__/reusable-block-delete-button.js.snap index fe0ee5827c4998..c31a0f8e9f9b46 100644 --- a/packages/editor/src/components/block-settings-menu/test/__snapshots__/reusable-block-delete-button.js.snap +++ b/packages/editor/src/components/block-settings-menu/test/__snapshots__/reusable-block-delete-button.js.snap @@ -1,11 +1,11 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`ReusableBlockDeleteButton matches the snapshot 1`] = ` - Remove from Reusable Blocks - + `; diff --git a/packages/editor/src/components/block-settings-menu/test/block-mode-toggle.js b/packages/editor/src/components/block-settings-menu/test/block-mode-toggle.js index 61f819da34dbad..a8d8e321eb1547 100644 --- a/packages/editor/src/components/block-settings-menu/test/block-mode-toggle.js +++ b/packages/editor/src/components/block-settings-menu/test/block-mode-toggle.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { shallow } from 'enzyme'; +import ShallowRenderer from 'react-test-renderer/shallow'; /** * Internal dependencies @@ -9,34 +9,40 @@ import { shallow } from 'enzyme'; import { BlockModeToggle } from '../block-mode-toggle'; describe( 'BlockModeToggle', () => { + function getShallowRenderOutput( element ) { + const renderer = new ShallowRenderer(); + renderer.render( element ); + return renderer.getRenderOutput(); + } + it( "should not render the HTML mode button if the block doesn't support it", () => { - const wrapper = shallow( + const wrapper = getShallowRenderOutput( ); - expect( wrapper.equals( null ) ).toBe( true ); + expect( wrapper ).toBe( null ); } ); it( 'should render the HTML mode button', () => { - const wrapper = shallow( + const wrapper = getShallowRenderOutput( ); - const text = wrapper.find( 'MenuItem' ).first().prop( 'children' ); + const text = wrapper.props.children; expect( text ).toEqual( 'Edit as HTML' ); } ); it( 'should render the Visual mode button', () => { - const wrapper = shallow( + const wrapper = getShallowRenderOutput( ); - const text = wrapper.find( 'MenuItem' ).first().prop( 'children' ); + const text = wrapper.props.children; expect( text ).toEqual( 'Edit visually' ); } ); diff --git a/packages/editor/src/components/block-settings-menu/test/reusable-block-convert-button.js b/packages/editor/src/components/block-settings-menu/test/reusable-block-convert-button.js index 84b8b7085f8ff2..1aabafc55ef925 100644 --- a/packages/editor/src/components/block-settings-menu/test/reusable-block-convert-button.js +++ b/packages/editor/src/components/block-settings-menu/test/reusable-block-convert-button.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { shallow } from 'enzyme'; +import ShallowRenderer from 'react-test-renderer/shallow'; /** * Internal dependencies @@ -9,40 +9,48 @@ import { shallow } from 'enzyme'; import { ReusableBlockConvertButton } from '../reusable-block-convert-button'; describe( 'ReusableBlockConvertButton', () => { + function getShallowRenderOutput( element ) { + const renderer = new ShallowRenderer(); + renderer.render( element ); + return renderer.getRenderOutput(); + } + it( 'should not render when isVisible false', () => { - const wrapper = shallow( + const wrapper = getShallowRenderOutput( ); - expect( wrapper.children() ).not.toExist(); + expect( wrapper ).toBe( null ); } ); it( 'should allow converting a static block to a reusable block', () => { const onConvert = jest.fn(); - const wrapper = shallow( + const wrapper = getShallowRenderOutput( ); - const button = wrapper.find( 'MenuItem' ).first(); - expect( button.children().text() ).toBe( 'Add to Reusable Blocks' ); - button.simulate( 'click' ); + expect( wrapper.props.children[ 1 ] ).toBeFalsy(); + const button = wrapper.props.children[ 0 ]; + expect( button.props.children ).toBe( 'Add to Reusable Blocks' ); + button.props.onClick(); expect( onConvert ).toHaveBeenCalled(); } ); it( 'should allow converting a reusable block to static', () => { const onConvert = jest.fn(); - const wrapper = shallow( + const wrapper = getShallowRenderOutput( ); - const button = wrapper.find( 'MenuItem' ).first(); - expect( button.children().text() ).toBe( 'Convert to Regular Block' ); - button.simulate( 'click' ); + expect( wrapper.props.children[ 0 ] ).toBeFalsy(); + const button = wrapper.props.children[ 1 ]; + expect( button.props.children ).toBe( 'Convert to Regular Block' ); + button.props.onClick(); expect( onConvert ).toHaveBeenCalled(); } ); } ); diff --git a/packages/editor/src/components/block-settings-menu/test/reusable-block-delete-button.js b/packages/editor/src/components/block-settings-menu/test/reusable-block-delete-button.js index 63caddb28a5911..9da36d7f24e601 100644 --- a/packages/editor/src/components/block-settings-menu/test/reusable-block-delete-button.js +++ b/packages/editor/src/components/block-settings-menu/test/reusable-block-delete-button.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { shallow } from 'enzyme'; +import ShallowRenderer from 'react-test-renderer/shallow'; import { noop } from 'lodash'; /** @@ -10,8 +10,14 @@ import { noop } from 'lodash'; import { ReusableBlockDeleteButton } from '../reusable-block-delete-button'; describe( 'ReusableBlockDeleteButton', () => { + function getShallowRenderOutput( element ) { + const renderer = new ShallowRenderer(); + renderer.render( element ); + return renderer.getRenderOutput(); + } + it( 'matches the snapshot', () => { - const wrapper = shallow( + const wrapper = getShallowRenderOutput( { it( 'should allow deleting a reusable block', () => { const onDelete = jest.fn(); - const wrapper = shallow( + const wrapper = getShallowRenderOutput( ); - wrapper.find( 'MenuItem' ).simulate( 'click' ); + wrapper.props.onClick(); expect( onDelete ).toHaveBeenCalledWith( 123 ); } ); } ); diff --git a/test/e2e/specs/__snapshots__/plugins-api.test.js.snap b/test/e2e/specs/__snapshots__/plugins-api.test.js.snap index c4f89eddc19fd3..0b62fc522bf7ef 100644 --- a/test/e2e/specs/__snapshots__/plugins-api.test.js.snap +++ b/test/e2e/specs/__snapshots__/plugins-api.test.js.snap @@ -1,3 +1,3 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Using Plugins API Sidebar Should open plugins sidebar using More Menu item and render content 1`] = `"
(no title)
Sidebar title plugin
"`; +exports[`Using Plugins API Sidebar Should open plugins sidebar using More Menu item and render content 1`] = `"
(no title)
Sidebar title plugin
"`; diff --git a/test/e2e/specs/links.test.js b/test/e2e/specs/links.test.js index 0bd1859b00249f..23546f8c72045f 100644 --- a/test/e2e/specs/links.test.js +++ b/test/e2e/specs/links.test.js @@ -192,17 +192,14 @@ describe( 'Links', () => { expect( await getEditedPostContent() ).toMatchSnapshot(); } ); - const toggleFixedToolbar = async ( b ) => { - await page.click( '.edit-post-more-menu button' ); - const button = ( await page.$x( "//button[contains(text(), 'Unified Toolbar')]" ) )[ 0 ]; - const buttonClassNameProperty = await button.getProperty( 'className' ); - const buttonClassName = await buttonClassNameProperty.jsonValue(); - const isSelected = buttonClassName.indexOf( 'is-selected' ) !== -1; - if ( isSelected !== b ) { - await button.click(); - } else { - await page.click( '.edit-post-more-menu button' ); - } + const toggleFixedToolbar = async ( isFixed ) => { + await page.evaluate( ( _isFixed ) => { + const { select, dispatch } = wp.data; + const isCurrentlyFixed = select( 'core/edit-post' ).isFeatureActive( 'fixedToolbar' ); + if ( isCurrentlyFixed !== _isFixed ) { + dispatch( 'core/edit-post' ).toggleFeature( 'fixedToolbar' ); + } + }, isFixed ); }; it( 'allows Left to be pressed during creation when the toolbar is fixed to top', async () => {