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

[image_picker] 🚧 Add support for web 🚧 #2767

Closed
wants to merge 15 commits into from

Conversation

ditman
Copy link
Member

@ditman ditman commented May 16, 2020

Description

This is a first pass at a version of the Image Picker plugin for web. There's a few changes:

  • Add a new cross-platform type to handle both dart:io and dart:html use cases: PickedFile. Thanks for the inspiration @rodydavis!
  • Add new methods to the ImagePicker Platform Interface API that return PickedFiles, and not "String" or dart:io "File".
  • @Deprecate old methods with references to the current ones.
  • Modify the example to use the new API.
  • Add a web implementation of the plugin.
    • It uses createObjectUrl so we don't need to load bytes manually, the browser keeps a reference to the picked files for when we need them, and data from those files can be accessed through a "network" request.

Demo

https://dit-picker-tests.web.app

Missing things / Known issues / Limitations

  • Unit/Integration Tests
  • Is there any opportunity to add revokeObjectUrl to free resources in the browser? This might be a memory leak in long-lived apps.
  • Safari is misbehaving with videos, but it should work (this might be a bug in the VideoPlayer plugin for web).
  • Also when taking pics on an iPhone, the aspect ratio is lost, and every pic is rendered in "landscape" :/
  • In general, only videos that can be opened directly by the browser can be previewed. There's no client-side transcoding of any kind. Not sure if this is detectable, but it'd be a nice exception to throw!

Related Issues

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.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • 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 signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).

The change is breaking, but backwards compatible. Users will need to migrate their apps to a new (non static) API, and a new return value from the plugin.

@ditman ditman added the WIP label May 16, 2020
@ditman ditman requested a review from cyanglaz May 16, 2020 01:13
@googlebot

This comment has been minimized.

@ditman
Copy link
Member Author

ditman commented May 16, 2020

(The Googlebot comment is not very relevant to this PR, since this is not what's going to be merged. This is the POC with all the changes required, that will be split into 3 PRs that need to be merged in order.)

@ditman ditman changed the title [image_picker] Add support for web [image_picker] 🚧 Add support for web 🚧 May 16, 2020
@m-j-g
Copy link

m-j-g commented May 18, 2020

@ditman demo looks perfect. nice work!

@ditman
Copy link
Member Author

ditman commented May 18, 2020

Currently writing tests for the image_picker_for_web package.

@rodydavis
Copy link
Contributor

rodydavis commented May 18, 2020

@ditman also added a new method and updated the example: ditman#366

I added this so in the future it would be easy to add to return types and combinations. For example we could add "imageVideo" as a return type to support the gallery picking any file and furthermore we could add "file" for picking documents.

This will greatly cut down on the boilerplate for the developer as more types are added.

I think this will be important as the plugin moves to desktop too and the types of files can be vast.

import './base.dart';

/// A PickedFile backed by a dart:io File.
class PickedFile extends PickedFileBase {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need 2 different PickedFiles?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we need them because only the appropriate one gets conditionally exported by the picked_file.dart file.

@milan-kit

This comment has been minimized.

Run with flutter test / flutter test --platform chrome
@ditman
Copy link
Member Author

ditman commented May 21, 2020

Hey @rodydavis, can you please hit #2791 with a message of consent towards the Googlebot? Thanks!

@Ahmadre
Copy link

Ahmadre commented May 22, 2020

Thanks guys, you're awesome ❤️

@rodydavis
Copy link
Contributor

rodydavis commented May 22, 2020

Sure thing 😎 and thanks @ditman for doing this PR!

@fvisticot
Copy link

I check the demo on an iPhone -> OK
But on chrome on MAC the camera is not started (can not take a picture from my MAC)

@ditman
Copy link
Member Author

ditman commented May 27, 2020

@fvisticot correct; desktop browsers don't support the capture attribute. I've updated the README of the plugin to reflect this, here. Look at the input file "capture" section.

Once the camera plugin is ported to the web, we could wire everything together in a big example that knows what's the best way of taking a picture, depending on the platform!

@ditman
Copy link
Member Author

ditman commented May 28, 2020

The latest version of image_picker (that uses the new API, and enables the web version) has just been published: v0.6.7

It contains information on migrating from the old API to the new one, after the deprecation of pickImage, pickVideo and LostDataResponse.

https://pub.dev/packages/image_picker#deprecation-warnings-in-pickimage-pickvideo-and-lostdataresponse

@ditman
Copy link
Member Author

ditman commented May 29, 2020

Published the PR introducing the web version of the plugin here: #2802

@ditman
Copy link
Member Author

ditman commented May 29, 2020

Closing in favor of: #2802

@ditman ditman closed this May 29, 2020
@mayank-dhanawade
Copy link

can someone give an example of how to make a common code for mobile and web for image picker??

@kmoreau
Copy link

kmoreau commented Jun 3, 2020

can someone give an example of how to make a common code for mobile and web for image picker??

Hello, i'm interesting about this example too :)
I think it's an line like :

pickedFile = await _picker.getImage(source: ImageSource.gallery);

but it's not work in web.

I will waiting the new merge :)

@ditman ditman deleted the image-picker-web branch August 25, 2020 22:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants