search

minaxorg / flutter_intro

A better way for new feature introduction and step-by-step users guide for your Flutter project.
MIT License
47 stars 36 forks source link

[Bug] overlayBuilder race condition? #23

Open EnduringBeta opened 3 months ago

EnduringBeta commented 3 months ago

Thank you for creating this package. I want to suggest that the documentation explain how best to show an intro on route load.

I tried to simply call Intro.start() in a addPostFrameCallback in initState for my route, but the intro didn't show reliably. I guessed a race condition, and I think I'm right. _IntroStepBuilderState and perhaps other places use addPostFrameCallback to build _stepsMap.

Once I added a Future.delayed() of 10ms, the guide shows reliably.

Is there a better technique? If not, this being in the readme would be helpful, I think.

hellohejinyu commented 3 months ago

https://github.com/minaxorg/flutter_intro/blob/f1ad3a9a3485752444487a73292cb3212c872ea9/example/lib/demo_usage.dart#L52-L69

Hello, it can be triggered in the onWidgetLoad of the first step.

EnduringBeta commented 3 months ago

Thank you for sharing this. The guide started properly, but steps 2 and 3 weren't loaded yet and did not show. I kept a Future.delayed inside onWidgetLoad for 10ms, and this fixed things again.

hellohejinyu commented 3 months ago

This seems to be a bug. Could you provide me with a minimal reproducible demo?

EnduringBeta commented 3 months ago

I will try, but it might be a few days.

EnduringBeta commented 3 months ago

https://github.com/EnduringBeta/flutter-bug/tree/flutter_intro

💪 I've managed to make a tiny app that has the bug. 🐛 I think this has to do with overlayBuilder.

Run this, and then tap the "Refresh" icon button in the top left. This will take you to the test route, where there are 2 intro steps, but only 1 will show (only the first time).

In the code, change useCustomOverlay to false, restart the app, and confirm that you see 2 intro steps.

My function _buildIntroOverlay will probably be of interest for you to look at, but I'm not sure if there's anything unusual in there. This may just be in the package's implementation when the custom overlay is used.

Edit: I changed the issue title

hellohejinyu commented 3 months ago

Thanks for the bug reproduction case! I checked it out, and yeah, there is an issue, but it's not super easy to fix. However, there's a pretty straightforward workaround what I mentioned earlier about calling start in the onWidgetLoad of the first step to start the guide as soon as the page loads was incorrect. Instead, you should call it in the onWidgetLoad of the last step in the render tree (usually the bottom-right step of the page). By this time, all elements on the page should be fully loaded so that you can get the correct count.

I'll keep thinking about how to solve this issue without causing any breaking changes. Also, what you mentioned earlier about using Future.delay is a good idea. 😝

EnduringBeta commented 3 months ago

Future.delay is okay, but it feels risky because of what I assume is a race condition.

Unfortunately, for my case, my last step is the "Back" button up in the app bar. So your assumption about "all elements being fully loaded" may not be true for me?

I'm not expecting a fix for this. I'm doing fine with what I have.

Often developers would know how many intro steps there will be on a route, I think? Could there be a bool flag that marks a group "loaded" because it reaches the expected number of steps? I'm just brainstorming.

At the very least, you can update the docs to share this bug/limitation so people are aware.