1124098 - Hide the Get More Languages link in devices other than phones

Status: RESOLVED FIXED

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

Description

[Staś Małolepszy :stas]

In bug 1115798 we're introducing a Get More Languages link in the Settings' Language panel as part of the UI implementation for language packages.

Since device types are a buildtime option, in their initial form langpacks won't contain translation strings for all devices.  In 2.2 we'll only have langpacks for phones.  Consequently we need a way to hide the Get More Languages link on other device types.

Comment 1

[Staś Małolepszy :stas]

One solution would be to extend the logic in https://github.com/stasm/gaia/blob/332f20c80ce79e91a05d0e7f9d6014c35b61e57b/build/webapp-optimize.js#L489-L500 to also remove other element types than <link>s.

A less invasive alternative would be to use device-specific CSS to show and hide the link.

Arthur, do you have a preference?

Comment 2

[Arthur Chen [:arthurcc][inactive after 6/30]]

device-specific CSS sounds a good option! Settings app uses templates so it would required extra effort in addition to the change of optimizeDeviceTypeCSS if we want data-device-type to be supported in a template.

Comment 3

[Staś Małolepszy :stas]

Attachment: Pull request

Hey Arthur, would you mind taking a look?

Comment 4

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8553651 [details] [review]
Pull request

We have to hide the <li> or there is an empty space left.

Comment 5

[Staś Małolepszy :stas]

Comment on attachment 8553651 [details] [review]
Pull request

I updated the PR to account for the blank space in <li>.

Comment 6

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8553651 [details] [review]
Pull request

r=me, thanks!

And I was wondering when will the marketplace app start to handle "marketplace-langpacks" activities? Is there any bug tracking it? If it will not be landed any soon, it would be better to hide the menu item or it feels like broken.

Comment 7

[Staś Małolepszy :stas]

Master: https://github.com/mozilla-b2g/gaia/commit/060ca176afb5479291f2a87e422e8f04fedfbf12

Comment 8

[Staś Małolepszy :stas]

(In reply to Arthur Chen [:arthurcc] from comment #6)

> And I was wondering when will the marketplace app start to handle
> "marketplace-langpacks" activities? Is there any bug tracking it? If it will
> not be landed any soon, it would be better to hide the menu item or it feels
> like broken.

Good point. I think the Marketplace team is close to finishing the implementation but I'm not sure when it'll be available in production. I'll ask and if needed, file a new bug to hide the link for a little while.

Comment 9

[Staś Małolepszy :stas]

Comment on attachment 8553651 [details] [review]
Pull request

[Approval Request Comment]

[Bug caused by] (feature/regressing bug #):  bug 1115798

[User impact] if declined:  the "Get More Languages" link will be visible on all devices, while langpacks are only going to be offered to phones initially.

[Testing completed]: on the device

[Risk to taking this patch] (and alternatives if risky):  low

[String changes made]: n/a

Comment 10

[Ryan VanderMeulen [:RyanVM]]

AFAICT, this depends on Bug 1115798, which isn't on v2.2?

Comment 11

[Staś Małolepszy :stas]

Yes, I must have forgotten to request the approval for bug 1115798.  Thanks, Ryan.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

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