Fix: Closing bottom sheet and removing FAB cause assertion failure #128566
Conversation
github-actions
bot
added
f: material design
|
Thanks for fixing this! @utisam We'll need a test for this fix. |
|
@TahaTesser Test was added at 02404bc The assertion failure was caused on the latest master channel. |
There was a problem hiding this comment.
I'm not really sure if this is the correct way to test this.
When I comment on these lines and just let pumpWidget, I get errors.
// setState(() => show = false);
// await tester.pump(); // start animation
// await tester.pump(const Duration(seconds: 1));
// expect(controller?.closed, completes);
// expect(find.byType(FloatingActionButton), findsNothing);There was a problem hiding this comment.
Is addPostFrameCallback necessary?
It'd be preferable to add and remove the bottom sheet by pressing buttons.
There was a problem hiding this comment.
Yeah, maybe can the FAB.onPressed trigger this change?
There was a problem hiding this comment.
Thanks for the comments. I rewrite the test using buttons.
Yeah, maybe can the FAB.onPressed trigger this change?
I also tried to trigger 2 changes at FAB.onPressed at once, but it does not reproduce the assertion error.
While rewriting the test, I found the problem is only that while the hiding FAB animation is running, another event starts the closing bottom animation. The reversed order (close bottom sheet → hide FAB) or both at once does not.
|
@Piinks |
Piinks
left a comment
Piinks
left a comment
There was a problem hiding this comment.
Thanks for raising this @TahaTesser. Welcome @utisam! Thanks for contributing!
Can you take a look at the test suggestion below? It may be that we just need a bit more context to understand the design of the test - thank you for including one by the way. :)
utisam
force-pushed
the
fix_fav_animation_assert
branch
from
7f95711 to
45c1a68
Compare
|
We'll need a second review from @Piinks before landing this. |
|
auto label is removed for flutter/flutter, pr: 128566, due to - The status or check suite Linux flutter_plugins has failed. Please fix the issues identified (or deflake) before re-applying this label. |
auto-submit
bot
removed
the
autosubmit
Piinks
force-pushed
the
fix_fav_animation_assert
branch
from
259e266 to
8b9c1fd
Compare
… at the same time
Co-authored-by: Taha Tesser <[email protected]>
Piinks
force-pushed
the
fix_fav_animation_assert
branch
from
8b9c1fd to
b781357
Compare
flutter/flutter@c40baf4...042c036 2023-06-22 [email protected] Remove unnecessary variable `_hasPrimaryFocus` (flutter/flutter#129066) 2023-06-22 [email protected] Fix Material 3 Scrollable `TabBar` (flutter/flutter#125974) 2023-06-22 [email protected] Fix: Closing bottom sheet and removing FAB cause assertion failure (flutter/flutter#128566) 2023-06-22 [email protected] Add `InputDecorationTheme.merge` (flutter/flutter#129011) 2023-06-22 [email protected] Roll Packages from 9af50d4 to 95bc1c6 (6 revisions) (flutter/flutter#129351) 2023-06-22 [email protected] Prevent crashes on range errors when selecting device (flutter/flutter#129290) 2023-06-22 [email protected] Revert "Roll Flutter Engine from 703c9a14ac7f to 8cc6d6d5efdb (1 revision)" (flutter/flutter#129353) 2023-06-22 [email protected] Roll Flutter Engine from 703c9a14ac7f to 8cc6d6d5efdb (1 revision) (flutter/flutter#129339) 2023-06-22 [email protected] Roll Flutter Engine from d9530e2b87de to 703c9a14ac7f (1 revision) (flutter/flutter#129337) 2023-06-22 [email protected] Roll Flutter Engine from c6251a69a09a to d9530e2b87de (1 revision) (flutter/flutter#129334) 2023-06-22 [email protected] Roll Flutter Engine from 08aaa88bf67f to c6251a69a09a (10 revisions) (flutter/flutter#129331) 2023-06-22 [email protected] Manual roll of packages to 9af50d4 (flutter/flutter#129328) 2023-06-21 [email protected] Roll Flutter Engine from 090fae83548a to 08aaa88bf67f (3 revisions) (flutter/flutter#129306) 2023-06-21 [email protected] [framework,web] add FlutterTimeline and semantics benchmarks that use it (flutter/flutter#128366) 2023-06-21 [email protected] Roll pub packages (flutter/flutter#128966) 2023-06-21 [email protected] Remove incorrect non-nullable assumption from `ShapeDecoration.lerp` (flutter/flutter#129298) 2023-06-21 [email protected] Gracefully handle negative position in getWordAtOffset (flutter/flutter#128464) 2023-06-21 [email protected] [flutter_tools] add a gradle error handler for could not open cache directory (flutter/flutter#129222) 2023-06-21 [email protected] Roll Flutter Engine from f973fb4636d3 to 090fae83548a (5 revisions) (flutter/flutter#129293) 2023-06-21 [email protected] Selection area right click behavior should match native (flutter/flutter#128224) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
3 participants
Fixes #128562
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.