Fix floating SnackBar throws when FAB is on the top by bleroux · Pull Request #129274 · flutter/flutter

bleroux · 2023-06-21T13:51:03Z

Description

This PR updates how a floating snack bar is positionned when a Scaffold defines a FAB with Scaffold.floatingActionButtonLocation sets to one of the top locations.

Before this PR:

When a FAB location is set to the top of the Scaffold, a floating SnackBar can't be displayed and an assert throws in debug mode.

After this PR:

When a FAB location is set to the top of the Scaffold, a floating SnackBar will be displayed at the bottom of the screen, above a NavigationBar for instance (the top FAB is ignored when computing the floating snack bar position).

Motivation

This is a edge case related to a discrepancy between the Material spec and the Flutter Scaffold customizability:

Material spec states that a floating SnackBar should be displayed above a FAB. But, in Material spec, FABs are expected to be on the bottom.
Since Support New and Custom FAB Locations #51465, Flutter Scaffold makes it valid to show a FAB on the top of the Scaffold.

Related Issue

fixes #128150

Tests

Adds 1 test.

Piinks · 2023-06-23T15:22:20Z

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

This should use a switch statement. Dart 3 makes it really quite nice!

For example:

final BorderRadius childrenGroupBorderRadius = switch (type) { CupertinoListSectionType.insetGrouped => _kDefaultInsetGroupedBorderRadius, CupertinoListSectionType.base => BorderRadius.zero, };

Nice! I switched to Dart 3 switch 😄

Piinks · 2023-07-10T22:11:38Z

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

These can use || so => false does not need to be repeated.

Piinks · 2023-07-10T22:13:19Z

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

Instead of a default case, can you enumerate the other values? That way if we ever add another case, we won't introduce a bug where it isn't accounted for. This is actually why we prefer to use switch statements with enums :)

https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#avoid-using-if-chains-or--or--with-enum-values

This is actually why we prefer to use switch statements with enums :)

I'm not used to rely on switch, thanks for your explanations 👍 .

In this particular case, it turned out that FloatingActionButtonLocation is not an enum (values are constants defined in an abstract class and they are initialized by instantiating various subclasses). I think we can't avoid adding a default case because this is not a real enum?
I pushed a commit where I enumerated all values (there are a lot ot them). It seems that this an edge case where maybe we can avoid listing all values for readability?

Oh wow I did not realize it was an enum! TIL something new! In that case (and sorry for not realizing it sooner) I would recommend throwing an error for the default case. That way, if we add a new location, there will be an error that let's us know we need to account for it here. :)

Piinks · 2023-07-18T17:22:53Z

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

So this instead could be that default case that presents an error. :)
I know it is very verbose, but it will keep future bugs away.

Piinks

LGTM!

…flutter#129274)

flutter/flutter@d7ed5dc...9def8f6 2023-07-25 [email protected] Proposal to add barrier configs for showDatePicker, showTimePicker and showAboutDialog. (flutter/flutter#130484) 2023-07-25 [email protected] Roll Flutter Engine from a7a842ee9ccd to 036c58f79307 (1 revision) (flutter/flutter#131244) 2023-07-25 [email protected] Roll Flutter Engine from 3baca2fe55c8 to a7a842ee9ccd (1 revision) (flutter/flutter#131243) 2023-07-25 [email protected] Roll Flutter Engine from 9a0192d965e0 to 3baca2fe55c8 (1 revision) (flutter/flutter#131241) 2023-07-25 [email protected] Roll Flutter Engine from ceb2674e82b4 to 9a0192d965e0 (3 revisions) (flutter/flutter#131230) 2023-07-25 [email protected] Roll Flutter Engine from 4fded78e5a01 to ceb2674e82b4 (2 revisions) (flutter/flutter#131229) 2023-07-25 [email protected] Roll Flutter Engine from ff02fa72acce to 4fded78e5a01 (2 revisions) (flutter/flutter#131225) 2023-07-24 [email protected] Roll Flutter Engine from a489c7496268 to ff02fa72acce (1 revision) (flutter/flutter#131221) 2023-07-24 [email protected] Roll Flutter Engine from 815b97157dc7 to a489c7496268 (3 revisions) (flutter/flutter#131218) 2023-07-24 [email protected] Roll Flutter Engine from 2b8d83fa20e3 to 815b97157dc7 (5 revisions) (flutter/flutter#131214) 2023-07-24 [email protected] Use toStringAsFixed in DecorationImage.toString (flutter/flutter#131026) 2023-07-24 [email protected] Roll Flutter Engine from aa876f6bec69 to 2b8d83fa20e3 (3 revisions) (flutter/flutter#131207) 2023-07-24 [email protected] Fix M3 TimePicker dial background uses incorrect color (flutter/flutter#131045) 2023-07-24 [email protected] Fix floating SnackBar throws when FAB is on the top (flutter/flutter#129274) 2023-07-24 [email protected] Update link to unbounded constraints error (flutter/flutter#131205) 2023-07-24 [email protected] Optimize SliverMainAxisGroup/SliverCrossAxisGroup paint function (flutter/flutter#129310) 2023-07-24 [email protected] [DropdownMenu] Close menu after editing is complete (flutter/flutter#130710) 2023-07-24 [email protected] Reduce usage of testUsingContext (flutter/flutter#131078) 2023-07-24 [email protected] Roll Packages from 2266a76 to 8028caf (13 revisions) (flutter/flutter#131196) 2023-07-24 [email protected] Fix material date picker behavior when changing year (flutter/flutter#130486) 2023-07-24 [email protected] Update Gallery demo app themes for Material3 compatibility (flutter/flutter#131093) 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

This reverts commit 97e0a05.

…e top" (#131303) Reverts #129274 temporarily putting up a revert in case a fix is difficult context: [b/293202068](http://b/293202068) youtube integration tests failed

## Description This PR is a reland of #129274 with a fix and new test related to the revert (#131303). It updates how a floating snack bar is positionned when a `Scaffold` defines a FAB with `Scaffold.floatingActionButtonLocation` sets to one of the top locations. **Before this PR:** - When a FAB location is set to the top of the `Scaffold`, a floating `SnackBar` can't be displayed and an assert throws in debug mode. **After this PR:** - When a FAB location is set to the top of the `Scaffold`, a floating `SnackBar` will be displayed at the bottom of the screen, above a `NavigationBar` for instance (the top FAB is ignored when computing the floating snack bar position). ![image](https://github.com/flutter/flutter/assets/840911/08fcee6c-b286-4749-ad0b-ba09e653bd94) ## Motivation This is a edge case related to a discrepancy between the Material spec and the Flutter `Scaffold` customizability: - Material spec states that a floating `SnackBar` should be displayed above a FAB. But, in Material spec, FABs are expected to be on the bottom. - Since #51465, Flutter `Scaffold` makes it valid to show a FAB on the top of the `Scaffold`. ## Related Issue fixes #128150 ## Tests Adds 2 tests.

## Description This PR updates how a floating snack bar is positionned when a `Scaffold` defines a FAB with `Scaffold.floatingActionButtonLocation` sets to one of the top locations. **Before this PR:** - When a FAB location is set to the top of the `Scaffold`, a floating `SnackBar` can't be displayed and an assert throws in debug mode. **After this PR:** - When a FAB location is set to the top of the `Scaffold`, a floating `SnackBar` will be displayed at the bottom of the screen, above a `NavigationBar` for instance (the top FAB is ignored when computing the floating snack bar position). ![image](https://github.com/flutter/flutter/assets/840911/08fcee6c-b286-4749-ad0b-ba09e653bd94) ## Motivation This is a edge case related to a discrepancy between the Material spec and the Flutter `Scaffold` customizability: - Material spec states that a floating `SnackBar` should be displayed above a FAB. But, in Material spec, FABs are expected to be on the bottom. - Since flutter#51465, Flutter `Scaffold` makes it valid to show a FAB on the top of the `Scaffold`. ## Related Issue fixes flutter#128150 ## Tests Adds 1 test.

…e top" (flutter#131303) Reverts flutter#129274 temporarily putting up a revert in case a fix is difficult context: [b/293202068](http://b/293202068) youtube integration tests failed

…#131475) ## Description This PR is a reland of flutter#129274 with a fix and new test related to the revert (flutter#131303). It updates how a floating snack bar is positionned when a `Scaffold` defines a FAB with `Scaffold.floatingActionButtonLocation` sets to one of the top locations. **Before this PR:** - When a FAB location is set to the top of the `Scaffold`, a floating `SnackBar` can't be displayed and an assert throws in debug mode. **After this PR:** - When a FAB location is set to the top of the `Scaffold`, a floating `SnackBar` will be displayed at the bottom of the screen, above a `NavigationBar` for instance (the top FAB is ignored when computing the floating snack bar position). ![image](https://github.com/flutter/flutter/assets/840911/08fcee6c-b286-4749-ad0b-ba09e653bd94) ## Motivation This is a edge case related to a discrepancy between the Material spec and the Flutter `Scaffold` customizability: - Material spec states that a floating `SnackBar` should be displayed above a FAB. But, in Material spec, FABs are expected to be on the bottom. - Since flutter#51465, Flutter `Scaffold` makes it valid to show a FAB on the top of the `Scaffold`. ## Related Issue fixes flutter#128150 ## Tests Adds 2 tests.

## Description This PR updates how a floating snack bar is positionned when a `Scaffold` defines a FAB with `Scaffold.floatingActionButtonLocation` sets to one of the top locations. **Before this PR:** - When a FAB location is set to the top of the `Scaffold`, a floating `SnackBar` can't be displayed and an assert throws in debug mode. **After this PR:** - When a FAB location is set to the top of the `Scaffold`, a floating `SnackBar` will be displayed at the bottom of the screen, above a `NavigationBar` for instance (the top FAB is ignored when computing the floating snack bar position). ![image](https://github.com/flutter/flutter/assets/840911/08fcee6c-b286-4749-ad0b-ba09e653bd94) ## Motivation This is a edge case related to a discrepancy between the Material spec and the Flutter `Scaffold` customizability: - Material spec states that a floating `SnackBar` should be displayed above a FAB. But, in Material spec, FABs are expected to be on the bottom. - Since flutter#51465, Flutter `Scaffold` makes it valid to show a FAB on the top of the `Scaffold`. ## Related Issue fixes flutter#128150 ## Tests Adds 1 test.

…e top" (flutter#131303) Reverts flutter#129274 temporarily putting up a revert in case a fix is difficult context: [b/293202068](http://b/293202068) youtube integration tests failed

…#131475) ## Description This PR is a reland of flutter#129274 with a fix and new test related to the revert (flutter#131303). It updates how a floating snack bar is positionned when a `Scaffold` defines a FAB with `Scaffold.floatingActionButtonLocation` sets to one of the top locations. **Before this PR:** - When a FAB location is set to the top of the `Scaffold`, a floating `SnackBar` can't be displayed and an assert throws in debug mode. **After this PR:** - When a FAB location is set to the top of the `Scaffold`, a floating `SnackBar` will be displayed at the bottom of the screen, above a `NavigationBar` for instance (the top FAB is ignored when computing the floating snack bar position). ![image](https://github.com/flutter/flutter/assets/840911/08fcee6c-b286-4749-ad0b-ba09e653bd94) ## Motivation This is a edge case related to a discrepancy between the Material spec and the Flutter `Scaffold` customizability: - Material spec states that a floating `SnackBar` should be displayed above a FAB. But, in Material spec, FABs are expected to be on the bottom. - Since flutter#51465, Flutter `Scaffold` makes it valid to show a FAB on the top of the `Scaffold`. ## Related Issue fixes flutter#128150 ## Tests Adds 2 tests.

github-actions bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Jun 21, 2023

bleroux changed the title ~~[WIP] Fix floating SnackBar throws when FAB is on the top~~ Fix floating SnackBar throws when FAB is on the top Jun 21, 2023

bleroux force-pushed the show_floating_snackbar_at_bottom_when_FAB_on_top branch from 0b94627 to 4f99d30 Compare June 21, 2023 14:13

bleroux requested a review from Piinks June 23, 2023 07:56

Piinks reviewed Jun 23, 2023

View reviewed changes

bleroux force-pushed the show_floating_snackbar_at_bottom_when_FAB_on_top branch 3 times, most recently from ee17aa8 to 8a0fa25 Compare June 26, 2023 09:15

bleroux requested a review from Piinks June 26, 2023 12:26

Piinks reviewed Jul 10, 2023

View reviewed changes

bleroux force-pushed the show_floating_snackbar_at_bottom_when_FAB_on_top branch from 8a0fa25 to 162fafc Compare July 11, 2023 08:59

bleroux requested a review from Piinks July 12, 2023 15:14

Piinks reviewed Jul 18, 2023

View reviewed changes

bleroux added 2 commits July 19, 2023 09:10

Fix floating SnackBar throws when FAB is on the top

7a1f533

Enumerates all FloatingActionButtonLocation values

350f4ba

bleroux force-pushed the show_floating_snackbar_at_bottom_when_FAB_on_top branch from 162fafc to 6813883 Compare July 19, 2023 07:19

Throw a FlutterError for default case

0bc5fe4

bleroux force-pushed the show_floating_snackbar_at_bottom_when_FAB_on_top branch from 6813883 to 0bc5fe4 Compare July 19, 2023 07:31

bleroux requested a review from Piinks July 19, 2023 08:40

Piinks approved these changes Jul 24, 2023

View reviewed changes

Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 24, 2023

auto-submit bot merged commit 97e0a05 into flutter:master Jul 24, 2023

bleroux deleted the show_floating_snackbar_at_bottom_when_FAB_on_top branch July 24, 2023 18:32

engine-flutter-autoroll mentioned this pull request Jul 25, 2023

Roll Flutter from d7ed5dc2e212 to c8b9b15e4cd1 (20 revisions) flutter/packages#4560

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 25, 2023

97e0a05 Fix floating SnackBar throws when FAB is on the top (flutter/…

ba77339

…flutter#129274)

engine-flutter-autoroll mentioned this pull request Jul 25, 2023

Roll Flutter from d7ed5dc2e212 to 9def8f6bc56b (21 revisions) flutter/packages#4561

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 25, 2023

97e0a05 Fix floating SnackBar throws when FAB is on the top (flutter/…

3929bf1

…flutter#129274)

XilaiZhang added a commit that referenced this pull request Jul 25, 2023

Revert "Fix floating SnackBar throws when FAB is on the top (#129274)"

d906fba

This reverts commit 97e0a05.

XilaiZhang mentioned this pull request Jul 25, 2023

[flutter roll] Revert "Fix floating SnackBar throws when FAB is on the top" #131303

Merged

bleroux restored the show_floating_snackbar_at_bottom_when_FAB_on_top branch July 28, 2023 08:39

bleroux deleted the show_floating_snackbar_at_bottom_when_FAB_on_top branch July 28, 2023 08:39

bleroux mentioned this pull request Jul 28, 2023

Reland - Fix floating SnackBar throws when FAB is on the top #131475

Merged

HansMuller mentioned this pull request Jul 28, 2023

Flutter Framework Team: Notable Commits #121415

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix floating SnackBar throws when FAB is on the top#129274

Fix floating SnackBar throws when FAB is on the top#129274
auto-submit[bot] merged 3 commits intoflutter:masterfrom
NevercodeHQ:show_floating_snackbar_at_bottom_when_FAB_on_top

bleroux commented Jun 21, 2023 •

edited

Loading

Uh oh!

Piinks Jun 23, 2023

Uh oh!

bleroux Jun 26, 2023

Uh oh!

Piinks Jul 10, 2023

Uh oh!

Piinks Jul 10, 2023

Uh oh!

bleroux Jul 11, 2023 •

edited

Loading

Uh oh!

Piinks Jul 18, 2023

Uh oh!

Piinks Jul 18, 2023

Uh oh!

Piinks left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

bleroux commented Jun 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation

Related Issue

Tests

Uh oh!

Piinks Jun 23, 2023

Choose a reason for hiding this comment

Uh oh!

bleroux Jun 26, 2023

Choose a reason for hiding this comment

Uh oh!

Piinks Jul 10, 2023

Choose a reason for hiding this comment

Uh oh!

Piinks Jul 10, 2023

Choose a reason for hiding this comment

Uh oh!

bleroux Jul 11, 2023 • 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!

Piinks Jul 18, 2023

Choose a reason for hiding this comment

Uh oh!

Piinks Jul 18, 2023

Choose a reason for hiding this comment

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bleroux commented Jun 21, 2023 •

edited

Loading

bleroux Jul 11, 2023 •

edited

Loading