fix: bug in the firebase login example

[feinstein]

I believe the firebase login example has a bug.

In case there's a login error, the bloc is emitting a new state with status: FormzSubmissionStatus.failure, and this state triggers a snackbar with a message.

If we change the email or password, every time we change them will trigger a new state with copyWith, so the FormzSubmissionStatus.failure status will be carried with the copyWith, triggering more snackbar messages, even though there might not be any errors with the email or password.

I think this example lacks an example on how to reset the status, so we don't keep triggering it.

I can see this being made in some ways:

1. After the snackbar is shown, the view calls a method in the cubit to reset the status.
2. We can emit 2 states at once, the first has the failure status, the second the status is reset. This will trigger the listener twice.
3. We can remember to set the status each time we emit a new state.

I find option 1 the best. Option 2 looks kinda weird, emitting 2 states at once, and can trigger more things in the listener if we are not careful with the listener code, so it could have side effects. Option 3 is very fragile in my opinion, it's very easy to forget to add a status in one of the state emissions.

I would like to know your thoughts regarding what's the best practice for resetting states @felangel, thanks!

[fernan542]

I think the BlocListener for the LoginState status should provide a listenWhen to prevent the SnackBar on spamming whenever there's a state changes coming from other props.

[feinstein]

I think this is helpful for many case, but I don't think this cover the case where we need to show the same error message again. If we don't clear it, the string will appear to be same and the error message won't appear.

[fernan542]

I mean, validate the errorMessage. Something like this:

BlocListener<LoginCubit, LoginState>(
      listenWhen: (previous, current) => previous.errorMessage != current.errorMessage 
        && current.errorMessage != null,
      listener: (context, state) {
        if (state.status.isFailure) {
          ScaffoldMessenger.of(context)
            ..hideCurrentSnackBar()
            ..showSnackBar(
              SnackBar(
                content: Text(state.errorMessage ?? 'Authentication Failure'),
              ),
            );
        }
      },

[feinstein]

Yeah, but how about when the user typed an invalid e email, then they fix it to a valid email, then they type an invalid email again?

If you don't remember to clear the error message when you have a valid email, you won't fire the snackbar when the user types the wrong email for the second time.

[fernan542]

By convention, that's a form-level error, based on the example app you referenced too. So in my opinion, you should trigger the error field of a TextFormField instead of showing a SnackBar.

[feinstein]

Ok, but that doesn't change the case here, where we show the error, or any kind of event doesn't change how events are managed.

We use bloc to separate the UI logic from the UI visualization. So the same event from the bloc can appear in the screen as a text field error, or a snackbar, or an alert, or a navigation to another page, or a red icon. That's the beauty of separating the screen from the bloc.

My point is, your solution doesn't fix the problem, because even if we filter the event, if we are not careful to reset it, it won't fire a second time, if you need an event to fire a second time. That's the main problem, and here we have a specific example of the problem in a form, but don't get too attached to the example.

[fernan542]

I'm not filtering the event, I'm filtering the expected state, making it predictable.

The easiest and clean approach for me is the second. Reset the status back to initial inside the finally clause after it emit failure since the "goal" of the catch clause is to simply tell the ui that there's something wrong happened within the bloc layer. We can prevent the BlocListener to be triggered twice by providing the listenWhen.

Let's see how Felix will answer this for convention.