CustomSelectControl: refactor with ariakit

[brookewp]

Context

Continuing with the exploration started by @ciampo in #41466, this issue will track the migration of CustomSelectControl to Ariakit.

Legacy: #55234
New version: #55790

Initial steps

• Analyze the legacy component, learn how it works, take note of its features
• Study Ariakit's Select component
• Look into ARIA roles
• Consider using WordPressComponentsProps CustomSelect: Add WordPressComponentsProps #56998
• Create a new, temporary component which should implement:
  • children (and a CustomSelectControlV2.Item component) to allow its consumers to specify select options
  • basic functionality (value / onChange / defaultValue / label / size props)
  • styles that match the legacy component
  • Storybook docs and examples (maybe one with post authors)
• Refine README, types stories, etc CustomSelect: Adapt component for legacy props #57902

V2 Legacy Wrapper

Completed in #57902

• Analyze the "API gap" between the legacy and the new component
• Come up with a strategy on how to bridge the gap
• Add support for the "monolithic" usage of the component with the options prop
• Bolster the legacy CustomSelectControl unit tests CustomSelectControl: add additional unit tests #56575
• Use the previously bolstered unit tests to certify that the new component behaves like the legacy one
• Align unit tests CustomSelectControl: align v1 and legacy v2 unit tests #62706
• Fix option extra attributes types and propagation to onChange callback and DOM elements CustomSelectControlV2: fix handling of extra attributes passed to options in the legacy adapter #62255
• Investigate potential issues when using v2 legacy adapter in controlled mode, as the component doesn't seem to update correctly when a new value is passed? (can be reproduced by following the testing instructions in CustomSelectControlV2: fix handling of extra attributes passed to options in the legacy adapter #62255 re. FontAppearanceControl in global styles) — fix in CustomSelectControl V2: fix setting initial value and reacting to external controlled updates #62733
• Double check initial item selection, including the fact that onChange doesn't fire on first render on v1 while it does on legacy v2 (WIP PR CustomSelectControlV2: Fix initial value not being set in the select for the V1 legacy adapter #62308, comment) — fix in CustomSelectControl V2: fix setting initial value and reacting to external controlled updates #62733
• Double check any styling issues as flagged in this comment and following conversation (plus check WIP in CustomSelectControlV2: Fix styling issues #62424)
  • CustomSelectControlV2: fix popover styles #62821
  • CustomSelectControlV2: Handle long strings in selected value #62198
  • CustomSelectControlV2: Add root element wrapper #62803
  • CustomSelectControl V2: allow wrapping item hint to new line #62848
  • CustomSelectControlV2: fix select popover content overflow #62844
  • CustomSelectControlV2: tweak item inline padding based on size #62850
  • CustomSelectControlV2: fix item styles #62825
  • CustomSelectControl V2: fix trigger text alignment in RTL languages #62869
  • CustomSelectControl V2 legacy adapter: fix trigger button font size #63131
  • CustomSelectControlV2: collapse checkmark space when unchecked #63229
  • CustomSelectControlV2: keep item checkmark top aligned #63230
• Align keyboard event propagation while focussing select popover (context) CustomSelectControl V2: prevent keyboard event propagation in legacy wrapper #62907
• Check arrow down behaviour CustomSelectControl V2: keep legacy arrow down behavior only for legacy wrapper #62919
• CustomSelectControl V2: fix labelling with a visually hidden label #63137
• Make the V2 legacy wrapper available via private APIs CustomSelectControlV2: expose legacy wrapper through private APIs #62936
• Swap all V1 usages with V2 legacy wrapper (remember to also check and possibly remove unnecessary style overrides)
  • FontSizePicker: use CustomSelectControl V2 legacy adapter #63134
  • Block position controls: use V2 legacy adapter instead of V1 CustomSelectControl #63139
  • DateFormatPicker: use CustomSelectControl V2 legacy adapter #63171
  • FontAppearanceControl: use CustomSelectControl V2 legacy adapter #63179
  • SpacingInputControl: use CustomSelectControl V2 legacy adapter #63190
• Check that there are no unknown style overrides remaining
• CustomSelectControlV2: do not flip popover if legacy adapter #63357
• CustomSelectControl V2 legacy adapter: stabilize experimental props #63248
• CustomSelectControlV2: animate select popover appearance #63343
• CustomSelectControl: switch to ariakit-based implementation #63258
• Prepare dev note

V2

To discuss

• Review API surface:
  • Based on how much we expect V2 default to diverge from V2 legacy wrapper, does it make sense to keep a common underlying implementation, since it comes at the cost of higher complexity in the internal implementation
    • if so, let's see if there is a better way to allow different behavior / styles between the two versions (we currently use React context + passing props to Styled components)
  • Should the components forward refs? If so, should they expose multiple refs? (ie. to the label, the button, and the popover)
  • How to differentiate between props for wrapper vs props for trigger? Should we have a trigger render prop, like we do for DropdownMenuV2 ? (see CustomSelectControlV2: Add root element wrapper #62803 (comment)))
• Popover behavior and APIS:
  • Should the popover be modal ? Regardless, should it always prevent body scroll?
  • Where should the popover render in the DOM ? Always inline? Always portal-ed ? If so, in which slot (see Popover-based UI: do not render in Popover.Slot by default #56482)? Should we expose any props?
    • This should influence / should be influenced by DropdownMenuV2 too
• Consider cloning + splitting V1 and V2 implementations, so that V2 can diverge without affecting V1
• Do we need an isMultiple prop? (especially when neither value nor defaultValue are specified, but also for TS ergonomics)
• Discuss whether it makes sense to tweak the size of each item based on the pointer type (smaller items for fine pointer control, larger items for coarser pointer)
• Discuss best approach to the select popover height (including the maximum and minimum heights) (see Improve CustomSelectControl's usability around max height #49110)
• Discuss best approach to the select popover flip behavior (especially in the light of CustomSelectControl V2 popover renders below position: sticky elements #63180)
• Consider adding new features (like exporting more subcomponents, separators, group and group labels, etc)
  • Including new Select-related components recently added to Ariakit
• Ask design for specs/review
  • Should it have more specific prefix/suffix structure, similarly to the new DropdownMenuItem ?
  • Checkmark/SelectItemCheck
    • should it be optional?
    • should it be instead exported as a subcomponent, and used by consumers when needed?
    • placement:

      Should the design for CustomSelectItem be inspired by the work? Specifically; placing the checkmark icon on the left?

  • review line height (see comment)

To implement

• Remove legacy adapter layer (make two separate exports) CustomSelectControlV2: Remove legacy adapter layer #59420
• Rename to CustomSelectControl and CustomSelectControl.Item CustomSelectControlV2: Rename for consistency #60178
• Should the list of items always have the same width as the closed component? CustomSelect: Adapt component for legacy props #57902
• Add compact size CustomSelect: Adapt component for legacy props #57902
• Focus / hover styles on both the items and the selected value CustomSelectControlV2: Use InputBase for styling #60261
• Try to make the TS types for the single/multiple selection modes be more ergonomic
• Update ariakit to latest version and see new Select components (see changelog)
• See what unit tests can be applied also to the v2 implementation (non legacy)
• Needs a utility padding wrapper like InputControlPrefixWrapper
• Allowing the trigger button to be disabled
• Improve CustomSelectControl's usability around max height #49110
• CustomSelectControl V2 popover renders below position: sticky elements #63180

To check before releasing

• Public-facing docs (Types, Storybook examples, READMEs)
• Export the component as private API, start experimenting in the editor
• Check with accessibility folks on this and for overall feedback

[brookewp]

@andrewhayward @afercia @alexstine

Hey folks! We're looking into improving the CustomSelectControl component. To start, we've created a basic experimental component based on Ariakit's Select.

Before getting too far into this, we wanted to check in with you all to hear your thoughts on this. Do you have any concerns with the existing CustomSelectControl or with Ariakit's Select? How does the new version of the new component feel so far?

---

Note

When testing the new version, the select's popover will appear to be misaligned. You can use the shortcut S to toggle the sidebar or open the story in its own window to see the correct styling.

[afercia]

@brookewp thanks for the ping.

Do you have any concerns with the existing CustomSelectControl

The only concern I have with the existing CustomSelectControl is that it seems to use a wrong ARIA role of listbox. The listbox role is more similar to a HTML select element with size attribute greater than one. Instead, what we want is to emulate a HTML select element with size one.

The Ariakit's Select implementation seems more correct to me, at a first glance. It uses a combobox role which is more appropriate. However, the combobox role has a few variants and several optional behaviors. it's a bit complex to understand. It's important to get it right since the start. Reference with examples in the ARIA Authoring Practices (APG): https://www.w3.org/WAI/ARIA/apg/patterns/combobox/

Some comboboxes allow users to type and edit text in the combobox and others do not.

To emulate a HTML select element
When a combobox does not support text input, it is referred to as select-only, meaning the only way users can set its value is by selecting a value in the popup. For example, in some browsers, an HTML select element with size="1" is presented to assistive technologies as a combobox.

This is the behavior we'd want to use for a HTML select-like component. All the properties to be used and intereaction are detailed in the APG. Example available here: https://www.w3.org/WAI/ARIA/apg/patterns/combobox/examples/combobox-select-only

Input with suggestions
Instead, when a combobox supports text input, it is referred to as editable. An editable combobox may either allow users to input any arbitrary value, or it may restrict its value to a discrete set of allowed values, in which case typing input serves to filter suggestions presented in the popup

This is the pattern we'd want to use for ComboboxControl and FormFieldToken, which is more commonly called 'autocompleter with suggestions'.

Initial testing

Quickly testing with Safari and VoiceOver:

• The default example seems to work okay to me.
• The Multi-Select example dosn't work correctly: the announcement of the selected options is wrong, at least with Safari and VoiceOver.
• The Controlled example: same, the announcement of the selected option is wrong.

[ciampo]

Thank you for looking in to this, @afercia ! And also for sharing some very useful links to the relevant parts of the specs.

The Ariakit's Select implementation seems more correct to me, at a first glance.

Thank you! That's good to know, especially since the new version of the component that Brooke has been working on is indeed based on the ariakit Select component family.

Quickly testing with Safari and VoiceOver:

• The default example seems to work okay to me.
• The Multi-Select example dosn't work correctly: the announcement of the selected options is wrong, at least with Safari and VoiceOver.
• The Controlled example: same, the announcement of the selected option is wrong.

Could you be more specific about what exactly is wrong vs the expected behaviour?

I'm also going to re-open this issue, since it was meant to be a tracking issue for @brookewp's ongoing work on those components.

[afercia]

I'm also going to re-open this issu

Oh sorry I think I closed it not intentionally, somehow.

Could you be more specific about what exactly is wrong vs the expected behaviour?

Well simply testing it with a screen reader clearly shows the bugs so I'm assuming this hasn't been tested properly.

In both the Multi-Select and the Controlled example, the 'selected' announcement is wrong. I think it's because of incorrect usage of aria-selected but I haven't looked into it in depth. I think this originates from some confusion about `aria-selected' as this attribute in some ARIA widgets configurations is meant to indicate the actually selected option while in other cases is meant to indicate the highlighted option for example while navigating with the arrow keys through a list of suggestions. More confuzion may arise when multi-selection comes into plau. The correct usage is well detailed in the ARIA Authoring Practices but it may be source of confusion.
I think it's an ariakit bug and should be fixed upstream. But, so far, we can't use the Multi-Select and the Controlled configurations.

I'm testing with:

• macOS Sonoma 14.1.1
• Safari Version 17.1 (19616.2.9.11.7)
• VoiceOer Version 10 (919.1)

Multi-Select

Initial announcement of the two selected options looks correct:

But, as soon as arrow keys are used to move to the other options, the announcement is wrong: 'flamingo pink' is not selected but it is announced as 'added to selection'.

or, it announces that a selection has bee 'replaces' while it is not true:

And again, moving through the options it announces 'removed from selection' or 'added to selection' when it's not true.

Controlled

This is a bit uncomfortable to test as there are a few containers with CSS overflow property that cut-off the dropdown. I removed those CSS using the dev tools inspector properties for testing purposes.

Again, navigating through the options, it may announce an option as 'selected' while it is not true:

[diegohaz]

Could you be more specific about what exactly is wrong vs the expected behaviour?

Well simply testing it with a screen reader clearly shows the bugs so I'm assuming this hasn't been tested properly.

In both the Multi-Select and the Controlled example, the 'selected' announcement is wrong. I think it's because of incorrect usage of aria-selected but I haven't looked into it in depth. I think this originates from some confusion about `aria-selected' as this attribute in some ARIA widgets configurations is meant to indicate the actually selected option while in other cases is meant to indicate the highlighted option for example while navigating with the arrow keys through a list of suggestions. More confuzion may arise when multi-selection comes into plau. The correct usage is well detailed in the ARIA Authoring Practices but it may be source of confusion. I think it's an ariakit bug and should be fixed upstream. But, so far, we can't use the Multi-Select and the Controlled configurations.

@afercia, thanks for testing. There's a bug in WebKit/Safari. I can reproduce the same behavior on the APG Multi-Select Listbox example.

Safari has known issues with aria-activedescendant. It appears the issue is being addressed for combobox widgets in the upcoming version. So, we can expect improvements in the near future.

For now, the aria-activedescendant can be disabled for the Ariakit Select component by setting the virtualFocus prop to false. This will enable the use of the roving tabindex approach, which seems to function well on Safari.