Set up basic i18n framework

[sirpengi]

Set up flutter_localizations integration and add initial test bundle of ARB files (with a few strings in English and Japanese) to support some strings in various contexts. The bindings are autogenerated on flutter run.

Fixes #275

---

Overall context

Per the discussion in #275 the approach here is to start with ARB files and have dart autogenerate the bindings. I believe this is the most straightforward way when connecting with a translation management system, as they output ARB files that we consume (this is also the same way web and mobile works but with .po or .json files, I believe).

The autogeneration is done automatically in flutter run, although one could manually run it using flutter gen-l10n.

Using in code

To utilize in our widgets you need to import the generated bindings:

import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';

And in your widget code pull the localizations out of the context:

  Widget build(BuildContext context) {
    final zulipLocalizations = ZulipLocalizations.of(context);

And finally access one of the generated properties: Text(zulipLocalizations.chooseAccountButtonAddAnAccount).

String that take placeholders are generated as functions that take arguments: zulipLocalizations.subscribedToNStreams(store.subscriptions.length)

Hack to enforce locale (for testing, etc)

To manually trigger a locale change for testing I've found it helpful to add the localeResolutionCallback in app.dart to enforce a particular locale:

    return GlobalStoreWidget(
      child: MaterialApp(
        title: 'Zulip',
        localizationsDelegates: ZulipLocalizations.localizationsDelegates,
        supportedLocales: ZulipLocalizations.supportedLocales,
        localeResolutionCallback: (locale, supportedLocales) {
          return const Locale("ja");
        },
        theme: theme,
        home: const ChooseAccountPage()));

(careful that returning a locale not in supportedLocales will crash, the default behavior ensures a fallback is always selected)

Tests

Widgets that access localization will fail if the root MaterialApp given in the setup isn't also set up with localizations. Make sure to add the right localizationDelegates and supportedLocales:

  await tester.pumpWidget(
    GlobalStoreWidget(
      child: MaterialApp(
        navigatorObservers: navigatorObserver != null ? [navigatorObserver] : [],
        localizationsDelegates: ZulipLocalizations.localizationsDelegates,
        supportedLocales: ZulipLocalizations.supportedLocales,
        home: PerAccountStoreWidget(

Screenshots

[gnprice]

Great! Thanks @sirpengi for setting this up.

I ran this on an Android phone, and then went and changed the system settings to make Japanese the preferred language, and the app automatically switched to using these Japanese translations. So that logic is working correctly.

I'm still reading through this and exploring, but some initial comments below.

[gnprice]

Let's put these somewhere outside of lib/, so that it's in a different subdirectory from our main source code.

In zulip-mobile we used to have the translation data inside the main source subdirectory (src/ there), and it was regularly annoying when grepping the code. If there's even one string that matches whatever you're searching for, you'll get a match in every one of the translations… and because we are fortunate to have people contributing translations for over 50 languages, that can easily fill one's screen. So I was very happy to get them out of there, in zulip/zulip-mobile@1fc26a5.

Perhaps assets/l10n/? Or assets/translations/?

[sirpengi]

I was just following the guide (their example config used lib/l10n) but I understand the argument here. I've also kept django templates out of module subdirectories for this very reason. I'll move to assets/l10n

[gnprice]

branch-structure nit: Let's add each of these translations to this .arb file in the same commit where we start using it. That seems like it'll make that commit have more complete context — the string and the use of it complement each other for understanding what's happening.

I think it'd be fine for the first commit to just introduce empty .arb files, and then the next commit can fill them in.

[gnprice]

What does the process look like when adding a new translated string?

I imagine in particular it includes:

• add something in app_en.arb
• ?? rerun code generation
• use the resulting getter or method in app code

Is it necessary to also add something to all the other .arb files, or will things fall back smoothly until we have a translation?

What's the most convenient way to get the generated code to refresh after adding to a .arb file?

[sirpengi]

flutter gen-l10n will recreate the bindings and my IDE picks it up after that straightaway. The generate command also runs automatically as part of flutter run so builds will show the most up to date strings. I only need to run flutter gen-l10n if I need my IDE to autocomplete a newly added string.

I've tested with missing strings and the situation there is it will use a fallback string. The generate command (or run) will then output a message alerting you that strings are missing:

bash-3.2$ fvm flutter gen-l10n
Because l10n.yaml exists, the options defined there will be used instead.
To use the command line arguments, delete the l10n.yaml file in the Flutter project.


"ja": 1 untranslated message(s).
To see a detailed report, use the untranslated-messages-file
option in the l10n.yaml file:
untranslated-messages-file: desiredFileName.txt
<other option>: <other selection>


This will generate a JSON format file containing all messages that
need to be translated.

It looks like we can get the untranslated messages into a JSON file using the untranslated-messages-file option, which I presume we somehow upload to the translation management service. I didn't add this setting or look into it as I believe it was out of scope for this issue.

[gnprice]

OK, finished studying this; here's a few more comments.

[gnprice]

Suggested change

	arb-dir: lib/l10n
	# Docs on this config file:
	# https://docs.flutter.dev/ui/accessibility-and-localization/internationalization#configuring-the-l10nyaml-file
	arb-dir: lib/l10n

That'll make it a bit smoother whenever we're adjusting these settings. (I came across that documentation because it's on the same page as the tutorial linked from #275, and I was rereading that.)

[gnprice]

From reading through those docs on this config file, one other option looks like it'd be good to add:

Suggested change

	template-arb-file: app_en.arb
	template-arb-file: app_en.arb
	required-resource-attributes: true

Requires all resource ids to contain a corresponding resource attribute.

By default, simple messages won’t require metadata, but it’s highly recommended as this provides context for the meaning of a message to readers.

We can always relent on that if we decide it's a pain. But my guess is that it won't actually be much of a burden to write a few words about what the message is for, at the time when we're adding it and it's right in front of us. (And I see you've consistently done that in the demo messages in this PR.)

[gnprice]

Mildly annoying that we have to do this, but oh well.

It does at least look like this doesn't introduce nondeterminism to the tests. I tried running them with a Japanese ambient locale:

$ LC_ALL=ja_JP.utf8 flutter test test/widgets/profile_test.dart

and they passed even though I'd added a check for the English version of a translated string:

--- a/test/widgets/profile_test.dart
+++ b/test/widgets/profile_test.dart
@@ -84,6 +84,7 @@ void main() {
 
       check(because: 'find user avatar', find.byType(Avatar).evaluate()).length.equals(1);
       check(because: 'find user name', find.text('test user').evaluate()).isNotEmpty();
+      check(find.text('Member').evaluate()).isNotEmpty();
     });
 
     testWidgets('page builds; profile page renders with profileData', (WidgetTester tester) async {

[sirpengi]

I did test various situations on a device (the phone with different supported locales, or using a primary locale not in the supported list) and it seemed to behave nicely (no crashing, reverts to English - although when I look through the locale selection code it seems to try to find adjacent languages or languages from the same country). I didn't think to see if the ambient locale affected the tests!

But, I'm not sure if flutter is picking up on that locale setting. If I do LC_ALL=ja_JP.utf8 flutter run it doesn't appear to have picked up the locale.

[gnprice]

But, I'm not sure if flutter is picking up on that locale setting.

If flutter test doesn't pick up on that setting, then that's great — what I was going for here was just that the tests won't behave differently on different people's development machines on account of their language settings.

If I do LC_ALL=ja_JP.utf8 flutter run it doesn't appear to have picked up the locale.

Yeah, that seems appropriate — the app should be using the locale configured on the target device, not on the development host. … I guess it's less clear what it should do in the desktop case, though, where those are the same, and that's likely the scenario you were trying.

[sirpengi]

@gnprice the changes have been incorporated and we're ready for another round!

[gnprice]

Thanks for the revision!

This code all now looks good.

Following up on #302 (comment) above, I think the one remaining thing I'd like before we merge this is to have a clear picture of what our workflow will look like as we continue adding new areas of UI that have new strings of their own. Specifically:

• Let's have a commit here that adds one or two strings (replacing existing hard-coded English strings) and doesn't add translations for them, only the English values. That'll be our normal workflow, so it'll make a helpful demo.
• Let's add a bit of contributor-facing documentation in a file docs/translation.md, with instructions for what one should do when adding a string in the UI. This information is basically present in your comment Set up basic i18n framework #302 (comment) plus the PR description, so this is a matter of putting it in one place and in the form of step-by-step instructions focused on that task.

Relatedly:

I've tested with missing strings and the situation there is it will use a fallback string. The generate command (or run) will then output a message alerting you that strings are missing:

bash-3.2$ fvm flutter gen-l10n
Because l10n.yaml exists, the options defined there will be used instead.
To use the command line arguments, delete the l10n.yaml file in the Flutter project.


"ja": 1 untranslated message(s).
To see a detailed report, use the untranslated-messages-file
option in the l10n.yaml file:
untranslated-messages-file: desiredFileName.txt
<other option>: <other selection>


This will generate a JSON format file containing all messages that
need to be translated.

That looks like an annoying amount of noise for flutter run to be spitting out. (And when I try it in my IDE now, I actually see several copies of that noise). I think having untranslated strings is going to be our steady state for the near term, after we sweep through to the new way of writing the code (#277) and before we wire up to Transifex or a competitor (#276). I think most likely in the long term that will also be the norm within a development branch that adds new strings in the UI, until it's merged and one of us maintainers runs the workflow to sync it with the #276 translation platform; that's been the case in zulip-mobile, anyway. So we don't want it to cause a loud warning.

At a quick test, if we set untranslated-messages-file then that warning message disappears — effectively the tool seems to consider that by writing the file it's gotten that message across. So let's set it to some file, and then make sure the file is ignored with .gitignore.

[sirpengi]

@gnprice ready again!

[gnprice]

Thanks! Merging, with an extra commit at the end making an editing pass over those handy new docs; please take a look.

(Also fixed a nit in the first commit message, per https://github.com/zulip/zulip-mobile/blob/main/docs/style.md#fixes-format :

$ git range-diff origin pr/302 @
1:  160bccfd2 ! 1:  c88b99052 intl: Set up basic i18n framework
    @@ Commit message
         Set up `flutter_localizations` integration and blank
         test bundle of ARB files.
     
    -    Fixes #275
    +    Fixes: #275
[…]

)