Page MenuHomePhabricator

ChipInput: implement basic screen reader support
Closed, ResolvedPublic5 Estimated Story Points

Description

Background

The current state of the ChipInput T337095 has some significant accessibility shortcomings:

  1. There is nothing semantically connecting the chips with the input
  2. It may be unclear to screen reader users how to add, remove, or edit a chip

User stories

As a screen-reader user of the ChipInput, I want to perceive the current values of the input and understand what they mean

As a screen-reader user of the ChipInput, I want to be able to easily add, remove, or edit a chip

Previous implementations

  • OOUI: TagMultiselectWidget

Design spec

Open questions

How should we implement this?

There are 2 main ways we could add information about the component that is accessible to screen readers:

  1. Use ARIA attributes and roles. This is what is done in the gmail email recipient UI
  2. Add visually-hidden information about the chips and how to interact with them

Acceptance criteria (or Done)

Design

  • If needed, update the component in the Figma library.

Code

  • Implement the improvements (exact updates TBD)

Event Timeline

CCiufo-WMF triaged this task as Medium priority.Aug 28 2023, 3:35 PM

The gmail UI wraps the list of chips in a div that is a sibling of the input. This div has:

  • role="listbox"
  • aria-multiselectable="true"
  • aria-orientation="horizontal"
  • aria-label="Search Field"
  • aria-activedescendant="ID of the fake-focused chip" (only if a chip is highlighted)

Each chip is then wrapped in a bunch of divs, one of which has:

  • role="option"
  • aria-describedby="ID of the delete button"
  • aria-selected="false" (changes to true when the chip is highlighted using the arrow keys)

The delete button has:

  • aria-hidden="true"
  • aria-label="Press delete to remove this chip"

The input and menu have normal ARIA attributes for an input+menu combo, except that the menu has aria-multiselectable="true", and menu items corresponding to chips that are already in the input have aria-selected="true" and are styled differently.

The chips are not focusable and can't be reached with the tab key. You can only fake-focus them by clicking on them, or by tabbing into the input and pressing the left arrow key. When you do this, the listbox that contains the chips is focused, its aria-activedescendant is set to point to the fake-focused chip, and that chip gets aria-selected="true".

I don't think we should emulate the gmail approach exactly, because I think the chips should be focusable and reachable with the tab key. But maybe we could borrow some aspects from it. How about this:

  • Wrap the chips in another div (we can't use the existing __chips div, because it also contains the input). On this div, set:
    • role="listbox"
    • aria-orientation="horizontal"
    • (rely on a wrapping Field component for labeling)
  • On each of the chips, set:
    • role="option"
    • aria-description="Press Enter to edit this chip, press Delete to remove"

One problem is that getting the internationalized text for "Press Enter to edit this chip, press Delete to remove" would need to be done through a required prop. We already have something like this in ChipInput (the label for the remove button), but this would add another property that the user would have to provide i18n for, and this is the most complex one so far. That might be OK for now, but it makes me want to consider T345386: i18n: Build a system to support common translated strings in components.

@Catrope is that enough to also tie the listbox of options to the input itself (and its label)?

I do think requiring users to provide text for the ARIA description, which should always say the same thing, is a bridge too far - resolving T345386 seems like the best path forward, and a default provided message for the aria-description here shouldn't be controversial (i.e. we shouldn't need it to be recreated for each context, the way other messages should be)

@Catrope is that enough to also tie the listbox of options to the input itself (and its label)?

It turns out I misread the HTML tree in Gmail. The outer listbox div is not a sibling of the input, it's the parent of the input. The input is a combobox which points to the menu, which is also a listbox.

I'll experiment with this structure and see if it produces reasonable screen reader behavior.

Change 954142 had a related patch set uploaded (by Catrope; author: Catrope):

[design/codex@main] [WIP] ChipInput: Add ARIA attributes

https://gerrit.wikimedia.org/r/954142

Testing this with a screen reader in BrowserStack it seems to work OK, and produces a similar experience to Gmail's, except that the chips are in the tab order (which I think is better).

However, I found the following issues:

  • aria-description is a newer ARIA attribute (in ARIA 1.3). For better browser support, we probably need to use aria-describedby instead, which requires creating a visually hidden element that contains the description.
  • The remove button has an aria-label, which is read out as part of the chip value. I think this is confusing since the remove button isn't focusable with Tab (although I think @Volker_E argued it needed a label since people could navigate to it using the arrow keys)
  • I set the description text to "Press Enter to edit this chip, press Delete to remove", but that's pretty long, and the comma causes the screen reader to pause for a long time
  • The combination of role="listbox" + aria-orientation="horizontal"` promises that the left/right arrow keys will move between options, but they don't, because T344848 isn't done yet
  • The input is missing a bunch of ARIA roles relating to it having a menu, because it doesn't have a menu yet (T345291)

Gmail addresses the remove button and description issues with a clever trick: it sets the "Press delete to remove this chip" text as the aria-label on the delete button, and then points the chip's aria-describedby to the delete button. It also sets aria-hidden="true" on this button. It appears the latter is necessary, otherwise the "Remove" label is read out twice.

Here's what the screen reader says when I interact with this prototype:

  • (After tabbing to "chip 1"): [high pitched sound] list [pause] chip 1 remove press enter to edit this chip [pause] press delete to remove 1 of 2
  • (Then tabbing to "chip 2"): chip 2 remove press enter to edit this chip [pause] press delete to remove 2 of 2
  • (Then tabbing to the input): edit blank
  • (Then tabbing out:) [low pitched sound] reset demo button
  • (Then tabbing back into the input:) [high pitched sound] list [pause] edit blank
  • (When pressing enter on "chip 2"): edit chip
  • (When pressing delete on "chip 2"): edit blank
  • (When pressing enter after typing something to create a new chip): (nothing)

Here's what it does for Gmail:

  • (After tabbing into the input:) [high pitched sound] to [pause] search field list [pause] combobox collapsed has autocomplete editable blank
  • (After pressing the left arrow key to highlight the last chip:) bar at wikimedia dot org press delete to remove this chip 2 of 2
  • (After pressing the left arrow key again to highlight the first chip:) foo at wikimedia dot org press delete to remove this chip 1 of 2
  • (After pressing delete:) combobox collapsed has autocomplete editable blank (note: if you press delete on a chip that isn't the last chip, the focus goes to the next chip, not to the input)
  • The remove button has an aria-label, which is read out as part of the chip value. I think this is confusing since the remove button isn't focusable with Tab (although I think @Volker_E argued it needed a label since people could navigate to it using the arrow keys)

Yeah, it really seems like this button needs to be aria-hidden.

  • I set the description text to "Press Enter to edit this chip, press Delete to remove", but that's pretty long, and the comma causes the screen reader to pause for a long time

We can work on the text and ask AFB about this...FWIW, I don't think we should the term "chip" at all. "Press Enter to edit or Delete to remove" might be enough, or we can call the chip an "item" if needed.

I think the length of my suggested sentence is fine...VoiceOver provides long-ish instructions sometimes, so I think this is fairly expected, but we should confirm with AFB.

  • The combination of role="listbox" + aria-orientation="horizontal"` promises that the left/right arrow keys will move between options, but they don't, because T344848 isn't done yet

Probably acceptable for MVP, but another good reason to move on T344848 when we can.

  • The input is missing a bunch of ARIA roles relating to it having a menu, because it doesn't have a menu yet (T345291)

It doesn't need any of them until we implement a menu though, right? At which point the input would need role="combobox"? Or are you talking about additional attributes of the listbox?


I tested your prototype with VoiceOver on Mac and there are a few differences in UX:

  • When separateInput="false", the input is considered an item in the listbox, so for a ChipInput with 2 chips you hear "item 1 of 3" when focused on the first chip and "item 3 of 3" when focused on the input
  • The input is noted as "dimmed"

These seem undesirable, but they are consistent with the output when traversing the gmail UI.

I think with:

  • The button aria-hidden
  • A more concise description for InputChips
  • Tackling the keyboard nav task fairly soon

this is okay to move forward. But I'd also like to know what Volker thinks, and definitely think we should test this with AFB.

  • The remove button has an aria-label, which is read out as part of the chip value. I think this is confusing since the remove button isn't focusable with Tab (although I think @Volker_E argued it needed a label since people could navigate to it using the arrow keys)

Yeah, it really seems like this button needs to be aria-hidden.

Agreed, I'll make that change.

  • I set the description text to "Press Enter to edit this chip, press Delete to remove", but that's pretty long, and the comma causes the screen reader to pause for a long time

We can work on the text and ask AFB about this...FWIW, I don't think we should the term "chip" at all. "Press Enter to edit or Delete to remove" might be enough, or we can call the chip an "item" if needed.

I think the length of my suggested sentence is fine...VoiceOver provides long-ish instructions sometimes, so I think this is fairly expected, but we should confirm with AFB.

Thanks for this suggestion, I'll incorporate this.

  • The combination of role="listbox" + aria-orientation="horizontal"` promises that the left/right arrow keys will move between options, but they don't, because T344848 isn't done yet

Probably acceptable for MVP, but another good reason to move on T344848 when we can.

Agreed.

  • The input is missing a bunch of ARIA roles relating to it having a menu, because it doesn't have a menu yet (T345291)

It doesn't need any of them until we implement a menu though, right? At which point the input would need role="combobox"? Or are you talking about additional attributes of the listbox?

No, that's right, we don't need this until we add a menu. I wasn't talking about anything special, just the usual attributes for inputs that have a menu attached to them (aria-controls, aria-owns, aria-autocomplete, aria-expanded, aria-activedescendant).


I tested your prototype with VoiceOver on Mac and there are a few differences in UX:

  • When separateInput="false", the input is considered an item in the listbox, so for a ChipInput with 2 chips you hear "item 1 of 3" when focused on the first chip and "item 3 of 3" when focused on the input

Interesting; I tested with NVDA on Windows, which didn't consider the input to be an item in the list.

  • The input is noted as "dimmed"

I have no idea what that means or what we are doing that could be causing this.

These seem undesirable, but they are consistent with the output when traversing the gmail UI.

I think with:

  • The button aria-hidden
  • A more concise description for InputChips
  • Tackling the keyboard nav task fairly soon

this is okay to move forward. But I'd also like to know what Volker thinks, and definitely think we should test this with AFB.

I agree with all of that. I'll update my patch, and then that can be the MVP implementation of ChipInput a11y. We can then refine that with Volker's input, and AFB's feedback later.

I have updated the patch to make these changes. To avoid the i18n issue I made the chip ARIA description a prop for now, replacing the removeButtonLabel prop. But we should aim to remove this prop as part of T345386, and if you don't like the new chipAriaDescription prop I can prioritize working on T345386 this week.

Since we only have one known users of this component right away, I think we can inform them of the non-ideal nature of this text both directly and in a comment in the docs (see my feedback on your patch). I would like to prioritize T345386 ASAP though, since it affects other components as well and would be a significant DX improvement.

Assigning to @Volker_E to review the merged solution at some point.

Change 954142 merged by jenkins-bot:

[design/codex@main] ChipInput: Add ARIA attributes

https://gerrit.wikimedia.org/r/954142

Volker_E renamed this task from FilterChipInput: implement basic screen reader support to ChipInput: implement basic screen reader support.Sep 8 2023, 12:52 AM
Volker_E updated the task description. (Show Details)
Volker_E updated the task description. (Show Details)
Volker_E changed the task status from Open to In Progress.Sep 8 2023, 3:20 AM

Change 956973 had a related patch set uploaded (by Eric Gardner; author: Eric Gardner):

[mediawiki/core@master] Update Codex from v0.18.0 to v0.19.0

https://gerrit.wikimedia.org/r/956973

Change 956973 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.18.0 to v0.19.0

https://gerrit.wikimedia.org/r/956973