Material Bottom Sheet Reveal/Dismiss animation uses a curved animation by guidezpl · Pull Request #51122 · flutter/flutter

guidezpl · 2020-02-20T14:04:30Z

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.

…om-sheet-curve

perclasson

Some style nits and comments

…om-sheet-curve

rami-a

LGTM with a couple comments

Zazo032 · 2020-02-21T09:37:54Z

@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
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

HansMuller · 2020-02-24T16:29:26Z

  final bool enableDrag;

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


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

HansMuller · 2020-02-24T16:38:23Z

+/// 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


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).

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?

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.

Sounds good

HansMuller

LGTM

guidezpl added 9 commits February 5, 2020 14:59

Merge changes from marcoms:bottom-sheet-curve

ad4cda0

add Material curves

8948052

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 p: material_ui material_ui package in flutter/packages framework flutter/packages/flutter repository. See also f: labels. labels Feb 20, 2020

googlebot added the cla: yes label Feb 20, 2020

perclasson reviewed Feb 20, 2020

View reviewed changes

Address feedback

dc200e2

Zazo032 reviewed Feb 20, 2020

View reviewed changes

Comment thread packages/flutter/lib/src/material/bottom_sheet.dart Outdated

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

4ac3a83

…om-sheet-curve

rami-a approved these changes Feb 20, 2020

View reviewed changes

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

Comment thread packages/flutter/lib/src/material/curves.dart Outdated

guidezpl added 2 commits February 20, 2020 16:08

Use describeIdentity instead of runtimeType

d8b8e90

Add comment & newline

aceec0d

rami-a reviewed Feb 20, 2020

View reviewed changes

Comment thread packages/flutter/lib/src/material/bottom_sheet.dart Outdated

Format documentation

2d5c9c1

Remove trailing space

4f7803e

guidezpl requested a review from HansMuller February 21, 2020 10:53

HansMuller reviewed Feb 24, 2020

View reviewed changes

guidezpl added 4 commits February 25, 2020 15:15

Update documentation

b8fe425

Specify import

3105215

Update Curve

b77a5d3

BottomSheetSuspendedCurve extends ParametricCurve instead of Curve

5410d52

guidezpl mentioned this pull request Feb 28, 2020

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 Mar 2, 2020

View reviewed changes

guidezpl added the waiting for tree to go green label Mar 2, 2020

fluttergithubbot merged commit ec64f93 into flutter:master Mar 2, 2020

guidezpl deleted the bottom-sheet-curve branch March 3, 2020 09:29

github-actions Bot locked as resolved and limited conversation to collaborators Aug 1, 2021

Conversation

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

Uh oh!

Description

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

perclasson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rami-a left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Zazo032 commented Feb 21, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HansMuller Feb 24, 2020

Choose a reason for hiding this comment

Uh oh!

guidezpl Feb 25, 2020

Choose a reason for hiding this comment

Uh oh!

HansMuller Feb 24, 2020

Choose a reason for hiding this comment

Uh oh!

guidezpl Feb 25, 2020

Choose a reason for hiding this comment

Uh oh!

HansMuller Feb 26, 2020

Choose a reason for hiding this comment

Uh oh!

guidezpl Feb 28, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

HansMuller left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

guidezpl commented Feb 20, 2020 •

edited

Loading