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

Status: RESOLVED FIXED

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

Description

[Eitan Isaacson [:eeejay]]

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

Comment 1

[jobara]

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 ?

Comment 2

[Yura Zenevich [:yzen] (please use "needinfo")]

(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).

Comment 3

[jobara]

Attachment: 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.

Comment 4

[Yura Zenevich [:yzen] (please use "needinfo")]

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.

Comment 5

[jobara]

Attachment: 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.

Comment 6

[jobara]

Yura, I created a new pull request with the changes you asked for and squashed them down into a single commit.

Comment 7

[Yura Zenevich [:yzen] (please use "needinfo")]

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.

Comment 8

[jobara]

(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.

Comment 9

[Yura Zenevich [:yzen] (please use "needinfo")]

Merged https://github.com/mozilla-b2g/gaia/commit/1d3e427b61fb72b6f6f7a20ce6176f5a0d6c9db5, Thanks!