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

Replace "chosen" script with a more accessible option #7722

Closed
robincornett opened this issue Apr 8, 2020 · 8 comments · Fixed by #8067
Closed

Replace "chosen" script with a more accessible option #7722

robincornett opened this issue Apr 8, 2020 · 8 comments · Fixed by #8067

Comments

@robincornett
Copy link
Contributor

Enhancement Request

...we should look at deprecating Chosen (which itself has been Deprecated https://github.com/harvesthq/chosen) and replacing it with a more maintained alternative, such as:

Related #2758 #4375

Originally posted by @spencerfinnell in #7583 (comment)

Justification or use case

Any select option using Chosen is currently not accessible to keyboard or screen reader users.

@spencerfinnell
Copy link
Contributor

Noting that @robincornett opened this issue as a place to discuss alternatives/replacements for Chosen and to keep #7583 as a tracking issue for removing existing unneeded usage.

@cklosowski
Copy link
Contributor

Is Select2 an option? It seems to still be maintained.

@robincornett
Copy link
Contributor Author

selectWoo is a select2 fork with accessibility related improvements and fixes so is probably a better option.

@cklosowski
Copy link
Contributor

I find it interesting that they chose to fork instead of contribute upstream. Maybe the Select2 team isn’t open to their suggestions.

@JJJ
Copy link
Contributor

JJJ commented Jul 23, 2020

I spent last night forking Chosen, here: https://github.com/JJJ/chosen

  • 25 pull requests merged, out of 55 (including Aria & Role support!)
  • Oldest pull request was 8 years old (yikes!)
  • It's fully accessible (probably!)
  • It's working (tests are failing tho!)
  • Styling modernized (kinda!)
  • Old stuff killed (ie8, Bower, weird build process, etc!)
  • New stuff we can do in the future (webpack, React!)

Please consider giving my fork a try. It is drop-in replacement for what is in EDD already, and I'd love some help getting the tests to pass 🤗

I find it interesting that they chose to fork instead of contribute upstream...

I chose(ha!) to fork Chosen because it was tagged as deprecated and unmaintained by the folks at Harvest. It sat for a few years with tons of comments and PRS, but nobody helping them.

robincornett added a commit that referenced this issue Jul 23, 2020
robincornett added a commit that referenced this issue Jul 23, 2020
@robincornett robincornett linked a pull request Jul 23, 2020 that will close this issue
robincornett added a commit that referenced this issue Jul 24, 2020
Pulled from Sugar Calendar; note that
.chosen-container-active.chosen-dropup styles still need to be handled.

#7722
robincornett added a commit that referenced this issue Jul 27, 2020
robincornett added a commit that referenced this issue Jul 27, 2020
@robincornett robincornett added this to the 3.0 milestone Jul 28, 2020
robincornett added a commit that referenced this issue Jul 28, 2020
robincornett added a commit that referenced this issue Jul 28, 2020
robincornett added a commit that referenced this issue Jul 29, 2020
robincornett added a commit that referenced this issue Aug 25, 2020
Pulled from Sugar Calendar (props @JJJ). This does not update the other
color schemes in the edd-admin-css and datepicker files.

#7722
ashleyfae added a commit that referenced this issue Sep 3, 2020
@spencerfinnell
Copy link
Contributor

I'm not sure we should close this out completely until Chosen adds mobile support: JJJ/chosen#33

Currently searching for Downloads (or anything dynamic) does not work on mobile.

@robincornett
Copy link
Contributor Author

Would it be worth opening a new issue, @spencerfinnell? I saw that John updated Chosen to 2.2.0 and what we've put into EDD 3.0 is 2.1.0, so an update would be needed anyway. Also, based on your comments there, this is a current issue with 2.9.x, yes?

@spencerfinnell
Copy link
Contributor

@robincornett Sure -- probably a good idea to track it separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants