-
Notifications
You must be signed in to change notification settings - Fork 27.3k
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
Change snack bar default hitTestBehavior to deferToChild when SnackBarThemeData.insetPadding is not null #148568
Conversation
…insetPadding is not null
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.
Hi @hany-achraf welcome!
@@ -387,7 387,7 @@ class SnackBar extends StatefulWidget { | |||
|
|||
/// Defines how the snack bar area, including margin, will behave during hit testing. | |||
/// | |||
/// If this property is null and [margin] is not null, then [HitTestBehavior.deferToChild] is used by default. | |||
/// If this property is null, and [margin] is not null or [snackBarTheme.insetPadding] is not null, then [HitTestBehavior.deferToChild] is used by default. |
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.
It looks like this is what is causing the doc reference failure. Can you take a look?
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.
Hi @Piinks! Thanks for flagging that issue. I've taken a look and applied the fix. All checks have now passed successfully. Please go ahead and verify that everything is working as expected on your end.
expect(completer.isCompleted, true); | ||
}); | ||
|
||
testWidgets('Cannot interact with widgets behind SnackBar if hitTestBehavior is set to HitTestBehavior.opaque even if insetPadding is set in SnackBarThemeData', (WidgetTester tester) async { |
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.
This passes without this change, is that expected?
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.
The behavior you're observing is expected. This PR modifies the default value of hitTestBehavior
to HitTestBehavior.deferToChild
instead of HitTestBehavior.opaque
when SnackBarThemeData.insetPadding
is not null. The purpose of this test is to ensure that even with SnackBarThemeData.insetPadding
set, the default value of hitTestBehavior
can still be overridden, which is consistent with the existing behavior before this PR.
The test passes both with and without this change because the intended functionality remains intact. I added this test after noticing a similar one in #127959 named "Can't tap on button behind snack bar defined by margin and HitTestBehavior.opaque" in the same snack_bar_test.dart
file. However, I'm unsure whether keeping this new test is necessary.
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.
Hi @Piinks! I've just removed that test in my latest commit on this PR. Please review and let me know if you'd like me to revert that change.
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.
Thanks for the update! This LGTM!
I will need to find a second reviewer, please hold. :)
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.
LGTM! Thank you for putting this together!
auto label is removed for flutter/flutter/148568, due to - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label. |
… SnackBarThemeData.insetPadding is not null (flutter/flutter#148568)
… SnackBarThemeData.insetPadding is not null (flutter/flutter#148568)
flutter/flutter@a1a33e6...c85fa6a 2024-05-29 [email protected] Clean leak in editable_text_test.dart. (flutter/flutter#149223) 2024-05-29 [email protected] Add tests for animated_switcher.0.dart API example. (flutter/flutter#149180) 2024-05-29 [email protected] Manual roll Flutter Engine from d0323905fc2f to b26e1b023cdb (16 revisions) (flutter/flutter#149220) 2024-05-29 [email protected] Change snack bar default hitTestBehavior to deferToChild when SnackBarThemeData.insetPadding is not null (flutter/flutter#148568) 2024-05-29 98614782 auto-submit[bot]@users.noreply.github.com Reverts "sliverGridDelegate mainAxisExtent add assert (#148470)" (flutter/flutter#149224) 2024-05-29 [email protected] sliverGridDelegate mainAxisExtent add assert (flutter/flutter#148470) 2024-05-29 [email protected] Fix `SearchAnchor` suggestions not refreshing after long API call (flutter/flutter#148767) 2024-05-28 737941 [email protected] Add link to golden file test docs in the framework gardener guide (flutter/flutter#149207) 2024-05-28 [email protected] Remove opt out for CurvedAnimation. (flutter/flutter#147863) 2024-05-28 31859944 [email protected] Fix the RenderFlex.computeDryBaseline implementation to match computeDistanceToActualBaseline (flutter/flutter#149062) 2024-05-28 [email protected] Clean leaky test. (flutter/flutter#149199) 2024-05-28 34871572 [email protected] Change `android_plugin_new_output_dir_test.dart` test description (flutter/flutter#149198) 2024-05-28 [email protected] fix M2 InputDecorator suffix icon doesn't turn red on error (flutter/flutter#149161) 2024-05-28 77919688 [email protected] Add selectionOverlayBuilder in CupertinoDatePicker and CupertinoTimer� (flutter/flutter#143079) 2024-05-28 [email protected] Mouse onEnter and onExit now support hovering stylus (flutter/flutter#149006) 2024-05-28 31859944 [email protected] Remove `TextEditingController` private member access (flutter/flutter#149042) 2024-05-28 [email protected] Roll Packages from b7bcb4b to a933c30 (1 revision) (flutter/flutter#149184) 2024-05-28 [email protected] Roll Flutter Engine from b1751088c7e9 to d0323905fc2f (2 revisions) (flutter/flutter#149169) 2024-05-28 [email protected] [tool] Use kebabCase directly (flutter/flutter#149150) 2024-05-28 [email protected] [wiki migration] Leftover wiki pages and links (flutter/flutter#148989) 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/ doc/main/autoroll/README.md
Thank you @Piinks and @MitchellGoodwin so much! |
…rThemeData.insetPadding is not null (flutter#148568) The PR changes the default value of hitTestBehavior in snack bars to `HitTestBehavior.deferToChild` when snackBarTheme.insetPadding is not null, so that widgets behind snack bars affected by the value set to insetPadding, remain interactive even while a snack bar is visible. This PR can be considered as an extension to what have been done in PR flutter#127959 which fixes the same problem but for individual snack bars with margin not being null. This PR works on the theme level. *List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.* flutter#148566
flutter/flutter@a1a33e6...c85fa6a 2024-05-29 [email protected] Clean leak in editable_text_test.dart. (flutter/flutter#149223) 2024-05-29 [email protected] Add tests for animated_switcher.0.dart API example. (flutter/flutter#149180) 2024-05-29 [email protected] Manual roll Flutter Engine from d0323905fc2f to b26e1b023cdb (16 revisions) (flutter/flutter#149220) 2024-05-29 [email protected] Change snack bar default hitTestBehavior to deferToChild when SnackBarThemeData.insetPadding is not null (flutter/flutter#148568) 2024-05-29 98614782 auto-submit[bot]@users.noreply.github.com Reverts "sliverGridDelegate mainAxisExtent add assert (#148470)" (flutter/flutter#149224) 2024-05-29 [email protected] sliverGridDelegate mainAxisExtent add assert (flutter/flutter#148470) 2024-05-29 [email protected] Fix `SearchAnchor` suggestions not refreshing after long API call (flutter/flutter#148767) 2024-05-28 737941 [email protected] Add link to golden file test docs in the framework gardener guide (flutter/flutter#149207) 2024-05-28 [email protected] Remove opt out for CurvedAnimation. (flutter/flutter#147863) 2024-05-28 31859944 [email protected] Fix the RenderFlex.computeDryBaseline implementation to match computeDistanceToActualBaseline (flutter/flutter#149062) 2024-05-28 [email protected] Clean leaky test. (flutter/flutter#149199) 2024-05-28 34871572 [email protected] Change `android_plugin_new_output_dir_test.dart` test description (flutter/flutter#149198) 2024-05-28 [email protected] fix M2 InputDecorator suffix icon doesn't turn red on error (flutter/flutter#149161) 2024-05-28 77919688 [email protected] Add selectionOverlayBuilder in CupertinoDatePicker and CupertinoTimer� (flutter/flutter#143079) 2024-05-28 [email protected] Mouse onEnter and onExit now support hovering stylus (flutter/flutter#149006) 2024-05-28 31859944 [email protected] Remove `TextEditingController` private member access (flutter/flutter#149042) 2024-05-28 [email protected] Roll Packages from b7bcb4b to a933c30 (1 revision) (flutter/flutter#149184) 2024-05-28 [email protected] Roll Flutter Engine from b1751088c7e9 to d0323905fc2f (2 revisions) (flutter/flutter#149169) 2024-05-28 [email protected] [tool] Use kebabCase directly (flutter/flutter#149150) 2024-05-28 [email protected] [wiki migration] Leftover wiki pages and links (flutter/flutter#148989) 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/ doc/main/autoroll/README.md
… SnackBarThemeData.insetPadding is not null (flutter/flutter#148568)
The PR changes the default value of hitTestBehavior in snack bars to
HitTestBehavior.deferToChild
when snackBarTheme.insetPadding is not null, so that widgets behind snack bars affected by the value set to insetPadding, remain interactive even while a snack bar is visible. This PR can be considered as an extension to what have been done in PR #127959 which fixes the same problem but for individual snack bars with margin not being null. This PR works on the theme level.List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.
#148566
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.