Adds callback onWillAcceptWithDetails in DragTarget. #131545

chinmoy12c · 2023-07-29T11:56:16Z

This PR adds onWillAcceptWithDetails callback to DragTarget to get information about offset.

Fixes: #131378

This PR is subject to changes based on #131542

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, or this PR is test-exempt.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Piinks · 2023-08-01T22:20:48Z

packages/flutter/lib/src/widgets/drag_target.dart

Is this similar to these two? if so, why do we not assert in this case that both are not set?

In the case of onWillAccept and onWillAcceptWithDetails there is a discrepancy, what if one returns false and the other returns true? This is not the case between onAccept and onAcceptWithDetails though since it would just trigger these calls and the user may very well implement two different functionalities in both callbacks, even though it does not make any sense in doing that. The question as to whether we "should" put an assert for these two as well is debatable.

Any views here @goderbauer ?

I think we should assert and make a plan to deprecate and migrate folks over to the newer APIs.

I think we should assert and make a plan to deprecate and migrate folks over to the newer APIs.

Should this new assertion for onWillAccept and onWillAcceptWithDetails be managed in a separate PR and issue?

Yeah, let's do that in a separate PR. I can if you'd like with the deprecations and dart fixes to migrate folks to the newer APIs. I think this change is ok to go ahead with.

Piinks · 2023-08-01T22:21:17Z

packages/flutter/lib/src/widgets/drag_target.dart

Is this meant to replace onWillAccept? Should onWillAccept be deprecated in favor of this new API?

Based on what is highlighted by @goderbauer here, onWillAceeptWithDetails would eventually replace onWillAccept completely.

Sounds good, when we run through triage today I will sync with @goderbauer on this. Before we land it, we should have a plan for deprecation and migration.

Which could be relatively simple for this type of change, dart fix can rename this for folks really easily. :)

packages/flutter/lib/src/widgets/drag_target.dart

Piinks

LGTM with a follow up PR to clean up the onAccept and onWillAccept. Let me know if you are interested in working on that. Thank you! :)

chinmoy12c · 2023-08-16T19:38:36Z

LGTM with a follow up PR to clean up the onAccept and onWillAccept. Let me know if you are interested in working on that. Thank you! :)

Sure I'll be happy to raise another PR for this :).

justinmc

LGTM 👍. +1 to deprecating in a follow-up.

packages/flutter/lib/src/widgets/drag_target.dart

Co-authored-by: Justin McCandless <[email protected]>

Piinks · 2023-08-25T20:19:57Z

Filed #133347 for follow up, this should be clear to land now. :)

auto-submit · 2023-08-25T20:21:00Z

auto label is removed for flutter/flutter/131545, due to - The status or check suite Linux web_canvaskit_tests_6 has failed. Please fix the issues identified (or deflake) before re-applying this label.

The status or check suite Linux web_canvaskit_tests_4 has failed. Please fix the issues identified (or deflake) before re-applying this label.
The status or check suite Linux web_canvaskit_tests_2 has failed. Please fix the issues identified (or deflake) before re-applying this label.
The status or check suite Linux web_canvaskit_tests_0 has failed. Please fix the issues identified (or deflake) before re-applying this label.
The status or check suite Linux web_tests_6 has failed. Please fix the issues identified (or deflake) before re-applying this label.
The status or check suite Linux web_tests_5 has failed. Please fix the issues identified (or deflake) before re-applying this label.
The status or check suite Linux web_tests_4 has failed. Please fix the issues identified (or deflake) before re-applying this label.
The status or check suite Linux web_tests_3 has failed. Please fix the issues identified (or deflake) before re-applying this label.
The status or check suite Linux web_tests_2 has failed. Please fix the issues identified (or deflake) before re-applying this label.
The status or check suite Linux web_tests_1 has failed. Please fix the issues identified (or deflake) before re-applying this label.
The status or check suite Linux web_tests_0 has failed. Please fix the issues identified (or deflake) before re-applying this label.
The status or check suite Linux analyze has failed. Please fix the issues identified (or deflake) before re-applying this label.
The status or check suite Linux skp_generator has failed. Please fix the issues identified (or deflake) before re-applying this label.

packages/flutter/lib/src/widgets/drag_target.dart

…/flutter#131545)

flutter/flutter@61d9f55...229b74d 2023-08-26 [email protected] Roll Flutter Engine from e280ed21f923 to c9b584d59219 (1 revision) (flutter/flutter#133380) 2023-08-26 [email protected] Roll Flutter Engine from 63fdf8a6701c to e280ed21f923 (3 revisions) (flutter/flutter#133378) 2023-08-26 [email protected] _SelectableFragment should dispatch creation in constructor. (flutter/flutter#133351) 2023-08-25 [email protected] Roll Flutter Engine from 53595c937df1 to 63fdf8a6701c (1 revision) (flutter/flutter#133366) 2023-08-25 [email protected] Roll Flutter Engine from 1ec7b89f3a6b to 53595c937df1 (4 revisions) (flutter/flutter#133362) 2023-08-25 [email protected] Roll Flutter Engine from 1471967afb9b to 1ec7b89f3a6b (6 revisions) (flutter/flutter#133355) 2023-08-25 [email protected] Fix locking to work with flutter and dart running simultaneously (flutter/flutter#133350) 2023-08-25 [email protected] Adds callback onWillAcceptWithDetails in DragTarget. (flutter/flutter#131545) 2023-08-25 [email protected] Remove deprecated onPlatformMessage from TestWindow and TestPlatformDispatcher (flutter/flutter#133183) 2023-08-25 [email protected] Remove deprecated androidOverscrollIndicator from ScrollBehaviors (flutter/flutter#133181) 2023-08-25 [email protected] Add an example showing how to use a MatrixTransition. (flutter/flutter#132874) 2023-08-25 [email protected] Roll Flutter Engine from 3dcd2179336d to 1471967afb9b (3 revisions) (flutter/flutter#133342) 2023-08-25 [email protected] Roll Flutter Engine from 33fca02451ef to 3dcd2179336d (3 revisions) (flutter/flutter#133340) 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

…/flutter#131545)

github-actions bot added the framework flutter/packages/flutter repository. See also f: labels. label Jul 29, 2023

chinmoy12c force-pushed the feature/add_on_will_accept_with_details branch from 87b239c to 2d880b9 Compare July 29, 2023 12:08

Piinks reviewed Aug 1, 2023

View reviewed changes

Piinks force-pushed the feature/add_on_will_accept_with_details branch from 2d880b9 to c90a846 Compare August 15, 2023 21:32

Piinks reviewed Aug 15, 2023

View reviewed changes

packages/flutter/lib/src/widgets/drag_target.dart Outdated Show resolved Hide resolved

Piinks reviewed Aug 15, 2023

View reviewed changes

packages/flutter/lib/src/widgets/drag_target.dart Outdated Show resolved Hide resolved

Piinks approved these changes Aug 15, 2023

View reviewed changes

justinmc approved these changes Aug 22, 2023

View reviewed changes

packages/flutter/lib/src/widgets/drag_target.dart Outdated Show resolved Hide resolved

Piinks reviewed Aug 25, 2023

View reviewed changes

packages/flutter/lib/src/widgets/drag_target.dart Outdated Show resolved Hide resolved

chinmoy12c and others added 4 commits August 25, 2023 15:16

Adds callback onWillAcceptWithDetails in DragTarget.

cce580f

Apply suggestions from code review

8cf8f2f

Update packages/flutter/lib/src/widgets/drag_target.dart

bba4e26

Co-authored-by: Justin McCandless <[email protected]>

Update packages/flutter/lib/src/widgets/drag_target.dart

85495fd

Piinks force-pushed the feature/add_on_will_accept_with_details branch from e6ea553 to 85495fd Compare August 25, 2023 20:16

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

Piinks mentioned this pull request Aug 25, 2023

Deprecate old accept callbacks in DragTarget #133347

Closed

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 25, 2023

Piinks reviewed Aug 25, 2023

View reviewed changes

packages/flutter/lib/src/widgets/drag_target.dart Outdated Show resolved Hide resolved

Update packages/flutter/lib/src/widgets/drag_target.dart

6bcc3b3

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

auto-submit bot merged commit 347f7ba into flutter:master Aug 25, 2023

engine-flutter-autoroll mentioned this pull request Aug 26, 2023

Roll Flutter from 61d9f5566559 to 229b74d98772 (13 revisions) flutter/packages#4776

Merged

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

347f7ba Adds callback onWillAcceptWithDetails in DragTarget. (flutter…

9d99f88

…/flutter#131545)

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

347f7ba Adds callback onWillAcceptWithDetails in DragTarget. (flutter…

799f80f

…/flutter#131545)

Adds callback onWillAcceptWithDetails in DragTarget. #131545

Adds callback onWillAcceptWithDetails in DragTarget. #131545

Uh oh!

Conversation

chinmoy12c commented Jul 29, 2023

Pre-launch Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chinmoy12c Aug 2, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

chinmoy12c commented Aug 16, 2023

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Piinks commented Aug 25, 2023

Uh oh!

auto-submit bot commented Aug 25, 2023

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chinmoy12c Aug 2, 2023 •

edited

Loading