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

BLoC state management modifications #461

Closed
wants to merge 6 commits into from

Conversation

vcadillog
Copy link
Contributor

Hi again, great work; I had made some changes for it to match with BLoC pattern; if you think that this can be helpful consider on merging or making a secondary tree, since I made some refactors and adding a controller.
Here you can see how I implemented it with firebase_auth.
https://github.com/vcadillog/flutter_firebase_login_bloc_pattern/blob/master/lib/login/view/login_form.dart
Some things to note when isBlocPattern = true (the flag for bloc). It skips all the error handling logic, since I do it in the BLoC repository layer and for that reason I create a Flushbar myself.

Additional changes included, for example in login_form.dart file:
176 await _submitController.forward();
177 setState(() => _isSubmitting = true);
This code potentially leads to a dispose controller error, because if the user taps fast enought the forgot password screen before the setState takes action it disposes the submitController; so I swapped the order to ensure _isSubmitting is true and disable undesired gestures.

@juliansteenbakker
Copy link
Collaborator

Thank you for your PR, however i won't be merging this, because it will add lots of boiler plate code for a specific plugin (BLOC) that not all people use.

@artem-bakuta
Copy link

Would be great to have something inside

@artem-bakuta
Copy link

artem-bakuta commented Feb 28, 2024

I have tried to use Completer<String?> to deliver future into onLogin, but it always throw me:

Unhandled Exception: Navigator operation requested with a context that does not include a Navigator.
The context used to push or pop routes from the Navigator must be that of a widget that is a descendant of a Navigator widget.

So solution out of the box should be great solution

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.

3 participants