1032261 - [GaiaMenu] Use locale strings inside of component folder

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Kevin Grandon :kgrandon]

In order to have external components one hurdle we need to solve is localized components. We need to solve this within gaia before we have a hope at externalizing components I believe. Ideally we should be able to ship a set of *.properties l10n files within each component.

Blocking bug 1013840 for now as I'm going to specifically prototype this with the gaia-menu component.

Comment 1

[Kevin Grandon :kgrandon]

Attachment: Packaged properties files with gaia-menu

Hey Stas, Gandalf - 

Could you guys take a look at this and let me know if you are ok with this route? Essentially I think there is value in packaging locale properties files within components. Just wanted to check with you guys to see if you are ok with it, and if it would work from the l10n process.

I think there are potentially some build system changes that we would need to make to get this fully working, but I just wanted to lay out the general thoughts to collect feedback first. Also would you guys know if there are any special steps we would need to take to get these strings into the hands of localizers from this new location?

Comment 2

[Kevin Grandon :kgrandon]

Comment on attachment 8448071 [details] [review]
Packaged properties files with gaia-menu

Also adding Francesco and Axel here as I would rather over-communicate this to make sure we get the necessary eyes on it. Please see comment 1.

Comment 3

[Axel Hecht [:Pike]]

Comment on attachment 8448071 [details] [review]
Packaged properties files with gaia-menu

From a file-path POV, this would work, but there are a things that make me sad.

Mostly, that for every element a page uses, it would need to include the localized strings.

IMHO, we should get to a place where the web component knows how to localize itself, and establishes its own localization context.

I hope that gandalf and stas have something constructive up their sleaves.

Comment 4

[Francesco Lodolo [:flod]]

Comment on attachment 8448071 [details] [review]
Packaged properties files with gaia-menu

(following the discussion, but not really helpful for feedback here)

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8448071 [details] [review]
Packaged properties files with gaia-menu

That's a great first step.

We're working on remodeling our internal API to have lightweight "context" or "bundle" (name tbd) per document. It means that we will have a separate one for main document and for each component.

This will give us ability to isolate web-component specific strings.

With this patch, we will be able to store web-component specific strings in their own properties files and once we get our internal remodel ready, we'll just move to that.

Comment 6

[Kevin Grandon :kgrandon]

Comment on attachment 8448071 [details] [review]
Packaged properties files with gaia-menu

Thanks for the feedback guys. I think I'd like to move forward isolating l10n with web componenhts, which should allow us to do more elegant things with them in the future. Zibi - any chance you would have some time to give this a review?

Comment 7

[Wilson Page [:wilsonpage]]

This is a great first step! :)

1. Have we established if translators are comfortable to start sending pull-requests for strings into external component repos? Are there any strong arguments against this?

2. Components are currently assuming that `navigator.mozL10n` is present without actually explicitly depending on it. If third parties are to start making use of these components, then localization will need to work out-of-the-box.

Comment 8

[Axel Hecht [:Pike]]

(In reply to Wilson Page [:wilsonpage] from comment #7)
> This is a great first step! :)
> 
> 1. Have we established if translators are comfortable to start sending
> pull-requests for strings into external component repos? Are there any
> strong arguments against this?

That's a process that's completely alien to that part of our community. The strings here should just be gaia strings.

We should be very mindful to not run into complexity hell here. Trying to localize a dozen versions of a dozen web components into 100 languages each is not a task we'll win.

Comment 9

[Kevin Grandon :kgrandon]

(In reply to Wilson Page [:wilsonpage] from comment #7)
> This is a great first step! :)
> 2. Components are currently assuming that `navigator.mozL10n` is present
> without actually explicitly depending on it. If third parties are to start
> making use of these components, then localization will need to work
> out-of-the-box.

One of the interesting things that we can do is to simply set the l10n-id and thanks to the mutation observer, I think that's all we need to do. If authors include mozL10n they'll get translations. If not they'll just get the default string. In this case we'll still need to include the default english strings in markup, but I think that may be acceptable for components (we were trying to remove default strings from html files).

Comment 10

[Kevin Grandon :kgrandon]

Comment on attachment 8448071 [details] [review]
Packaged properties files with gaia-menu

Adding Cristian for a review for homescreen pieces, and Yuren for the simple build change. Would you guys have a minute to review this? Thanks!

Comment 11

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8448071 [details] [review]
Packaged properties files with gaia-menu

That looks good. My only concern is that since MO doesn't reach to ShadowRoot (and in the future we will want to have separate MO on component's root) don't you need to manually trigger mozL10n.translateFragment() on the node?

I didn't run the code myself, so if it works without that, I'm definitely happy with the mozL10n API usage here.

Comment 12

[Kevin Grandon :kgrandon]

Comment on attachment 8448071 [details] [review]
Packaged properties files with gaia-menu

(In reply to Zibi Braniecki [:gandalf] from comment #11)
> Comment on attachment 8448071 [details] [review]
> Packaged properties files with gaia-menu
> I didn't run the code myself, so if it works without that, I'm definitely
> happy with the mozL10n API usage here.

Thanks for the review. I will setup a quick test to make sure everything was working, I did a few runs and l10n was working well. Perhaps it was only the initial population though, and not localized events. I'll check before landing. Clearing review on Cristian for now.

Comment 13

[Wilson Page [:wilsonpage]]

I believe we've established that localizers are unable to work out of any other repo than Gaia. So for that reason is there any value in this patch?

Perhaps we should look at external components shipping with just default strings but provided an interface to easily plug-in more advanced l10n solutions within the app.

In reality there are going to be very few web-components that require localization. Most web-component's content will be provided by the user in the form of projected content. It is their job to localize this. I can see there being more `aria-label` type stuff than real content.

We should also talk about when it is/isn't worthwhile making a component external to Gaia. I know myself and Kevin feel differently about this, but I don't see much value in having a component like <gaia-grid> be external. It's very specific to the homescreen, and not very re-usable. <gaia-header>, <gaia-button> or <gaia-checkbox> on the other hand can be used in almost any app, and are highly re-useable.

It is more work to manage an external component, so I'm not sure we should invest that work if re-use is unlikely. Just a gut instinct, prepared to be proven wrong on this :)

Comment 14

[Kevin Grandon :kgrandon]

(In reply to Wilson Page [:wilsonpage] from comment #13)
> I believe we've established that localizers are unable to work out of any
> other repo than Gaia. So for that reason is there any value in this patch?

I think having self-contained strings within the component is very compelling and makes our codebase much cleaner. It may be possible to have external components and cleanly splice together locales and components during a build step, but I think that gets very messy very quickly. Also your external example has the possibility of different versions being used in different apps, how would we handle different versions of shared/locale files if this were the case?


> In reality there are going to be very few web-components that require
> localization.

I still disagree here. I think we should be ready for a lot of components, and dozens that require localization.


> We should also talk about when it is/isn't worthwhile making a component
> external to Gaia. I know myself and Kevin feel differently about this, but I
> don't see much value in having a component like <gaia-grid> be external.
> It's very specific to the homescreen, and not very re-usable. <gaia-header>,
> <gaia-button> or <gaia-checkbox> on the other hand can be used in almost any
> app, and are highly re-useable.

This is interesting, and something worth exploring. Special handling of two different sets of component infrastructure does seem unnecessarily complex to me though. This is another reason why I suggest exporting the basic components from gaia for third party developers to reuse if that is the main goal, which is what happens in the prototype here: https://github.com/gaia-elements.

Comment 15

[Wilson Page [:wilsonpage]]

(In reply to Kevin Grandon :kgrandon from comment #14)
> your external example has the possibility of different versions being used
> in different apps, how would we handle different versions of shared/locale
> files if this were the case?

Locale files are already in a way 'versioned'. When strings are changed or added we have to use a new key. This is so that discrepancies can be spotted in other locales and updated.

In an ideal world I would like the localization to be shipped with components, but the complexity of the l10n workflow seems to permit this.

> This is interesting, and something worth exploring. Special handling of two
> different sets of component infrastructure does seem unnecessarily complex
> to me though. This is another reason why I suggest exporting the basic
> components from gaia for third party developers to reuse if that is the main
> goal, which is what happens in the prototype here:
> https://github.com/gaia-elements.

Although exporting the components out of Gaia does make them slightly more accessible to third-party developers, it's a bit broken. Third-party devs (Mozilla or not) can't contribute back to these repositories and we lose all the versioning benefits we would have gained from importing external components into Gaia.

We will still have 20+ apps depending on HEAD of each component. When any Gaia dev makes a significant change to any component, they will have to check 20+ apps to confirm they haven't broken anything. As an application owner this is a risk! It is this reason alone that myself and my team use `shared/` as an absolute last resort. We want control over our application and to mitigate risk.

I want these shared components to succeed. The current `shared/` setup makes it dead easy to make changes to components sometimes  this is a good thing, but often it is bad. Moving to external repos and versioning components is more work, but we gain stability, control, explicit dependency management, openness and external contributions. IMO a small price to pay for a shed load of greatness!

We should test the water and be prepared to try a few approaches before we settle on something that works best for everyone.

Comment 16

[Axel Hecht [:Pike]]

I disagree, I don't see how using 15 different versions of a piece of code across 20 different apps is going to get us to a release-worthy piece of software.

CCing relmans to see how they feel about herding those cats, and if they have a plan for this.

Comment 17

[Wilson Page [:wilsonpage]]

(In reply to Axel Hecht [:Pike] from comment #16)
> I disagree, I don't see how using 15 different versions of a piece of code
> across 20 different apps is going to get us to a release-worthy piece of
> software.

The point is that apps 'pull' the updates when they're ready, and these updates aren't 'pushed' upon them. We would never ship a release if apps didn't look visually consistent. If you haven't worked from the component author perspective, you probably aren't aware of how difficult if is to make changes to code shared by 20+ apps. It's stifling our progression and innovation.

Comment 18

[Kevin Grandon :kgrandon]

If we have to make these changes anyway to *all* apps for a release, I think this is just increase in work over a release instead of being something manageable. 

If in production we will only have one version, it seems to defeat the purpose of having versions in the first place. What happens when 10 apps upgrade, 10 apps are lagging, and our release deadline is coming - do we backout the other 10 apps? I think we will be in a much worse place than we are today. If it is purely an implementation workflow thing, there are ways to solve that outside of versioning that we have not yet explored.

Comment 19

[Kevin Grandon :kgrandon]

Since I've been asked to land bug 1037864, I will land this soon as there is a dependency on this bug.

Regarding of where the actual component lives, I'm still open to externalizing things depending on the feedback we get for the 2.1 plan.

Comment 20

[Kevin Grandon :kgrandon]

In master: https://github.com/mozilla-b2g/gaia/commit/47c39cf470ce1a6cc7ace4de4a3e5465ae9e1511