From 967c90f6762378d5b8af2b267c8076933aea0127 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Fri, 10 Dec 2021 15:18:39 +1100 Subject: [PATCH 01/13] Try using Popover component in custom select control --- .../src/custom-select-control/index.js | 123 ++++++++++-------- .../src/custom-select-control/style.scss | 6 +- 2 files changed, 75 insertions(+), 54 deletions(-) diff --git a/packages/components/src/custom-select-control/index.js b/packages/components/src/custom-select-control/index.js index 90b5bbf2115e20..a58bae8c67070e 100644 --- a/packages/components/src/custom-select-control/index.js +++ b/packages/components/src/custom-select-control/index.js @@ -12,7 +12,19 @@ import { __, sprintf } from '@wordpress/i18n'; /** * Internal dependencies */ -import { Button, VisuallyHidden } from '../'; +import { Button, Popover, VisuallyHidden } from '../'; + +const OptionList = ( { anchorRect, isOpen, children } ) => { + if ( ! isOpen ) { + return children; + } + + return ( + + { children } + + ); +}; const itemToString = ( item ) => item?.name; // This is needed so that in Windows, where @@ -104,6 +116,7 @@ export default function CustomSelectControl( { ) { delete menuProps[ 'aria-activedescendant' ]; } + return (
) } - - +
+ + +
    + { isOpen && + items.map( ( item, index ) => ( + // eslint-disable-next-line react/jsx-key +
  • + { item.name } + { item.__experimentalHint && ( + + { item.__experimentalHint } + + ) } + { item === selectedItem && ( + + ) } +
    • + ) ) } +
+ +
); } diff --git a/packages/components/src/custom-select-control/style.scss b/packages/components/src/custom-select-control/style.scss index f3175331336bfd..543615d4d1da40 100644 --- a/packages/components/src/custom-select-control/style.scss +++ b/packages/components/src/custom-select-control/style.scss @@ -2,6 +2,10 @@ position: relative; } +.components-custom-select-control__inner { + display: inline-block; +} + .components-custom-select-control__label { display: block; margin-bottom: $grid-unit-10; @@ -52,7 +56,7 @@ min-width: 100%; overflow: auto; padding: 0; - position: absolute; + margin: 0; z-index: z-index(".components-popover"); } From 73b385b2e8d11083690c4d72689f686773e8f2a8 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Fri, 10 Dec 2021 15:26:00 +1100 Subject: [PATCH 02/13] Remove anchorRect prop --- packages/components/src/custom-select-control/index.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/components/src/custom-select-control/index.js b/packages/components/src/custom-select-control/index.js index a58bae8c67070e..77552c60830356 100644 --- a/packages/components/src/custom-select-control/index.js +++ b/packages/components/src/custom-select-control/index.js @@ -14,16 +14,12 @@ import { __, sprintf } from '@wordpress/i18n'; */ import { Button, Popover, VisuallyHidden } from '../'; -const OptionList = ( { anchorRect, isOpen, children } ) => { +const OptionList = ( { isOpen, children } ) => { if ( ! isOpen ) { return children; } - return ( - - { children } - - ); + return { children }; }; const itemToString = ( item ) => item?.name; From 0bdd715caa260b0573a0b78bbfc74c9627ab9201 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Mon, 13 Dec 2021 17:34:25 +1100 Subject: [PATCH 03/13] Switch to using the DropdownMenu component --- .../src/custom-select-control/index.js | 182 ++++++------------ .../src/custom-select-control/style.scss | 9 - 2 files changed, 61 insertions(+), 130 deletions(-) diff --git a/packages/components/src/custom-select-control/index.js b/packages/components/src/custom-select-control/index.js index 77552c60830356..b83f1d4625c229 100644 --- a/packages/components/src/custom-select-control/index.js +++ b/packages/components/src/custom-select-control/index.js @@ -1,7 +1,6 @@ /** * External dependencies */ -import { useSelect } from 'downshift'; import classnames from 'classnames'; /** @@ -12,83 +11,25 @@ import { __, sprintf } from '@wordpress/i18n'; /** * Internal dependencies */ -import { Button, Popover, VisuallyHidden } from '../'; +import { DropdownMenu, MenuGroup, MenuItem, VisuallyHidden } from '../'; -const OptionList = ( { isOpen, children } ) => { - if ( ! isOpen ) { - return children; - } +const itemToString = ( item ) => item?.name; - return { children }; +const POPOVER_PROPS = { + className: 'block-editor-block-settings-menu__popover', + position: 'bottom right', + isAlternate: true, }; -const itemToString = ( item ) => item?.name; -// This is needed so that in Windows, where -// the menu does not necessarily open on -// key up/down, you can still switch between -// options with the menu closed. -const stateReducer = ( - { selectedItem }, - { type, changes, props: { items } } -) => { - switch ( type ) { - case useSelect.stateChangeTypes.ToggleButtonKeyDownArrowDown: - // If we already have a selected item, try to select the next one, - // without circular navigation. Otherwise, select the first item. - return { - selectedItem: - items[ - selectedItem - ? Math.min( - items.indexOf( selectedItem ) + 1, - items.length - 1 - ) - : 0 - ], - }; - case useSelect.stateChangeTypes.ToggleButtonKeyDownArrowUp: - // If we already have a selected item, try to select the previous one, - // without circular navigation. Otherwise, select the last item. - return { - selectedItem: - items[ - selectedItem - ? Math.max( items.indexOf( selectedItem ) - 1, 0 ) - : items.length - 1 - ], - }; - default: - return changes; - } -}; -export default function CustomSelectControl( { +export default function CustomSelectControlWithDropdownMenu( { className, hideLabelFromVision, label, describedBy, options: items, onChange: onSelectedItemChange, - value: _selectedItem, + value: selectedItem, } ) { - const { - getLabelProps, - getToggleButtonProps, - getMenuProps, - getItemProps, - isOpen, - highlightedIndex, - selectedItem, - } = useSelect( { - initialSelectedItem: items[ 0 ], - items, - itemToString, - onSelectedItemChange, - ...( typeof _selectedItem !== 'undefined' && _selectedItem !== null - ? { selectedItem: _selectedItem } - : undefined ), - stateReducer, - } ); - function getDescribedBy() { if ( describedBy ) { return describedBy; @@ -102,16 +43,23 @@ export default function CustomSelectControl( { return sprintf( __( 'Currently selected: %s' ), selectedItem.name ); } - const menuProps = getMenuProps( { - className: 'components-custom-select-control__menu', - 'aria-hidden': ! isOpen, - } ); - // We need this here, because the null active descendant is not fully ARIA compliant. - if ( - menuProps[ 'aria-activedescendant' ]?.startsWith( 'downshift-null' ) - ) { - delete menuProps[ 'aria-activedescendant' ]; - } + const toggleProps = { + // This is needed because some speech recognition software don't support `aria-labelledby`. + 'aria-label': label, + 'aria-labelledby': undefined, + className: 'components-custom-select-control__button', + isSmall: true, + describedBy: getDescribedBy(), + children: ( + <> + { itemToString( selectedItem ) } + + + ), + }; return (
{ hideLabelFromVision ? ( - - { label } - + { label } ) : ( /* eslint-disable-next-line jsx-a11y/label-has-associated-control, jsx-a11y/label-has-for */ ) } -
- - -
    - { isOpen && - items.map( ( item, index ) => ( - // eslint-disable-next-line react/jsx-key -
  • + { ( { isOpen, onClose } ) => + isOpen && ( + + { items.map( ( item, index ) => ( + { + onSelectedItemChange( { + selectedItem: item, + } ); + onClose(); + } } + icon={ item === selectedItem && check } > { item.name } { item.__experimentalHint && ( @@ -179,17 +124,12 @@ export default function CustomSelectControl( { { item.__experimentalHint } ) } - { item === selectedItem && ( - - ) } -
    • + ) ) } -
- -
+ + ) + } +
); } diff --git a/packages/components/src/custom-select-control/style.scss b/packages/components/src/custom-select-control/style.scss index 543615d4d1da40..8874c84bc0c792 100644 --- a/packages/components/src/custom-select-control/style.scss +++ b/packages/components/src/custom-select-control/style.scss @@ -61,17 +61,10 @@ } .components-custom-select-control__item { - align-items: center; - display: grid; - grid-template-columns: auto auto; - list-style-type: none; padding: $grid-unit-10; cursor: default; line-height: $icon-size + $grid-unit-05; - &.has-hint { - grid-template-columns: auto auto 30px; - } &.is-highlighted { background: $gray-300; } @@ -79,8 +72,6 @@ color: $gray-700; text-align: right; padding-right: $grid-unit-05; - } - .components-custom-select-control__item-icon { margin-left: auto; } From 2b58993fc76d6ff5a446e46c9224f612e1eb7a6c Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Tue, 14 Dec 2021 16:14:49 +1100 Subject: [PATCH 04/13] Explore overriding role attributes to semantically indicate that we are using approximating select/option --- .../components/src/custom-select-control/index.js | 12 +++++++++--- packages/components/src/dropdown-menu/index.js | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/components/src/custom-select-control/index.js b/packages/components/src/custom-select-control/index.js index b83f1d4625c229..b69cef26df19ce 100644 --- a/packages/components/src/custom-select-control/index.js +++ b/packages/components/src/custom-select-control/index.js @@ -11,7 +11,7 @@ import { __, sprintf } from '@wordpress/i18n'; /** * Internal dependencies */ -import { DropdownMenu, MenuGroup, MenuItem, VisuallyHidden } from '../'; +import { DropdownMenu, MenuItem, VisuallyHidden } from '../'; const itemToString = ( item ) => item?.name; @@ -61,6 +61,10 @@ export default function CustomSelectControlWithDropdownMenu( { ), }; + const menuProps = { + role: 'select', + }; + return (
{ ( { isOpen, onClose } ) => isOpen && ( - + <> { items.map( ( item, index ) => ( ) ) } - + ) } diff --git a/packages/components/src/dropdown-menu/index.js b/packages/components/src/dropdown-menu/index.js index 85983f92305aca..dddaa5fd00b828 100644 --- a/packages/components/src/dropdown-menu/index.js +++ b/packages/components/src/dropdown-menu/index.js @@ -134,7 +134,7 @@ function DropdownMenu( dropdownMenuProps ) { ); return ( - + { isFunction( children ) ? children( props ) : null } { flatMap( controlSets, ( controlSet, indexOfSet ) => controlSet.map( ( control, indexOfControl ) => ( From acdb9c10c7e27238ec3f0daa238779a6b22de223 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Tue, 14 Dec 2021 16:18:53 +1100 Subject: [PATCH 05/13] Rename select role to listbox --- packages/components/src/custom-select-control/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/custom-select-control/index.js b/packages/components/src/custom-select-control/index.js index b69cef26df19ce..b134639c512d50 100644 --- a/packages/components/src/custom-select-control/index.js +++ b/packages/components/src/custom-select-control/index.js @@ -62,7 +62,7 @@ export default function CustomSelectControlWithDropdownMenu( { }; const menuProps = { - role: 'select', + role: 'listbox', }; return ( From 0304b706042704440fd792cb31af8aba787bffe8 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Wed, 15 Dec 2021 17:03:57 +1100 Subject: [PATCH 06/13] Switch back to using Popover component, add useResizeObserver for setting correct width of dropdown --- .../src/custom-select-control/index.js | 211 ++++++++++++------ .../src/custom-select-control/style.scss | 20 +- 2 files changed, 154 insertions(+), 77 deletions(-) diff --git a/packages/components/src/custom-select-control/index.js b/packages/components/src/custom-select-control/index.js index b134639c512d50..5457ffa748237a 100644 --- a/packages/components/src/custom-select-control/index.js +++ b/packages/components/src/custom-select-control/index.js @@ -1,35 +1,112 @@ /** * External dependencies */ +import { useSelect } from 'downshift'; import classnames from 'classnames'; /** * WordPress dependencies */ +import { useResizeObserver } from '@wordpress/compose'; +import { useRef } from '@wordpress/element'; import { Icon, check, chevronDown } from '@wordpress/icons'; import { __, sprintf } from '@wordpress/i18n'; /** * Internal dependencies */ -import { DropdownMenu, MenuItem, VisuallyHidden } from '../'; +import { Button, Popover, VisuallyHidden } from '../'; -const itemToString = ( item ) => item?.name; +const OptionList = ( { anchorRef, isOpen, children } ) => { + if ( ! isOpen ) { + return children; + } -const POPOVER_PROPS = { - className: 'block-editor-block-settings-menu__popover', - position: 'bottom right', - isAlternate: true, + return ( + + { children } + + ); }; -export default function CustomSelectControlWithDropdownMenu( { +const itemToString = ( item ) => item?.name; +// This is needed so that in Windows, where +// the menu does not necessarily open on +// key up/down, you can still switch between +// options with the menu closed. +const stateReducer = ( + { selectedItem }, + { type, changes, props: { items } } +) => { + switch ( type ) { + case useSelect.stateChangeTypes.ToggleButtonKeyDownArrowDown: + // If we already have a selected item, try to select the next one, + // without circular navigation. Otherwise, select the first item. + return { + selectedItem: + items[ + selectedItem + ? Math.min( + items.indexOf( selectedItem ) + 1, + items.length - 1 + ) + : 0 + ], + }; + case useSelect.stateChangeTypes.ToggleButtonKeyDownArrowUp: + // If we already have a selected item, try to select the previous one, + // without circular navigation. Otherwise, select the last item. + return { + selectedItem: + items[ + selectedItem + ? Math.max( items.indexOf( selectedItem ) - 1, 0 ) + : items.length - 1 + ], + }; + default: + return changes; + } +}; +export default function CustomSelectControl( { className, hideLabelFromVision, label, describedBy, options: items, onChange: onSelectedItemChange, - value: selectedItem, + value: _selectedItem, } ) { + const { + getLabelProps, + getToggleButtonProps, + getMenuProps, + getItemProps, + isOpen, + highlightedIndex, + selectedItem, + } = useSelect( { + initialSelectedItem: items[ 0 ], + items, + itemToString, + onSelectedItemChange, + ...( typeof _selectedItem !== 'undefined' && _selectedItem !== null + ? { selectedItem: _selectedItem } + : undefined ), + stateReducer, + } ); + + const anchorRef = useRef(); + + const [ + buttonResizeListener, + { width: buttonWidth }, + ] = useResizeObserver(); + function getDescribedBy() { if ( describedBy ) { return describedBy; @@ -43,27 +120,19 @@ export default function CustomSelectControlWithDropdownMenu( { return sprintf( __( 'Currently selected: %s' ), selectedItem.name ); } - const toggleProps = { - // This is needed because some speech recognition software don't support `aria-labelledby`. - 'aria-label': label, - 'aria-labelledby': undefined, - className: 'components-custom-select-control__button', - isSmall: true, - describedBy: getDescribedBy(), - children: ( - <> - { itemToString( selectedItem ) } - - - ), - }; - - const menuProps = { - role: 'listbox', - }; + const menuProps = getMenuProps( { + className: 'components-custom-select-control__menu', + 'aria-hidden': ! isOpen, + style: { + width: buttonWidth, + }, + } ); + // We need this here, because the null active descendant is not fully ARIA compliant. + if ( + menuProps[ 'aria-activedescendant' ]?.startsWith( 'downshift-null' ) + ) { + delete menuProps[ 'aria-activedescendant' ]; + } return (
{ hideLabelFromVision ? ( - { label } + + { label } + ) : ( /* eslint-disable-next-line jsx-a11y/label-has-associated-control, jsx-a11y/label-has-for */ ) } - - { ( { isOpen, onClose } ) => - isOpen && ( - <> - { items.map( ( item, index ) => ( - + + +
    + { isOpen && + items.map( ( item, index ) => ( + // eslint-disable-next-line react/jsx-key +
  • { - onSelectedItemChange( { - selectedItem: item, - } ); - onClose(); - } } - icon={ item === selectedItem && check } + } ) } > { item.name } { item.__experimentalHint && ( @@ -130,12 +202,17 @@ export default function CustomSelectControlWithDropdownMenu( { { item.__experimentalHint } ) } - + { item === selectedItem && ( + + ) } +
    • ) ) } - - ) - } - +
+ +
); } diff --git a/packages/components/src/custom-select-control/style.scss b/packages/components/src/custom-select-control/style.scss index 8874c84bc0c792..8ef38c5c4fc3fc 100644 --- a/packages/components/src/custom-select-control/style.scss +++ b/packages/components/src/custom-select-control/style.scss @@ -2,10 +2,6 @@ position: relative; } -.components-custom-select-control__inner { - display: inline-block; -} - .components-custom-select-control__label { display: block; margin-bottom: $grid-unit-10; @@ -45,15 +41,10 @@ display: none; } - // Block UI appearance. - border: $border-width solid $gray-900; - background-color: $white; - border-radius: $radius-block-ui; outline: none; transition: none; - max-height: 400px; - min-width: 100%; + min-width: 130px; overflow: auto; padding: 0; margin: 0; @@ -61,10 +52,17 @@ } .components-custom-select-control__item { + align-items: center; + display: grid; + grid-template-columns: auto auto; + list-style-type: none; padding: $grid-unit-10; cursor: default; line-height: $icon-size + $grid-unit-05; + &.has-hint { + grid-template-columns: auto auto 30px; + } &.is-highlighted { background: $gray-300; } @@ -72,6 +70,8 @@ color: $gray-700; text-align: right; padding-right: $grid-unit-05; + } + .components-custom-select-control__item-icon { margin-left: auto; } From b59b6a7becf51c0c9659a29cf42eaafdfcada42e Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Wed, 15 Dec 2021 17:14:58 +1100 Subject: [PATCH 07/13] Revert change to DropdownMenu component --- packages/components/src/dropdown-menu/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/dropdown-menu/index.js b/packages/components/src/dropdown-menu/index.js index dddaa5fd00b828..85983f92305aca 100644 --- a/packages/components/src/dropdown-menu/index.js +++ b/packages/components/src/dropdown-menu/index.js @@ -134,7 +134,7 @@ function DropdownMenu( dropdownMenuProps ) { ); return ( - + { isFunction( children ) ? children( props ) : null } { flatMap( controlSets, ( controlSet, indexOfSet ) => controlSet.map( ( control, indexOfControl ) => ( From b8fc4d53912468d6f6ec3c0a7c5a1395fd198006 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Wed, 15 Dec 2021 17:17:51 +1100 Subject: [PATCH 08/13] Remove unnecessary nesting --- .../src/custom-select-control/index.js | 114 +++++++++--------- 1 file changed, 56 insertions(+), 58 deletions(-) diff --git a/packages/components/src/custom-select-control/index.js b/packages/components/src/custom-select-control/index.js index 5457ffa748237a..1ab1acf830d8b0 100644 --- a/packages/components/src/custom-select-control/index.js +++ b/packages/components/src/custom-select-control/index.js @@ -155,64 +155,62 @@ export default function CustomSelectControl( { { label } ) } -
- - -
    - { isOpen && - items.map( ( item, index ) => ( - // eslint-disable-next-line react/jsx-key -
  • - { item.name } - { item.__experimentalHint && ( - - { item.__experimentalHint } - - ) } - { item === selectedItem && ( - - ) } -
    • - ) ) } -
- -
+ + +
    + { isOpen && + items.map( ( item, index ) => ( + // eslint-disable-next-line react/jsx-key +
  • + { item.name } + { item.__experimentalHint && ( + + { item.__experimentalHint } + + ) } + { item === selectedItem && ( + + ) } +
    • + ) ) } +
+ ); } From 509e015b9a95eb1b558e570446007fa6c3eb3b32 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Thu, 16 Dec 2021 17:39:58 +1100 Subject: [PATCH 09/13] Add anchorRef and merge refs with downshift's toggle button ref to allow calculating popover position based on the size of the button --- .../src/custom-select-control/index.js | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/packages/components/src/custom-select-control/index.js b/packages/components/src/custom-select-control/index.js index 1ab1acf830d8b0..fdb5fe36e7b31f 100644 --- a/packages/components/src/custom-select-control/index.js +++ b/packages/components/src/custom-select-control/index.js @@ -7,7 +7,7 @@ import classnames from 'classnames'; /** * WordPress dependencies */ -import { useResizeObserver } from '@wordpress/compose'; +import { useMergeRefs, useResizeObserver } from '@wordpress/compose'; import { useRef } from '@wordpress/element'; import { Icon, check, chevronDown } from '@wordpress/icons'; import { __, sprintf } from '@wordpress/i18n'; @@ -27,6 +27,7 @@ const OptionList = ( { anchorRef, isOpen, children } ) => { className="components-custom-select-control__popover" isAlternate={ true } position={ 'bottom center' } + shouldAnchorIncludePadding > { children } @@ -124,7 +125,7 @@ export default function CustomSelectControl( { className: 'components-custom-select-control__menu', 'aria-hidden': ! isOpen, style: { - width: buttonWidth, + minWidth: buttonWidth, }, } ); // We need this here, because the null active descendant is not fully ARIA compliant. @@ -134,6 +135,19 @@ export default function CustomSelectControl( { delete menuProps[ 'aria-activedescendant' ]; } + const toggleButtonProps = getToggleButtonProps( { + // This is needed because some speech recognition software don't support `aria-labelledby`. + 'aria-label': label, + 'aria-labelledby': undefined, + className: 'components-custom-select-control__button', + isSmall: true, + describedBy: getDescribedBy(), + } ); + + // Merge the toggle button's ref and the anchorRef so that the anchorRef can be + // used for calculating the Popover position based on the size of the button. + const buttonRef = useMergeRefs( [ toggleButtonProps.ref, anchorRef ] ); + return (
) } -