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

Accessible <select> elem solution #2013

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

Conversation

alademann
Copy link

Proposed solution for Issue 264

Instead of taking the "deep dive" into making the Chosen markup accessible, my proposed solution takes a much simpler approach centered around the principles of progressive enhancement.

Instead of using display: none on the converted <select> element, I have used a utility CSS class that follows accepted best practices for hiding content so that it is accessible to screen readers. In addition to this, I have added an aria-hidden=true to the <div class="chosen-container"> element.

The combination of these two simple changes makes for an accessible solution because:

  • The <select> element will be the control used by screen readers
  • The <div class="chosen-container"> element will be ignored by screen readers

Obviously a fully-accessible listbox solution would be ideal - but IMO, this change will go a long way to making existing Chosen implementations accessible today.

sass/chosen.scss Outdated
@@ -417,3 +417,15 @@ $chosen-sprite-retina: image-url('chosen-sprite@2x.png') !default;
}
}
/* @end */

/* @group Accessibility helpers */
.sr-only {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please prefix the class with chosen- to avoid conflicts with other libraries.

@alademann
Copy link
Author

Good call @stof - done.

@stof
Copy link
Collaborator

stof commented Aug 19, 2014

There is a PR starting to add accessibility to Chosen: #2047

I guess both PRs should not be merged, right ? What do you think about the accessibility PR ?

@alademann
Copy link
Author

@stof if that PR is going in any time soon, then no - it doesn't make sense to merge this one. But if it could take a month or two... why not gain the improvements from this, and simply remove them as part of the changes implemented in #2047

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.

3 participants