long1eu / flutter_i18n

This plugin create a binding between your translations from .arb files and your Flutter app.
Apache License 2.0
251 stars 55 forks source link

Add ability to change locale manually #97

Closed mrkpatchaa closed 5 years ago

mrkpatchaa commented 5 years ago

Inspired from https://marketplace.visualstudio.com/items?itemName=esskar.vscode-flutter-i18n-json. Related to #65

mrkpatchaa commented 5 years ago

I've updated the code.

mrkpatchaa commented 5 years ago

Hi @noordawod any update on this ? Thanks.

noordawod commented 5 years ago

What do you think @long1eu ?

long1eu commented 5 years ago

The way this changes the language is misleading. Normally we rely on the localisation Widget to tell the whole Widget tree what the locale is. This implementation seems more like a hack and doesn't follow the way Flutter normally works.

noordawod commented 5 years ago

@long1eu

I'm in agreement. I was hesitant to merge this in so I'm happy you have doubts too.

Although @rmkpatchaa worked tirelessly to fine-tune the pr according to my input, I vote against merging it in. You?

mrkpatchaa commented 5 years ago

@long1eu thanks for the input. Unfortunately I'm not experienced in flutter dev. So yes maybe the solution is not ideal. And maybe someone could help with that. I think it is a feature that is need with a plugin like this one.

noordawod commented 5 years ago

I'm closing this because the maintainers think it's out of scope.

mrkpatchaa commented 5 years ago

Oh I'm a little bit confused... 😕 The i18n.dart file is generated by the extension.

How could we implement the language switch without editing that file ? And if edited, it will be overwritten the next time you add a string resource...

Actually I was testing all the modifications I made to the extension by editing my local i18n.dart file, but I know it will be overwritten next time I add a new translation key...

mrkpatchaa commented 5 years ago

The point is not about merging my PR. The point is "how do we do that?" if things do not change.

long1eu commented 5 years ago

You need to change the locale value of your MaterialApp or WidgetApp. Make your root Widget stateful and use setState to change this value.

On Fri, 4 Oct 2019 at 20:19, Raymond Médédé KPATCHAA < notifications@github.com> wrote:

Oh I'm a little bit confused... 😕 The i18n.dart file is generated by the extension.

How could we implement the language switch without editing that file ? And if edited, it will be overwritten the next time you add a string resource...

Actually I was testing all the modifications I made to the extension by editing my local i18n.dart file, but I know it will be overwritten next time I add a new translation key...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/long1eu/flutter_i18n/pull/97?email_source=notifications&email_token=ACG2OVQTUCIRRWPQOUYGKH3QM53J3A5CNFSM4I3UTW3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAMKL3Q#issuecomment-538486254, or mute the thread https://github.com/notifications/unsubscribe-auth/ACG2OVRRATGTOUPZUBXRN7DQM53J3ANCNFSM4I3UTW3A .

mrkpatchaa commented 5 years ago

Ok, I've tried with a custom class and it worked. One thing though If we want to test on supported locales, the _isSupported method is always called with true.

@override
  bool isSupported(Locale locale) => _isSupported(locale, true);

This could be written

@override
  bool isSupported(Locale locale) => _isSupported(locale, locale.countryCode?.isNotEmpty);

Thanks.

noordawod commented 5 years ago

I believe this was done in this fashion because of backward-compatibility. It may break apps if we change the implementation now.

mrkpatchaa commented 5 years ago

OK. Thanks for the input. BTW it's mentioned that you're working on a VSCode extension. Is it ready yet for testing ? Thanks.

long1eu commented 5 years ago

Not yet unfortunately. Do you have knowledge of building a vccode extension?

On Fri, 4 Oct 2019 at 22:49, Raymond Médédé KPATCHAA < notifications@github.com> wrote:

OK. Thanks for the input. BTW it's mentioned that you're working on a VSCode extension. Is it ready yet for testing ? Thanks.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/long1eu/flutter_i18n/pull/97?email_source=notifications&email_token=ACG2OVXW2YSZULK3BSCLMM3QM6M4TA5CNFSM4I3UTW3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAMWZJA#issuecomment-538537124, or mute the thread https://github.com/notifications/unsubscribe-auth/ACG2OVTFWQWALJ2YUIRNUCTQM6M4TANCNFSM4I3UTW3A .

mrkpatchaa commented 5 years ago

Unfortunately no. But maybe I could make some time to learn by helping on this.