Support language settings #1513

PIG208 · 2025-05-16T00:56:46Z

Screenshots

settings	languages

Since we don't have enough translated strings for RTL languages, the following screenshots are with Arabic added temporarily.

diff --git a/lib/model/localizations.dart b/lib/model/localizations.dart
index 8644ad0ec..e2ef0ddf0 100644
--- a/lib/model/localizations.dart
+++ b/lib/model/localizations.dart
@@ -49,6 +49,7 @@ final kSelfnamesByLocale = <Locale, String>{
   Locale('pl'): 'Polski',
   Locale('ru'): 'Русский',
   Locale('uk'): 'Українська',
+  Locale('ar'): 'العربية',
 };
 
 extension ZulipLocalizationsHelper on ZulipLocalizations {
@@ -69,6 +70,8 @@ extension ZulipLocalizationsHelper on ZulipLocalizations {
         return languageRu;
       case 'uk':
         return languageUk;
+      case 'ar':
+        return 'Arabic';
       default:
         throw ArgumentError.value(locale, 'locale');
     }

LTR	RTL settings	RTL languages

Notice that the selfnames are always shown in their own locale.

preview documentation change

Fixes: #1139

chrisbobbe · 2025-05-21T21:21:55Z

When I rebase atop main, there's some failures at the first commit:

$ tools/check analyze l10n
Running analyze...
Analyzing zulip-flutter...                                              

  error • Missing concrete implementations of 'getter abstract class
         ZulipLocalizations.languageEn', 'getter abstract class ZulipLocalizations.languagePl',
         'getter abstract class ZulipLocalizations.languageRu', and 'getter abstract class
         ZulipLocalizations.languageUk' • lib/generated/l10n/zulip_localizations_de.dart:8:7 •
         non_abstract_class_inherits_abstract_member

1 issue found. (ran in 3.0s)
Running l10n...
Error: there were updates to l10n:
 M lib/generated/l10n/zulip_localizations_de.dart

FAILED: analyze l10n

To rerun the suites that failed, run:
  $ tools/check analyze l10n

Could you rebase and fix those please?

PIG208 · 2025-05-21T21:31:18Z

Thanks. Just updated the PR!

chrisbobbe

Thanks, this is an exciting feature!!

Small comments below.

And a question: what should happen, and what happens, if the language setting in the database isn't one of the languages we offer in the settings page? I think this could happen if we remove a language setting (e.g. if we become more strict about when to offer a language in the UI) or we change the language tag for a set of translations. If we need to do a database migration, let's document that, or perhaps the app can just notice the inconsistency and unset the setting.

chrisbobbe · 2025-05-21T23:47:30Z

docs/translation.md

+ARB files for new languages are automatically created in Pull Requests generated
+[the update-translations GitHub workflow](/.github/workflows/update-translations.yml).
+However, this won't make them appear in settings.


nits:

Is the first sentence missing a word? ("generated by the […]")

I would say either "PRs" or "pull requests" (not capitalized if spelled out)

Instead of "appear in settings", how about "appear in the in-app settings UI", for explicitness.

chrisbobbe · 2025-05-21T23:48:36Z

docs/translation.md

+When a language has a good percentage of strings translated, we should add it to
+settings.  First, if the language tag is 'en-GB', [add a string](#add-string)
+named 'languageNameEnGb'.
+
+Then, update [localizations.dart](/lib/model/localizations.dart) to include
+the new language in `kSelfnamesByLocale` and `localeDisplayName`, following
+the instructions there.


I think these two instructions would read more easily in a numbered or bulleted list.

chrisbobbe · 2025-05-22T00:01:02Z

docs/translation.md

+[the update-translations GitHub workflow](/.github/workflows/update-translations.yml).
+However, this won't make them appear in settings.
+
+When a language has a good percentage of strings translated, we should add it to


Does Weblate show the percentage? Let's try to match what zulip-mobile uses for a threshold; I think it might be 5%?

chrisbobbe · 2025-05-22T00:02:19Z

lib/model/localizations.dart

+/// [ZulipLocalizations.supportedLocales] can include languages that only have
+/// a few strings translated.  This map should only inlcude sufficiently
+/// translated locales from [ZulipLocalizations.supportedLocales].


Suggested change

/// [ZulipLocalizations.supportedLocales] can include languages that only have

/// a few strings translated. This map should only inlcude sufficiently

/// translated locales from [ZulipLocalizations.supportedLocales].

/// This map only includes some of [ZulipLocalizations.supportedLocales];

/// it includes languages that have substantially complete translations.

/// For what counts as substantially translated, see docs/translation.md.

(and then make sure docs/translation.md answers the question)

chrisbobbe · 2025-05-22T00:05:54Z

lib/model/localizations.dart

+/// The map should be sorted by selfname, to help users find their
+/// language in the UI.  When in doubt how to sort (like between different
+/// scripts, or in scripts you don't know), try to match the order found in
+/// other UIs, like for choosing a language in your phone's system settings.


Does a map have an order? Maybe we should hard-code a List<(Locale, String)>, which is definitely ordered, and compute kSelfnamesByLocale from that?

And indeed the commit message mentions making a list, but I guess the commit doesn't actually do that 🙂:

i18n: Maintain a list of supported languages with metadata

chrisbobbe · 2025-05-22T00:11:48Z

lib/model/localizations.dart

+      case 'en':
+        return languageEn;
+      case 'pl':
+        return languagePl;
+      case 'ru':
+        return languageRu;
+      case 'uk':
+        return languageUk;
+      default:
+        throw ArgumentError.value(locale, 'locale');


Ah (following from my comment above): or we have a list where each entry contains the language tag, Locale, and languageEn/etc.? And from that list, we generate kSelfnamesByLocale and a map to support this localeDisplayName method?

chrisbobbe · 2025-05-22T00:14:34Z

lib/model/database.dart

+  // TODO(upstream): support Locale.fromLanguageTag:
+  //   https://github.com/flutter/flutter/issues/143491


Let's remember to add this to

☂️ Upstream Flutter issues affecting Zulip #1077

This method looks like it's basically an implementation of the desired upstream feature — is that right?

In that case let's keep the TODO but it can instead read like:

// TODO(upstream): send this as a factory Locale.fromLanguageTag: // https://github.com/flutter/flutter/issues/143491

and no need to add it to that umbrella list — the issue isn't really affecting us, it's just an opportunity to send something useful upstream 🙂

chrisbobbe · 2025-05-22T00:20:53Z

lib/model/database.dart

+  ///    the method implements a subset of this EBNF grammar.
+  // TODO(upstream): support Locale.fromLanguageTag:
+  //   https://github.com/flutter/flutter/issues/143491
+  Locale _fromLanguageTag(String languageTag) {


Could you add a comment saying how you produced this implementation? I'm not sure if it was written from scratch looking at a spec, or if it's a simplified version of something in Locale.fromLanguageTag, etc.

This was written from scratch to support parsing Unicode Language Identifier, whose grammar is defined here:
https://www.unicode.org/reports/tr35/#Unicode_language_identifier

How about this:

/// Parse a Unicode BCP 47 Language Identifier into [Locale]. /// /// Throw when it fails to convert [languageTag] into a [Locale]. /// /// This supports parsing a Unicode Language Identifier returned from /// [Locale.toLanguageTag]. /// /// This implementation refers to a part of /// [this EBNF grammar](https://www.unicode.org/reports/tr35/#Unicode_language_identifier), /// assuming the identifier is valid without /// [unicode_variant_subtag](https://www.unicode.org/reports/tr35/#unicode_variant_subtag). /// /// This doesn't check if the [languageTag] is a valid identifier, (i.e., when /// this returns without errors, the identifier is not necessarily /// syntactically well-formed or valid).

chrisbobbe · 2025-05-22T00:28:18Z

lib/widgets/settings.dart

@@ -17,17 +21,28 @@ class SettingsPage extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    final zulipLocalizations = ZulipLocalizations.of(context);
+
+    Widget? subtitle;


As a variable in SettingsPage.build, this looks like it should have a more specific name—it's not the page's subtitle, but the subtitle for something on a specific item in the page.

PIG208 · 2025-05-22T01:20:19Z

And a question: what should happen, and what happens, if the language setting in the database isn't one of the languages we offer in the settings page? I think this could happen if we remove a language setting (e.g. if we become more strict about when to offer a language in the UI) or we change the language tag for a set of translations. If we need to do a database migration, let's document that, or perhaps the app can just notice the inconsistency and unset the setting.

Yeah, that's a good point. In case of removal without replacement, we will just assume the language is not set, until the user chooses one from the settings page — that's the scenario the test "handle unsupported (but valid) locale stored in database" covers.

When we replace a language tag with another, e.g. replacing zh-Hant with zh-Hant-TW we can probably maintain maps of names we have used before for each language tag, and write back its latest form to the database if zh-Hant is used.

I'm not sure how common it is going to be for us to repurpose a language tag like zh-Hant when it goes out of favor. When that happens, we can probably just bump the schema version and write a migration for it, without actually updating the database schema.

This is similar to what we did in zulip-mobile: https://github.com/zulip/zulip-mobile/blob/91f5c3289/src/settings/languages.js The difference here being that the display names come from a live ZulipLocalizations instance, so we can't just hardcode the list with literal strings.

An alternative to implementing _fromLanguageTag is using the LocaleParser from package:intl/locale.dart. In this case though, the values to parse always come from `toLanguageTag`, so we can utilize that knowledge to get away with a more simple implementation, that should also be handy upstream. The locale stored in the database is not guaranteed to be one of our supported value, especially if we need to rename locales in the future. See discussion: zulip#1513 (comment)

This doesn't really have user-facing changes yet, because they cannot set the language. In this case, ZulipApp.locale is null, and localization will follow system setting, like we did before this change. This behavior is an improvement compared to the legacy app, which just uses English (en) when language is not set.

Since there is no Figma design for the settings page yet, the design is kept simple while mostly matching zulip-mobile: we show both selfname and name of each available language option, and leave out the search funtionality. We don't allow unsetting the language once it is set, but that can easily change. Fixes: zulip#1139

PIG208 · 2025-05-22T23:06:48Z

Thanks for the review! I have updated the PR and left a TODO for migrations. I think the current behavior should be forward-compatible, and we can add migrations or locale-alias mappings later on, if needed.

gnprice · 2025-05-24T00:36:06Z

to repurpose a language tag like zh-Hant when it goes out of favor

I expect it will never happen that we take a language tag which used to mean one thing, and later re-use the same tag to mean a different thing. That's because these tags follow an Internet-wide standard (https://en.wikipedia.org/wiki/IETF_language_tag); and although a migration to free up an old name is an option for our app's local database, it's not an option for such a distributed standard. So those standard tags will never reuse an old name with a new meaning.

PIG208 force-pushed the pr-lang branch 2 times, most recently from 67e995b to fa118d1 Compare May 16, 2025 21:40

PIG208 marked this pull request as ready for review May 16, 2025 21:43

PIG208 requested a review from chrisbobbe May 16, 2025 21:50

PIG208 assigned chrisbobbe May 16, 2025

PIG208 added the maintainer review PR ready for review by Zulip maintainers label May 16, 2025

PIG208 force-pushed the pr-lang branch 2 times, most recently from 28e2263 to c62f77f Compare May 16, 2025 21:55

PIG208 force-pushed the pr-lang branch from c62f77f to 9175db3 Compare May 21, 2025 21:30

chrisbobbe reviewed May 22, 2025

View reviewed changes

PIG208 added 4 commits May 22, 2025 18:02

PIG208 force-pushed the pr-lang branch from 9175db3 to bdd7d9f Compare May 22, 2025 23:05

gnprice mentioned this pull request May 29, 2025

Add language setting #1461

Closed

		// TODO(upstream): support Locale.fromLanguageTag:
		// https://github.com/flutter/flutter/issues/143491

Support language settings #1513

Are you sure you want to change the base?

Support language settings #1513

Uh oh!

Conversation

PIG208 commented May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrisbobbe commented May 21, 2025

Uh oh!

PIG208 commented May 21, 2025

Uh oh!

chrisbobbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PIG208 commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PIG208 commented May 22, 2025

Uh oh!

gnprice commented May 24, 2025

Uh oh!

Uh oh!

PIG208 commented May 16, 2025 •

edited

Loading

PIG208 commented May 22, 2025 •

edited

Loading