Add localization #2748

mosuem · 2023-12-08T11:24:52Z

Currently still depends on package:flutter_messages, which is developed by me. We should think about the long-term solution here. Also, RTL languages such as arabic reveal some UI bugs (check this by setting the locale to ar).

cc @devoncarew @parlough

I’ve reviewed the contributor guide and applied the relevant portions to this PR.

Contribution guidelines:

See our contributor guide for general expectations for PRs.
Larger or significant changes should be discussed in an issue before creating a PR.
Contributions to our repos should follow the Dart style guide and use dart format.
Most changes should add an entry to the changelog and may need to rev the pubspec package version.
Changes to packages require corresponding tests.

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

devoncarew

Depending on where you are with the new localization packages I think its reasonable to land something like this in DartPad.

We probably also want some text in the README.md, "To regenerate the localization files, run dart run build_runner build --delete-conflicting-outputs.".

pkgs/sketch_pad/pubspec.yaml

devoncarew · 2023-12-11T18:47:48Z

pkgs/sketch_pad/pubspec.yaml


 flutter:
  uses-material-design: true
  assets:
    - assets/dart_logo_128.png
    - assets/flutter_logo_192.png
+    - lib/l10n/en.json


Not for this PR, but we should add some docs to the repo about how to contribute a new localization; this will be a good way to get more people involved.

pkgs/sketch_pad/lib/l10n/en.flutter.g.dart

pkgs/sketch_pad/lib/l10n/en.g.dart

pkgs/sketch_pad/lib/l10n/de.json

devoncarew · 2023-12-11T18:55:35Z

pkgs/sketch_pad/lib/main.dart

@@ -137,6 +138,14 @@ class _DartPadAppState extends State<DartPadApp> {
      routerConfig: router,
      debugShowCheckedModeBanner: false,
      themeMode: themeMode,
+      localizationsDelegates: [...MessagesLocalizations.localizationsDelegates],
+      supportedLocales: const [
+        Locale('en'), // English


Not for this PR, but it may be nice to have some CI checks that this list matches the arbs files available, and the list of json files in the flutter assets section.

I don't know how Flutter currently handles this matching between supportedLocales and actually existing .arb files. @HansMuller?

pkgs/sketch_pad/lib/main.dart

devoncarew · 2023-12-11T20:13:58Z

It looks like the editor overlay buttons should be docked on the right, even for rtl locales?

mosuem · 2023-12-13T16:07:23Z

It looks like the editor overlay buttons should be docked on the right, even for rtl locales?

I don't know how developers with RTL languages as the set locale code - @Manishearth?

Manishearth · 2023-12-13T16:42:16Z

It shouldn't overlap the text, it's a code box and the code is LTR.

If you want to support RTL programming languages it gets a bit more theoretical. Dart has a default LTR direction and should stay that way even when RTL text is used in it, since its keywords are LTR.

mosuem · 2023-12-13T16:43:41Z

It shouldn't overlap the text, it's a code box and the code is LTR.

If you want to support RTL programming languages it gets a bit more theoretical. Dart has a default LTR direction and should stay that way even when RTL text is used in it, since its keywords are LTR.

Thanks! I think it's just about RTL languages. So then always aligning these buttons to the right seems like the correct choice.

devoncarew · 2023-12-13T23:12:31Z

This PR should address the issue w/ the buttons in a rtl locale: #2754.

parlough · 2023-12-14T04:47:53Z

@mosuem This is really exciting, thanks for working on this (and the underlying tooling).

I'm curious if you'd be comfortable with some form of this landing soonish? It doesn't seem super invasive since there's not a lot of strings in the app, and I think we would be fine with making changes as needed, at least for now. That way we can: be better prepared for any upstream/longer-term solution, engage the community with localizations, and have a simple test-bed for at least some portions of your work.

mosuem · 2023-12-14T12:18:34Z

I'm curious if you'd be comfortable with some form of this landing soonish?

Sure! This would first need a button to switch locale at some place - any suggestions where?

devoncarew · 2023-12-14T17:30:03Z

This would first need a button to switch locale at some place - any suggestions where?

Perhaps to the right of the channel selector widget? We do have about as much UI clutter as we want in DartPad's UI - part of its strength is its simple UI.

We could also consider moving the theme toggle and the locale selector to the overflow menu. But we could start with the locale selector on the bottom toolbar and see how well that works.

parlough · 2023-12-14T17:34:37Z

I think a little globe icon without text (but still a semantic label) to the right of the channel selector makes a lot of sense!

If it doesn't work out, we can definitely move it to the overflow menu. I imagine it will be relatively rare to switch.

Side question: Even if we don't add any client analytics, does the hosting setup show the traffic to individual files? If so, we could use the downloads of the different json files as a useful reference.

parlough

As I said, I'm super excited for this. Thanks so much!

I'm all for this landing if @devoncarew is happy, but I have a few questions more about the underlying tooling, nothing blocking this.

Could you make sure each string is configurable and setup for at least German too?

pkgs/sketch_pad/lib/l10n/en.flutter.g.dart

pkgs/sketch_pad/lib/l10n/en.g.dart

pkgs/sketch_pad/lib/l10n/en.json

pkgs/sketch_pad/lib/main.dart

pkgs/sketch_pad/pubspec.yaml

pkgs/sketch_pad/lib/main.dart

devoncarew

I think there's no open questions w/ this PR? lgtm; excited about the change to get code completion on messages ('Reload' ==> messages.reload).

pkgs/sketch_pad/lib/l10n/en.g.dart

Xazin

Some thoughts from a stranger.

Xazin · 2024-02-05T22:36:02Z

pkgs/sketch_pad/README.md

+* Set the `"locale"` key in the file to `ar`.
+* Add the translated strings to the file. Use the descriptions in `en.arb` as a reference.
+* Add the locale to the `supportedLocales` in `main.dart`.
+* Add `lib/ar.json` to the assets in the `pubspec.yaml`


Shouldn't this be

Suggested change

* Add `lib/ar.json` to the assets in the `pubspec.yaml`

* Add `lib/l10n/ar.json` to the assets in the `pubspec.yaml`

Xazin · 2024-02-05T22:48:45Z

pkgs/sketch_pad/lib/main.dart

      if (result.source == value) {
-        appModel.editorStatus.showToast('No formatting changes');
+        appModel.editorStatus
+            // ignore: use_build_context_synchronously
+            .showToast(context.messagesLocalizations!.noFormattingChanges);
      } else {
-        appModel.editorStatus.showToast('Format successful');
+        appModel.editorStatus
+            // ignore: use_build_context_synchronously
+            .showToast(context.messagesLocalizations!.formatSuccesful);
        appModel.sourceCodeController.text = result.source;
      }
    } catch (error) {
-      appModel.editorStatus.showToast('Error formatting code');
+      appModel.editorStatus
+          // ignore: use_build_context_synchronously
+          .showToast(context.messagesLocalizations!.errorFormatting);
      appModel.appendLineToConsole('Formatting issue: $error');


No need to ignore this lint, use the mounted check instead:

String? toastMessage; ... if (result.source == value) { toastMessage = mounted ? context.messagesLocalizations.noFormattingChanges : null; } else { toastMessage = mounted ? context.messagesLocalizations.formatSuccesful : null; appModel.sourceCodeController.text = result.source; } } catch (error) { toastMessage = mounted ? context.messagesLocalizations.errorFormatting : null; appModel.appendLineToConsole('Formatting issue: $error'); } if (toastMessage?.isNotEmpty ?? false) { appModel.editorStatus.showToast(toastMessage); }

Or

if (mounted) { if (result.source == value) { appModel.editorStatus .showToast(context.messagesLocalizations!.noFormattingChanges); } else { appModel.editorStatus .showToast(context.messagesLocalizations!.formatSuccesful); appModel.sourceCodeController.text = result.source; } } } catch (error) { if (mounted) { appModel.editorStatus .showToast(context.messagesLocalizations!.errorFormatting); appModel.appendLineToConsole('Formatting issue: $error'); } }

Xazin · 2024-02-05T22:49:01Z

pkgs/sketch_pad/lib/main.dart

+      appModel.editorStatus
+          // ignore: use_build_context_synchronously
+          .showToast(context.messagesLocalizations!.compilationFailed);


Same here, can use a mounted check instead.

Xazin · 2024-02-05T22:49:54Z

pkgs/sketch_pad/lib/main.dart

-                Text('Privacy notice'),
-                SizedBox(width: denseSpacing),
-                Icon(Icons.launch, size: 16),
+                Text(MessagesLocalizations.of(context)!.privacyNotice),


When to prefer MessageLocalizations.of(context) over context.messageLocalizations?

Xazin · 2024-02-05T22:50:20Z

pkgs/sketch_pad/lib/main.dart

+            // ignore: use_build_context_synchronously
+            _handleSelection(appServices, selection, context);


Mounted check if (selection != null && mounted)

mosuem added 6 commits December 8, 2023 11:33

Add english arb file

c8fa740

Add Flutter localizations

4a41aba

Add test locale

f84ae43

Add localization example

ebd0586

Switch to template

5d01ab4

Make final

58aef90

devoncarew reviewed Dec 11, 2023

View reviewed changes

mosuem mentioned this pull request Dec 13, 2023

Small fixes to package:messages dart-lang/i18n#760

Merged

1 task

devoncarew mentioned this pull request Dec 13, 2023

ensure the run button displays on the RHS in rtl locales #2754

Merged

1 task

Change deps

948bd91

mosuem added 2 commits December 14, 2023 13:28

Regenerate

4852c20

Add readme

3a8333c

mosuem marked this pull request as ready for review December 14, 2023 12:32

mosuem added 2 commits December 14, 2023 13:32

Remove unsupported locales

c52f6e3

Fix readme

ba05735

parlough approved these changes Dec 14, 2023

View reviewed changes

Add button to change locale

3cc8300

mosuem commented Dec 15, 2023

View reviewed changes

pkgs/sketch_pad/lib/main.dart Outdated Show resolved Hide resolved

mosuem added 3 commits January 2, 2024 15:13

Add build verify

fe95aa0

Remove toast

deea7c0

Rev flutter_messages

58ba66b

mosuem and others added 4 commits January 3, 2024 11:43

Merge branch 'main' into addLocalization

77cbabd

Fix

3fa10c6

Fix language selection

4a6497a

Add translations

ba00e09

mosuem mentioned this pull request Jan 3, 2024

Updates to messages dart-lang/i18n#762

Merged

1 task

devoncarew approved these changes Jan 3, 2024

View reviewed changes

devoncarew reviewed Jan 3, 2024

View reviewed changes

pkgs/sketch_pad/lib/l10n/en.g.dart Outdated Show resolved Hide resolved

Update pkgs/sketch_pad/lib/l10n/en.g.dart

dc42857

Xazin reviewed Feb 5, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add localization #2748

Add localization #2748

mosuem commented Dec 8, 2023 •

edited

devoncarew left a comment

devoncarew Dec 11, 2023

devoncarew Dec 11, 2023

mosuem Dec 13, 2023

devoncarew commented Dec 11, 2023

mosuem commented Dec 13, 2023

Manishearth commented Dec 13, 2023

mosuem commented Dec 13, 2023

devoncarew commented Dec 13, 2023

parlough commented Dec 14, 2023 •

edited

mosuem commented Dec 14, 2023 •

edited

devoncarew commented Dec 14, 2023

parlough commented Dec 14, 2023 •

edited

parlough left a comment •

edited

devoncarew left a comment

Xazin left a comment

Xazin Feb 5, 2024

Xazin Feb 5, 2024 •

edited

Xazin Feb 5, 2024

Xazin Feb 5, 2024

Xazin Feb 5, 2024

	* Add `lib/ar.json` to the assets in the `pubspec.yaml`
	* Add `lib/l10n/ar.json` to the assets in the `pubspec.yaml`

		// ignore: use_build_context_synchronously
		_handleSelection(appServices, selection, context);

Add localization #2748

Are you sure you want to change the base?

Add localization #2748

Conversation

mosuem commented Dec 8, 2023 • edited

devoncarew left a comment

Choose a reason for hiding this comment

devoncarew Dec 11, 2023

Choose a reason for hiding this comment

devoncarew Dec 11, 2023

Choose a reason for hiding this comment

mosuem Dec 13, 2023

Choose a reason for hiding this comment

devoncarew commented Dec 11, 2023

mosuem commented Dec 13, 2023

Manishearth commented Dec 13, 2023

mosuem commented Dec 13, 2023

devoncarew commented Dec 13, 2023

parlough commented Dec 14, 2023 • edited

mosuem commented Dec 14, 2023 • edited

devoncarew commented Dec 14, 2023

parlough commented Dec 14, 2023 • edited

parlough left a comment • edited

Choose a reason for hiding this comment

devoncarew left a comment

Choose a reason for hiding this comment

Xazin left a comment

Choose a reason for hiding this comment

Xazin Feb 5, 2024

Choose a reason for hiding this comment

Xazin Feb 5, 2024 • edited

Choose a reason for hiding this comment

Xazin Feb 5, 2024

Choose a reason for hiding this comment

Xazin Feb 5, 2024

Choose a reason for hiding this comment

Xazin Feb 5, 2024

Choose a reason for hiding this comment

mosuem commented Dec 8, 2023 •

edited

parlough commented Dec 14, 2023 •

edited

mosuem commented Dec 14, 2023 •

edited

parlough commented Dec 14, 2023 •

edited

parlough left a comment •

edited

Xazin Feb 5, 2024 •

edited