Closed
Bug 1102506
Opened 10 years ago
Closed 9 years ago
Settings screen icon button labels should be set via aria-label instead of 0 font sized text.
Categories
(Firefox OS Graveyard :: Gaia::Calendar, defect)
Firefox OS Graveyard
Gaia::Calendar
Tracking
(b2g-v2.2 fixed, b2g-master fixed)
RESOLVED
FIXED
2.2 S6 (20feb)
People
(Reporter: obara.justin, Assigned: obara.justin)
References
Details
(Keywords: access)
Attachments
(1 file)
46 bytes,
text/x-github-pull-request
|
mmedeiros
:
review+
yzen
:
a11y-review+
bajaj
:
approval-gaia-v2.2+
|
Details | Review |
The icon button labels shouldn't be specified as text nodes in the spans. Rather they should be specified using an aria-label attribute. <div role="toolbar"> <button class="settings toolbar-item"> <span class="icon" data-l10n-id="advanced-settings-short">Settings</span> </button> <button class="sync update toolbar-item"> <span class="icon" data-l10n-id="drawer-sync-button">Sync</span> </button> </div>
Attachment #8552767 -
Flags: a11y-review?(yzenevich)
Comment 2•9 years ago
|
||
Looks good, some comments in Github. Flip back the review on once you make the changes. Thanks!
Updated•9 years ago
|
Attachment #8552767 -
Flags: a11y-review?(yzenevich)
Attachment #8552767 -
Flags: a11y-review?(yzenevich)
Comment 3•9 years ago
|
||
Comment on attachment 8552767 [details] [review] Rather than having the text as invisible text nodes. The labels for the sync and settings buttons in the calendar settings are now set using an aria-label. Looks good once the last comments are addressed. Feel free to file a followup for back button stuff. Once you updated the PR, please ask mmedeiros@mozilla.com for a review. Thanks!
Attachment #8552767 -
Flags: a11y-review?(yzenevich) → a11y-review+
Updated•9 years ago
|
Attachment #8552767 -
Flags: review?(mmedeiros)
Comment 4•9 years ago
|
||
Comment on attachment 8552767 [details] [review] Rather than having the text as invisible text nodes. The labels for the sync and settings buttons in the calendar settings are now set using an aria-label. LGTM! thanks.
Attachment #8552767 -
Flags: review?(mmedeiros) → review+
Keywords: checkin-needed
Updated•9 years ago
|
Keywords: checkin-needed
Comment 5•9 years ago
|
||
Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.
Comment 6•9 years ago
|
||
https://github.com/mozilla-b2g/gaia/commit/e4956b2ecd5357ba4aa7604ca2ab244b01366f7a
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment 7•9 years ago
|
||
Comment on attachment 8552767 [details] [review] Rather than having the text as invisible text nodes. The labels for the sync and settings buttons in the calendar settings are now set using an aria-label. [Approval Request Comment] Fixes the way screen reader labels are set in settings. [Bug caused by] (feature/regressing bug #): improvement, not a bug [User impact] if declined: If declined, the labels will remain being set incorrectly. [Testing completed]: on device [Risk to taking this patch] (and alternatives if risky): low [String changes made]: https://github.com/mozilla-b2g/gaia/pull/27580/files#diff-97b7fed57927556b91fb72d4d1b8c8b5
Attachment #8552767 -
Flags: approval-gaia-v2.2?
Updated•9 years ago
|
Attachment #8552767 -
Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Comment 8•9 years ago
|
||
v2.2: https://github.com/mozilla-b2g/gaia/commit/afc63e5c1a40b3587846cadb348996fc01998883
You need to log in
before you can comment on or make changes to this bug.
Description
•