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

Conversation

shjortConcordium
Copy link
Contributor

@shjortConcordium shjortConcordium commented Nov 30, 2023

Purpose

Setup internationalization, and show of usage. (using terms and condition screen)

Changes

  • Setup internationalization using flutter_localizations and intl.
  • Replace raw text in terms and conditions screen with localization.
  • Use localization in test instead of raw text.

Checklist

  • My code follows the style of this project.
  • The code compiles without warnings.
  • I have performed a self-review of the changes.
  • I have documented my code, in particular the intent of the
    hard-to-understand areas.
  • (If necessary) I have updated the CHANGELOG.

@shjortConcordium shjortConcordium marked this pull request as ready for review December 4, 2023 09:47
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Consider add an extension like this

import 'package:flutter/material.dart';
import 'package:flutter_gen/gen_l10n/app_localizations.dart';

extension AppLocalizationsExt on BuildContext {
  AppLocalizations get l10n => AppLocalizations.of(this);
}

Then localized strings can be get by using context.l10n.initializing instead of AppLocalizations.of(context).initializing.

@@ -0,0 +1,10 @@
{
"initializing": "Initializing...",
Copy link

@ghost ghost Dec 5, 2023

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

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? 🤔

"initializing": "Initializing...",
"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

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)

@@ -0,0 +1,10 @@
{
"initializing": "Initializing...",
Copy link
Contributor

@rh-concordium rh-concordium Dec 6, 2023

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

"initializing": "Initializing...",
"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
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

l10n.yaml Show resolved Hide resolved
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants