Closed Bug 1115642 Opened 7 years ago Closed 7 years ago

[Stingray] Card name does not change when locale changes

Categories

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

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: dwi2, Assigned: dwi2)

References

Details

(Whiteboard: [ft:conndevices][Stingray-Branch])

Attachments

(1 file)

We should fetch app name from manifest every time we need them.
Summary: [Stingray] [Home] App name does not change when locale changes → [Stingray] Card name does not change when locale changes
We should take all type of cards into consideration too. 
This includes: Application, AppBookmark, Folder, Deck.
See Also: → 1121816
Assignee: nobody → tzhuang
Status: NEW → ASSIGNED
We have two cases for card name localization:
1. Original card names, which are fetched from manifest or default names of new folders. We should localize these names.
2. User changed name via rename function. In this case, we should respect the name user give, leave it as is and save it as {raw: 'given name'} in data structure. 

And we still have one issue here. We put 'name' label in smart-button element in Home app. However, there are other DOM elements reside in smart-button, which restrict us from using l10n pattern in [1]. If we set textContent of smart-button, it will throw exceptions like: 
  setTextContent is deprecated (https://bugzil.la/1053629). Setting text content of elements with 
  child elements is no longer supported by l10n.js.

I think we need some guidance from Zibi. Given restriction above, how do we localize content for case 1?


[1]: https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/localization_code_best_practices#Writing_APIs_that_operate_on_L10nIDs
Flags: needinfo?(gandalf)
Thanks for reaching out guys! I appreciate your attention to get this right :)

(In reply to Tzu-Lin Huang [:dwi2][:tzhuang] from comment #3)
> We have two cases for card name localization:
> 1. Original card names, which are fetched from manifest or default names of
> new folders. We should localize these names.
> 2. User changed name via rename function. In this case, we should respect
> the name user give, leave it as is and save it as {raw: 'given name'} in
> data structure.

That sounds right. I imagine you storing sth like:

card1.name = {'raw': "My custom name"}
card2.name = 'homeCardName'

that later gets into UI via:

if (typeof(name) === 'object') {
  if ('raw' in name) {
    node.textContent = name.raw;
  }
} else {
  node.setAttribute('data-l10n-id', name);
}

> And we still have one issue here. We put 'name' label in smart-button
> element in Home app. However, there are other DOM elements reside in
> smart-button, which restrict us from using l10n pattern in [1]. If we set
> textContent of smart-button, it will throw exceptions like: 
>   setTextContent is deprecated (https://bugzil.la/1053629). Setting text
> content of elements with 
>   child elements is no longer supported by l10n.js.
> 
> I think we need some guidance from Zibi. Given restriction above, how do we
> localize content for case 1?

Right, so because of the design of mozL10n, we want to think of node localization as a singular unit. You assign l10n-id and the engine localizes the node. For that reason, we cannot have mixed nodes with content from l10n and the developer.

Although I don't see any example of rich content inside the smart-button here [0] I assume that you want to something like:

<smart-button>
  <span data-l10n-id="myButton">Localizable button text</span>
  <img src="./icon.png"/>
</smart-button>

and then your span is properly localized and you can pull it into shadow dom.

That's what we do in shared/elements/*.

Does it help? If you need more guidance, I'll be happy to answer any questions!

Does it help?

[0] https://github.com/mozilla-b2g/gaia/blob/master/tv_apps/tv_shared/components/smart-button/examples/index.html#L37-L43
Flags: needinfo?(gandalf)
It helps a lot. 
I know how to fix this one now, thanks
Comment on attachment 8564906 [details] [review]
[gaia] dwi2:bug1115642 > mozilla-b2g:master

Card name could come from two cases: 
1. If it is a application or deck (which is actually also an app), the localized name is come from its manifest.
2. If it has l10nId (usually in case of folder), we should use form like {id: 'l10nId'} and set its data-l10n-id to let l10n works.

We encapsulate the logic in cardManager.resolveCardName. Also, in order to let l10n works as comment 4 suggested, we separate `name` label from smart-button itself and put it as a child element under smart-button.
Attachment #8564906 - Flags: review?(im)
Attachment #8564906 - Flags: review?(gandalf)
Attachment #8564906 - Flags: review?(gandalf) → review+
Comment on attachment 8564906 [details] [review]
[gaia] dwi2:bug1115642 > mozilla-b2g:master

Looks good to me. But adding an extra element to smart-button may affect the animation. We should ask Sung about this, too.
Attachment #8564906 - Flags: review?(im)
Attachment #8564906 - Flags: review+
Attachment #8564906 - Flags: feedback?(suchiu)
Comment on attachment 8564906 [details] [review]
[gaia] dwi2:bug1115642 > mozilla-b2g:master

It looks good to me! The animation transform is applied on smart-button, so adding extra element on smart-button should not affect the animation.
Attachment #8564906 - Flags: feedback?(suchiu) → feedback+
Thanks everyone.

I've addressed comments and rebased it to latest master.

Gaia-Try result:
https://treeherder.mozilla.org/#/jobs?repo=gaia-try&revision=af33ff46e544
Keywords: checkin-needed
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.
Component: Gaia → Gaia::TV
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Daisuke-san,

This patch makes default apps or decks localizable in Smart-Home. I'll suggest you to pick this patch. Thanks.
Flags: needinfo?(938.daisuke)
Whiteboard: [ft:conndevices] → [ft:conndevices][Stingray-Branch]
Hi Tzu-Lin

Stingray has no UI to change locale on b2g(can change by module out of b2g) and
when locale changed, restart b2g.
So currently no need to introduce your patch.

Thanks
Flags: needinfo?(938.daisuke)
You need to log in before you can comment on or make changes to this bug.