Material Bottom Sheet Reveal/Dismiss animation uses a curved animation#51122

Merged

fluttergithubbot merged 21 commits intoflutter:masterfrom

guidezpl:bottom-sheet-curve

Mar 2, 2020

Member

guidezpl commented Feb 20, 2020 •

edited

Loading

Description

Ensures all bottom sheets use non-linear animations for entrance and exits, while still allowing dragging (linearly) bottom sheets. Fixes the animation durations to match the spec.

Before (in slow motion):

After (in slow motion):1

Related Issues

Closes #19469
Future work #51627

Tests

I added the following tests:

A test to ensure that persistent bottom sheets animate non-linearly
A test to ensure that modal bottom sheets animate non-linearly

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.

guidezpl added 9 commits

February 5, 2020 14:59


          Merge changes from marcoms:bottom-sheet-curve

ad4cda0


          add Material curves


          Fix modal bottom sheet duration and easing

1c1b3ce


          Fix typo

c4f6827


          Fix standard bottom sheet easing

126d702


          Merge branch 'master' of https://github.com/flutter/flutter into bott…

3b035ea

…om-sheet-curve


          Remove extraneous exit curve

1357c63


          Rename persistentBottomSheetCurve to standardBottomSheetCurve

72d6dcd


          Add tests

d482801

fluttergithubbot added f: material design framework labels

googlebot added the cla: yes label

perclasson reviewed

View reviewed changes

Contributor

perclasson left a comment

Some style nits and comments

packages/flutter/lib/src/material/scaffold.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/material/scaffold.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/material/scaffold.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/material/curves.dart Show resolved Hide resolved

packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/material/curves.dart Outdated Show resolved Hide resolved


          Address feedback

dc200e2

Zazo032 reviewed

View reviewed changes

packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved


          Merge branch 'master' of https://github.com/flutter/flutter into bott…

4ac3a83

…om-sheet-curve

rami-a approved these changes

View reviewed changes

Contributor

rami-a left a comment

LGTM with a couple comments

packages/flutter/lib/src/material/bottom_sheet.dart Show resolved Hide resolved

packages/flutter/lib/src/material/curves.dart Outdated Show resolved Hide resolved

guidezpl added 2 commits

February 20, 2020 16:08


          Use describeIdentity instead of runtimeType

d8b8e90


          Add comment & newline

aceec0d

rami-a reviewed

View reviewed changes

packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved


          Format documentation

2d5c9c1

Contributor

Zazo032 commented Feb 21, 2020

@guidezpl the analyzer is unhappy about this:

▌15:22:08▐ Trailing spaces...
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
/tmp/flutter sdk/packages/flutter/lib/src/material/bottom_sheet.dart:500: trailing U+0020 space character
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━


          Remove trailing space

4f7803e

guidezpl requested a review from HansMuller

February 21, 2020 10:53

HansMuller reviewed

View reviewed changes

packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/material/bottom_sheet.dart Show resolved Hide resolved

packages/flutter/lib/src/material/bottom_sheet.dart Show resolved Hide resolved

packages/flutter/lib/src/material/curves.dart Outdated Show resolved Hide resolved

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

                 final bool enableDrag;
+                /// Called when the user begins dragging the bottom sheet vertically.
+                final BottomSheetDragStartHandler onDragStart;

Contributor

HansMuller Feb 24, 2020

The new callbacks should explain how they'd typically be used and how they relate to the existing onClosing and enableDrag parameters.

Member Author

guidezpl Feb 25, 2020

Done

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

+              /// straight line, and the top-right quarter will contain the entire contents of
+              /// [Curves.easeOut].
+              ///
+              /// This is useful in situations where a widget must track the user's finger

Contributor

HansMuller Feb 24, 2020

This curve appears to fit a common idiom - linear drag then cuved animated fling - that's not specific to BottomSheet. Rather than defining it as specific to BottomSheet now, and then later as a generic utility with a different name, can we keep the API private for now and then make it public later? We'd have to include a copy of _BottomSheetSuspendedCurve in scaffold.dart for now. Making this public would require adding some usage examples to the docs and maybe retrofitting some existing code to use it (and to prove that it's generally useful).

Member Author

guidezpl Feb 25, 2020

I don't mind adding examples and putting it in material/curves.dart. I think this would be cleaner and a future PR could use it in more places that exhibit this kind of behaviour. What do you think?

Contributor

HansMuller Feb 26, 2020

I think that expanding on this part of the PR, by adding a track-fling curve and some examples to curves.dart in a future PR, is a good idea. So long as the curve is (the two copies of the curve are) private now.

Member Author

guidezpl Feb 28, 2020

Sounds good

packages/flutter/lib/src/material/bottom_sheet.dart Show resolved Hide resolved

packages/flutter/lib/src/material/curves.dart Show resolved Hide resolved

guidezpl added 4 commits

February 25, 2020 15:15


          Update documentation

b8fe425


          Specify import


          Update Curve

b77a5d3


          BottomSheetSuspendedCurve extends ParametricCurve instead of Curve

5410d52

guidezpl mentioned this pull request

Look into making _BottomSheetSuspendedCurve public #51627

Closed

guidezpl added 2 commits

February 28, 2020 11:08


          Make BottomSheetSuspendedCurve private

ea3d006


          Fix test

ec9e65c

HansMuller approved these changes

View reviewed changes

Contributor

HansMuller left a comment

LGTM

guidezpl added the waiting for tree to go green label

fluttergithubbot merged commit ec64f93 into flutter:master

guidezpl deleted the bottom-sheet-curve branch

March 3, 2020 09:29

github-actions bot locked as resolved and limited conversation to collaborators

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

f: material design framework