felangel / flow_builder

Flutter Flows made easy! A Flutter package which simplifies navigation flows with a flexible, declarative API.
https://pub.dev/packages/flow_builder
MIT License
399 stars 65 forks source link

Back button not visible #36

Open e200 opened 3 years ago

e200 commented 3 years ago

Describe the bug The back button is not visible for the first page, it's only visible for the second and up

To Reproduce Steps to reproduce the behavior: With this Flow widget you will see the issue:

class ServiceFlow extends StatefulWidget {
  static const route = '/service_flow';

  @override
  _ServiceFlowState createState() => _ServiceFlowState();
}

class _ServiceFlowState extends State<ServiceFlow> {
  final _flowController = FlowController(const ServiceFlowState());

  @override
  Widget build(BuildContext context) {
    return FlowBuilder<ServiceFlowState>(
      controller: _flowController,
      onGeneratePages: _onGenerateRoutes,
    );
  }

  List<Page> _onGenerateRoutes(state, pages) {
    return [
      AppPage(
        child: ServiceCategoryListPage(
          onSelect: (category) {
            _flowController.update((value) {
              return value.copyWith(serviceCategoryId: 1);
            });
          },
        ),
      ),
      if (state.serviceCategoryId != null)
        AppPage(
          child: ServiceListPage(
            onSelect: (service) {
              _flowController.complete((value) => value.copyWith(serviceId: 1));
            },
          ),
        ),
    ];
  }

  @override
  void dispose() {
    super.dispose();

    _flowController.dispose();
  }
}

Expected behavior The arrow button to be visible for wach flow state.

Screenshots

https://user-images.githubusercontent.com/20504726/108818386-925e8680-75b9-11eb-9c85-56f6d24373e6.mp4

Logs Run flutter analyze and attach any output of that command below.

Analyzing foo...                                                        

   info • Sort pub dependencies • pubspec.yaml:28:3 • sort_pub_dependencies
   info • Sort pub dependencies • pubspec.yaml:39:3 • sort_pub_dependencies

2 issues found. (ran in 3.1s)

Paste the output of running `

flutter doctor -v` here.

[✓] Flutter (Channel beta, 1.26.0-17.6.pre, on Linux, locale pt_PT.UTF-8)
    • Flutter version 1.26.0-17.6.pre at /home/eleandro/fvm/versions/beta
    • Framework revision a29104a69b (7 days ago), 2021-02-16 09:26:56 -0800
    • Engine revision 21fa8bb99e
    • Dart version 2.12.0 (build 2.12.0-259.12.beta)

[✓] Android toolchain - develop for Android devices (Android SDK version 30.0.3)
    • Android SDK at /home/eleandro/Android/Sdk
    • Platform android-30, build-tools 30.0.3
    • Java binary at: /snap/android-studio/current/android-studio/jre/bin/java
    • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b3-6222593)
    • All Android licenses accepted.

[✓] Android Studio
    • Android Studio at /snap/android-studio/current/android-studio
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • android-studio-dir = /snap/android-studio/current/android-studio
    • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b3-6222593)

[✓] IntelliJ IDEA Community Edition (version 2020.3)
    • IntelliJ at /opt/Intellij
    • Flutter plugin version 53.0.5
    • Dart plugin version 203.6912

[✓] Connected device (1 available)
    • SM G920V (mobile) • 05157df5e88ac22a • android-arm64 • Android 7.0 (API 24)

• No issues found!

Additional context:

Incredibly the same doesn't happen if we add another empty page before the others:

List<Page> _onGenerateRoutes(state, pages) {
    return [
      const AppPage(child: SizedBox.shrink()),
      AppPage(
        child: ServiceCategoryListPage(
          onSelect: (category) {
            _flowController.update((value) {
              return value.copyWith(serviceCategoryId: 1);
            });
          },
        ),
      ),
      if (state.serviceCategoryId != null)
        AppPage(
          child: ServiceListPage(
            onSelect: (service) {
              _flowController.complete((value) => value.copyWith(serviceId: 1));
            },
          ),
        ),
     ];
  }
}

I think the problem is with the code that checks the length of pages, I'm not sure.

felangel commented 3 years ago

Hi @e200 👋 Thanks for opening an issue!

This is working as expected because your FlowBuilder is a disconnected flow so the first page doesn't know about where it came from and whether it should show a back button.

You can add a BackButton as needed to the first page:

Scaffold(
  appBar: AppBar(
    leading: BackButton(onPressed: () => context.flow<MyFlowState>().complete(),
    ...
  ),
  ...
)

Hope that helps 👍

e200 commented 3 years ago

This makes all sense.

But if FlowBuilder provided a way to toggle this option, would be really useful because not every flow may require the button to be visible (hard coded). What do you think?

kranfix commented 3 years ago

I agree with @e200. The FlowBuilder or FlowController could have a field allowPop.

spiritinlife commented 3 years ago

Agree on this. I would prefer not to add navigation logic in my screen because it limits the usecases of the screen and how it can be built. I think the allowPop would be a nice addition.

feronetick commented 2 years ago

For this case we use this workaround:

class FlowPage<T> extends Page<T> {
  const FlowPage({
    required this.child,
    this.maintainState = true,
    this.onPop,
    super.key,
    super.name,
    super.arguments,
    super.restorationId,
  });

  final Widget child;
  final bool maintainState;
  final void Function(BuildContext context)? onPop;

  @override
  Route<T> createRoute(BuildContext context) {
    return _PageBasedFlowPageRoute<T>(page: this);
  }
}

class _PageBasedFlowPageRoute<T> extends PageRoute<T>
    with MaterialRouteTransitionMixin<T> {
  _PageBasedFlowPageRoute({
    required FlowPage<T> page,
  }) : super(settings: page) {
    assert(opaque);
  }

  FlowPage<T> get _page => settings as FlowPage<T>;

  @override
  Widget buildContent(BuildContext context) {
    if (_page.onPop != null) {
      return WillPopScope(
        child: _page.child,
        onWillPop: () async {
          _page.onPop?.call(context);
          return true;
        },
      );
    }
    return _page.child;
  }

  @override
  bool get willHandlePopInternally => _page.onPop != null;

  @override
  bool get maintainState => _page.maintainState;

  @override
  String get debugLabel => '${super.debugLabel}(${_page.name})';
}

Now we can add onPop handler to FlowPage:

onGenerateRoutes(state, pages) {
    return [
        FlowPage(
            onPop: (context) => context.flow<RegistrationFlowState>().complete(),
            child: FirstPageInStack(),
        ),

       // ....
    ];
}

Back button appears automatically because willHandlePopInternally flag is set while onPop handler provided. Hope this helps someone.

This workaround breaks iOS pop gestures: https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/cupertino/route.dart#L194