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

Prevent repeated calls of "onComplete" #73

Closed
Lootwig opened this issue Nov 11, 2021 · 5 comments · Fixed by #74
Closed

Prevent repeated calls of "onComplete" #73

Lootwig opened this issue Nov 11, 2021 · 5 comments · Fixed by #74
Assignees
Labels
bug Something isn't working

Comments

@Lootwig
Copy link
Contributor

Lootwig commented Nov 11, 2021

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
Copy link
Owner

Hi @Lootwig 👋
Thanks for opening an issue!

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

@felangel felangel self-assigned this Nov 11, 2021
@felangel felangel added the question Further information is requested label Nov 11, 2021
@Lootwig
Copy link
Contributor Author

Lootwig commented Nov 11, 2021

  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 felangel added the investigating Investigating a reported issue label Nov 11, 2021
@felangel
Copy link
Owner

felangel commented Nov 11, 2021

@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
Copy link
Contributor Author

Lootwig commented Nov 11, 2021

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
Copy link
Owner

felangel commented Nov 11, 2021

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 👍

@felangel felangel added bug Something isn't working and removed question Further information is requested investigating Investigating a reported issue labels Nov 11, 2021
@felangel felangel added this to To do in flow_builder via automation Nov 11, 2021
@felangel felangel moved this from To do to Review in progress in flow_builder Nov 11, 2021
flow_builder automation moved this from Review in progress to Done Nov 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

Successfully merging a pull request may close this issue.

2 participants