From badb51d0144383e47d63d7ca3dd9e6a3e1fac1f0 Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 5 Dec 2019 02:27:31 -0300 Subject: [PATCH 1/7] Initial refactor on ToolbarButton --- packages/components/src/index.js | 2 + packages/components/src/index.native.js | 2 + .../accessible-toolbar-button-container.js | 26 ------- ...essible-toolbar-button-container.native.js | 6 -- .../components/src/toolbar-button/index.js | 75 ++++++++----------- .../toolbar-group/toolbar-group-collapsed.js | 8 +- .../src/toolbar-icon-button/index.js | 29 +++++++ packages/components/src/toolbar-item/index.js | 36 +++++++++ .../src/toolbar-item/index.native.js | 16 ++++ .../components/src/toolbar/stories/index.js | 57 +++++++++----- packages/components/src/toolbar/test/index.js | 6 +- 11 files changed, 160 insertions(+), 103 deletions(-) delete mode 100644 packages/components/src/toolbar-button/accessible-toolbar-button-container.js delete mode 100644 packages/components/src/toolbar-button/accessible-toolbar-button-container.native.js create mode 100644 packages/components/src/toolbar-icon-button/index.js create mode 100644 packages/components/src/toolbar-item/index.js create mode 100644 packages/components/src/toolbar-item/index.native.js diff --git a/packages/components/src/index.js b/packages/components/src/index.js index b89847f5a184e..d483175b93dc1 100644 --- a/packages/components/src/index.js +++ b/packages/components/src/index.js @@ -72,6 +72,8 @@ export { default as ToggleControl } from './toggle-control'; export { default as Toolbar } from './toolbar'; export { default as ToolbarButton } from './toolbar-button'; export { default as ToolbarGroup } from './toolbar-group'; +export { default as __experimentalToolbarIconButton } from './toolbar-icon-button'; +export { default as __experimentalToolbarItem } from './toolbar-item'; export { default as Tooltip } from './tooltip'; export { default as TreeSelect } from './tree-select'; export { default as VisuallyHidden } from './visually-hidden'; diff --git a/packages/components/src/index.native.js b/packages/components/src/index.native.js index 7cda193234fcd..a2f71ecbeaa1a 100644 --- a/packages/components/src/index.native.js +++ b/packages/components/src/index.native.js @@ -7,6 +7,8 @@ export { default as Dropdown } from './dropdown'; export { default as Toolbar } from './toolbar'; export { default as ToolbarButton } from './toolbar-button'; export { default as ToolbarGroup } from './toolbar-group'; +export { default as __experimentalToolbarIconButton } from './toolbar-icon-button'; +export { default as __experimentalToolbarItem } from './toolbar-item'; export { default as Icon } from './icon'; export { default as IconButton } from './icon-button'; export { default as Spinner } from './spinner'; diff --git a/packages/components/src/toolbar-button/accessible-toolbar-button-container.js b/packages/components/src/toolbar-button/accessible-toolbar-button-container.js deleted file mode 100644 index 5d026c4b83e8c..0000000000000 --- a/packages/components/src/toolbar-button/accessible-toolbar-button-container.js +++ /dev/null @@ -1,26 +0,0 @@ -/** - * External dependencies - */ -import { useToolbarItem } from 'reakit/Toolbar'; - -/** - * WordPress dependencies - */ -import { Children, cloneElement, useContext } from '@wordpress/element'; - -/** - * Internal dependencies - */ -import ToolbarContext from '../toolbar-context'; - -function AccessibleToolbarButtonContainer( props ) { - const accessibleToolbarState = useContext( ToolbarContext ); - const childButton = Children.only( props.children ); - - // https://reakit.io/docs/composition/#props-hooks - const itemHTMLProps = useToolbarItem( accessibleToolbarState, childButton.props ); - - return
{ cloneElement( childButton, itemHTMLProps ) }
; -} - -export default AccessibleToolbarButtonContainer; diff --git a/packages/components/src/toolbar-button/accessible-toolbar-button-container.native.js b/packages/components/src/toolbar-button/accessible-toolbar-button-container.native.js deleted file mode 100644 index 7fe5e6657bcd1..0000000000000 --- a/packages/components/src/toolbar-button/accessible-toolbar-button-container.native.js +++ /dev/null @@ -1,6 +0,0 @@ -/** - * Internal dependencies - */ -import ToolbarButtonContainer from './toolbar-button-container'; - -export default ToolbarButtonContainer; diff --git a/packages/components/src/toolbar-button/index.js b/packages/components/src/toolbar-button/index.js index 0dd438b47260e..a7f48c3a63f19 100644 --- a/packages/components/src/toolbar-button/index.js +++ b/packages/components/src/toolbar-button/index.js @@ -3,18 +3,13 @@ */ import classnames from 'classnames'; -/** - * WordPress dependencies - */ -import { useContext } from '@wordpress/element'; - /** * Internal dependencies */ import IconButton from '../icon-button'; -import ToolbarContext from '../toolbar-context'; -import AccessibleToolbarButtonContainer from './accessible-toolbar-button-container'; import ToolbarButtonContainer from './toolbar-button-container'; +import Button from '../button'; +import ToolbarItem from '../toolbar-item'; function ToolbarButton( { containerClassName, @@ -28,48 +23,42 @@ function ToolbarButton( { isDisabled, extraProps, children, + ...props } ) { - // It'll contain state if `ToolbarButton` is being used within - // `` - const accessibleToolbarState = useContext( ToolbarContext ); - - const button = ( - { - event.stopPropagation(); - if ( onClick ) { - onClick( event ); - } - } } - className={ classnames( - 'components-toolbar__control', - className, - { 'is-active': isActive } - ) } - aria-pressed={ isActive } - disabled={ isDisabled } - { ...extraProps } - /> - ); - - if ( accessibleToolbarState ) { + if ( icon ) { return ( - - { button } - + + { + event.stopPropagation(); + if ( onClick ) { + onClick( event ); + } + } } + className={ classnames( 'components-toolbar__control', className, { + 'is-active': isActive, + } ) } + aria-pressed={ isActive } + disabled={ isDisabled } + { ...extraProps } + /> + { children } + ); } - // ToolbarButton is being used outside of the accessible Toolbar return ( - - { button } - { children } - + + { ( toolbarItemProps ) => } + ); } diff --git a/packages/components/src/toolbar-group/toolbar-group-collapsed.js b/packages/components/src/toolbar-group/toolbar-group-collapsed.js index 8e684a7cae72a..7e084770ec1eb 100644 --- a/packages/components/src/toolbar-group/toolbar-group-collapsed.js +++ b/packages/components/src/toolbar-group/toolbar-group-collapsed.js @@ -1,8 +1,3 @@ -/** - * External dependencies - */ -import { ToolbarItem } from 'reakit/Toolbar'; - /** * WordPress dependencies */ @@ -13,6 +8,7 @@ import { useContext } from '@wordpress/element'; */ import DropdownMenu from '../dropdown-menu'; import ToolbarContext from '../toolbar-context'; +import ToolbarItem from '../toolbar-item'; function ToolbarGroupCollapsed( { controls = [], @@ -40,7 +36,7 @@ function ToolbarGroupCollapsed( { if ( accessibleToolbarState ) { return ( // https://reakit.io/docs/composition/#render-props - + { ( toolbarItemHTMLProps ) => renderDropdownMenu( toolbarItemHTMLProps ) } ); diff --git a/packages/components/src/toolbar-icon-button/index.js b/packages/components/src/toolbar-icon-button/index.js new file mode 100644 index 0000000000000..cf983e31ebd27 --- /dev/null +++ b/packages/components/src/toolbar-icon-button/index.js @@ -0,0 +1,29 @@ +/** + * External dependencies + */ +import classnames from 'classnames'; + +/** + * WordPress dependencies + */ +import { forwardRef } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import ToolbarItem from '../toolbar-item'; +import IconButton from '../icon-button'; + +function ToolbarIconButton( props, ref ) { + const className = classnames( + 'components-toolbar-icon-button', + props.className + ); + return ( + + { ( toolbarItemProps ) => } + + ); +} + +export default forwardRef( ToolbarIconButton ); diff --git a/packages/components/src/toolbar-item/index.js b/packages/components/src/toolbar-item/index.js new file mode 100644 index 0000000000000..00170174387bd --- /dev/null +++ b/packages/components/src/toolbar-item/index.js @@ -0,0 +1,36 @@ +/** + * External dependencies + */ +import { useToolbarItem } from 'reakit/Toolbar'; + +/** + * WordPress dependencies + */ +import { forwardRef, useContext } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import ToolbarContext from '../toolbar-context'; + +function ToolbarItem( { children, ...props }, ref ) { + const accessibleToolbarState = useContext( ToolbarContext ); + // https://reakit.io/docs/composition/#props-hooks + const itemProps = useToolbarItem( accessibleToolbarState, { ...props, ref } ); + + if ( typeof children !== 'function' ) { + // eslint-disable-next-line no-console + console.warn( '`ToolbarItem` is a generic headless component that accepts only function children props' ); + return null; + } + + if ( ! accessibleToolbarState ) { + // eslint-disable-next-line no-console + console.warn( '`ToolbarItem` should be rendered within ``' ); + return null; + } + + return children( itemProps ); +} + +export default forwardRef( ToolbarItem ); diff --git a/packages/components/src/toolbar-item/index.native.js b/packages/components/src/toolbar-item/index.native.js new file mode 100644 index 0000000000000..9819d0ed02d83 --- /dev/null +++ b/packages/components/src/toolbar-item/index.native.js @@ -0,0 +1,16 @@ +/** + * WordPress dependencies + */ +import { forwardRef } from '@wordpress/element'; + +function ToolbarItem( { children, ...props }, ref ) { + if ( typeof children !== 'function' ) { + // eslint-disable-next-line no-console + console.warn( '`ToolbarItem` is a generic headless component that accepts only function children props' ); + return null; + } + + return children( { ...props, ref } ); +} + +export default forwardRef( ToolbarItem ); diff --git a/packages/components/src/toolbar/stories/index.js b/packages/components/src/toolbar/stories/index.js index 102f924f5a1a0..264338d102491 100644 --- a/packages/components/src/toolbar/stories/index.js +++ b/packages/components/src/toolbar/stories/index.js @@ -2,7 +2,15 @@ * Internal dependencies */ import Toolbar from '../'; -import { SVG, Path, ToolbarButton, ToolbarGroup } from '../../'; +import { + SVG, + Path, + ToolbarButton, + ToolbarGroup, + __experimentalToolbarIconButton as ToolbarIconButton, + __experimentalToolbarItem as ToolbarItem, + DropdownMenu, +} from '../../'; export default { title: 'Components|Toolbar', component: Toolbar }; @@ -20,22 +28,30 @@ export const _default = () => { // id is required for server side rendering - + + + + + { ( toggleProps ) => ( + + ) } + - - - - + Text + + + { export const withoutGroup = () => { return ( // id is required for server side rendering - - - - + + + + ); }; diff --git a/packages/components/src/toolbar/test/index.js b/packages/components/src/toolbar/test/index.js index 2d454d1a8bff3..da443da540891 100644 --- a/packages/components/src/toolbar/test/index.js +++ b/packages/components/src/toolbar/test/index.js @@ -7,15 +7,15 @@ import { mount } from 'enzyme'; * Internal dependencies */ import Toolbar from '../'; -import ToolbarButton from '../../toolbar-button'; +import ToolbarIconButton from '../../toolbar-icon-button'; describe( 'Toolbar', () => { describe( 'basic rendering', () => { it( 'should render a toolbar with toolbar buttons', () => { const wrapper = mount( - - + + ); const control1 = wrapper.find( 'button[aria-label="control1"]' ); From 31dcf580d16cc2a81a8ac00bc8712ab6d778743d Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 5 Dec 2019 03:05:06 -0300 Subject: [PATCH 2/7] Update storyshots --- storybook/test/__snapshots__/index.js.snap | 348 +++++++++++---------- 1 file changed, 175 insertions(+), 173 deletions(-) diff --git a/storybook/test/__snapshots__/index.js.snap b/storybook/test/__snapshots__/index.js.snap index b34d7ad67005b..b80103ddce8df 100644 --- a/storybook/test/__snapshots__/index.js.snap +++ b/storybook/test/__snapshots__/index.js.snap @@ -3385,48 +3385,11 @@ exports[`Storyshots Components|Toolbar Default 1`] = ` >
-
- -
-
-
-
- -
-
- -
-
+
-
-
-
+ -
-
-`; - -exports[`Storyshots Components|Toolbar Without Group 1`] = ` -