bahricanyesil / flutter-animated-login

Fully responsive ready-made both login and signup screen with soft and pleasant animations.
https://pub.dev/packages/animated_login
MIT License
150 stars 40 forks source link

Few suggestions #4

Closed wer-mathurin closed 2 years ago

wer-mathurin commented 2 years ago

Hi @bahricanyesil ,

very good first implementation of the library.

Here what I noticed, it can be more helpfully if you name your parameter typedef ChangeLanguageCallback = void Function(String?); => typedef ChangeLanguageCallback = void Function(String? language)

So the IDE will generate a more meanfull variable name instead of p0 :-) This apply to most of the callbacks defined.

Also here I would find be more usefull to return the LanguageOption? instead of a String?
typedef ChangeLanguageCallback = void Function(String?); => typedef ChangeLanguageCallback = void Function(LanguageOption ? option)

Also for LanguageOption it would replace it by:

const LanguageOption({
    required this.value,
    required this.languageLabel,
    this.iconPath,
  });

This will allow to be very generic, and use the value store in the LanguageOption to other layers (ex: a cubit responsible to change the locale, etc...)

Let me know if you want me to do a pull request.

Best reards, Louis-Michel

bahricanyesil commented 2 years ago

Hi @wer-mathurin ,

Thank you for such an effort to use, review and give feedback. It's very valuable for me.

1- Yeah, you're right. I missed that auto-generated parameter name part.

2- Yes, it should be LanguageOptioninstead of String. While converting the logic to a single language text to the LanguageOption model, I missed that part. I forgot to open an issue, thank you for reminding me.

3- I will change the field names in the LanguageOption class.

If you just want to do a pull request, you can do it, no problem. However, I fixed them for this issue. I referenced this issue in my commit and also referenced it in the pull request. You can check it and if there is no problem, with the other PRs, it will be live in the next version.

Thank you for pointing them out, I'm very appreciated.

Best regards, Bahrican Yeşil