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

Status: RESOLVED FIXED

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

Description

[Paco Rampas [:paco]]

Attachment: merge-localization.png

Comment 1

[Paco Rampas [:paco]]

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?

Comment 2

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

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.

Comment 3

[Paco Rampas [:paco]]

Attachment: patch in github

Comment 4

[Doug Sherk (:drs) (inactive)]

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.

Comment 5

[Paco Rampas [:paco]]

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.

Comment 6

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

Added some clarification comments in the PR.

Comment 7

[Paco Rampas [:paco]]

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.

Comment 8

[Doug Sherk (:drs) (inactive)]

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.

Comment 9

[Paco Rampas [:paco]]

Attachment: patch in github

Comment 10

[Paco Rampas [:paco]]

Attachment: patch in github

Comment 11

[Paco Rampas [:paco]]

Comment on attachment 8522896 [details] [review]
patch in github

I leave a comment on github. Thanks.

Comment 12

[Paco Rampas [:paco]]

merged: 05d80f0af85c690e098828ed3e548b144afd904b