Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[image_picker] Updated pickImage and pickVideo docs to expose the possible errors that can be thrown #2662

Closed
joaomagfreitas wants to merge 5 commits into from
Closed

[image_picker] Updated pickImage and pickVideo docs to expose the possible errors that can be thrown #2662

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7888bac
Updated pickImage and pickVideo docs to expose the possible errors th…
joaomagfreitas Apr 17, 2020
6c73545
Updated plugin version on pubspec.yaml and CHANGELOG.md
joaomagfreitas Apr 17, 2020
72eac69
Updated how pickImage and pickVideo docs were referencing PlatformExc…
joaomagfreitas Apr 21, 2020
a0de9d0
Update pickImage and pickVideo docks to reference that due to an unkn…
joaomagfreitas Apr 21, 2020
b6e908d
Merge branch 'master' into image_picker/add-missing-doc
joaomagfreitas Apr 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/image_picker/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.6.5+1

* Expose errors thrown in `pickImage` and `pickVideo` docs.

## 0.6.5

* Set maximum duration for video recording.
Expand Down
12 changes: 10 additions & 2 deletions packages/image_picker/lib/image_picker.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ class ImagePicker {
static const MethodChannel _channel =
MethodChannel('plugins.flutter.io/image_picker');

/// Returns a [File] object pointing to the image that was picked.
/// Returns a [File] object pointing to the image that was picked or throws
Copy link
Contributor

@cyanglaz cyanglaz Apr 20, 2020

Choose a reason for hiding this comment

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

nits:

Returns a [File] object pointing to the image that was picked.
Throws ...

Also, the message might be a little too definitive as there might be other platform exception thrown due to some error caused in the platform. (Like some iOS errors unknown to us), we might want to re-word it a little. Maybe something like "The method could throw a [PlatformException] if..."

Copy link
Contributor Author

@joaomagfreitas joaomagfreitas Apr 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @cyanglaz

Thanks for the feedback! I update the docs to explicit that the method could throw a PlatformException but left the description of the possible exceptions, because I think it is necessary for developers to be aware of the common exceptions that they might have to deal with when reading the documentation. If on the method docs is not the best place, could it be explicit instead on README.md?

Thanks,
freitzzz

Copy link
Contributor

@cyanglaz cyanglaz Apr 21, 2020

Choose a reason for hiding this comment

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

Sorry I didn't make myself clear enough. I meant what you have done is mostly ok, just that we need to indicate there might be other errors besides the ones that you have there.

Copy link
Contributor Author

@joaomagfreitas joaomagfreitas Apr 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry I didn't make myself clear enough. I meant what you have done is mostly ok, just that we need to indicate there might be other errors besides the ones that you have there.

Got it! Just updated to reference that PlatformException could also be thrown due to an unknow error. Is it ok for now, or do you think it needs to be rephrased?

Copy link
Contributor Author

@joaomagfreitas joaomagfreitas May 8, 2020

Choose a reason for hiding this comment

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

@cyanglaz Let me know if you still want these changes

Copy link
Contributor

@cyanglaz cyanglaz May 8, 2020

Choose a reason for hiding this comment

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

absolutely! If you could rebase and resolve the conflicts, I will land this! Thanks again.

/// a [PlatformException] if the app does not have permission to access
/// the camera or photos gallery, no camera is available, plugin is already in use,
/// temporary file could not be created (iOS only) or plugin activity could not
/// be allocated (Android only).
///
/// The `source` argument controls where the image comes from. This can
/// be either [ImageSource.camera] or [ImageSource.gallery].
Expand Down Expand Up @@ -96,7 +100,11 @@ class ImagePicker {
return path == null ? null : File(path);
}

/// Returns a [File] object pointing to the video that was picked.
/// Returns a [File] object pointing to the video that was picked or throws
/// a [PlatformException] if the app does not have permission to access
/// the camera or photos gallery, no camera is available, plugin is already in use,
/// temporary file could not be created and video could not be cached (iOS only)
/// or plugin activity could not be allocated (Android only).
///
/// The [source] argument controls where the video comes from. This can
/// be either [ImageSource.camera] or [ImageSource.gallery].
Expand Down
2 changes: 1 addition & 1 deletion packages/image_picker/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: image_picker
description: Flutter plugin for selecting images from the Android and iOS image
library, and taking new pictures with the camera.
homepage: https://github.com/flutter/plugins/tree/master/packages/image_picker
version: 0.6.5
version: 0.6.5+1

flutter:
plugin:
Expand Down