-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Deprecates onWillAccept and onAccept callbacks in DragTarget. #133691
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
Deprecates onWillAccept and onAccept callbacks in DragTarget. #133691
Conversation
5ab998e to
a01c49b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Piinks, even though this resolves the simple renaming issue, I wonder what might happen when the code is a bit more complex. Let's say that data used to be an 'int' in the previous 'onAccept' and the code was something like:
onAccept: (data) => {
myVar += data;
}
Ideally, after switching to onAcceptWithDetails this should be changed to:
onAcceptWithDetails: (details) => {
myVar += details.data;
}
since details is not an int now but a DragTargetDetails<int>. Can this be also handled by a dart fix? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this is something the user would have to update. Thankfully though they would only need to add the details. on the front.
9e0817f to
d371697
Compare
d371697 to
e9358f7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since cupertino exports the widgets library, we'll want to include it too.
These uris are used to determine if a fix might be needed in a given file based on the imports, that is why we have tests for each uri. So for this one, the tests will also need a material and cupertino test set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this is something the user would have to update. Thankfully though they would only need to add the details. on the front.
e9358f7 to
73764b7
Compare
73764b7 to
2f08c71
Compare
Piinks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks very much for your patience! I've been in and out of office a lot lately. 🙏
LGTM! Thanks for following up on this!
|
See https://discord.com/channels/608014603317936148/611619879153565715/1154915603577049162 for contributor access invite. I really appreciate your contributions over the years, thanks for continuing to make Flutter better. |
2f08c71 to
3c0417f
Compare
3c0417f to
e516541
Compare
|
auto label is removed for flutter/flutter/133691, due to - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label. |
|
I've removed the autosubmit because I am curious for your thoughts on #133136. I noticed there is a bit of a conundrum for folks that may have apps that allow the data to be nullable. The new methods that provide the details do not support data being nullable. WDYT for folks that are migrating this change and have a case where data is null? |
Hmm yes, that would indeed break their code after the migration. IMO it has become quite difficult to decide on a way to handle this without breaking things. I feel if the newer callbacks had supported nullable data and offset this could have been avoided in the newer callbacks 🤔. Should we make the decision now to change the newer callbacks making them accept nullable fields (Although the newer callbacks are out in the |
Piinks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your patience while I was out! Yeah, I have long felt like there's this sticky conundrum here where we allow for null in some parts of the API and don't in others. Since we'll be moving folks over to a new API, let's go ahead and see if we can fix this the right way while we have the opportunity.
Should we make the decision now to change the newer callbacks making them accept nullable fields?
Can you go ahead and stage this and I can run extra testing to find out just how breaking it will be (hopefully not too breaking since these callbacks are new)? Then based on the results we can communicate the change before landing it.
Thanks so much for embracing the yak shave on this one!
Roll Flutter from f662150 to e8c2bb1 (66 revisions) flutter/flutter@f662150...e8c2bb1 2023-11-14 tessertaha@gmail.com Reland "Update `framework_test.dart` to remove `ButtonBar` usage and remove references from other clases (#137550) (flutter/flutter#137753) 2023-11-14 jacksongardner@google.com Consume flutter.js from the engine artifacts. (flutter/flutter#137113) 2023-11-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 995ebcefb0d4 to 1b3fd80812c3 (1 revision) (flutter/flutter#138415) 2023-11-14 engine-flutter-autoroll@skia.org Roll Packages from 17bd92e to 428ba3e (2 revisions) (flutter/flutter#138412) 2023-11-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 712094481217 to 995ebcefb0d4 (1 revision) (flutter/flutter#138409) 2023-11-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 2c54b5f70c94 to 712094481217 (1 revision) (flutter/flutter#138401) 2023-11-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from d100e8912eb0 to 2c54b5f70c94 (4 revisions) (flutter/flutter#138399) 2023-11-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 74a9de45f128 to d100e8912eb0 (3 revisions) (flutter/flutter#138383) 2023-11-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 77b952f3add4 to 74a9de45f128 (3 revisions) (flutter/flutter#138382) 2023-11-14 79099771+peterabrahamdev@users.noreply.github.com Fixing typo (flutter/flutter#138253) 2023-11-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 046ec85dffc6 to 77b952f3add4 (1 revision) (flutter/flutter#138377) 2023-11-13 engine-flutter-autoroll@skia.org Roll Flutter Engine from db6da000a17e to 046ec85dffc6 (5 revisions) (flutter/flutter#138375) 2023-11-13 goderbauer@google.com Finally remove analysis_options_user.yaml (flutter/flutter#138261) 2023-11-13 43759233+kenzieschmoll@users.noreply.github.com Add a DevTools section to CONTRIBUTING.md (flutter/flutter#137193) 2023-11-13 104349824+huycozy@users.noreply.github.com Update DraggableScrollableSheet docs to reflect API change (flutter/flutter#136471) 2023-11-13 gspencergoog@users.noreply.github.com Clean up synonyms, key code generation. (flutter/flutter#138192) 2023-11-13 engine-flutter-autoroll@skia.org Roll Flutter Engine from 74ba6c17a488 to db6da000a17e (2 revisions) (flutter/flutter#138364) 2023-11-13 engine-flutter-autoroll@skia.org Roll Flutter Engine from 5d62f1a2392a to 74ba6c17a488 (1 revision) (flutter/flutter#138362) 2023-11-13 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7de793d2bb68 to 5d62f1a2392a (1 revision) (flutter/flutter#138358) 2023-11-13 polinach@google.com Upgrade leak tracker. (flutter/flutter#138283) 2023-11-13 engine-flutter-autoroll@skia.org Roll Flutter Engine from fe11f3a46bac to 7de793d2bb68 (2 revisions) (flutter/flutter#138353) 2023-11-13 engine-flutter-autoroll@skia.org Roll Packages from a682189 to 17bd92e (2 revisions) (flutter/flutter#138347) 2023-11-13 engine-flutter-autoroll@skia.org Roll Flutter Engine from a18ee3c7f57a to fe11f3a46bac (2 revisions) (flutter/flutter#138344) 2023-11-12 engine-flutter-autoroll@skia.org Roll Flutter Engine from 828e4dbf6693 to a18ee3c7f57a (7 revisions) (flutter/flutter#138332) 2023-11-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from e2e07eab35ec to 828e4dbf6693 (1 revision) (flutter/flutter#138282) 2023-11-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 1c29ce15c528 to e2e07eab35ec (1 revision) (flutter/flutter#138280) 2023-11-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from aa6753fdbb51 to 1c29ce15c528 (1 revision) (flutter/flutter#138277) 2023-11-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 00db306f6f7b to aa6753fdbb51 (1 revision) (flutter/flutter#138269) 2023-11-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9d8a1125640d to 00db306f6f7b (7 revisions) (flutter/flutter#138266) 2023-11-10 15619084+vashworth@users.noreply.github.com Only run tests on macOS 12 (flutter/flutter#138260) 2023-11-10 kristijan.zic@gmail.com Fixes vscode path installed via snap (flutter/flutter#136997) 2023-11-10 rossllewallyn@proton.me Docs typo: comprised -> composed (flutter/flutter#137896) 2023-11-10 chinmoy12c@gmail.com Deprecates onWillAccept and onAccept callbacks in DragTarget. (flutter/flutter#133691) 2023-11-10 chris@bracken.jp [macOS] Suppress Xcode 15 createItemModels warning (flutter/flutter#138243) 2023-11-10 engine-flutter-autoroll@skia.org Roll Flutter Engine from 275ddb296ec9 to 9d8a1125640d (1 revision) (flutter/flutter#138252) 2023-11-10 sokolovskyi.konstantin@gmail.com GestureRecognizer should dispatch creation and disposal events. (flutter/flutter#138223) 2023-11-10 engine-flutter-autoroll@skia.org Roll Flutter Engine from 5c2e16c5a95a to 275ddb296ec9 (1 revision) (flutter/flutter#138249) 2023-11-10 engine-flutter-autoroll@skia.org Roll Packages from b69f54e to a682189 (3 revisions) (flutter/flutter#138239) 2023-11-10 engine-flutter-autoroll@skia.org Roll Flutter Engine from e5b75177ac8e to 5c2e16c5a95a (1 revision) (flutter/flutter#138247) 2023-11-10 42216813+eliasyishak@users.noreply.github.com `CommandResultEvent` migrated (flutter/flutter#138165) 2023-11-10 john@johnmccutchan.com Fix #128925 by properly setting the Android Event Source (flutter/flutter#138241) 2023-11-10 engine-flutter-autoroll@skia.org Roll Flutter Engine from 77349dc8e27b to e5b75177ac8e (2 revisions) (flutter/flutter#138244) 2023-11-10 engine-flutter-autoroll@skia.org Roll Flutter Engine from a5eab0d281fe to 77349dc8e27b (4 revisions) (flutter/flutter#138237) 2023-11-10 42216813+eliasyishak@users.noreply.github.com Update analytics constructor to include `FLUTTER_HOST` (flutter/flutter#138107) 2023-11-10 srawlins@google.com Prepare the analyze_once test for removal of analysis_options_user support (flutter/flutter#138229) 2023-11-10 engine-flutter-autoroll@skia.org Roll Flutter Engine from b020893cba29 to a5eab0d281fe (5 revisions) (flutter/flutter#138231) ...
This PR deprecates
onWillAcceptandonAcceptoveronWillAcceptWithDetailsandonAcceptWithDetails.Fixes: #133347
Pre-launch Checklist
///).