Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Accessibility #236

Closed
ericgio opened this issue Sep 29, 2017 · 16 comments
Closed

Improve Accessibility #236

ericgio opened this issue Sep 29, 2017 · 16 comments
Labels

Comments

@ericgio
Copy link
Owner

ericgio commented Sep 29, 2017

Feedback from @RichCaloggero:

General

  • Add live region which will receive messages about number of matches
    • add div containing aria-live="{count} matches" somewhere on the page
    • move it off-screen to hide it from sighted users
    • Update on every character, but only if a minimum amount of time has elapsed since previous update (about 0.33 seconds to 0.5 seconds seems like a reasonable minimum interval)

Single-select

  • Hide hint input with aria-hidden since it shows up as a second input field which is confusing. It isn't in the tab order, but screen reader users can still see it.

Multi-select

  • Allow arrowing into the list as with the single select case
  • Clicking a list item removes it from the list and places it before the input control
    • should either place it's string value in the input field or use aria-describedby or aria-labelledby to link the chosen results with the input field

Tokens

  • Make delete button focusable so token can be deleted via keyboard
  • Better label ("delete" instead of "times")
@RichCaloggero
Copy link

Update on every character, but only if a minimum amount of time has elapsed since previous update

My mistake. This would be what you want if you were doing list filtering (i.e. type a string and narrow down the list as you type). For strictly character-by-character autocomplete, I don't think the time limit is necessary. You do want to provide a count of matches which updates on every character however. See this widget for an example of what I mean (try it with a screen reader such as VoiceOver which can be found on every mac):
http://www.jqueryui.com/autocomplete

should either place it's string value in the input field or use aria-describedby or aria-labelledby to link the chosen results with the input field

This is a bit hard to describe unless you know what it is like to use a screen reader.

Firstly, the screen reader user only "sees" exactly one thing at any given time, i.e. the thing just navigated to. This means that if one chooses a value from this widget in multiselect mode:

  • user navigates to the list of matches in some way
  • navigates the list item by item until one is found
  • press enter on that selection
  • repeat until all desired values are selected
  • presses shift+tab and sees that the input field remains empty

Where are the chosen results? Turns out they are placed before the input field. The user will probably eventually find them, but it's certainly not an intuitive setup. The sighted user takes one glance on the page and sees them immediately!

One possible solution would be to make a list of checkboxes and allow the user to check the ones she wants to select.

Another idea might be to consider all matches to the typeahead selected (probably not expected or desired behavior).

You could place the selected results in a native select list and move it off-screen. Sighted users could use the method you currently use, and screen reader users would see the results in a native select list, which is easily navigated. If you take this approach, hide the current results panel via aria-hidden="true" so it becomes invisible to screen reader users, and add tabindex="-1" to all focussable items in this panel so that screen reader users won't be able to mistakenly tab to them.

@daniel-z
Copy link

daniel-z commented Oct 4, 2017

Based in WCAG 2.0 Level A,
In case that we don't want to include a label for the input we should add a title attribute to them.

I think that's a feature to add to this as well, I had to make a workaround on this to be able to meet the standard.

@ericgio
Copy link
Owner Author

ericgio commented Oct 19, 2017

@daniel-z: Arbitrary attributes such as title can be added to the input as of v2.0.0-alpha.1 using the inputProps prop.

@ericgio
Copy link
Owner Author

ericgio commented Oct 23, 2017

@RichCaloggero: As of v2.0.0-alpha.6 I've made the following accessibility enhancements:

  • Add aria-hidden attribute to input hint
  • Make both the input and token clear buttons keyboard-accessible
  • Add better labels via aria-label
  • Add aria-live field that:
    • Lists the number of results if the menu is open
    • Lists the highlighted value when keying through the menu
    • Lists the number of selections after a selection is made

I think this mostly addresses the issues you raised, and I've tried testing as best I can using VoiceOver. However, I'd love your feedback as to whether the changes satisfy what you had in mind.

@RichCaloggero
Copy link

This is good. However, a few adjustments:

  • add aria-atomic="true" to the element with aria-live
  • do not write currently highlighted menu item to aria-live region (highlighted item is already announced by screen reader)
  • perhaps instead of separate remove buttons for each selection in multiselect mode, give each selected item tabindex="0" which allows them to take keyboard focus, and remove when clicked using generic "click" event which also fires on enter key

Aside from the comments above, I'm also hearing some very strange speech -- choppy, as if pieces of messages are written, then output pauses, then more bits of the message are written. I can't describe it... You need to try with NVDA+Firefox on windows to hear it.
I think not echoing the highlighted item to the live region may fix this issue; try that and I'll retest.

@ericgio
Copy link
Owner Author

ericgio commented Oct 30, 2017

@RichCaloggero: v2.0.0-rc.1 incorporates your feedback:

  • add aria-atomic="true" to the element with aria-live

Done.

  • do not write currently highlighted menu item to aria-live region (highlighted item is already announced by screen reader)

Done. When I tested using VoiceOver, I was no longer hearing the highlighted item announced, so I'm assuming I either don't have my accessibility settings configured correctly, or more advanced setups do this. Trusting your feedback here.

  • perhaps instead of separate remove buttons for each selection in multiselect mode, give each selected item tabindex="0" which allows them to take keyboard focus, and remove when clicked using generic "click" event which also fires on enter key

I removed the ability to focus the individual remove button. The user can focus each selection individually and just hit "delete" to remove the selection.

@ericgio ericgio changed the title Accessibility Issues Improve Accessibility Oct 30, 2017
@RichCaloggero
Copy link

Focus

If I navigate away from the input field and past the list, then attempt to navigate back into the list from the bottom end, my focus is grabbed and sent back to the input element. Do not do this; the user may want to navigate manually or perhaps reread the screen reader message which appears following the list.

Single select

This seems to work well now.

Multiselect

This should work in a similar way to the single select, i.e.:

  • type "m" into input,
  • press downArrow to begin navigating list,
  • press some key (usually some version of space or enter) to toggle selection on current element,
  • continue navigating or accept selection.

However, typing into the input and pressing down / up arrow does nothing; screen reader simply rereads the input field's contents.
The live region does trigger telling me the number of matches.

I can navigate manually to the list and click a button (list item). When I do, the live region triggers and focus is bounced back to the input field. Now I have to manually find the list again and find another selection...

Perhaps you could make a somewhat different keyboard interface for multiselect:

  • allow navigating the list as in single select case
  • toggle selection with spacebar
  • press enter to accept and exit widget

You could still provide the selected elements above / before the input as you are doing now, but the keyboard user would not have to use this to manage selection; can be done directly from the list as described above.

To see keyboard behavior, go here (not pretty looking, but exhibits the correct behavior):
https://richcaloggero.github.io/autocomplete/autocomplete.html

  • check "multiselect" checkbox
  • type "c" into the input field
  • press downArrow to navigate
  • press spacebar to toggle an option
  • press enter to exit

@ericgio
Copy link
Owner Author

ericgio commented Oct 30, 2017

Thanks for the in-depth comments.

Just so we're on the same page, were you testing the package directly or the examples? It turns out the 2.0.0-rc.1 package may be stale (separate issue I'm investigating), but the latest 2.0 example should have all the changes: http://ericgio.github.io/react-bootstrap-typeahead/2.0/.

If I navigate away from the input field and past the list, then attempt to navigate back into the list from the bottom end, my focus is grabbed and sent back to the input element. Do not do this; the user may want to navigate manually or perhaps reread the screen reader message which appears following the list.

I don't understand this comment. My interpretation is that the user tabs past the input, then backward-tabs into the input. What would the expected behavior be there? Note that the menu is not intended to be focusable; if you tab out of the input, the menu closes and focus moves on.

However, typing into the input and pressing down / up arrow does nothing; screen reader simply rereads the input field's contents.

I think I just need to populate the input with the value of the selection here as I'm doing in the single-select case.

I can navigate manually to the list and click a button (list item). When I do, the live region triggers and focus is bounced back to the input field. Now I have to manually find the list again and find another selection

Yeah, that's just how the component works. The menu closes when a selection is made and the input remains focused. Typing more characters re-opens the menu with newly filtered results. I would consider this a general usability thing and not specifically related to accessibility.

@bamab
Copy link

bamab commented Feb 9, 2018

I am on version 2.3.0 I still see no way to add aria-label text. I just started using this component but am having push-back because my app is failing 508 scans. Is an aria-label prop coming in the foreseeable future? Thanks.

@ericgio
Copy link
Owner Author

ericgio commented Feb 11, 2018

@bamab: What part of the component (ie: which underlying HTML element) are you trying to add an aria-label to?

@ericgio ericgio added the a11y label Feb 11, 2018
@bamab
Copy link

bamab commented Feb 12, 2018

The input element. When I run Wave on the page I get "A form control does not have a corresponding label" error.

label-error

@ericgio
Copy link
Owner Author

ericgio commented Feb 12, 2018

@bamab: You can use inputProps to do that:

<Typeahead
  inputProps={{'aria-label': 'my-label'}}
  options={options}
/>

@bamab
Copy link

bamab commented Feb 12, 2018

Thanks, that works great. One other question, is there a way to apply the aria-label to the inner div input - the hint (class="rbt-input-hint")? This may not be doable but I thought I'd ask (because someone asked me...). If it cannot be done, I think I can get this passed as a false positive.

@bamab
Copy link

bamab commented Feb 21, 2018

One other question, is there a way to apply the aria-label to the inner div input - the hint (class="rbt-input-hint")? This may not be doable but I thought I'd ask (because someone asked me...).

Can this be accomplished?

@ericgio
Copy link
Owner Author

ericgio commented Feb 21, 2018

@bamab: No, not possible. That input has aria-hidden="true" though; is that not enough?

@ericgio
Copy link
Owner Author

ericgio commented Mar 19, 2018

v2.6.0 includes some major improvements to a11y. I believe I've more or less brought the component inline with the combobox authoring practices. I've also fixed the biggest outstanding issue described in this thread, which is that the menu options weren't being announced by screenreaders.

I'm going to consider this issue closed. Please open a new issue for any a11y problems you encounter.

@ericgio ericgio closed this as completed Mar 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants