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

Improve mutually exclusive screen reader alerts #263

Merged
merged 20 commits into from
Mar 5, 2019

Conversation

bameyrick
Copy link
Contributor

@bameyrick bameyrick commented Feb 28, 2019

I've updated the mutually exclusive component to improve the screen reader alerts for when inputs/checkboxes are programatically cleared.

It would be worth having a play with a few of the different mutually exclusive example types using a screen reader to see how the alerts sound. One to be aware of is the "please specify" input in the "Other" option in mutually exclusive checkboxes, although that may be an issue with the "other" checkbox/radio option itself rather than the mutually exclusive code.

Will resolve #270
Will resolve #271

@bameyrick
Copy link
Contributor Author

@codecov
Copy link

codecov bot commented Feb 28, 2019

Codecov Report

Merging #263 into master will increase coverage by 0.38%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #263      +/-   ##
=========================================
+ Coverage   95.92%   96.3%   +0.38%     
=========================================
  Files          15      15              
  Lines         564     595      +31     
=========================================
+ Hits          541     573      +32     
+ Misses         23      22       -1
Impacted Files Coverage Δ
src/components/textarea/character-limit.js 100% <100%> (ø) ⬆️
...omponents/mutually-exclusive/mutually-exclusive.js 100% <100%> (ø) ⬆️
src/components/header/header-nav.js 100% <0%> (+4.34%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f5ad5c3...8b0b6e8. Read the comment docs.

@jrbarnes9
Copy link
Contributor

Copy link
Contributor

@jrbarnes9 jrbarnes9 left a comment

Choose a reason for hiding this comment

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

  • Could we change alert for the ‘or’ option dependent on whether or not selecting it would actually deselect the other checkboxes? Currently always says "selecting this will uncheck all other checkboxes" ...it ideally shouldn’t read the same thing out after the user has checked it.
  • If the ‘or’ checkbox is selected, selecting an option checkbox doesn’t inform the user that the ‘or’ checkbox has been unselected.
  • When you select 'or' option whilst 'other' is selected and has a value it reads: "Other deselected, please specify deselected" …it would read better as just e.g. ‘Other, [other input value] deselected’

@bameyrick
Copy link
Contributor Author

@jrbarnes9 I've changed it to only say "selecting this will uncheck all other checkboxes" if checkboxes have been checked.

I get an alert for "no central heating deselected" when i check another options so it could be a device specific issue... what are you testing with?

As for the other field, I think we will have to address that as a fix for checkboxes and radios as they have no link between other and the input field it shows at the moment. I've raised an issue here: #272

@bameyrick bameyrick requested a review from jrbarnes9 March 4, 2019 14:18
@jrbarnes9
Copy link
Contributor

jrbarnes9 commented Mar 4, 2019

@jrbarnes9 I've changed it to only say "selecting this will uncheck all other checkboxes" if checkboxes have been checked.

I get an alert for "no central heating deselected" when i check another options so it could be a device specific issue... what are you testing with?

As for the other field, I think we will have to address that as a fix for checkboxes and radios as they have no link between other and the input field it shows at the moment. I've raised an issue here: #272

  1. Good, this works well now.
  2. Seems fine now, perhaps I missed it first time round, was using chrome with mac voiceover.
  3. Sure, makes sense.

@jrbarnes9
Copy link
Contributor

When using the feedback text area example, the character count is read out after the 'I don't want to provide feedback' checkbox is selected.

@jrbarnes9
Copy link
Contributor

Still a spacing issue:...
screenshot 2019-03-04 at 14 28 47

@jrbarnes9
Copy link
Contributor

Mutually exclusive duration... "[legend] deselected" is read out twice after selecting the checkbox. One for each field cleared presumably. Preferably should just read this out once to indicate all fields in the field_group have been deselected.

@bameyrick
Copy link
Contributor Author

@bameyrick
Copy link
Contributor Author

Mutually exclusive duration... "[legend] deselected" is read out twice after selecting the checkbox. One for each field cleared presumably. Preferably should just read this out once to indicate all fields in the field_group have been deselected.

I've updated this to read out the title of the abbreviation so it should read something like "Years cleared. Months cleared" rather than the [legend] deselected. I'm not sure whether saying "[legend] cleared" and not mentioning which fields were cleared in the group 'e.g. years and months here, or day, month, year in the date input' would be clear enough for screen reader users.

Co-Authored-By: bameyrick <bameyrick@gmail.com>
@bameyrick bameyrick merged commit 47879b3 into master Mar 5, 2019
@bameyrick bameyrick deleted the 52-mutually-exclusive-aria-live-isnt-very-good branch March 5, 2019 14:37
boxadesign pushed a commit that referenced this pull request Feb 17, 2023
* Prevent screen reader from reading Or

* Prevent screen reader from saying "Table end" when selecting a label in a field__item

* Improve aria-live for mutually exclusive

* Ensure alert message is read by screen reader and fix spacing of checkbox / radios

* Fix mutually exclusive fields layout

* Fix layout of mutually exclusive checkbox and change deselection message to only fire when relevent

* Ensure characters remaining isnt read out when textarea cleared programatically

* Ensure "Select all that apply" message isnt part of legend

* Fix layout of mutually exclusive prefixed/suffixed field

* Update all input types to be able to be run in "questionMode" as this will be required for mutually exclusive questions

* Fix mutually exlusive for inputs with prefix/suffix and no label

* Fix tagName detection

* Update radios and checkboxes to prevent screen reader from focusing on inner label elements, but still read the labels

* Fix clear meassage for prefixed/suffixed inputs not in a group

* Fix label for mutiple suffixed/prefixed and add unit test

* Fix char limit unit tests

* Improve char limit test

* Correct spelling

Co-Authored-By: bameyrick <bameyrick@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants