Try adding a keyboard-navigable font size selector with visual feedback

[youknowriad]

Very early exploration based on one of the codepen examples here #16473

[enriquesanchez]

I just ran a test with VoiceOver + Safari (Mac):

• The component is announced as "Custo, Font Size, Menu pop-up combo box, Font Size, Font Size, group". I think this could be improved if we avoid repeating "Font Size". Also, I'm not sure why it announces it as "Custo" instead of "Custom". But I've seen this happen before in other cases and think it's a VoiceOver issue and not specific to this component.

• The component is initialized with "Custom" as default. If I'm not mistaken, we should initialize it with "Normal" and display that as the current state.

• When selecting an option other than "Custom", the new state is not reflected in the component. "Custom" stays visible all the time.

• After selecting an option from the dropdown, my keyboard focus is lost and I seem to be taken back to the start of the HTML document, making it really hard to go back to where I was.

[tellthemachines]

In addition to @enriquesanchez's comments above, a quick test on Windows 10 with Firefox + NVDA:

• Control is announced as "font size combobox collapsed", as opposed to the native select which is announced as " choose preset combobox normal collapsed". The "font size" bit is the button aria label, so that's easy to change back to "choose preset", and then we'll get less of the repetition Enrique mentioned. (The first "font size" in his tests would be coming from the fieldset legend.)
  We'll still need to find a way to announce the currently selected option though.
• Tabbing into the control and then pressing arrow up or arrow down is setting the button's aria-expanded to true but doesn't allow further interaction with the control. We probably don't want that to happen.
• This one is not happening all the time, but I've managed to reproduce it in Safari + VO too: after selecting an option with either space or enter, the dropdown doesn't close, and the only way to exit it is to select another option (it usually closes by itself as soon as the second option is selected).

I haven't really looked into the code yet so might have further feedback in the next few days 🙂

[tellthemachines]

Update on this: I have removed the combobox role (and associated aria-expanded state) as it is causing VoiceOver to read out the button text incorrectly. For some reason VO cuts off the last letter of the text when a button is assigned the combobox role. Here's a codepen to showcase the issue.
Not sure it's worth reporting though; the spec tells us that a combobox must contain a text input as well as a listbox, so assigning the role to a button element should be invalid.

VoiceOver is now behaving a little better: "Choose preset. Popup button. Font size. Font size. Group." (It only repeats the "Font size" bit when entering the fieldset)

NVDA is still not announcing the current value of the button; it now reads out "Choose preset button submenu".

Would love some further feedback on this.

NOTE: I haven't touched the focus management issues yet so keyboard interaction is still pretty broken. Will work on that next.

[enriquesanchez]

Thanks @tellthemachines! This is looking so much better already.

I tested your latest updates on both NVDA (Firefox) and VO (Safari).

On NVDA + Firefox:

• Component is announced as "Font Size grouping, Font Size combo box collapsed".
• If I hit up/down arrows I hear "expanded", as expected. However, the list does not appear. I need to hit enter for it to open.
• Upon entering the list of options I hear: "Choose Font Size list".

I wasn't able to focus on any of the available options with keyboard only, but I assume this is the part that you mentioned still needs some work.

On VO + Safari:

• Component is announced as "Choose Preset, pop up button, Font Size, Font Size, group"
• Hitting enter opens the list and announces "Choose Font Size, group, main"
• I was able to interact with the options on the list and select one. However once an option is selected, the selection is not announced.

[tellthemachines]

I think I've addressed most of the issues with focus management and labelling of the custom select control.
One side effect of trying to reproduce native select behaviour on Windows is that now you can also pick options with the arrow keys on Mac 😁
Feedback and testing with miscellaneous AT welcome!

[talldan]

It'd be interesting to know the reasoning behind removing this code. I'm not sure what it does, but I think it terms of documenting its removal some details in the PR description would be great.

[youknowriad]

Yes, this seems dangerous for me. This means if we close a modal/popover programmatically (not by focusing something else explicitly) and the focus is outside of that modal/popover when we do so, the focus will return to the button that opened the popover) which IMO is not what we want right?

[tellthemachines]

I'm not sure what it does, but I think it terms of documenting its removal some details in the PR description would be great.

What it currently does is return if no element within the wrapped component is focused, so that focus goes back to the document body. I can't think of any situation where we would want this to happen, especially when using a component the sole purpose of which is to return focus to the previously focused element when closing a modal or popover. So I'm particularly curious to know why it was added in the first place. I know it's part of this changeset but I can't reproduce the issue it fixes after my changes.

Note that this return doesn't get triggered when we deliberately move focus to a specific element, such as when we use the Block Navigator. It only gets triggered when focus is for some reason lost while we are still inside the wrapped component. This is something that, for the sake of accessibility, should never happen, so I'm not sure that we should be catering to it.

Removing this code fixes the issue of focus being lost when closing the font size dropdown because in the focusActiveItem function focus is blurred from the dropdown just before closing. I'm not sure why we're managing focus with this function though; I thought NavigableMenu would be enough to provide keyboard navigation with the arrow keys? Perhaps @youknowriad could shed some light on this.

Happy to hear further feedback and suggestions!

[gziolo]

I think this check exists to address the issues that there might be several nested components that are wrapped with withFocusReturn and all of them can respond when they get unmounted. I suspect, this is a way to ensure that it's handled only once in such a case. This should be confirmed though.

[tellthemachines]

I think this check exists to address the issues that there might be several nested components that are wrapped with withFocusReturn and all of them can respond when they get unmounted.

Hmmm I don't think that's what it does, because if we have nested withFocusReturns (such as this font size picker nested in the sidebar) the parent keeps tracking focus even when it's inside the child component, so I wouldn't expect focus to be lost in the parent unless it were also lost in the child.

We don't seem to have any instance of a parent popover/withFocusReturn-wrapped component that closes when a child popover is closed (that's probably an accessibility anti-pattern anyway), but I have verified that, with the current changes, when opening the font size picker and then closing the sidebar with it still open, focus is correctly transferred to the open sidebar button.

[enriquesanchez]

Hi @tellthemachines! 👋

I did another round of testing today. Here are my results:

Test with Dragon 15 (Windows + Firefox):
I was not able to interact with the component. It looks like that's because the visible label "Preset Size" is not explicitly assigned to the menu button:

<label class="components-font-size-picker__select-label">
	Preset Size
	<button type="button" aria-haspopup="listbox" class="components-button components-font-size-picker__select-selector is-button is-default is-large">Normal</button>
</label>

I tried assigning the label explicitly:

<label class="components-font-size-picker__select-label" for="button_id">
	Preset Size
	<button type="button" id="button_id" aria-haspopup="listbox" class="components-button components-font-size-picker__select-selector is-button is-default is-large">Regular</button>
</label>

..and I still couldn't interact with it.

I then tried also adding an aria-label to the button:

<label class="components-font-size-picker__select-label" for="button_id">
	Preset Size
	<button type="button" id="button_id" aria-haspopup="listbox" aria-label="Preset Size" class="components-button components-font-size-picker__select-selector is-button is-default is-large">Large</button>
</label>

... and it worked! 🎉 🎉 🎉

What do you think about making this change?

Test with NVDA (Windows + Firefox):

When focusing on the element, NVDA announces:

Font Size grouping
clickable
Preset Size Large ▼
button subMenu

When interacting with it with keyboard, I wasn't able to just use up or down arrows to select between options, clicking down takes me to the next input field instead. I had to click Enter to open the list. This is what NVDA announced then:

Choose Font Size list
Regular (Selected) 2 of 5
Large not selected 3 of 5
Custom not selected 5 of 5

It properly indicated number of options, which one was selected and which one I had just selected 👍

The popover stayed open after selection, I was expecting for it to close. I had to click esc twice for it to close.

I ran another test with the small changes I proposed previously for Dragon (explicit label and aria-label added). NVDA announced the component as:

Font Size grouping
clickable
Preset Size
button subMenu

And when interacting with it:

Choose Font Size list
Large (Selected) 3 of 5
Regular not selected 2 of 5
Regular (Selected)
selected

It still announced items and selection to my expectations.

Test with VoiceOver (Mac + Safari):

VO announces the component as:

Normal ▼
Preset Size
popup button
Font Size
Font Size
group

When interacting with it:

Using up and down to select an option without opening the listfelt a bit buggy. I could see that the selection was changing but the focus was moving to different parts of the UI.

I opened the list with Enter:

Choose font size
group
main

...and was able to select an option from the list. VO properly announced the number of available options and which one was selected.

Normal (Selected), 2 of 5
Large, 3 of 5
Huge, 4 of 5

When selecting an option, VO announced:

Huge ▼
Preset Size
popup button
Font Size
Font Size

I'm not sure why "Font Size" is being repeated twice. This did not happen with NVDA.

[tellthemachines]

Thanks for testing everything so thoroughly @enriquesanchez !

Re adding an aria-label to the button, sure, let's do that 😄

The up/down arrow navigation issue with NVDA is a tricky one. What's happening is NVDA is not going into forms mode when entering the component, so the arrow keys are working as expected in browse mode. However, triggering forms mode by pressing Insert + Space causes the arrow keys to navigate the options but NVDA doesn't read them out, so that's not terribly good either.

I'm wondering if we should stop pretending that this is a select and abandon the arrow navigation altogether. The other issue with it is that I can't think of how to make it work on Windows only, and not on any of the Mac browsers.

I can reproduce the issue with the popover staying open after an option is selected, on Firefox with and without NVDA and from time to time on Safari, but I haven't found the cause yet. Will investigate further.

VoiceOver repeating the fieldset legend (the "Font Size" bit) is IIRC a bug on their side, and I think it's already been reported to them.

[enriquesanchez]

I'm wondering if we should stop pretending that this is a select and abandon the arrow navigation altogether.

@tellthemachines I agree 100% with this. From previous attempts and testing, I think it's clear that getting full parity with a native select is just not going to happen.

I think we can view this from the lens of a menu button (like the ones we already have on the block toolbar). From what I've gathered, the trick with the menu button is how to display the current state of the button (Regular, Large, etc.) while also having an obvious accessible name that users of speech recognition software could still interact with. This is where I think having a visible label ("Preset Size") and assigning that same label to the button (aria-label = "Preset Size") could work, or at least, it worked on my previous test.

With Dragon, if I say "click Preset Size" I should be able to open the menu button, even though the actual real label of the button is "Normal" (from <button>Normal</button>).

Not sure if you've come across this article: https://inclusive-components.design/menus-menu-buttons/ . In particular, there's a couple of sections called 'The "choose" event' and 'Persisting choices' (almost at the end of the article) that have examples very similar to what we want here.

[tellthemachines]

Not sure if you've come across this article: https://inclusive-components.design/menus-menu-buttons/ . In particular, there's a couple of sections called 'The "choose" event' and 'Persisting choices' (almost at the end of the article) that have examples very similar to what we want here.

@enriquesanchez I tried implementing the menu-menuitemradio pattern described in that article and it messes up our current focus handling with the arrow keys completely, on both NVDA and VoiceOver.

I have removed arrow key navigation when the dropdown is closed and added the aria-label to the button, but still trying to figure out why the dropdown doesn't always close on selection with Firefox and Safari.

I will probably not be able to work on this for the next few days. The PR is updated with my latest changes if anyone wants to pick it up in the meantime.

Note: some of the e2e tests are failing because they're still expecting the old <select> markup.

[gziolo]

As discussed on WordPress Slack, this one should be closed in favor of #17926. /cc @youknowriad