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

SelectControl should probably use onChange instead of onBlur #2227

Closed
mkaz opened this issue Aug 4, 2017 · 4 comments
Closed

SelectControl should probably use onChange instead of onBlur #2227

mkaz opened this issue Aug 4, 2017 · 4 comments
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Enhancement A suggestion for improvement.

Comments

@mkaz
Copy link
Member

mkaz commented Aug 4, 2017

When implementing SelectControl in Inspector Controls for the Gallery links, I noticed that using the onBlur method requires clicking out of the select being selected, even after a change is made in order to get the Update button to become clickable and save the change.

The onBlur should be switched to onChange which should resolve

@afercia
Copy link
Contributor

afercia commented Aug 5, 2017

Selects behavior is different across operating systems. See related core Trac ticket:

Review the usage of the change event on select elements
https://core.trac.wordpress.org/ticket/40925

And see what happens on Windows when using change on a Select:
https://cloudup.com/iuFxQ7CkA7k

Ideally, all select elements should just use a button.

@afercia afercia added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Jan 18, 2018
@jeffpaul jeffpaul added the [Type] Enhancement A suggestion for improvement. label Jan 18, 2018
@youknowriad
Copy link
Contributor

Added some notes in #2000 explaining why I prefer using onChange over onBlur in the SelectControl component. I'm closing this issue for now.

@afercia
Copy link
Contributor

afercia commented Jan 30, 2018

Worth noting all the a11y recommendations here to not use onChange apply only when the change triggers an unexpected change of context. For example, when only selecting an option from a select would change a view. That wouldn't be acceptable for a11y, for the reasons explained above and also in the jsx-a11y docs. However, I haven't find a similar case in Gutenberg so far.

If I'm missing something, and there are cases of unexpected changes, then the related select shouldn't use onChange.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

5 participants