w3c / accname

Accessible Name and Description Computation
https://w3c.github.io/accname/
60 stars 23 forks source link

update accname 'flash screen' example to use native clickable label element #206

Closed cookiecrook closed 8 months ago

cookiecrook commented 9 months ago

Closes #65


Preview | Diff

scottaohara commented 9 months ago

@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:

<label for="flash" id=label>
  <input type="checkbox" id="flash" aria-labelledby="label number times">
  Flash the screen 
</label>
<label for=number>
  <input type="text" value="5" aria-label="number of times" id=number> <span id=times>times</span>.
</label>

but a label is only allowed to contain a single labellable element, and if it weren't for the for attribute pointing to the checkbox, this would start breaking in other ways which wouldn't produce the described accName - but the use of for isn't referenced in the 'consider this' text as being necessary to make this work as the example says it will.

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. ☹️

cookiecrook commented 9 months ago

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 for/id relationship is explicitly defined. I don't think any accessibility implementations would be tripped up by the inclusion of the second input, which also has an explicitly defined aria-label.

But that said, I agree we shouldn't use an invalid example.

cookiecrook commented 9 months ago

@scottaohara how one of these options

  1. Avoid the second <input> element in the <label> element; use an ARIA textbox instead.

    <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>
  2. Like yours, but avoid the <label> elements entirely, since the controls use aria-labelledby and aria-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>
scottaohara commented 9 months ago

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.

cookiecrook commented 9 months ago

Recycling @scottaohara's review with the new diff.

cookiecrook commented 9 months ago

I really appreciate the first one, specifically as a "html doesn't allow this? oh yeah?" it made me grin.

It also keeps the "clickable" functionality of the HTML <label> 👍

cookiecrook commented 8 months ago

@accdc @pkra @spectranaut @jnurthen Please merge?

accdc commented 8 months ago

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.

cookiecrook commented 8 months ago

I'm not sure how to resolve the branch merge conflicts though

I didn't see any merge conflicts. Merge was successful. Deleting branch.

cookiecrook commented 8 months ago

Oh... I see. @jnurthen forward merged main into this one instead of a rebase squash. Thanks.