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

Accessibility updates #264 - Add aria attributes to better support screen readers and keyboard navigation #2596

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

cooperfellows
Copy link

Summary

This adds a good chunk of ARIA attributes and other related attributes to the chosen elements to move towards making Chosen more accessible.

ARIA Labels and Other Helpful Attributes

Managing State

  • Managing the aria-expanded attribute
    • In order to indicate when the search results should be relevant to assitive technology, we need to toggle the aria-expanded attribute as fields are activated/deactivated.
    • The easiest way to do this (AFAIK) is to adjust the attribute during the close_field and activate_field methods.
    • A simple switch from true to false or false to true in each of these methods is sufficient enough to keep the state updated

A complete description of the changes made, with the reasoning behind each attribute is available here:

References

@tjschuck
Copy link
Member

Thanks @cooperfellows!

Just linking a couple more related PRs: #2013 and #2047

@cooperfellows
Copy link
Author

cooperfellows commented Apr 27, 2016

@tjschuck Happy to help, but I think there is a bug in the chosen.prototype.coffee. The build process flagged in in the detailed output, but claims it passed anyway. Let me take a look at it and get back to you.

TypeError: 'undefined' is not a function (evaluating 'this.search_field.attr("aria-expanded", true)') at

TypeError: 'undefined' is not a function (evaluating 'this.result_highlight.attr("id")') at

@stof
Copy link
Collaborator

stof commented Apr 27, 2016

.attr is a jquery method. Using it in the prototype version is wrong.

@stof
Copy link
Collaborator

stof commented Apr 27, 2016

btw, you used the proper Prototype methods in other places dealing with attributes, so this looks weird to me.

@cooperfellows
Copy link
Author

Yea, I'm not a Prototype guy, I am seeing some other issues with the attribute values. It looks like I should be passing string "true" and string "false" not the boolean. At least according to:

Working on an update now.

@cooperfellows
Copy link
Author

I'm sorry this got so ugly. I was trying to finish this up before I take off for a week so I've been rushing things a bit more than I would like to. I missed one other .attr call which I've now corrected.

Let me know what else you want from my end, happy to help when I can. Thanks for the response and the pointers.

@miconley
Copy link

@pfiller Have you and your team taken a look? My team prefers to use this library but cannot until it's A11y-ready. Thanks!

@latagore latagore mentioned this pull request Dec 22, 2016
@arvisto
Copy link

arvisto commented Dec 4, 2017

Hi Cooperfellows, could you give an update on your plans for the future of this pull request?

@cooperfellows
Copy link
Author

Hi @arvisto I haven't been keeping this up to date with any other changes in Chosen. I have been using a version with all the changes I made in this PR in several of our production applications without any known issues.

To be clear though, not hearing about any issues does not mean there are none. Unfortunately I am too busy to make any updates to this for at least the next 6 months (some major projects at work). So for now this will most likely remain stagnant.

@arvisto
Copy link

arvisto commented Dec 15, 2017

@cooperfellows Thank you for the update. Hopefully these changes get merged in at some point, chosen would benefit greatly from adhering to accessibility standards, as most Canadian companies that have anything to do with the government have to adhere by law. But I think I'm preaching to the choir.

@ckpicker
Copy link

@cooperfellows Nicely done! Any idea when this is going to be merged? I would love to start using this asap.

Changes inspired by
kirstenmalin@4d0da63

Use the label element, aria-label, or aria-labelledby attributes from
the select box for the text input field.
Add attributes and ids such that a screen reader can know which element
in the search results is highlighted so that it can be read:
Attributes on the text input:
aria-haspopup="true"
aria-expanded="true" (needs to be changed dynamically as the field is
activated/deactivated)
role="combobox"
aria-autocomplete="list"
aria-owns="id_of_list_of_options"
aria-activedescendant="id_of_highlighted_option" (change this value
programatically when the highlighted option changes)
aria-labeledby="id_of_field_label"
Attributes on the list of options:
id for use in the aria-owns attribute on the input
Attributes on each option:
role="option"
id for use in the aria-activedescendant attribute on the input
Changes inspired by
kirstenmalin@4d0da63

Use the label element, aria-label, or aria-labelledby attributes from
the select box for the text input field.
Add attributes and ids such that a screen reader can know which element
in the search results is highlighted so that it can be read:
Attributes on the text input:
aria-haspopup="true"
aria-expanded="false" (needs to be changed dynamically as the field is
activated/deactivated)
role="combobox"
aria-autocomplete="list"
aria-owns="id_of_list_of_options"
aria-activedescendant="id_of_highlighted_option" (change this value
programatically when the highlighted option changes)
aria-labeledby="id_of_field_label"
Attributes on the list of options:
id for use in the aria-owns attribute on the input
Attributes on each option:
role="option"
id for use in the aria-activedescendant attribute on the input
This is simply a pass through to the winnow_results function. There is
an A11Y reason behind this, when a page loads, and chosen is activated,
the results list is empty, so the aria labels are pointing to an empty
list, which is flagged as an error in many web scanners. As soon as the
results list is built (the first time the field is activated) the
scanners stop complaining.

The reason I chose to add it as a trigger, is for dynamic select lists.
Normally, chosen:updated would be used when options are inserted, the
problem is that you can't use chosen:updated until the results list has
been built once, because it doesn't actually generate the results list.

I have decided NOT to force a build on initialization to avoid adding
overhead for those who don't need the A11Y work. Instead, the trigger
needs to be called in your script wherever Chosen is initialized.

If there is a better way to build the options list than this, please let
me know, I am admittedly new to the Chosen source code.
Default to busy since there are no results until the field is activated.
Upon activation of the field, set aria-busy to false.
Also add missing role="listbox" to chosen-results element
Since we are using aria-busy, there is no reason to build out the
results early like before.
Was not hitting the right element in previous commit.
Fix incorrect syntax in prototype.coffee file when writing attribute
values.  (.writeAtribute, not .attr)

According to prototype source
(https://github.com/sstephenson/prototype/blob/5fddd3e/src/prototype/dom/dom.js#L2385)
passing boolean false will remove the attribute, and passing boolean
true will set the value of the attribute to the attribute name, not the
value. So pass string "true" and string "false" instead.
Needed to fix prototype readAttribute call. Too used to jQuery...
Not all browsers are implementing this yet
@cooperfellows cooperfellows changed the title A11y updates #264 Accessibility updates #264 - Add aria attributes to better support screen readers and keyboard navigation Jul 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants