-
Notifications
You must be signed in to change notification settings - Fork 22
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
update accname 'flash screen' example to use native clickable label element #206
Conversation
@cookiecrook @MelSumner should we call out the fact that this example is invalid HTML? i mean, it's far easier to use this pattern than the 'correctly nested' markup pattern, which could look like:
but a label is only allowed to contain a single labellable element, and if it weren't for the sorry, really don't mean to be so pedantic about this, but it does just strike me as odd for a spec to use an invalid example without at least mentioning it is invalid. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just making this an official 'review' per my above comment.
again not in opposition to the example, i just think it needs to state it is invalid markup so as to not have this spec seemingly imply something is valid which html says is not.
I'm only slightly embarrassed to say I didn't think about the fact that two inputs in a label would make it invalid, especially since the But that said, I agree we shouldn't use an invalid example. |
@scottaohara how one of these options
<label for="flash">
<input type="checkbox" id="flash">
Flash the screen <span tabindex="0" role="textbox" aria-label="number of times" contenteditable>5</span> times.
</label>
<input type="checkbox" id="flash" aria-labelledby="flash number times">
<span id="flash">Flash the screen</span>
<input id="number" type="text" value="5" aria-label="number of times">
<span id="times">times.</span> |
fwiw, @cookiecrook, i agree - it 'should/could' to be "OK" - though i believe I've run into some windows based voice dictation software falling down on this sort of thing, where screen readers generally handle it ok these days. anyway... appreciate your alternatives. and yeah, i think either of those examples works. I really appreciate the first one, specifically as a "html doesn't allow this? oh yeah?" it made me grin. |
Recycling @scottaohara's review with the new diff. |
It also keeps the "clickable" functionality of the HTML |
@accdc @pkra @spectranaut @jnurthen Please merge? |
I'm happy merging since we all appear to be in agreement. I'm not sure how to resolve the branch merge conflicts though so will need someone's assistance who has a better handle on Git than I. |
I didn't see any merge conflicts. Merge was successful. Deleting branch. |
Oh... I see. @jnurthen forward merged main into this one instead of a rebase squash. Thanks. |
Closes #65
Preview | Diff