Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable translation without context #191

Closed
wants to merge 4 commits into from

Conversation

MilosKarakas
Copy link

Hi, I just completed the PR that enables translation with FlutterI18n instance, and without context. Basically I made BuildContext nullable in 4 functions that required it: translate(..), plural(..), currentLocale(..) and refresh(..). I added an optional named parameter FlutterI18n? instance to all of these, and have added it to I18nText and I18nPlural widgets as well. I decided to go with this kind of implementation to avoid adding additional 4 functions to the FlutterI18n class. IMHO it would make it too bloated. I am not completely satisfied with this solution either, since you need to pass null for BuildContext each time you translate without it, but I didn't have any better ideas how to implement it.

I have added tests as well.

Now you can register a FlutterI18n instance, and call for translator, for instance, somewhere in your data layer, like this:

FlutterI18n instance = GetIt.instance.get();
String translatedText = FlutterI18n.translate(null, 'translation_key', instance: instance);  

Whenever you pass an instance that is not null it will avoid trying to use the context. When instance is not passed context is used. Developers must take care not to set both BuildContext and instance to null.

@MilosKarakas
Copy link
Author

@ilteoood Any idea when you could review this PR? Thanks.

@MilosKarakas
Copy link
Author

@ilteoood Hey, do you have any feedback on this?

@ilteoood
Copy link
Owner

Hi @MilosKarakas,
Sorry but I'm in honeymoon rn with limited internet connection.

I'll check it when I'll go home, I promise.

@MilosKarakas
Copy link
Author

Hi @ilteoood , thanks for the response. Have a nice time during honeymoon 😄

@ilteoood
Copy link
Owner

ilteoood commented Jun 5, 2022

Hi @MilosKarakas,
I've looked at this implementation and, sincerely I don't like it too.

It's too complicated and do too much stuff for a single method.
I've not finished yet, but here is my proposal:
#198

Can you look at it and give me a suggestion pls?

Matteo

@MilosKarakas
Copy link
Author

Hey @ilteoood I'll take a look until the end of next week and give you my opinion.

Regards,
Miloš

@MilosKarakas
Copy link
Author

Sorry, I'm still overloaded with work. I'll let you know as soon as I am done looking into it.

@ilteoood ilteoood closed this Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants