Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Report progress on Dismissible update callback #95504

Merged
merged 7 commits into from Feb 5, 2022
Merged

Report progress on Dismissible update callback #95504

merged 7 commits into from Feb 5, 2022
+55 −1

Conversation

@cachapa
Copy link
Contributor

@cachapa cachapa commented Dec 18, 2021 •
edited

This PR adds a progress parameter to the Dismissible onUpdate callback.
The value represents how far the dismissible child has been dragged in its parent container as a ratio value between 0.0 and 1.0, which can be useful to e.g. fade elements in sync with the gesture.

This implementation also correctly reports the position during the return animation when the dismiss is unsuccessful.

This PR partly solves #21230 (3rd. point) and closes #74582

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.
  • All existing and new tests are passing.
@goderbauer goderbauer requested a review from Piinks Jan 5, 2022
Piinks
Piinks reviewed Jan 7, 2022
View changes
packages/flutter/lib/src/widgets/dismissible.dart Show resolved Hide resolved
packages/flutter/test/widgets/dismissible_test.dart
@@ -12,6 +12,7 @@ const DismissDirection defaultDismissDirection = DismissDirection.horizontal;
const double crossAxisEndOffset = 0.5;
bool reportedDismissUpdateReached = false;
bool reportedDismissUpdatePreviousReached = false;
double reportedDismissUpdateProgress = 0.0;
Copy link
Contributor

@Piinks Piinks Jan 7, 2022

Choose a reason for hiding this comment

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

Can you add tests for non-zero cases?

Copy link
Contributor Author

@cachapa cachapa Jan 10, 2022

Choose a reason for hiding this comment

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

I went back to check and it turns out I forgot to update the value using the callback.
It's fixed now, I'll commit shortly.

Copy link
Contributor

@Piinks Piinks Jan 10, 2022

Choose a reason for hiding this comment

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

Can you add another test that validates it reports the right value somewhere in between 0 and 1? Thanks!

Copy link
Contributor Author

@cachapa cachapa Jan 11, 2022

Choose a reason for hiding this comment

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

Done.

Piinks
Piinks reviewed Jan 10, 2022
View changes
packages/flutter/lib/src/widgets/dismissible.dart Show resolved Hide resolved
packages/flutter/lib/src/widgets/dismissible.dart Show resolved Hide resolved
packages/flutter/test/widgets/dismissible_test.dart
@@ -12,6 +12,7 @@ const DismissDirection defaultDismissDirection = DismissDirection.horizontal;
const double crossAxisEndOffset = 0.5;
bool reportedDismissUpdateReached = false;
bool reportedDismissUpdatePreviousReached = false;
double reportedDismissUpdateProgress = 0.0;
Copy link
Contributor

@Piinks Piinks Jan 10, 2022

Choose a reason for hiding this comment

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

Can you add another test that validates it reports the right value somewhere in between 0 and 1? Thanks!

Piinks
Piinks approved these changes Jan 21, 2022
View changes
Copy link
Contributor

@Piinks Piinks left a comment

This LGTM! Thank you for the contribution!
I've reached out to the team for a secondary review.

@cachapa
Copy link
Contributor Author

@cachapa cachapa commented Jan 21, 2022

Thanks. I'm sorry it took so long, I didn't notice that the tests were failing since last week.

Hixie
Hixie approved these changes Jan 26, 2022
View changes
Copy link
Member

@Hixie Hixie left a comment

LGTM

@cachapa
Copy link
Contributor Author

@cachapa cachapa commented Feb 5, 2022

I'm kind of new to this, is there anything else required from my side?

@Hixie
Copy link
Member

@Hixie Hixie commented Feb 5, 2022

Nope sorry, we just didn't notice it was ready to land! My bad!

@fluttergithubbot fluttergithubbot merged commit 9d2a294 into flutter:master Feb 5, 2022
66 checks passed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 5, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this issue Feb 5, 2022
@engine-flutter-autoroll
9d2a29449 Report progress on Dismissible update callback (flutter/flu…
39b7b39 
…tter#95504)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 5, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this issue Feb 5, 2022
@engine-flutter-autoroll
9d2a29449 Report progress on Dismissible update callback (flutter/flu…
10e5daa 
…tter#95504)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 6, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this issue Feb 6, 2022
@engine-flutter-autoroll
9d2a29449 Report progress on Dismissible update callback (flutter/flu…
3df97c3 
…tter#95504)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 6, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this issue Feb 6, 2022
@engine-flutter-autoroll
9d2a29449 Report progress on Dismissible update callback (flutter/flu…
1c4a8ac 
…tter#95504)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 6, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this issue Feb 6, 2022
@engine-flutter-autoroll
9d2a29449 Report progress on Dismissible update callback (flutter/flu…
cc16257 
…tter#95504)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 6, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this issue Feb 6, 2022
@engine-flutter-autoroll
9d2a29449 Report progress on Dismissible update callback (flutter/flu…
5a9890a 
…tter#95504)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 6, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this issue Feb 6, 2022
@engine-flutter-autoroll
9d2a29449 Report progress on Dismissible update callback (flutter/flu…
d21c0e0 
…tter#95504)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hixie Hixie

@Piinks Piinks

Assignees
No one assigned
Labels
f: material design framework severe: new feature waiting for tree to go green
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

expose the moveController of the Dismissible widget
4 participants
@cachapa @Hixie @Piinks @fluttergithubbot