Skip to content
This repository has been archived by the owner on Feb 9, 2024. It is now read-only.

Setup Internationalization #26

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Setup Internationalization #26

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a154c3a
Setup Internationalization
Nov 30, 2023
98af20b
Fix localization
Dec 1, 2023
3890f38
Merge intro text paragraphs
Dec 6, 2023
b21c989
Stop using localization outside materialApp + fix error in README
Dec 6, 2023
eb038ae
Appease linter
Dec 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions l10n.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
arb-dir: lib/l10n
template-arb-file: app_en.arb
output-localization-file: app_localizations.dart
nullable-getter: false
rh-concordium marked this conversation as resolved.
Show resolved Hide resolved
10 changes: 10 additions & 0 deletions lib/l10n/app_en.arb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"initializing": "Initializing...",
Copy link

@ghost ghost Dec 5, 2023 •
edited by ghost
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I seems like this file could quite fast get messy if we don’t have some structure.

@TouHouNo or @prinshamlet do you have any good advice or experience on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably good to have some convention in the names for the structure that includes "module names", like using "intro_page__text_name" 🤔
@rh-concordium might also have some insight on this.

Copy link
Contributor

@rh-concordium rh-concordium Dec 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I would suggest grouping related texts keys according to functionality using dot notation. E.g., "onboarding.intro_text", "onboarding.terms_and_conditions", etc. Then you could use additional additional dots if you want to further subgroup your functionality/texts.

I wouldn't use dots for general terms like "Ok", "Continue", etc, though

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the dot notation, but it seems impossible with the current setup.

Generating synthetic localizations package failed with 1 error:

Exception: Invalid ARB resource name "terms_and_conditions.before_you_begin" in LocalFile:
'/home/hjort/Projects/concordium-wallet/lib/l10n/app_en.arb'.
Resources names must be valid Dart method names: they have to be camel case, cannot start with a number or
underscore, and cannot contain non-alphanumeric characters.

We can do camelCase with underscore as the separator: onboarding_termsAndConditions? 🤔

"cont": "Continue",
"before_you_begin": "Before you begin",
"intro_text0": "Before you start using the Concordium Mobile Wallet, you have to set up a passcode and optionally biometrics.",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use more descriptive names instead of _0 and _1?

Copy link
Contributor Author

@shjortConcordium shjortConcordium Dec 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm not sure there are more descriptive names, it's just one text split into two paragraphs.
I think the right thing is merging them? 🤔
(I've tried doing that)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicking: Is "cont" a common abreviation for "Continue" at concordium? Otherwise, I would just use the full name as the key for readability

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not a common abbreviation.
I did it because both Continue and continue are "bad" keys. Continue is rejected by the generator and continue causes compile issues because it is a keyword.
Maybe there is a better solution though? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PS: In my old job we were using string xml files, which is why we could get away with dots and we could use whatever keys we liked (and the reason we were using string xml files was that we had a huge language file already 5000+ words that was shared with other native apps)

"intro_text1": "It is very important that you keep your passcode safe, because it is the only way to access your accounts. Concordium is not able to change your passcode or help you unlock your wallet if you lose your passcode.",
"read_and_agree": "I have read and agree to the ",
"terms_and_conditions": "Terms and Conditions v{v}",
"previously_accepted": " (you previously accepted v{v})."
}
20 changes: 12 additions & 8 deletions lib/main.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import 'package:concordium_wallet/theme.dart';
import 'package:concordium_wallet/types/future_value.dart';
import 'package:flutter/material.dart';
import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:flutter_gen/gen_l10n/app_localizations.dart';

void main() {
runApp(const App());
Expand Down Expand Up @@ -55,10 +56,13 @@ class App extends StatelessWidget {
child: _WithSelectedNetwork(
initialNetwork: initialNetwork,
child: _WithTermsAndConditionAcceptance(
child: MaterialApp(
routes: appRoutes,
theme: concordiumTheme(),
)),
child: MaterialApp(
localizationsDelegates: AppLocalizations.localizationsDelegates,
supportedLocales: AppLocalizations.supportedLocales,
routes: appRoutes,
theme: concordiumTheme(),
),
),
),
);
}
Expand Down Expand Up @@ -196,14 +200,14 @@ class _Initializing extends StatelessWidget {

@override
Widget build(BuildContext context) {
return const Column(
return Column(
mainAxisAlignment: MainAxisAlignment.center,
crossAxisAlignment: CrossAxisAlignment.center,
children: [
CircularProgressIndicator(),
SizedBox(height: 16),
const CircularProgressIndicator(),
const SizedBox(height: 16),
// Setting text direction is required because we're outside 'MaterialApp' widget.
Text('Initializing...', textDirection: TextDirection.ltr),
Text(AppLocalizations.of(context).initializing, textDirection: TextDirection.ltr),
],
);
}
Expand Down
20 changes: 10 additions & 10 deletions lib/screens/terms_and_conditions/screen.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import 'package:concordium_wallet/state/terms_and_conditions.dart';
import 'package:flutter/material.dart';
import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:flutter_svg/flutter_svg.dart';
import 'package:flutter_gen/gen_l10n/app_localizations.dart';

class TermsAndConditionsScreen extends StatefulWidget {
final TermsAndConditions validTermsAndConditions;
Expand Down Expand Up @@ -53,18 +54,17 @@ class _TermsAndConditionsScreenState extends State<TermsAndConditionsScreen> {
const SizedBox(height: 24),
Center(
child: Text(
'Before you begin',
AppLocalizations.of(context).before_you_begin,
style: Theme.of(context).textTheme.displaySmall,
),
),
Container(
padding: const EdgeInsets.all(16),
child: const Column(
child: Column(
children: [
Text('Before you start using the Concordium Mobile Wallet, you have to set up a passcode and optionally biometrics.'),
SizedBox(height: 9),
Text(
'It is very important that you keep your passcode safe, because it is the only way to access your accounts. Concordium is not able to change your passcode or help you unlock your wallet if you lose your passcode.'),
Text(AppLocalizations.of(context).intro_text0),
const SizedBox(height: 9),
Text(AppLocalizations.of(context).intro_text1),
],
),
),
Expand All @@ -84,9 +84,9 @@ class _TermsAndConditionsScreenState extends State<TermsAndConditionsScreen> {
text: TextSpan(
style: Theme.of(context).textTheme.bodySmall,
children: [
const TextSpan(text: 'I have read and agree to the '),
TextSpan(text: AppLocalizations.of(context).read_and_agree),
TextSpan(
text: 'Terms and Conditions v${widget.validTermsAndConditions.version}',
text: AppLocalizations.of(context).terms_and_conditions(widget.validTermsAndConditions.version),
style: const TextStyle(
color: Colors.indigo,
fontWeight: FontWeight.bold,
Expand All @@ -95,7 +95,7 @@ class _TermsAndConditionsScreenState extends State<TermsAndConditionsScreen> {
switch (widget.acceptedTermsAndConditionsVersion) {
null => const TextSpan(text: '.'),
String v => TextSpan(
text: ' (you previously accepted v$v).',
text: AppLocalizations.of(context).previously_accepted(v),
style: Theme.of(context).textTheme.bodySmall,
),
},
Expand All @@ -113,7 +113,7 @@ class _TermsAndConditionsScreenState extends State<TermsAndConditionsScreen> {
const SizedBox(height: 9),
ElevatedButton(
onPressed: _onAcceptButtonPressed(context),
child: const Text('Continue'),
child: Text(AppLocalizations.of(context).cont),
),
],
),
Expand Down
13 changes: 13 additions & 0 deletions pubspec.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,11 @@ packages:
url: "https://pub.dev"
source: hosted
version: "2.0.3"
flutter_localizations:
dependency: "direct main"
description: flutter
source: sdk
version: "0.0.0"
flutter_svg:
dependency: "direct main"
description:
Expand Down Expand Up @@ -407,6 +412,14 @@ packages:
description: flutter
source: sdk
version: "0.0.0"
intl:
dependency: "direct main"
description:
name: intl
sha256: "3bc132a9dbce73a7e4a21a17d06e1878839ffbf975568bc875c60537824b0c4d"
url: "https://pub.dev"
source: hosted
version: "0.18.1"
io:
dependency: transitive
description:
Expand Down
4 changes: 4 additions & 0 deletions pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ dependencies:
package_info_plus: ^4.2.0
hive: ^2.2.3
hive_flutter: ^1.1.0
flutter_localizations:
sdk: flutter
intl: ^0.18.1

dev_dependencies:
flutter_test:
Expand All @@ -68,6 +71,7 @@ dev_dependencies:

# The following section is specific to Flutter packages.
flutter:
generate: true

# The following line ensures that the Material Icons font is
# included with your application, so that you can use the icons in
Expand Down
1 change: 1 addition & 0 deletions scripts/license.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ packageLicenseOverride:
flutter_web_plugins: BSD-3-Clause
flutter_driver: BSD-3-Clause
integration_test: BSD-3-Clause
flutter_localizations: BSD-3-Clause
fuchsia_remote_debug_protocol: BSD-3-Clause

omitDisclaimer:
Expand Down
6 changes: 5 additions & 1 deletion test/helpers.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import 'package:flutter/material.dart';
import 'package:flutter_gen/gen_l10n/app_localizations.dart';

/// Wrap MaterialApp and Scaffold around the given widget.
Widget wrapMaterial({required Widget? child}) => MaterialApp(home: Scaffold(body: child));
Widget wrapMaterial({required Widget? child}) => MaterialApp(
localizationsDelegates: AppLocalizations.localizationsDelegates,
supportedLocales: AppLocalizations.supportedLocales,
home: Scaffold(body: child));
10 changes: 4 additions & 6 deletions test/terms_and_conditions_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import 'package:concordium_wallet/screens/terms_and_conditions/screen.dart';
import 'package:concordium_wallet/services/wallet_proxy/model.dart';
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:flutter_gen/gen_l10n/app_localizations_en.dart';

import 'helpers.dart';

Expand Down Expand Up @@ -52,8 +53,7 @@ void main() {
await tester.pumpWidget(wrapMaterial(child: tacScreen));

// Act
// TODO use internationalized version here.
await tester.tap(find.text("Continue", findRichText: true));
await tester.tap(find.text(AppLocalizationsEn().cont, findRichText: true));

// Assert
expect(checked, false);
Expand All @@ -68,8 +68,7 @@ void main() {

await tester.pump();

// TODO use internationalized version here.
await tester.tap(find.text("Continue", findRichText: true));
await tester.tap(find.text(AppLocalizationsEn().cont, findRichText: true));

await tester.pump();

Expand All @@ -95,8 +94,7 @@ void main() {
when(() => launcher.launch(uri)).thenAnswer((_) => Future.value(true));

// Act
// TODO use internationalized version here.
await tester.tap(find.textContaining("I have read and agree to the", findRichText: true));
await tester.tap(find.textContaining(AppLocalizationsEn().read_and_agree, findRichText: true));

// Assert
verify(() => launcher.launch(uri)).called(1);
Expand Down