Closed Bug 1069836 Opened 5 years ago Closed 5 years ago

[Dialer][Call Screen] Improve the localizantion of call options in call screen (follow up of 977588)

Categories

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

x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: paco, Assigned: paco)

References

Details

Attachments

(2 files, 2 obsolete files)

82.06 KB, image/png
Details
46 bytes, text/x-github-pull-request
drs
: review+
Details | Review
Attached image merge-localization.png
No description provided.
We have doubts for how fix the localization. I opened this follop up for fix it. Here you can see the discussion in github.

https://github.com/mozilla-b2g/gaia/pull/22348#discussion-diff-17464123R226

What is the best option for fix it?
Flags: needinfo?(yzenevich)
Summary: [Dialer][Call Screen] Improve the localizantion of call options in call screen → [Dialer][Call Screen] Improve the localizantion of call options in call screen (follow up of 977588)
See Also: → 977588
Left a comment in Github too:

If the button you are providing the text for is an icon you should just use either:
* aira-label attribute with the localized text as a value on the button itself (simplest)
* or, as Doug mentioned, aria-labelledby attribute with the id of the text node as a value (however if the text node is there only to be used as a label use the first option)
Hope this helps.
Flags: needinfo?(yzenevich)
Assignee: nobody → pacorampas
Attached file patch in github (obsolete) —
Attachment #8515929 - Flags: review?(drs.bugzilla)
Comment on attachment 8515929 [details] [review]
patch in github

As Yura said, this is not the way to do it. The `aria-label` attribute's value must be the localized text, not the l10n id. I can't think of a way to use `aria-label` without requiring some JS, so it might be better to use the second method that I described, which would look like this:

```html
<span class="hide" id="toggleKeypadLabel" data-l10n-id="toggleKeypad"></span>
<button id="keypad-visibility" class="co-advanced-option grid center" aria-labelledby="toggleKeypadLabel">
<!-- ... -->
```

Please feedback? Yura for the next version of the patch. He might have some way to use `aria-label` that doesn't require extra JS.
Attachment #8515929 - Flags: review?(drs.bugzilla) → review-
Comment on attachment 8515929 [details] [review]
patch in github

Hello,

Regarding comment 2 and comment 4, could you check the patch and give us your feedback about if it is the best solution? 

Thanks.
Attachment #8515929 - Flags: feedback?(yzenevich)
Added some clarification comments in the PR.
Attachment #8515929 - Flags: feedback?(yzenevich)
Comment on attachment 8515929 [details] [review]
patch in github

Hi Doug, 

I have fixed the patch acording with suggestions of yura. Could you re-review it?  

Thanks.
Attachment #8515929 - Flags: review- → review?(drs.bugzilla)
Comment on attachment 8515929 [details] [review]
patch in github

I think there was a misunderstanding which Yura has helped clear up over IRC. Please see my comments on the PR.
Attachment #8515929 - Flags: review?(drs.bugzilla) → review-
Attached file patch in github (obsolete) —
Attachment #8515929 - Attachment is obsolete: true
Attachment #8522802 - Flags: review?(gtorodelvalle)
Attachment #8522802 - Flags: review?(gtorodelvalle)
Attached file patch in github
Attachment #8522802 - Attachment is obsolete: true
Comment on attachment 8522896 [details] [review]
patch in github

I leave a comment on github. Thanks.
Attachment #8522896 - Flags: review?(drs.bugzilla)
Attachment #8522896 - Flags: review?(drs.bugzilla) → review+
merged: 05d80f0af85c690e098828ed3e548b144afd904b
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.