Skip to content

Commit

Permalink
Add non-listbox functionality back to CircularOptionPicker (#54290)
Browse files Browse the repository at this point in the history
* Refactoring `CircularOptionPicker`

Added some tests and a new story to go with the changes.

* Updating `CircularOptionPicker` consumers

They all need to pass through the new `asButtons`/`disableLooping` prop following the `CircularOptionPicker` refactor.

* Updating CHANGELOG.md

* Fixing circular dependency

* Updating subcomponent filenames

* Simplifying `selectedIconProps` logic

* Refactoring `commonProps`

* Refactoring render functions into components

* Letting `Button` deal with classes

* Splitting state props into multiple effects

* Reverting `disableLooping` prop to `loop`

* Adding note explaining `aria-pressed={null}`

* Fixing some typing, removing `any`

* Reverting back to manual `is-pressed` style

* Refactoring `Omit`

* Updating `color-palette` snapshot

* Wrapping Option implementations with `forwardRef`

* Overriding `WithLoopingDisabled` code sample to show `loop`

* Refactoring `asButtons` typing

* Refactoring `compositeState` accessors

* Updating various README docs with new props

* Adding note to explain class additions

* Refactoring `listbox` implementation to keep `children` out of option list

* Re-refactoring `listbox` implementation

* Removing redundant `@default`

---------

Co-authored-by: Marco Ciampini <marco.ciampo@gmail.com>
  • Loading branch information
Andrew Hayward and ciampo authored Sep 15, 2023
1 parent 069a8f8 commit d2e3e29
Show file tree
Hide file tree
Showing 22 changed files with 898 additions and 335 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -199,43 +199,46 @@ exports[`ColorPaletteControl matches the snapshot 1`] = `
</div>
</div>
<div
aria-label="Custom color picker."
class="components-circular-option-picker"
id="option-picker-0"
role="listbox"
>
<div
class="components-circular-option-picker__swatches"
aria-label="Custom color picker."
id="components-circular-option-picker-0"
role="listbox"
>
<div
class="components-circular-option-picker__option-group components-circular-option-picker__swatches"
class="components-circular-option-picker__swatches"
>
<div
class="components-circular-option-picker__option-wrapper"
class="components-circular-option-picker__option-group components-circular-option-picker__swatches"
>
<button
aria-label="Color: red"
aria-selected="true"
class="components-button components-circular-option-picker__option is-pressed"
id="option-picker-0-0"
role="option"
style="background-color: rgb(255, 0, 0); color: rgb(255, 0, 0);"
tabindex="0"
type="button"
/>
<svg
aria-hidden="true"
fill="#000"
focusable="false"
height="24"
viewBox="0 0 24 24"
width="24"
xmlns="http://www.w3.org/2000/svg"
<div
class="components-circular-option-picker__option-wrapper"
>
<path
d="M16.7 7.1l-6.3 8.5-3.3-2.5-.9 1.2 4.5 3.4L17.9 8z"
<button
aria-label="Color: red"
aria-selected="true"
class="components-button components-circular-option-picker__option is-pressed"
id="components-circular-option-picker-0-0"
role="option"
style="background-color: rgb(255, 0, 0); color: rgb(255, 0, 0);"
tabindex="0"
type="button"
/>
</svg>
<svg
aria-hidden="true"
fill="#000"
focusable="false"
height="24"
viewBox="0 0 24 24"
width="24"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M16.7 7.1l-6.3 8.5-3.3-2.5-.9 1.2 4.5 3.4L17.9 8z"
/>
</svg>
</div>
</div>
</div>
</div>
Expand Down
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
- `BorderControl`: Apply proper metrics and simpler text ([#53998](https://github.com/WordPress/gutenberg/pull/53998)).
- `FormTokenField`: Update styling for consistency and increased visibility ([#54402](https://github.com/WordPress/gutenberg/pull/54402)).
- `Tooltip`: Add new `hideOnClick` prop ([#54406](https://github.com/WordPress/gutenberg/pull/54406)).
- `CircularOptionPicker`: Add option to use previous non-listbox behaviour, for contexts where buttons are more appropriate than a list of options ([#54290](https://github.com/WordPress/gutenberg/pull/54290)).
- `DuotonePicker/ColorListPicker`: Adds appropriate labels to 'Duotone Filter' color pickers ([#54468](https://github.com/WordPress/gutenberg/pull/54468)).

### Bug Fix
Expand Down
14 changes: 14 additions & 0 deletions packages/components/src/circular-option-picker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,20 @@ The child elements.

- Required: No

### `asButtons`: `boolean`

Whether the control should present as a set of buttons, each with its own tab stop.

- Required: No
- Default: `false`

### `loop`: `boolean`

Prevents keyboard interaction from wrapping around. Only used when `asButtons` is not true.

- Required: No
- Default: `true`

## Subcomponents

### `CircularOptionPicker.ButtonAction`
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/**
* External dependencies
*/
import classnames from 'classnames';

/**
* Internal dependencies
*/
import Button from '../button';
import Dropdown from '../dropdown';
import type { DropdownLinkActionProps } from './types';
import type { WordPressComponentProps } from '../ui/context';
import type { ButtonAsButtonProps } from '../button/types';

export function DropdownLinkAction( {
buttonProps,
className,
dropdownProps,
linkText,
}: DropdownLinkActionProps ) {
return (
<Dropdown
className={ classnames(
'components-circular-option-picker__dropdown-link-action',
className
) }
renderToggle={ ( { isOpen, onToggle } ) => (
<Button
aria-expanded={ isOpen }
aria-haspopup="true"
onClick={ onToggle }
variant="link"
{ ...buttonProps }
>
{ linkText }
</Button>
) }
{ ...dropdownProps }
/>
);
}

export function ButtonAction( {
className,
children,
...additionalProps
}: WordPressComponentProps< ButtonAsButtonProps, 'button', false > ) {
return (
<Button
className={ classnames(
'components-circular-option-picker__clear',
className
) }
variant="tertiary"
{ ...additionalProps }
>
{ children }
</Button>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* WordPress dependencies
*/
import { createContext } from '@wordpress/element';

/**
* Internal dependencies
*/
import type { CircularOptionPickerContextProps } from './types';

export const CircularOptionPickerContext =
createContext< CircularOptionPickerContextProps >( {} );
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/**
* External dependencies
*/
import classnames from 'classnames';

/**
* Internal dependencies
*/
import type { OptionGroupProps } from './types';

export function OptionGroup( {
className,
options,
...additionalProps
}: OptionGroupProps ) {
const role =
'aria-label' in additionalProps || 'aria-labelledby' in additionalProps
? 'group'
: undefined;

return (
<div
{ ...additionalProps }
role={ role }
className={ classnames(
'components-circular-option-picker__option-group',
'components-circular-option-picker__swatches',
className
) }
>
{ options }
</div>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
/**
* External dependencies
*/
import classnames from 'classnames';
import type { ForwardedRef } from 'react';

/**
* WordPress dependencies
*/
import { useInstanceId } from '@wordpress/compose';
import { forwardRef, useContext, useEffect } from '@wordpress/element';
import { Icon, check } from '@wordpress/icons';

/**
* Internal dependencies
*/
import { CircularOptionPickerContext } from './circular-option-picker-context';
import Button from '../button';
import { CompositeItem } from '../composite';
import Tooltip from '../tooltip';
import type {
OptionProps,
CircularOptionPickerCompositeState,
CircularOptionPickerContextProps,
} from './types';

const hasSelectedOption = new Map();

function UnforwardedOptionAsButton(
props: {
id?: string;
className?: string;
isPressed?: boolean;
},
forwardedRef: ForwardedRef< any >
) {
return <Button { ...props } ref={ forwardedRef }></Button>;
}

const OptionAsButton = forwardRef( UnforwardedOptionAsButton );

function UnforwardedOptionAsOption(
props: {
id: string;
className?: string;
isSelected?: boolean;
context: CircularOptionPickerContextProps;
},
forwardedRef: ForwardedRef< any >
) {
const { id, className, isSelected, context, ...additionalProps } = props;
const { isComposite, ..._compositeState } = context;
const compositeState =
_compositeState as CircularOptionPickerCompositeState;
const { baseId, currentId, setCurrentId } = compositeState;

useEffect( () => {
// If we call `setCurrentId` here, it doesn't update for other
// Option renders in the same pass. So we have to store our own
// map to make sure that we only set the first selected option.
// We still need to check `currentId` because the control will
// update this as the user moves around, and that state should
// be maintained as the group gains and loses focus.
if ( isSelected && ! currentId && ! hasSelectedOption.get( baseId ) ) {
hasSelectedOption.set( baseId, true );
setCurrentId( id );
}
}, [ baseId, currentId, id, isSelected, setCurrentId ] );

return (
<CompositeItem
{ ...additionalProps }
{ ...compositeState }
as={ Button }
id={ id }
// Ideally we'd let the underlying `Button` component
// handle this by passing `isPressed` as a prop.
// Unfortunately doing so also sets `aria-pressed` as
// an attribute on the element, which is incompatible
// with `role="option"`, and there is no way at this
// point to override that behaviour.
className={ classnames( className, {
'is-pressed': isSelected,
} ) }
role="option"
aria-selected={ !! isSelected }
ref={ forwardedRef }
/>
);
}

const OptionAsOption = forwardRef( UnforwardedOptionAsOption );

export function Option( {
className,
isSelected,
selectedIconProps = {},
tooltipText,
...additionalProps
}: OptionProps ) {
const compositeContext = useContext( CircularOptionPickerContext );
const { isComposite, baseId } = compositeContext;
const id = useInstanceId(
Option,
baseId || 'components-circular-option-picker__option'
);

const commonProps = {
id,
className: 'components-circular-option-picker__option',
...additionalProps,
};

const optionControl = isComposite ? (
<OptionAsOption
{ ...commonProps }
context={ compositeContext }
isSelected={ isSelected }
/>
) : (
<OptionAsButton { ...commonProps } isPressed={ isSelected } />
);

return (
<div
className={ classnames(
className,
'components-circular-option-picker__option-wrapper'
) }
>
{ tooltipText ? (
<Tooltip text={ tooltipText }>{ optionControl }</Tooltip>
) : (
optionControl
) }
{ isSelected && <Icon icon={ check } { ...selectedIconProps } /> }
</div>
);
}
Loading

1 comment on commit d2e3e29

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flaky tests detected in d2e3e29.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6200275396
📝 Reported issues:

Please sign in to comment.