Use link role instead of button for open action in cards view

RESOLVED FIXED

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: eeejay, Assigned: obara.justin, Mentored)

Tracking

({access})

unspecified
All
Gonk (Firefox OS)
access
Bug Flags:
a11y-review +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [b2ga11y p=1][good first bug][lang=html][lang=js])

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
By convention, we want all actions that open an app to be links instead of buttons.

Updated

4 years ago
Mentor: yzenevich
Whiteboard: [b2ga11y p=1] → [b2ga11y p=1][good first bug]

Updated

4 years ago
Whiteboard: [b2ga11y p=1][good first bug] → [b2ga11y p=1][good first bug][lang=html][lang=js]
(Assignee)

Comment 1

4 years ago
Yura, can I take a look at this issue. I was looking into the code a bit and wonder if it is just the role that needs to be changed https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/card.js#L73 or does this button also need to be have a role or element changed as well https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/card.js#L82-L83 ?
(In reply to obara.justin from comment #1)
> Yura, can I take a look at this issue. I was looking into the code a bit and
> wonder if it is just the role that needs to be changed
> https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/card.js#L73
> or does this button also need to be have a role or element changed as well
> https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/card.js#L82-
> L83 ?

Yep, its yours!
Yes the former role needs to be updated. In terms of the other button, we were thinking of hiding it from the screen reader completely (aria-hidden) because the sighted user does not differentiate it as a button (so I would get rid of the role there completely).
Assignee: nobody → obara.justin
(Assignee)

Comment 3

4 years ago
Created attachment 8499995 [details] [review]
Changes the button role on the screenshotView to link, and adds the aria-hidden=true attribute to the iconButton to hide it from AT's. Added tests to ensure the aria properties in the cards view.
Attachment #8499995 - Flags: review?(yzenevich)
Comment on attachment 8499995 [details] [review]
Changes the button role on the screenshotView to link, and adds the aria-hidden=true attribute to the iconButton to hide it from AT's. Added tests to ensure the aria properties in the cards view.

a11y-r+ me with a small fix (mentioned in the pull-request). Also could you please squash the PR into 1 commit, thanks! Marking Alive for review.
Attachment #8499995 - Flags: review?(yzenevich)
Attachment #8499995 - Flags: review?(alive)
Attachment #8499995 - Flags: a11y-review+
(Assignee)

Comment 5

4 years ago
Created attachment 8500810 [details] [review]
Changes the button role on the screenshotView to link, and adds the aria-hidden=true attribute to the iconButton to hide it from AT's. Added tests to ensure the aria properties in the cards view.
(Assignee)

Comment 6

4 years ago
Yura, I created a new pull request with the changes you asked for and squashed them down into a single commit.
Flags: a11y-review+
Attachment #8499995 - Flags: review?(alive) → review+
Hi Justin,
Could you rebase your pull request as it seems it can not be applied any more + some tests were failing (not your fault).
Thanks.
Flags: needinfo?(obara.justin)
(Assignee)

Comment 8

4 years ago
(In reply to Yura Zenevich [:yzen] from comment #7)
> Hi Justin,
> Could you rebase your pull request as it seems it can not be applied any
> more + some tests were failing (not your fault).
> Thanks.

Yura, I've rebased with master again. Had a conflict that I had to resolve, but it should be working now. I've updated the pull request.
Flags: needinfo?(obara.justin)
Merged https://github.com/mozilla-b2g/gaia/commit/1d3e427b61fb72b6f6f7a20ce6176f5a0d6c9db5, Thanks!
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.