Closed Bug 1069380 Opened 5 years ago Closed 5 years ago

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

Categories

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

All
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

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

References

Details

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

Attachments

(2 files)

By convention, we want all actions that open an app to be links instead of buttons.
Mentor: yzenevich
Whiteboard: [b2ga11y p=1] → [b2ga11y p=1][good first bug]
Whiteboard: [b2ga11y p=1][good first bug] → [b2ga11y p=1][good first bug][lang=html][lang=js]
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
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+
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)
(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
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.