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

[Bug] overlayBuilder race condition? #23

Open
EnduringBeta opened this issue Jul 18, 2024 · 7 comments
Open

[Bug] overlayBuilder race condition? #23

EnduringBeta opened this issue Jul 18, 2024 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@EnduringBeta
Copy link

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

icon: IntroStepBuilder(
order: 1,
text: 'Welcome to flutter_intro',
padding: const EdgeInsets.only(
bottom: 20,
left: 16,
right: 16,
top: 8,
),
onWidgetLoad: () {
Intro.of(context).start();
},
builder: (context, key) => Icon(
Icons.home,
key: key,
),
),
),

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

@EnduringBeta
Copy link
Author

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

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

@hellohejinyu hellohejinyu self-assigned this Jul 20, 2024
@EnduringBeta
Copy link
Author

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

@EnduringBeta
Copy link
Author

EnduringBeta commented Jul 25, 2024

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

@EnduringBeta EnduringBeta changed the title [Suggestion] addPostFrameCallback delay documentation [Bug] overlayBuilder race condition? Jul 25, 2024
@hellohejinyu
Copy link
Member

hellohejinyu commented Jul 29, 2024

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. 😝

@hellohejinyu hellohejinyu added the bug Something isn't working label Jul 29, 2024
@EnduringBeta
Copy link
Author

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.

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
None yet
Development

No branches or pull requests

2 participants