[image_picker] added methodCall arg: deleteCapturedOriginalIfScaled#1463
[image_picker] added methodCall arg: deleteCapturedOriginalIfScaled#1463sensiblearts wants to merge 2 commits intoflutter:masterfrom
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
|
I signed it!
…On Sun, Apr 7, 2019 at 9:37 PM googlebot ***@***.***> wrote:
Thanks for your pull request. It looks like this may be your first
contribution to a Google open source project (if not, look below for help).
Before we can look at your pull request, you'll need to sign a Contributor
License Agreement (CLA).
📝 *Please visit https://cla.developers.google.com/
<https://cla.developers.google.com/> to sign.*
Once you've signed (or fixed any issues), please reply here (e.g. I
signed it!) and we'll verify it.
------------------------------
What to do if you already signed the CLA Individual signers
- It's possible we don't have your GitHub username or you're using a
different email address on your commit. Check your existing CLA data
<https://cla.developers.google.com/clas> and verify that your email is
set on your git commits
<https://help.github.com/articles/setting-your-email-in-git/>.
Corporate signers
- Your company has a Point of Contact who decides which employees are
authorized to participate. Ask your POC to be added to the group of
authorized contributors. If you don't know who your Point of Contact is,
direct the Google project maintainer to go/cla#troubleshoot (Public
version <https://opensource.google.com/docs/cla/#troubleshoot>).
- The email used to register you as an authorized contributor must be
the email used for the Git commit. Check your existing CLA data
<https://cla.developers.google.com/clas> and verify that your email is
set on your git commits
<https://help.github.com/articles/setting-your-email-in-git/>.
- The email used to register you as an authorized contributor must
also be attached to your GitHub account
<https://github.com/settings/emails>.
ℹ️ *Googlers: Go here
<https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Fflutter%2Fplugins%2Fpull%2F1463>
for more info*.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1463 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAxJwuXv2YBnkfC7JclXgIl8qOQYgqvbks5vep1xgaJpZM4chIoS>
.
--
</>
David Alm
|
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
|
@sensiblearts Thank you for providing this PR. I understand your needs to save the original image. However, I am not confident enough to take a public API change that only adds new features to Android right now. I will take some time to review the possibility of doing this on iOS side and maybe provide a solution to benefit both platforms. If you do need this urgently, I suggest you to make the change on your own fork. Or you can use the earlier version of the image picker. I have created an issue to track this flutter/flutter#30788, you can follow the issue and get updates on progress of this fix. I will leave this PR open for now, if you'd like to provide the changes on iOS and also adding tests to this change, you are welcome to do so and I am happy to review the changes. |
|
@cyanglaz Thank you for the quick reply. I will eventually get to iOS version myself, as well as tests, but it will be a month or so. However, I just discovered that I need to make a breaking change, and I doubt whether it will be general enough to pull; consequently, I am assuming that I will be maintaining my own fork indefinitely. FYI, here is the breaking change in my fork: Problem to solve:
Whereas I can derive the original filename from the scaled filename in case of capture, I cannot do this if the user picks a random file in a folder. Therefore, the simplest way to get this string is to have the plugin return it. So, I modified class PickedFile {
PickedFile({this.original, this.scaled, this.video});
File original;
File scaled;
File video;
}
class ImagePicker {
//...
static Future<PickedFile> pickImage({
/...
final Map<dynamic, dynamic> pickedPath = await _channel.invokeMethod(
'pickImage',
//...
);
final PickedFile pf = PickedFile(
original: pickedPath["original"] == null
? null
: File(pickedPath["original"]),
scaled:
pickedPath["scaled"] == null ? null : File(pickedPath["scaled"]),
video: null);
return pf;
}
static Future<PickedFile> pickVideo({
//...
final Map<dynamic, dynamic> pickedPath = await _channel.invokeMethod(
'pickVideo',
//...
final PickedFile pf = PickedFile(
original: null,
scaled: null,
video: pickedPath["video"] == null ? null : File(pickedPath["video"]));
return pf;
}
}This code is in a separate branch called 'picked_file' and it is solving this problem described above. Thanks for your time, |
|
@sensiblearts |
Description
Your image_picker changelog
broke my business logic.
The fix is a very small change: add a methodCall param. I have only fixed Android:
(Specifically, my business logic needs: I was using the image_picker to capture scaled images (e.g., maxWidth = 600.0) but also keeping the original (non-scaled) image long enough to copy it to external storage; after I copied it, I manually deleted the original image.)
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.///).flutter analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?