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

Fix accessibility of first-time-flow terms checkboxes #7501

Merged
merged 1 commit into from
Nov 23, 2019

Conversation

whymarrh
Copy link
Contributor

See ARIA: checkbox role for context.

This PR fixes one part of the accessibility of our checkboxes in the on-boarding flow. We were previously creating custom checkboxes without the correct assistive properties. In part, this now allows keyboard users to tab to and check the checkbox.

@Gudahtt
Copy link
Member

Gudahtt commented Nov 23, 2019

I wonder - could we instead replace this with an HTML checkbox?

From that MDN article, under "Best Practices":

The first rule of ARIA is: if a native HTML element or attribute has the semantics and behavior you require, use it instead of re-purposing an element and adding an ARIA role, state or property to make it accessible. As such, it is recommended to use the native HTML checkbox using form control instead of recreating a checkbox's functionality with JavaScript and ARIA.

@whymarrh
Copy link
Contributor Author

We would have a hard time styling the native checkbox to look like the one we have the design. I believe that's what lead to the custom implementation.

@Gudahtt
Copy link
Member

Gudahtt commented Nov 23, 2019

Hmm. Is it that hard? I think something like this would work: http://jsfiddle.net/go392m5s/
Except you'd set a background-image for the :checked state.

I knew this used to be difficult, but I thought custom styles were easy today because native styles can be disabled with appearance: none.

Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

Looks good!

It would be nice to try using an HTML input, and moving this to a reusable component. But we can consider those another time.

@whymarrh
Copy link
Contributor Author

Yeah I think this is the best immediate change, we should definitely consider replacing theses.

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

Successfully merging this pull request may close these issues.

2 participants