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

Make old JAWS recognise disabled buttons #532

Merged
merged 1 commit into from
Jul 24, 2017
Merged

Conversation

selfthinker
Copy link
Contributor

@selfthinker selfthinker commented Jul 14, 2017

What problem does the pull request solve?

JAWS 15 and below don't recognise the disabled attribute on buttons.
This adds the aria-disabled attribute to make disabled buttons compatible with older screen readers.

This does not apply to other disabled elements. I also tested the new disabled radio buttons and checkboxes, and they are fine in older JAWS versions without this change.

How has this been tested?

I tested with JAWS 14, 15 and 17.
JAWS 17 is fine without the change and reads e.g. "Save button - unavailable" but all the others read "Save button" as if it was a normal button, not making it clear that you cannot use it. Adding aria-disabled fixes that. It seems redundant but is similar to our practice to add role="main" to our <main> element to support older browsers.

I couldn't test with JAWS 16 yet. In case that also doesn't support disabled, I will update the commit message and this PR accordingly.

What type of change is it?

  • Bug fix (non-breaking change which fixes an issue)

Has the documentation been updated?

I'm not sure if this needs to be documented?
I have added an item to the page.

@gemmaleigh gemmaleigh temporarily deployed to govuk-elements-review-pr-532 July 14, 2017 16:57 Inactive
@gemmaleigh
Copy link
Contributor

Thanks @selfthinker!

For the documentation - could you please add another list item to the list of bullet points about disabled buttons:

  • disabled buttons must use thearia-disabled attribute for older screen readers

JAWS 15 and below don't recognise the `disabled` attribute on buttons.
This adds the `aria-disabled` attribute to make
disabled buttons compatible with older screen readers.
@selfthinker
Copy link
Contributor Author

I have added an item to the list. I have shortened your proposed sentence because we rarely use the word "must" (or "should" etc) within Elements and I thought it sounded a bit too formal for that bit.

@selfthinker
Copy link
Contributor Author

Unfortunately, we can currently not test in JAWS 16. I've asked in the x-gov slack channel if anyone can test it. Otherwise, I don't think it's super important. It would just be good to know which JAWS version started to support the disabled attribute on buttons. But this could be merged without that knowledge. Let's wait until tomorrow.

@selfthinker
Copy link
Contributor Author

@accessiblewebuk just tested and confirmed that JAWS 16 handles the buttons correctly, so it is indeed "JAWS 15 and below" as stated in the commit message and description above.

@edwardhorsford
Copy link
Contributor

Would it be good to be specific about what screen readers and versions we're doing this for? I feel like this could be helpful in some years time when we can take a view if we still support those screen readers.

@selfthinker
Copy link
Contributor Author

I was specific in the commit message ("JAWS 15 and below"), which people should always be able to find. Or did you mean making it more prominent in the guidance?

@edwardhorsford
Copy link
Contributor

I mean making it explicit in the guidance - but it's only a suggestion.

@gemmaleigh
Copy link
Contributor

I'm happy to merge, before I do - let me know if you would prefer to use "older screen readers" or "JAWS 15 and below".

@selfthinker
Copy link
Contributor Author

We don’t necessarily know that it’s just older JAWS as we haven’t tested older versions of other screen readers. (Although the reason for that is that older versions of other screen readers are not used as much.)
We are also not specific in other areas either (e.g. with role="main" or many JavaScripts), so that would be inconsistent. Although, it would still be a good idea to generally do it, but it might be quite a lot of work to do it consistently.

@gemmaleigh
Copy link
Contributor

Thanks @selfthinker, will merge as-is.

@gemmaleigh gemmaleigh merged commit d4f8159 into master Jul 24, 2017
@gemmaleigh gemmaleigh deleted the disabled-buttons branch July 24, 2017 08:09
@36degrees 36degrees mentioned this pull request Oct 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants