Skip to content

Tap on button behind snack bar defined by margin#127959

Merged
auto-submit[bot] merged 2 commits intoflutter:masterfrom
lsaudon:snackbar-margin-tap
Jul 14, 2023
Merged

Tap on button behind snack bar defined by margin#127959
auto-submit[bot] merged 2 commits intoflutter:masterfrom
lsaudon:snackbar-margin-tap

Conversation

@lsaudon
Copy link
Copy Markdown
Contributor

@lsaudon lsaudon commented May 31, 2023

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.
#78537
#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.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels May 31, 2023
@github-actions github-actions bot removed the framework flutter/packages/flutter repository. See also f: labels. label May 31, 2023
@arxdeus
Copy link
Copy Markdown

arxdeus commented Jun 22, 2023

why not also let the developer choose the hitTestBehavior parameter himself?

#114810 - like here

@lsaudon
Copy link
Copy Markdown
Contributor Author

lsaudon commented Jun 22, 2023

why not also let the developer choose the hitTestBehavior parameter himself?

#114810 - like here

Because I don't want to have to know that I have to put in a hitTestBehavior when I change the margin.
I don't find that logical.
Leaving the choice means that I fix the bug only for those who know about it.

I can do this

behavior: widget.hitTestBehavior ?? widget.margin != null
            ? HitTestBehavior.deferToChild
            : HitTestBehavior.opaque,

@bleroux
Copy link
Copy Markdown
Contributor

bleroux commented Jun 23, 2023 •
edited
Loading

I can do this

behavior: widget.hitTestBehavior ?? widget.margin != null
            ? HitTestBehavior.deferToChild
            : HitTestBehavior.opaque,

I too think it would be great to update the PR this way.

@github-actions github-actions bot added the framework flutter/packages/flutter repository. See also f: labels. label Jun 23, 2023
bleroux
bleroux reviewed Jun 23, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@bleroux bleroux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor comments on the tests code.

@lsaudon
Copy link
Copy Markdown
Contributor Author

lsaudon commented Jun 23, 2023

I can do this

behavior: widget.hitTestBehavior ?? widget.margin != null
            ? HitTestBehavior.deferToChild
            : HitTestBehavior.opaque,

I too think it would be great to update the PR this way.

I added it.

bleroux
bleroux approved these changes Jun 23, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@bleroux bleroux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Thanks @lsaudon and @hot-moms

cc @justinmc for a review (because you reviewed #114810)

@bleroux bleroux requested a review from justinmc June 23, 2023 09:48
justinmc
justinmc approved these changes Jun 26, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 Good call adding the hitTestBehavior parameter as well.

bleroux
bleroux reviewed Jul 11, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@bleroux bleroux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lsaudon Thanks for you patience, just another step left to polish the comments.

@bleroux bleroux added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 14, 2023
@auto-submit auto-submit bot merged commit c3cd016 into flutter:master Jul 14, 2023
This was referenced Jul 14, 2023
Closed
Open
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 14, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 14, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
7f7a57e 
…tter#127959)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 14, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 14, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
b22f47b 
…tter#127959)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 14, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 14, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
9af75f6 
…tter#127959)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 14, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 14, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
279d6e4 
…tter#127959)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 14, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 14, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
f322632 
…tter#127959)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 14, 2023
Closed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 16, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 16, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
b3a69fe 
…tter#127959)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 17, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 17, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
1630d9d 
…tter#127959)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 17, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 17, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
bf16cee 
…tter#127959)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 17, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 17, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
9e70e8d 
…tter#127959)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 17, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 17, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
ab3bba0 
…tter#127959)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 17, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 17, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
dd7e346 
…tter#127959)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 17, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 17, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
47a0920 
…tter#127959)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 17, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
e9b80b4 
…tter#127959)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 17, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
c0264d9 
…tter#127959)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
4fa69fc 
…tter#127959)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
626ef2f 
…tter#127959)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
7108418 
…tter#127959)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
b58b621 
…tter#127959)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
f2cd6d0 
…tter#127959)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
ede2d61 
…tter#127959)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
5ca2e47 
…tter#127959)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 18, 2023
@engine-flutter-autoroll
c3cd016 Tap on button behind snack bar defined by margin (flutter/flu…
ada3bde 
…tter#127959)
@HansMuller HansMuller mentioned this pull request Jul 21, 2023
Closed
LouiseHsu pushed a commit to LouiseHsu/flutter that referenced this pull request Jul 31, 2023
@lsaudon @LouiseHsu
Tap on button behind snack bar defined by margin (flutter#127959)
7a5981c 
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].*
@lsaudon lsaudon deleted the snackbar-margin-tap branch September 15, 2023 07:47
@hany-achraf hany-achraf mentioned this pull request May 17, 2024
Merged
9 tasks
auto-submit bot pushed a commit that referenced this pull request May 29, 2024
@hany-achraf
Change snack bar default hitTestBehavior to deferToChild when SnackBa…
9ea9726 
…rThemeData.insetPadding is not null (#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
victorsanni pushed a commit to victorsanni/flutter that referenced this pull request May 31, 2024
@hany-achraf @victorsanni
Change snack bar default hitTestBehavior to deferToChild when SnackBa…
fa19af3 
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bleroux bleroux bleroux approved these changes

@justinmc justinmc justinmc approved these changes

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lsaudon @arxdeus @bleroux @justinmc