felangel / bloc

A predictable state management library that helps implement the BLoC design pattern
https://bloclibrary.dev
MIT License
11.76k stars 3.39k forks source link

How would I use Navigator with this? #63

Closed QoLTech closed 5 years ago

QoLTech commented 5 years ago

For example, I have a button that would normally have the Navigator call in the onPressed. Using this Bloc pattern, I would dispatch that button click event and then what? What runs the Navigator call?

felangel commented 5 years ago

@QoLTech if navigation is the only thing happening on a button press, I wouldn't use a Bloc at all because Navigation is not business logic and should be done by the UI layer.

If you have business logic on a button press and need to navigate based on some dynamic information then I would do the navigation again in the presentation layer (widget) in response to a success state like:

@override
Widget build(BuildContext context) {
  return BlocBuilder(
    bloc: _bloc,
    builder: (context, state) {
      if (state is Success) {
        SchedulerBinding.instance.addPostFrameCallback((_) {
          Navigator.of(context).push(...);
        });
        _bloc.dispatch(NavigationComplete()); // to reset the state and avoid an infinite loop
      }
      ...
    },
  );
}

At the end of the day blocs should just be converting Events to States and all navigation should be done outside of the Bloc in order for it to be decoupled from the presentation layer.

QoLTech commented 5 years ago

@felangel That's great, just needed clarification. Thanks for all the help.

This is an amazing package, keep it up!

felangel commented 5 years ago

@QoLTech No problem and thanks! 💯

tayelno commented 5 years ago

Can anyone provide a complete example on this, because when I tried this

        SchedulerBinding.instance.addPostFrameCallback((_) {
          Navigator.of(context).push(...);
        });

The app goes into infinite loop repeating the BlocBuilder builder function as well as the Navigator push function, if you need more info about the app please do tell, I am stuck with this.

import 'package:customer/common/common.dart';
import 'package:customer/src/blocs/authentication/authentication.dart';
import 'package:customer/src/screens/phone_signin/phone_login.dart';
import 'package:flutter/material.dart';
import 'package:flutter/scheduler.dart';
import 'package:flutter_bloc/flutter_bloc.dart';

class PhoneLoginForm extends StatefulWidget {
  final PhoneLoginBloc phoneLoginBloc;
  final AuthenticationBloc authenticationBloc;

  PhoneLoginForm({
    Key key,
    @required this.phoneLoginBloc,
    @required this.authenticationBloc,
  }) : super(key: key);

  @override
  State<PhoneLoginForm> createState() => _PhoneLoginFormState();
}

class _PhoneLoginFormState extends State<PhoneLoginForm> {
  PhoneLoginBloc get _phoneLoginBloc => widget.phoneLoginBloc;

  @override
  Widget build(BuildContext context) {
    return BlocBuilder<PhoneLoginEvent, PhoneLoginState>(
      bloc: _phoneLoginBloc,
      builder: (
        BuildContext context,
        PhoneLoginState state,
      ) {
        print(state);
        if (state is PhoneLoginVerificationError) {
         SchedulerBinding.instance.addPostFrameCallback((_) {
            Scaffold.of(context).showSnackBar(
              SnackBar(
                content: Text('${state.error}'),
                backgroundColor: Colors.red,
              ),
            );
          });
        }
        if (state is PhoneLoginCodeSent) {
          SchedulerBinding.instance.addPostFrameCallback((_) {
            Navigator.of(context).push(
              MaterialPageRoute(
                builder: (context) {
                  return VerifyCodeForm(phoneLoginBloc: _phoneLoginBloc);
                },
              ),
            );
            return;
          });
        }
        if (state is PhoneLoginLoading) {
          return LoadingIndicator();
        }
        return SendSMSForm(phoneLoginBloc: _phoneLoginBloc);
      },
    );
  }
}

I know the first guess is that I am pushing new states to the Bloc, but I debugged the App, and it never hits the PhoneLoginBloc mapEventToState function


@override
  Stream<PhoneLoginState> mapEventToState(
    PhoneLoginState currentState,
    PhoneLoginEvent event,
  ) async* {
    if (event is SendSMSButtonClick) {
      String phoneNumber = event.phoneNumber;
      _signInWithPhoneNumber(phoneNumber);
      yield PhoneLoginLoading();
    }

    if (event is SMSCodeSent) {
      String verificationId = event.verificationId;
      yield PhoneLoginCodeSent(verificationId: verificationId);
    }

    if (event is PhoneVerificationError) {
      yield PhoneLoginVerificationError(error: event.error);
    }

    if (event is PhoneVerificationComplete) {
      FirebaseUser auth = event.auth;
      authBloc.dispatch(Login(auth: auth));
      yield PhoneLoginInitial();
    }
  }
felangel commented 5 years ago

@SaMiCoOo this is happening because when navigating flutter is likely rebuilding the PhoneLoginForm and since the state hasn't changed it keeps pushing new routes over and over again.

In order to avoid this behavior I would recommend dispatching an event to reset the state before the navigation occurs.

Something like:

        if (state is PhoneLoginCodeSent) {
          // dispatch here to reset PhoneLoginFormState
          SchedulerBinding.instance.addPostFrameCallback((_) {
            Navigator.of(context).push(
              MaterialPageRoute(
                builder: (context) {
                  return VerifyCodeForm(phoneLoginBloc: _phoneLoginBloc);
                },
              ),
            );
            return;
          });
        }

If you are able to share the full source I can help you suggest ways to avoid having this issue in the first place.

Let me know if that helps.

tayelno commented 5 years ago

@felangel thanks for the quick response, I made it work by making another State that fires this navigation. here is a gist that is a mirror of the code

https://gist.github.com/SaMiCoOo/da0d388c6d6534f9f3f88ddaf244c4f3

But if possible can you add to the login example the navigation handling part, or provide me with a quick example? Because having to add a new State to Navigate, and a new Event to Reset the state after Navigation feels like complicating a simple operation

felangel commented 5 years ago

@SaMiCoOo sorry for the delayed response! I took a look at your gist and what you've done is totally fine however if there is literally no business logic happening in response to the PhoneLoginCodeSent event then there's no point to have to go through the bloc.

I would just do the navigation in response to the user input without a bloc and then you no longer have to worry about navigating and resetting the state. Let me know if that makes sense.

TheHemantKaushik commented 5 years ago

Hi @felangel !!

It would be great if you create some listener for this kind of listening. RxCommandListener is a good example to listen RxCommand. We can listen any command in initState() and close listener in dispose().

Reference: https://pub.dartlang.org/packages/rx_command https://pub.dartlang.org/packages/rx_widgets

felangel commented 5 years ago

@TheHemantKaushik thanks! I'll take a look 💯 I know this is definitely an area that can be improved.

edenchen123 commented 5 years ago

@QoLTech if navigation is the only thing happening on a button press, I wouldn't use a Bloc at all because Navigation is not business logic and should be done by the UI layer.

If you have business logic on a button press and need to navigate based on some dynamic information then I would do the navigation again in the presentation layer (widget) in response to a success state like:

@override
Widget build(BuildContext context) {
  return BlocBuilder(
    bloc: _bloc,
    builder: (context, state) {
      if (state is Success) {
        SchedulerBinding.instance.addPostFrameCallback((_) {
          Navigator.of(context).push(...);
        });
        _bloc.dispatch(NavigationComplete()); // to reset the state and avoid an infinite loop
      }
      ...
    },
  );
}

At the end of the day blocs should just be converting Events to States and all navigation should be done outside of the Bloc in order for it to be decoupled from the presentation layer.

I really don't like this solution, the build method has the code of Navigation and Rendering together. if you handle this not correct you will get a dirty error. Is it possible to add a listener(rx) to the bloc, so we can move the logic outside of build method, initState is an idea place

block.loginClicked$.success(...navigate to home page)

felangel commented 5 years ago

@edenchen123 this is really out-dated.

Please refer to the Navigation Recipe for the recommended approach.

Hope that helps 👍

basnetjiten commented 5 years ago

Hi @felangel . I have been following the Navigation Recipe you mentioned. But there seems to be a weird issue. BlocBuilder builds the next page say NexPage() which is a statefull widget inside same Page. When I try to Navigation it complains about setState or marksNeedsBuilds() called during build. However if I return like this return NextPage(). It renders this page inside the same Page

undamatlamanikanta commented 5 years ago

The app goes into infinite loop repeating the StreamBuilder builder function as well as the Navigator push function, if you need more info about the app please do tell, I am stuck with this.

felangel commented 5 years ago

@undamatlamanikanta you should not be navigating inside StreamBuilder or BlocBuilder. Instead, if you need to navigate based on a bloc state you should do the navigation inside BlocListener. Please look at the Route Navigation Recipe.

undamatlamanikanta commented 5 years ago

ok thank you for your support

On Fri, Jul 12, 2019 at 5:43 PM Felix Angelov notifications@github.com wrote:

@undamatlamanikanta https://github.com/undamatlamanikanta you should not be navigating inside StreamBuilder or BlocBuilder. Instead, if you need to navigate based on a bloc state you should do the navigation inside BlocListener. Please look at the navigation recipe linked above for an example.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/felangel/bloc/issues/63?email_source=notifications&email_token=AMIXAJEINIJSVIHJR3GZ7X3P7BYONA5CNFSM4GRAVDF2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZZSTXY#issuecomment-510863839, or mute the thread https://github.com/notifications/unsubscribe-auth/AMIXAJHUGZ6V4PW5YXNCVA3P7BYONANCNFSM4GRAVDFQ .

vitor-gyant commented 5 years ago

I know this is an old discussion but I found that https://dev.to/pedromassango/flutter-bloc-a-better-way-to-handle-navigation-4fig is a better way to manage this. Moreover, it allow us to apply logic before navigation is a centralised bloc and there is no need to manage reset state which IMO makes code hard to follow.

Allan-Nava commented 4 years ago

I know this is an old discussion but I found that https://dev.to/pedromassango/flutter-bloc-a-better-way-to-handle-navigation-4fig is a better way to manage this. Moreover, it allow us to apply logic before navigation is a centralised bloc and there is no need to manage reset state which IMO makes code hard to follow.

Is the correct way? I need to implement the navigation but with the simple

Navigator.push(
        context,
        MaterialPageRoute(
            builder: (context) => new FeedPage() ),
      );

this give me an error:

flutter: Another exception was thrown:         BlocProvider.of() called with a context that does not contain a Bloc of type FeedBloc.
felangel commented 4 years ago

Hi @Allan-Nava you need to use the parent context

Navigator.push(
        context,
        MaterialPageRoute(
            builder: (_) => BlocProvider.value(value: BlocProvider.of<MyBloc>(context), child: FeedPage() ),
      );

Check out the bloc access recipe for more information.

Allan-Nava commented 4 years ago

Hi @Allan-Nava you need to use the parent context

Navigator.push(
        context,
        MaterialPageRoute(
            builder: (_) => BlocProvider.value(value: BlocProvider.of<MyBloc>(context), child: FeedPage() ),
      );

Check out the bloc access recipe for more information.

Thank's for the fastly answer

I fixed with ur example of cart and weather