Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: Loading builder for Route #3113
feat: Loading builder for Route #3113
Changes from 7 commits
d2a9a88
87b7c6c
a8ea30c
8bf35d1
b9eaf29
624755f
7008bab
c5a465b
784fab9
a1d2a25
76bab2a
0eae558
b9686a5
b82150f
3b8b049
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think I follow why this doesn't work, what are you getting here currently?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry , False is being expected for 503, as for 507 it does pass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is false expected for 503? Since the route is added it should be showing the
loadingComponent
right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just found, I made an error in the checking at line 514 is not accurate in this commit, it should be expecting True, but I checked there for False and it passed
That means my loading component was never mounted even before and after the main component was mounted?
Maybe, that mean my implementation may have some error ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing that due to how
Future.delayed
works in tests theloadingComponent
was added and removed in the same tick. You could pass in a future to the_HeavyComponent
that you manually complete after you have checked that theloadingComponent
is mounted.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you have any more time to look at this @PistonShot1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delays got caught up with another project, but I am trying to implement the approach you mentioned , to pass a future that I can manually complete, I am not exactly sure how to implement this . I am assuming with
Completer
or sort ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah,
Completer
should workThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the test for checking if the
loadingComponent
was mounted and remove, both doesn't pass (at line 499 and at line 509), as I have commented, it is some how implying that the loadingPage was never mounted. I am not sure why, but the changes I added atpackages/flame/lib/src/components/route.dart
(line 146-151), supposed have mounted and removed after awaiting_page!.loaded;