AusDTO / gov-au-ui-kit

MOVED TO https://github.com/govau/uikit/
https://github.com/govau/uikit/
MIT License
19 stars 12 forks source link

refactor radio and checkbox icons to use floats rather than position … #456

Closed simonschwartz closed 7 years ago

simonschwartz commented 7 years ago

Bugfix - Fix screen reader compatibility with radio and checkbox inputs

Description

A DTA Identity project identified that the UI Kit checkbox/radio labels were not being read out by a screen reader. I replicated this issue using VoiceOver on MacOSX 10.10.5

The screen reader currently just reads 'Checkbox checked/un-checked' so the user has no idea what the value of the input they are selecting is.

The cause of this issue was the use of position: relative on the <label> element.

Proposal

This PR proposes replacing css position properties on the <label> element with float: left and negative margin. Using negative margins isn't great - is there a better way?

Additional information

I tested the Checkbox and Radio inputs on gov.uk and they have the same issue. If we find a nice solution we could send a PR to the gov.uk style guide.

Definition of Done

klepas commented 7 years ago

@AndrewArch and I had a look at this.

I was using VO from OS X 10.12.1 and couldn’t replicate the issue directly.

We did discover that VO plainly seems to skip the legend; it seems to handle the labels fine, but never seems to read out of the legend, unless explicitly selected via the cursor… which defeats the purpose… and even then it’s treated as plain text, and not a legend or in any other way associated to the form it’s in.

From some digging this seems to be a bug with VO back from 2014. 18F have run into this issue also: https://github.com/18F/web-design-standards/issues/792

We overcame the issue by explicitly associating the id of a legend of a given fieldset to the input via aria-describedby. For example:

<form>
  <fieldset>
    <legend id="foo">Which electronic devices do you own?</legend>
    <input id="phone" name="reply" type="checkbox" aria-describedby="foo" value="phone"/>
    <label for="phone">Phone</label>
    <input id="tablet" name="reply" type="checkbox" aria-describedby="foo" value="tablet"/>
    <label for="tablet">Tablet</label>
    <input id="laptop" name="reply" type="checkbox" aria-describedby="foo" value="laptop"/>
    <label for="laptop">Laptop</label>
    <input id="desktop" name="reply" type="checkbox" aria-describedby="foo" value="desktop"/>
    <label for="desktop">Desktop computer</label>
  </fieldset>
</form>

This is apparently non-standard, and not needed by other screen readers.

It’s worth noting we also discovered inconsistent results with NVDA: it would read out the legend and labels without a problem for the radio buttons, but not consistently for the checkboxes. It would read the checkbox legend after each label when using the above markup, and otherwise it appeared inconsistent.

Additional information

I’ve also spun up an instance that applies your SASS edits + a partial edit to the checkbox HTML (adding id and aria-describedby): http://dg-checkbox-radio-a11y-test.apps.staging.digital.gov.au/components/forms-buttons/index.html#checkbox-input

Where to from here?

I propose:

simonschwartz commented 7 years ago

I tried the markup in your previous post @klepas but am still getting the same error where the label isn't being read out by the screen reader. I did some further research and it seems this may be a bug with Chrome.

http://stackoverflow.com/questions/40846788/why-does-relative-positioning-on-a-label-make-the-label-content-invisible-to-the

Can confirm that this bug isn't happening in Firefox for me with current UI Kit markup

klepas commented 7 years ago

@simonschwartz Hey, thanks for the further info. I was silly and only tested this in Chrome, which explains why I couldn’t reproduce your error the first time, instead finding a related bug. Lel. :)

I’m keen now to merge these changes given that it fixes this problem until this is fixed in Chrome directly.

I’m gonna do some more testing with FF and Safari. I want to investigate if we should update the markup and our guidance to overcome this 3 year old VoiceOver bug. Once done I’ll report back and then I’ll do the work to merge this.

Thanks again.

klepas commented 7 years ago

I’ve adjusted the spacing a bit, and cleaned up a few unnecessary padding and media queries.

klepas commented 7 years ago

Just adding some before | after screenshots from the padding edits to the radio buttons and checkboxes:

Before

screen shot 2017-01-10 at 10 21 42 am

screen shot 2017-01-10 at 10 21 58 am


After

screen shot 2017-01-10 at 10 22 10 am

screen shot 2017-01-10 at 10 22 28 am

gazhands commented 7 years ago

Great work!