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

[image_picker_for_web] Added support for maxWidth, maxHeight and imageQuality #4389

Merged
merged 37 commits into from
Oct 18, 2021

Conversation

balvinderz
Copy link
Contributor

@balvinderz balvinderz commented Sep 27, 2021

Adds support for maxWidth, maxHeight and imageQuality in image_picker_for_web

Fixes flutter/flutter#78170

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 relevant style guides and ran the auto-formatter. (Note that unlike the flutter/flutter repo, the flutter/plugins repo does use dart 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.

@balvinderz
Copy link
Contributor Author

Implementation is based on this : https://labs.madisoft.it/javascript-image-compression-and-resizing/

There are some limitations in this.
imageQuality only works for jpeg images.
I could not find a way to make this work for gif images.

@cyanglaz cyanglaz requested review from ditman and removed request for cyanglaz September 27, 2021 18:48
Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @balvinderz! I think that this one is a little bit too green, and it needs some changes to the approach to make this more modular and testable!

In my opinion, the image resizing code should be extracted from the ImagePickerPlugin class completely. The only thing the plugin knows about resizing images is that it has an instance of an ImageResizer object that accepts and returns XFile instances.

I think the following steps are needed:

  • Create a ImageResizer component that deals with images in XFile instances, and resizes them according to the parameters passed by the user.
    • 99% of the code of this component already exists in the _getCompressedUri and _calculateSize methods. Having your own object will allow you to design it more carefully.
  • Have an instance of that ImageResizer inside of the ImagePickerPlugin, so it can be:
    • Used when needed to resize images (normal operation)
    • Injectable from tests so it can be mocked out if needed (testability)
  • Use the ImageResizer in the pickImage/pickImages methods, if the user wants a resized output
    • Do not pass the image resizing parameters (maxWidth, maxHeight, imageQuality) to the generic methods that pick files and convert them to a XFile!
  • Unit tests for the new ImageResizer component
  • Unit tests for the new resize images (when the correct parameters are passed) feature.

@balvinderz balvinderz changed the title [WIP] [image_picker_for_web] Added support for maxWidth, maxHeight and imageQuality [image_picker_for_web] Added support for maxWidth, maxHeight and imageQuality Sep 30, 2021
@balvinderz
Copy link
Contributor Author

balvinderz commented Sep 30, 2021

Thanks for the contribution @balvinderz! I think that this one is a little bit too green, and it needs some changes to the approach to make this more modular and testable!

In my opinion, the image resizing code should be extracted from the ImagePickerPlugin class completely. The only thing the plugin knows about resizing images is that it has an instance of an ImageResizer object that accepts and returns XFile instances.

I think the following steps are needed:

  • Create a ImageResizer component that deals with images in XFile instances, and resizes them according to the parameters passed by the user.

    • 99% of the code of this component already exists in the _getCompressedUri and _calculateSize methods. Having your own object will allow you to design it more carefully.
  • Have an instance of that ImageResizer inside of the ImagePickerPlugin, so it can be:

    • Used when needed to resize images (normal operation)
    • Injectable from tests so it can be mocked out if needed (testability)
  • Use the ImageResizer in the pickImage/pickImages methods, if the user wants a resized output

  • Do not pass the image resizing parameters (maxWidth, maxHeight, imageQuality) to the generic methods that pick files and convert them to a XFile!

  • Unit tests for the new ImageResizer component

  • Unit tests for the new resize images (when the correct parameters are passed) feature.

I think I have done all the steps that are mentioned in this.

@google-cla

This comment has been minimized.

@google-cla google-cla bot added cla: no and removed cla: yes labels Oct 16, 2021
@ditman

This comment has been minimized.

@google-cla google-cla bot added cla: yes and removed cla: no labels Oct 16, 2021
@ditman
Copy link
Member

ditman commented Oct 16, 2021

I've rebased this branch with the latest master, there was a small breakage in CI that required the latest code to pass.

@ditman ditman added waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. and removed waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. labels Oct 16, 2021
@ditman
Copy link
Member

ditman commented Oct 16, 2021

(It's too late to land this. I'll merge on monday.)

@ditman ditman added the waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. label Oct 18, 2021
@fluttergithubbot fluttergithubbot merged commit aae841a into flutter:master Oct 18, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 18, 2021
camsim99 pushed a commit to camsim99/plugins that referenced this pull request Oct 18, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 18, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 19, 2021
@balvinderz
Copy link
Contributor Author

Thanks for the dash @ditman 😄

@balvinderz balvinderz deleted the compress_file_web branch October 19, 2021 17:25
@ditman
Copy link
Member

ditman commented Oct 19, 2021

@balvinderz highly deserved! ;)

@ditman
Copy link
Member

ditman commented Oct 20, 2021

Because of a problem in our CI, this had to be released manually (I just did). This should now be available to everybody from pub.

NickalasB added a commit to NickalasB/plugins that referenced this pull request Oct 21, 2021
* master:
  [webview] Fix typos in the README (flutter#4249)
  [google_sign_in] add serverAuthCode to GoogleSignInAccount (flutter#4180)
  [ci] Update macOS Cirrus image to Xcode 13 (flutter#4429)
  [shared_preferences] Switch to new analysis options (flutter#4384)
  [flutter_plugin_android_lifecycle] remove placeholder dart file (flutter#4413)
  [camera] Run iOS methods on UI thread by default (flutter#4140)
  [ci] Always run all `format` steps (flutter#4427)
  [flutter_plugin_tools] Fix license-check on Windows (flutter#4425)
  [google_maps_flutter] Clean Java test, consolidate Marker example. (flutter#4400)
  [image_picker][android] suppress unchecked warning (flutter#4408)
  [ci] Replace Firebase Test Lab deprecated Pixel 4 device with Pixel 5 (flutter#4436)
  [image_picker_for_web] Added support for maxWidth, maxHeight and imageQuality  (flutter#4389)
  Bump compileSdkVersion to 31 (flutter#4432)
  [camera] Update [CameraOrientationTests testOrientationNotifications] unit test to work on Xcode 13 (flutter#4426)
  Update integration_test README  (flutter#3824)
  [webview_flutter] Adjust test URLs again (flutter#4407)
  [google_sign_in] Add serverAuthCode attribute to google_sign_in_platform_interface user data (flutter#4179)
  [camera] Add filter for unsupported cameras on Android (flutter#4418)
NickalasB added a commit to NickalasB/plugins that referenced this pull request Oct 21, 2021
* master:
  [webview] Fix typos in the README (flutter#4249)
  [google_sign_in] add serverAuthCode to GoogleSignInAccount (flutter#4180)
  [ci] Update macOS Cirrus image to Xcode 13 (flutter#4429)
  [shared_preferences] Switch to new analysis options (flutter#4384)
  [flutter_plugin_android_lifecycle] remove placeholder dart file (flutter#4413)
  [camera] Run iOS methods on UI thread by default (flutter#4140)
  [ci] Always run all `format` steps (flutter#4427)
  [flutter_plugin_tools] Fix license-check on Windows (flutter#4425)
  [google_maps_flutter] Clean Java test, consolidate Marker example. (flutter#4400)
  [image_picker][android] suppress unchecked warning (flutter#4408)
  [ci] Replace Firebase Test Lab deprecated Pixel 4 device with Pixel 5 (flutter#4436)
  [image_picker_for_web] Added support for maxWidth, maxHeight and imageQuality  (flutter#4389)
  Bump compileSdkVersion to 31 (flutter#4432)
  [camera] Update [CameraOrientationTests testOrientationNotifications] unit test to work on Xcode 13 (flutter#4426)
  Update integration_test README  (flutter#3824)
  [webview_flutter] Adjust test URLs again (flutter#4407)
  [google_sign_in] Add serverAuthCode attribute to google_sign_in_platform_interface user data (flutter#4179)
  [camera] Add filter for unsupported cameras on Android (flutter#4418)

# Conflicts:
#	packages/webview_flutter/webview_flutter/CHANGELOG.md
#	packages/webview_flutter/webview_flutter/pubspec.yaml
yasargil added a commit to yasargil/plugins that referenced this pull request Oct 25, 2021
* master: (364 commits)
  Use OpenJDK 11 in CI jobs  (flutter#4419)
  [google_sign_in] remove the commented out code in tests (flutter#4442)
  [webview] Fix typos in the README (flutter#4249)
  [google_sign_in] add serverAuthCode to GoogleSignInAccount (flutter#4180)
  [ci] Update macOS Cirrus image to Xcode 13 (flutter#4429)
  [shared_preferences] Switch to new analysis options (flutter#4384)
  [flutter_plugin_android_lifecycle] remove placeholder dart file (flutter#4413)
  [camera] Run iOS methods on UI thread by default (flutter#4140)
  [ci] Always run all `format` steps (flutter#4427)
  [flutter_plugin_tools] Fix license-check on Windows (flutter#4425)
  [google_maps_flutter] Clean Java test, consolidate Marker example. (flutter#4400)
  [image_picker][android] suppress unchecked warning (flutter#4408)
  [ci] Replace Firebase Test Lab deprecated Pixel 4 device with Pixel 5 (flutter#4436)
  [image_picker_for_web] Added support for maxWidth, maxHeight and imageQuality  (flutter#4389)
  Bump compileSdkVersion to 31 (flutter#4432)
  [camera] Update [CameraOrientationTests testOrientationNotifications] unit test to work on Xcode 13 (flutter#4426)
  Update integration_test README  (flutter#3824)
  [webview_flutter] Adjust test URLs again (flutter#4407)
  [google_sign_in] Add serverAuthCode attribute to google_sign_in_platform_interface user data (flutter#4179)
  [camera] Add filter for unsupported cameras on Android (flutter#4418)
  ...

# Conflicts:
#	packages/google_maps_flutter/google_maps_flutter/android/src/main/java/io/flutter/plugins/googlemaps/GoogleMapController.java
amantoux pushed a commit to amantoux/plugins that referenced this pull request Dec 11, 2021
KyleFin pushed a commit to KyleFin/plugins that referenced this pull request Dec 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes p: image_picker platform-web 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
Development

Successfully merging this pull request may close these issues.

[Image Picker] picked file on web isn't being compressed
3 participants