[Snackbar] Add hitTestBehavior property to fix blocking user hits issue
#114810
Conversation
This is a simple solution to allow users themselves to set the desired behavior of intercepting user hits from Snackbar Snackbar under the hood uses the Dismissible widget, which has HitTestBehavior set to HitTestBehavior.opaque by default, which affects the entire area, including margins
flutter-dashboard
bot
added
f: material design
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Github suggested some people to add as reviewers so I added them. I'm not familiar with this code so I am not the right person to tag here. You really should fix all of the problems first before asking for reviews. It looks like your CLA signing failed and there are some code formatting errors that are causing the analyzer to fail the checks. |
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
Thanks for the PR!
Maybe @darrenaustin has thoughts about whether or not we should add this parameter, but it seems reasonable to me at first glance, besides some comments I left below.
However, the main thing this PR is missing is a test! Can you take a look at adding a test to snack_bar_test.dart?
|
@justinmc hi, any news on it? It doesn't require any tests, because it just exposes access to the idk why but CI failed with unrelevant error with this PR ( |
|
@MindMayhem This will definitely need a test of some sort, because otherwise when we refactor this code we might accidentally revert your change and not even notice. The tests make sure that we keep your change intact when doing larger changes. |
|
@Hixie understand your point, but honestly, I have never written tests, so i will need a lil help with it |
|
@MindMayhem Start with this page in the wiki, and then take a look at the existing tests in in snack_bar_test.dart. |
|
@MindMayhem I'm going to close this PR for now to get it off our review queue, but please don't hesitate to submit a new PR if you have the time to get back to it. In the meantime I'll mention this PR on the issue so that anyone else who wants to pick it up can do so. Thanks again for your contribution! |
If the margin is used, set the `HitTestBehavior` to `deferToChild`. *List which issues are fixed by this PR. You must list at least one issue.* flutter#78537 flutter#114810 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
4 participants
Allow users themselves to set the desired behavior of intercepting user hits from
SnackbarSnackbar under the hood uses the
Dismissiblewidget, which has HitTestBehavior set toHitTestBehavior.opaqueby default, which affects the entire area, including marginsFixes:
Pre-launch Checklist
///).