Step 1: SnackBarBehavior.floating offset fix - Soft breaking change #50597

shihaohong · 2020-02-11T21:13:49Z

Description

Relands #47616, but as an opt-in change to allow developers to migrate to the fixed version. The original PR caused golden tests to fail since the SnackBar's offset was updated.

Screenshots

Before fix

After fix

Migration guide

Set all instances of Scaffold.shouldSnackBarIgnoreFABRect to true after this PR is merged.
Fix all failing tests. There are likely two main types:
a) Fix all golden tests to expect the new change.
b) Fix any widget tests that expect the SnackBar to appear higher than it should. The difference should simply be the size of the floating action button's rect.
Once Scaffold.shouldSnackBarIgnoreFABRect is set to true by default (in a subsequent PR), remove the parameter from all instances of Scaffold.

Related Issues

Addresses #47202
Addresses #43716

Tests

I added the following tests:

Same tests as original PR, but with the shouldSnackBarIgnoreFABRect flag turned on.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I signed the CLA.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

No, no existing tests failed, so this is not a breaking change.
Yes, this is a breaking change. If not, delete the remainder of this section.
- I wrote a design doc and migration guide: https://flutter.dev/go/floating-snackbar-offset

Now the test checks certain position Y of FAB

added tests for testing height of SnackBar.

…n' to group 'SnackBar position' improved test by removing padding details, and renaming.

… group "SnackBar position" and improved test for testing with universal behavior

…-soft-break

shihaohong · 2020-02-11T21:14:00Z

cc/ @filaps If you're okay with these changes, could you consent to the CLA? Please feel free to make any suggestions to the PR if you have any as well

googlebot · 2020-02-11T21:14:02Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

dnfield · 2020-02-11T22:19:04Z

packages/flutter/lib/src/material/scaffold.dart

+    'is present when it is not. This parameter will be removed. '
+    'This feature was deprecated after v1.15.3.'
+  )
+  final bool shouldSnackBarIgnoreFABRect;


Nit - why not just make this static?

How would that work? Would all Flutter apps then simply set Scaffold.shouldSnackBarIgnoreFABRect = true before they call runApp?

Right. It's not something we expect people to want to use, let alone to use differently for different scaffolds.

Presumably they could even set it after runApp, but they should probably do it before any builds kick off to avoid weirdness.

filaps · 2020-02-12T14:18:47Z

packages/flutter/lib/src/material/scaffold.dart

+  /// This flag is deprecated and fixes and issue with incorrect clipping
+  /// and positioning of the [SnackBar] set to [SnackBarBehavior.floating].
+  @Deprecated(
+    'Fixes a bug that that fixes clipping and positioning of SnackBar '


nit:

Suggested change

'Fixes a bug that that fixes clipping and positioning of SnackBar '

'Fixes a bug that fixes clipping and positioning of SnackBar '

I realized the wording in general was off, so I changed it to the following if that's okay:

'Fixes a bug that incorrectly clips and positions SnackBar. It ' 'previously incorrectly offsets itself assuming a floating action button ' 'is present even when it is not. This parameter will be removed. ' 'This feature was deprecated after v1.15.3.'

filaps · 2020-02-12T14:23:22Z

@googlebot I consent.

googlebot · 2020-02-12T14:23:33Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

filaps · 2020-02-12T14:42:42Z

LGTM.
Thank you so much for help with "breaking change process"!

…-soft-break

dnfield · 2020-02-12T20:41:19Z

packages/flutter/lib/src/material/scaffold.dart

    @required this.isSnackBarFloating,
    @required this.extendBody,
    @required this.extendBodyBehindAppBar,
+    @required this.shouldSnackBarIgnoreFABRect,


Let's ditch this and just acccess the static property.

dnfield · 2020-02-12T20:42:58Z

packages/flutter/lib/src/material/scaffold.dart

+    'Fixes a bug that incorrectly clips and positions SnackBar. It '
+    'previously incorrectly offsets itself assuming a floating action button '
+    'is present even when it is not. This parameter will be removed. '
+    'This feature was deprecated after v1.15.3.'


nit: maybe something like:

This property controls whether to clip and position the snackbar as if there is always a floating action button, even if one is not present. It exists to provide backwards compatibility to ease migrations, and will eventually be removed.

dnfield

LGTM with some nits

filaps and others added 7 commits February 6, 2020 21:46

renamed some tests in snack_bar_test.dart

2e17635

improved one test from snack_bar_test.dart

15292f1

Now the test checks certain position Y of FAB

fixed issue where snack bar was higher then should be

1dbd586

added tests for testing height of SnackBar.

moved test 'Floating SnackBar is positioned above FloatingActionButto…

19ad3f2

…n' to group 'SnackBar position' improved test by removing padding details, and renaming.

moved test "SnackBar bottom padding is not consumed by viewInsets" to…

66c0d19

… group "SnackBar position" and improved test for testing with universal behavior

Merge branch 'master' of github.com:flutter/flutter into snackbar-fix…

de48df7

…-soft-break

Add soft breaking change flag

7110b5a

fluttergithubbot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Feb 11, 2020

googlebot added the cla: no label Feb 11, 2020

shihaohong changed the title ~~Snackbar fix soft break~~ Step 1: SnackBarBehavior.floating offset fix - Soft breaking change Feb 11, 2020

shihaohong added the work in progress; do not review label Feb 11, 2020

shihaohong requested review from HansMuller and dnfield February 11, 2020 22:09

dnfield reviewed Feb 11, 2020

View reviewed changes

Make flag static

efbe774

filaps reviewed Feb 12, 2020

View reviewed changes

googlebot added cla: yes and removed cla: no labels Feb 12, 2020

shihaohong added 2 commits February 12, 2020 08:10

Improve deprecation warning

8e42ebf

Merge branch 'master' of github.com:flutter/flutter into snackbar-fix…

cab8003

…-soft-break

shihaohong requested a review from dnfield February 12, 2020 19:45

shihaohong removed the work in progress; do not review label Feb 12, 2020

dnfield reviewed Feb 12, 2020

View reviewed changes

dnfield approved these changes Feb 12, 2020

View reviewed changes

Code review

5065de1

shihaohong merged commit 1f3d423 into flutter:master Feb 13, 2020

shihaohong mentioned this pull request Mar 6, 2020

Step 2: SnackBarBehavior.floating offset fix by default #52136

Closed

11 tasks

shihaohong deleted the snackbar-fix-soft-break branch April 3, 2020 19:07

This was referenced Apr 16, 2020

Step 2: SnackBarBehavior.floating offset fix by default #54985

Merged

Step 3: Removes temporary flag for SnackBarBehavior.floating offset fix #55064

Merged

perclasson mentioned this pull request Jun 15, 2020

Fix reorderable listview onreorder callback #59103

Closed

13 tasks

github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2021

	'Fixes a bug that that fixes clipping and positioning of SnackBar '
	'Fixes a bug that fixes clipping and positioning of SnackBar '

Step 1: SnackBarBehavior.floating offset fix - Soft breaking change #50597

Step 1: SnackBarBehavior.floating offset fix - Soft breaking change #50597

Uh oh!

Conversation

shihaohong commented Feb 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Screenshots

Before fix

After fix

Migration guide

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

shihaohong commented Feb 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

googlebot commented Feb 11, 2020

Uh oh!

dnfield Feb 11, 2020

Choose a reason for hiding this comment

Uh oh!

shihaohong Feb 11, 2020

Choose a reason for hiding this comment

Uh oh!

dnfield Feb 11, 2020

Choose a reason for hiding this comment

Uh oh!

shihaohong Feb 11, 2020

Choose a reason for hiding this comment

Uh oh!

filaps Feb 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shihaohong Feb 12, 2020

Choose a reason for hiding this comment

Uh oh!

filaps commented Feb 12, 2020

Uh oh!

googlebot commented Feb 12, 2020

Uh oh!

filaps commented Feb 12, 2020

Uh oh!

dnfield Feb 12, 2020

Choose a reason for hiding this comment

Uh oh!

dnfield Feb 12, 2020

Choose a reason for hiding this comment

Uh oh!

dnfield left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

shihaohong commented Feb 11, 2020 •

edited

Loading

shihaohong commented Feb 11, 2020 •

edited

Loading

filaps Feb 12, 2020 •

edited

Loading