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

[image_picker] Image picker fix alert #3881

Merged
merged 10 commits into from
May 17, 2021

Conversation

ydag
Copy link
Contributor

@ydag ydag commented May 12, 2021

Since UIAlertView is deprecated I implemented UIAlertController with a preferredStyle of UIAlertControllerStyleAlert instead. I also made some small fixes.

Related issues :

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

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 and the C++, Objective-C, Java style guides. (Note that unlike the flutter/flutter repo, the flutter/plugins repo does use dart format. See plugin_tool format)
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the 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.

ydag added 4 commits May 11, 2021 20:58
Since UIAlertView is deprecated I implemented UIAlertController with a preferredStyle of UIAlertControllerStyleAlert instead.
ydag added 2 commits May 17, 2021 09:45
# Conflicts:
#	packages/image_picker/image_picker/CHANGELOG.md
#	packages/image_picker/image_picker/ios/Classes/FLTImagePickerPlugin.m
@ydag ydag force-pushed the image_picker_fix_alert branch from f421063 to 40decd7 Compare May 17, 2021 07:46
Copy link
Contributor

@mvanbeusekom mvanbeusekom left a comment

Choose a reason for hiding this comment

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

I have a question regarding a double if (granted) { test in the code, and I feel the version number should not be updated as a feature but rather a patch as the functionality doesn't change but instead just use a different SDK internally.

Comment on lines 221 to 222
if (granted) {
if (granted) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the double if (granted) { check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is fixed.

@@ -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/image_picker
version: 0.7.5+1
version: 0.7.6
Copy link
Contributor

Choose a reason for hiding this comment

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

Since there's no public API change, it should be 0.7.5+2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

@@ -1,3 +1,6 @@
## 0.7.6
* Implement UIAlertController with a preferredStyle of UIAlertControllerStyleAlert since UIAlertView is deprecated.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add back-ticks to indicate the UIAlertController, UIAlertControllerStyleAlert and UIAlertView are code constructs:

Suggested change
* Implement UIAlertController with a preferredStyle of UIAlertControllerStyleAlert since UIAlertView is deprecated.
* Implement `UIAlertController` with a preferredStyle of `UIAlertControllerStyleAlert` since `UIAlertView` is deprecated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

Copy link
Contributor

@mvanbeusekom mvanbeusekom left a comment

Choose a reason for hiding this comment

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

LGTM, I noticed that @stuartmorgan also wanted to do a review so let's wait for him before we proceed.

I was a bit to quick, the CHANGELOG.md contains the wrong version number.

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

LGTM once the CHANGELOG is fixed; sorry I forgot about this review last week, I meant to do it after the meeting.

Copy link
Contributor

@mvanbeusekom mvanbeusekom left a comment

Choose a reason for hiding this comment

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

LGTM

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 19, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 19, 2021
fotiDim pushed a commit to fotiDim/plugins that referenced this pull request Sep 13, 2021
@mvanbeusekom mvanbeusekom deleted the image_picker_fix_alert branch September 21, 2021 09:33
jolmiracle pushed a commit to miracle-as/webview_flutter that referenced this pull request Sep 24, 2021
# Conflicts:
#	packages/image_picker/image_picker/CHANGELOG.md
#	packages/image_picker/image_picker/pubspec.yaml
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes p: image_picker platform-ios waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
4 participants