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

docs: Add role="switch" to switches #34824

Merged
merged 2 commits into from Sep 9, 2021
Merged

docs: Add role="switch" to switches #34824

merged 2 commits into from Sep 9, 2021

Conversation

coliff
Copy link
Contributor

@coliff coliff commented Aug 26, 2021

Fixes: #34818

@XhmikosR
Copy link
Member

@coliff please check the new HTML errors. If there's something wrong, please file a bug report to update vnu-jar and or the specs.

@patrickhlauke
Copy link
Member

probably slight tweak to my original wording (now that i'm reading it in context)

to convey the nature of the control in browser/assistive technology combinations that support it.

ffoodd
ffoodd previously approved these changes Aug 27, 2021
Copy link
Member

@ffoodd ffoodd left a comment

Choose a reason for hiding this comment

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

Seems good to me, according to @patrickhlauke's recommandation 👌

Thanks!

@XhmikosR XhmikosR dismissed ffoodd’s stale review August 27, 2021 05:34

Tests don't pass...

@coliff
Copy link
Contributor Author

coliff commented Sep 1, 2021

The accessibility tests are failing with this error:

"file:/home/runner/work/bootstrap/bootstrap/_site/docs/5.1/forms/checks-radios/index.html":571.3-571.92: error: Element "input" is missing required attribute "aria-checked".
"file:/home/runner/work/bootstrap/bootstrap/_site/docs/5.1/forms/checks-radios/index.html":575.3-575.100: error: Element "input" is missing required attribute "aria-checked".
"file:/home/runner/work/bootstrap/bootstrap/_site/docs/5.1/forms/checks-radios/index.html":579.3-579.102: error: Element "input" is missing required attribute "aria-checked".
"file:/home/runner/work/bootstrap/bootstrap/_site/docs/5.1/forms/checks-radios/index.html":583.3-583.117: error: Element "input" is missing required attribute "aria-checked".

So it's suggesting we add aria-checked="false" to switches which are off and aria-checked="true" to switches are which are on. That would have to be done with JavaScript.

https://developer.mozilla.org/en-US/docs/Web/API/Element/ariaChecked

However, it also states on that page in the Note at the top that that we should use type="checkbox" (which we do) as this element has built in semantics and does not require ARIA attributes. So maybe this is a bug in the validator. What do you think @patrickhlauke ?

UPDATE: I posted an issue on the validator GitHub project. validator/validator#1221

@patrickhlauke
Copy link
Member

patrickhlauke commented Sep 1, 2021

yes this seems to be a shortcoming of the validator, as you don't need aria-checked for native <input type="checkbox">

https://www.w3.org/TR/html-aria/#docconformance

Authors SHOULD NOT use the aria-checked attribute on input type=checkbox elements.

[...]

Note: The HTML checked attribute can be used instead of the aria-checked attribute for menuitemcheckbox, option or switch roles when used on type=checkbox.

@XhmikosR
Copy link
Member

XhmikosR commented Sep 1, 2021 via email

@patrickhlauke
Copy link
Member

yeah, i did after posting the comment here validator/validator#1221 (comment)

@XhmikosR
Copy link
Member

XhmikosR commented Sep 9, 2021

This is weird, now we no longer get the errors but the issue wasn't fixed upstream :S

No idea what's happening...

v5.1.2 automation moved this from In progress to Ready to merge Sep 9, 2021
@XhmikosR
Copy link
Member

XhmikosR commented Sep 9, 2021 via email

@coliff coliff requested a review from a team as a code owner September 9, 2021 11:30
@XhmikosR XhmikosR merged commit cb87ed2 into twbs:main Sep 9, 2021
v5.1.2 automation moved this from Ready to merge to Done Sep 9, 2021
@coliff coliff deleted the patch-1 branch September 9, 2021 11:38
@XhmikosR XhmikosR changed the title docs: Add role="switch" to switches docs: Add role="switch" to switches Sep 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
v5.1.2
  
Done
Development

Successfully merging this pull request may close these issues.

Add role="switch" to switches in example code?
5 participants