Closed Bug 1018220 Opened 11 years ago Closed 11 years ago

Add missing aria attributes to the FTU app screens.

Categories

(Firefox OS Graveyard :: Gaia::First Time Experience, defect)

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: yzen, Assigned: yzen)

References

Details

(Keywords: access)

Attachments

(1 file)

Right now there are plenty of places where the FTU app is missing/misusing DOM and aria attributes. They need to be fixed.
Component: Gaia → Gaia::First Time Experience
Attached file Github PR (deleted) —
Attachment #8431589 - Flags: review?(fernando.campo)
Comment on attachment 8431589 [details] Github PR Sorry for the delay, but I had to learn a lot of new things about aria to review this :D So, from my little knowledge, code looks good, but I'm worried about the new strings added, so I'm asking other reviews from l10n team. I think some of them they should be a little more descriptive [networksList = 'Found'] Also, how can I test this? activating the screen reader on the phone? can I do that on nightly too?
Attachment #8431589 - Flags: review?(l10n)
Attachment #8431589 - Flags: review?(fernando.campo)
Attachment #8431589 - Flags: review+
(In reply to Fernando Campo (:fcampo) from comment #2) > Comment on attachment 8431589 [details] > Github PR > > Sorry for the delay, but I had to learn a lot of new things about aria to > review this :D NP :) > > So, from my little knowledge, code looks good, but I'm worried about the new > strings added, so I'm asking other reviews from l10n team. I think some of > them they should be a little more descriptive [networksList = 'Found'] I'm actually trying to keep things fairly short because the screen reader announced more things other than just the text, labels, etc. For example if we hit the list , like in this case screen reader will say: "Found list" > > Also, how can I test this? activating the screen reader on the phone? can I > do that on nightly too? Ya you can totally test it on the phone, emulator. If you use the phone, just toggle volume up and volume down buttons (hardware) 3 times and you should hear the instructions. If you are on the desktop emulator, devtools have a screen reader tab.
(In reply to Yura Zenevich [:yzen] from comment #3) > (In reply to Fernando Campo (:fcampo) from comment #2) > > I'm actually trying to keep things fairly short because the screen reader > announced more things other than just the text, labels, etc. For example if > we hit the list , like in this case screen reader will say: > > "Found list" I ll change it to Networks Found as suggested
Comment on attachment 8431589 [details] Github PR On top of what flod said, please add comments to strings that are used for a11y, that they're used for accessibility. On fx os, localizers are searching the short strings, but for a11y, that's not really required, and good text might be even more powerful. Thus, keeping those apart through comments is a good idea. f- based on flod's comments, too.
Attachment #8431589 - Flags: review?(l10n) → review-
Comment on attachment 8431589 [details] Github PR Addressed l10n comments.
Attachment #8431589 - Flags: review- → review?(community)
There's no reason to just randomly pick bugzilla accounts for stuff. Please don't abuse the community@localization.bugs account.
Attachment #8431589 - Flags: review?(community) → review?(l10n)
(In reply to Axel Hecht [:Pike] from comment #7) > There's no reason to just randomly pick bugzilla accounts for stuff. > > Please don't abuse the community@localization.bugs account. Sorry, accidentally picked the auto-completed :l10n entry.
About the comment: instead of just "#Accessibility", I would explain what it means. For example: > The following strings are not visible and used for accessibility (screen reader) Reference for the future about :l10n, open to suggestions for improvements ;-) https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_best_practices#Bugzilla_and_l10n
(In reply to Francesco Lodolo [:flod] from comment #9) > About the comment: instead of just "#Accessibility", I would explain what it > means. For example: > > > The following strings are not visible and used for accessibility (screen reader) Updated the comment as you suggested. > > Reference for the future about :l10n, open to suggestions for improvements > ;-) > https://developer.mozilla.org/en-US/docs/Mozilla/Localization/ > Localization_best_practices#Bugzilla_and_l10n
Flags: needinfo?(francesco.lodolo)
LGTM, only one additional comment: when there's only one string affected, it'd be better to use # LOCALIZATION NOTE(StringID): etc.
Flags: needinfo?(francesco.lodolo)
(In reply to Francesco Lodolo [:flod] from comment #11) > LGTM, only one additional comment: when there's only one string affected, > it'd be better to use > # LOCALIZATION NOTE(StringID): etc. Done :).
Flags: needinfo?(francesco.lodolo)
I'm discussing with other folks in #l10n about the best comment we can use in these cases, but for that we need to clarify a bit more what screen reader is capable of, and if there are some suggestions for accessibility that we should give localizers (in comments or other places). For sure we're not in a good place in terms of consistency, basically each a11y string across apps has a different comment.
Flags: needinfo?(francesco.lodolo)
After some discussion on IRC, we decided to use this comment for a11y strings (avoiding "accessibility", which is a bit too jargon). # The following strings are spoken by screen readers and not shown on the screen. Either as a group comment from more strings, or in the format # LOCALIZATION NOTE(ID): etc. for one string. We'll decide later where to place some guidelines about localizing these strings.
Comment on attachment 8431589 [details] Github PR Looks good to me with minor nit on one comment. Sorry it took so much time, and thanks for the full explanation.
Attachment #8431589 - Flags: review?(l10n) → review+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: