Closed
Bug 1069380
Opened 10 years ago
Closed 10 years ago
Use link role instead of button for open action in cards view
Categories
(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)
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.
Updated•10 years ago
|
Mentor: yzenevich
Whiteboard: [b2ga11y p=1] → [b2ga11y p=1][good first bug]
Updated•10 years ago
|
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 ?
Comment 2•10 years ago
|
||
(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
Attachment #8499995 -
Flags: review?(yzenevich)
Comment 4•10 years ago
|
||
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+
Updated•10 years ago
|
Attachment #8499995 -
Flags: review?(alive) → review+
Comment 7•10 years ago
|
||
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)
Comment 9•10 years ago
|
||
Merged https://github.com/mozilla-b2g/gaia/commit/1d3e427b61fb72b6f6f7a20ce6176f5a0d6c9db5, Thanks!
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•