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
389 stars 63 forks source link

Prevent repeated calls of "onComplete" #73

Closed Lootwig closed 2 years ago

Lootwig commented 2 years ago

Once a flow was completed, navigating back through the flow by popping causes the onComplete handler to fire again on every pop.

I can prevent this by manually wrapping my callback code into a check whether the current state is in fact the "last" state before actually executing its logic, but would prefer if this was built-in, e.g. by setting the private _completed property to false automatically anytime a _history state gets restored.

Am I missing something that would make this a bad idea?

felangel commented 2 years ago

Hi @Lootwig 👋 Thanks for opening an issue!

Can you provide a link to a minimal reproduction sample? Thanks!

Lootwig commented 2 years ago
  1. Run
  2. Press "next"
  3. Press "complete" -> console logs execution of onComplete
  4. Press back button in appbar -> console logs execution of onComplete
import 'package:flow_builder/flow_builder.dart';
import 'package:flutter/material.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
        primarySwatch: Colors.blue,
      ),
      home: const MyHomePage(),
    );
  }
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({Key? key}) : super(key: key);

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  final FlowController<int> _controller = FlowController(0);

  @override
  Widget build(BuildContext context) {
    return FlowBuilder<int>(
      controller: _controller,
      onComplete: (_) {
        print('COMPLETING FLOW');
      },
      onGeneratePages: (state, pages) {
        return [
          ...Iterable<int>.generate(state + 1)
              .map((_) => const MaterialPage(child: FlowWidget())),
        ];
      },
    );
  }
}

class FlowWidget extends StatelessWidget {
  const FlowWidget({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    final flow = context.flow<int>();
    return Scaffold(
      appBar: AppBar(),
      body: Center(
        child: ElevatedButton(
          onPressed: () {
            flow.state < 1
                ? flow.update((state) => state + 1)
                : flow.complete();
          },
          child: Text(flow.state < 1 ? 'NEXT' : 'COMPLETE'),
        ),
      ),
    );
  }
}
felangel commented 2 years ago

@Lootwig I took a closer look and was able to reproduce the issue. Once you complete a FlowController you will not be able to interact with the controller anymore. If you override onComplete then FlowBuilder assumes that you will handle dismissing the flow and handling the result manually. In this case, nothing happens because you're just printing but in practice you should either redirect the user to some other route with the result or remove onComplete and let FlowBuilder handle it for you.

I will push a fix to remove the controller listener after it has been completed.

Let me know if that helps and thanks for reporting this!

Lootwig commented 2 years ago

Interesting, that's basically saying that for my particular use case, where a user might go back to a particular step of the flow and make changes, completing is not an intended operation in the first place?

E.g. whatever I'm doing in the last step should be a result of just another update?

felangel commented 2 years ago

Interesting, that's basically saying that for my particular use case, where a user might go back to a particular step of the flow and make changes, completing is not an intended operation in the first place?

E.g. whatever I'm doing in the last step should be a result of just another update?

Yup that’s correct! If you don’t want the flow to end then you can just use the update API 👍