1102506 - Settings screen icon button labels should be set via aria-label instead of 0 font sized text.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Calendar, defect)

Description

[jobara]

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>

Comment 1

[jobara]

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

Comment 2

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

Looks good, some comments in Github. Flip back the review on once you make the changes. Thanks!

Comment 3

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

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 [e-mail] for a review. Thanks!

Comment 4

[Miller Medeiros [:millermedeiros]]

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.

Comment 5

[GitHub Autolander Bot]

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

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

https://github.com/mozilla-b2g/gaia/commit/e4956b2ecd5357ba4aa7604ca2ab244b01366f7a

Comment 7

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

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

Comment 8

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/afc63e5c1a40b3587846cadb348996fc01998883