WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
9.99k stars 4.01k forks source link

CustomSelectControl: refactor with ariakit #55023

Open brookewp opened 9 months ago

brookewp commented 9 months ago

Context

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

Legacy: https://github.com/WordPress/gutenberg/pull/55234 New version: https://github.com/WordPress/gutenberg/pull/55790

Initial steps

V2 Legacy Wrapper

Completed in #57902

V2

To discuss

To implement

To check before releasing

brookewp commented 7 months ago

@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 commented 7 months ago

@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:

ciampo commented 7 months ago

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 commented 7 months ago

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:

Multi-Select

Initial announcement of the two selected options looks correct:

Screenshot 2023-11-30 at 09 59 09

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'.

Screenshot 2023-11-30 at 09 59 25

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

Screenshot 2023-11-30 at 10 26 04

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:

Screenshot 2023-11-30 at 10 04 08

diegohaz commented 7 months ago

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.

ciampo commented 7 months ago

Thank you Diego for the reply!

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.

Do you suggest, for now, always setting virtualFocus: false to provide a better, more consistent experience across browsers?

alexstine commented 7 months ago

The Safari bug is a really bad one. It also affected the block autocompleter.

https://github.com/WordPress/gutenberg/pull/54902

Safari needs to keep support for ARIA attributes, that's the least Apple could do.

afercia commented 7 months ago

Safari has known issues with aria-activedescendant.

Yes that's a known problem but I don't think it's the only one.

Latest Safari version seems to made things even worse. However, we already went through listboxes, comboboxes, autotompleters, aria-activedescendant, and 'aria-selected` a few times. For example, we had an issue with the UrlInput in https://github.com/WordPress/gutenberg/issues/47147 and we fixed it in https://github.com/WordPress/gutenberg/pull/47148. At that time, the UrlInput worked with Safari and VoiceOver. Instead, now it doesn't work. Other autocompleters don't work with Safari / VoiceOver, for example the Command center, see https://github.com/WordPress/gutenberg/pull/52930. Although the combobox with autocompleter is a different pattern than a Listbox, I suspect the root problem is the same. These ARIA patterns are extremely delicate and even a small detail could make a difference. Even the markup structure, like unexpected wrappers could have an impact.

Still, it's important to note that the 'Add new tag' combobox with suggestions does work with Safari / Voiceover. It does use aria-activedescendant, it does use aria-selected, and it uses the old ARIA 1.0 pattern with arira-owns instead of aria-controls. It does not use the speak workaround implemented in https://github.com/WordPress/gutenberg/pull/54902. It just works out of the box.

Since this combobox + listbox does work, I'd suggest to investigate in depth why it works and why the other ones don't. There must be something that we are missing (and I couldn't find it out so far).

Screenshot of the Tag suggestions working with Safari / Voiceover:

Screenshot 2023-12-01 at 10 12 11

Tested with: macOS Sonoma 14.1.1 Safari Version 17.1 (19616.2.9.11.7) VoiceOver Version 10 (919.1)

ciampo commented 7 months ago

it's important to note that the 'Add new tag' combobox with suggestions does work with Safari / Voiceover

That would be the FormTokenField component, which you can see in isolation in Storybook

alexstine commented 7 months ago

I believe it works likely because the controlled descendant appears directly after the input in the DOM.

afercia commented 7 months ago

I believe it works likely because the controlled descendant appears directly after the input in the DOM.

Yes it is possible. Also, I seem to recall Safari / VoiceOver don't work at all with options nested within groups.