Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Web Camera - use file input #1856

Merged
merged 16 commits into from Jul 7, 2020
Merged

Web Camera - use file input #1856

merged 16 commits into from Jul 7, 2020

Conversation

mlynch
Copy link
Contributor

@mlynch mlynch commented Aug 7, 2019

This PR adds support for the native file input method for picking/taking a photo. This makes it possible to select a photo from the filesystem or use the native prompt provided on mobile.

Because this experience is likely a bit closer to what users would expect on mobile/desktop browsers, I've also changed the default so that the PWA Elements system is opt-in instead of the default if installed. Technically, this is a breaking change, but I don't think it warrants a major release.

Reader

Move PWA Elements to be opt-in

Fix up base64 method and such
@mlynch
Copy link
Contributor Author

mlynch commented Aug 7, 2019

I need to do one more thing: hide the input so it doesn't mess w/ any UI.

@jcesarmobile
Copy link
Member

I think you can’t click it if hidden, but you can probably use absolute position to put it outside of the visible area

Copy link
Member

@jcesarmobile jcesarmobile left a comment

Choose a reason for hiding this comment

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

Added a few comments

core/src/web/camera.ts Outdated Show resolved Hide resolved
core/src/core-plugin-definitions.ts Outdated Show resolved Hide resolved
core/src/web/camera.ts Outdated Show resolved Hide resolved
core/src/web/camera.ts Outdated Show resolved Hide resolved
@aaronksaunders
Copy link

when will this be merged?

@rdlabo
Copy link
Contributor

rdlabo commented Mar 25, 2020

This is great function . I looks forward to merge this🎉

@bijanmmarkes
Copy link

When will this be merged? 👍 @mlynch

@rdlabo
Copy link
Contributor

rdlabo commented May 20, 2020

I noticed that iOS Safari may not work input of createElement...
: https://stackoverflow.com/questions/47664777/javascript-file-input-onchange-not-working-ios-safari-only

User must prepare real DOM in index.html.

<div style="width: 0; height: 0; overflow: hidden;">
  <input id="browserPhotoUploader" type="file" accept="image/*" />
</div>

I think this is not smart. Is there any good way?

@alexcroox
Copy link

Any movement on this 10 month PR? I'd like to use file input for both web and native app.

@imhoffd imhoffd self-assigned this Jun 30, 2020
mlynch and others added 5 commits June 30, 2020 19:02
Co-authored-by: jcesarmobile <jcesarmobile@gmail.com>
Co-authored-by: jcesarmobile <jcesarmobile@gmail.com>
@mlynch
Copy link
Contributor Author

mlynch commented Jul 1, 2020

Alright this is updated, working, and ready for review

core/src/web/camera.ts Outdated Show resolved Hide resolved
@imhoffd imhoffd requested a review from jcesarmobile July 1, 2020 20:25
@imhoffd
Copy link
Contributor

imhoffd commented Jul 1, 2020

Looks good! @jcesarmobile want to approve?

Copy link
Member

@jcesarmobile jcesarmobile left a comment

Choose a reason for hiding this comment

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

Added two comments.

Also, CameraDirection description says it's iOS only, but this PR seems to add support for web, so should be mentioned.

core/src/web/camera.ts Outdated Show resolved Hide resolved
core/src/core-plugin-definitions.ts Show resolved Hide resolved
@imhoffd imhoffd requested a review from jcesarmobile July 6, 2020 21:49
Copy link
Member

@jcesarmobile jcesarmobile left a comment

Choose a reason for hiding this comment

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

You missed my comment about CameraDirection, I've added that information in a new commit.

Using the input as default will "break" apps that already configured and use pwa-elements, so the default behavior should be changed or the PR should be sent to v3 branch.

Copy link
Member

@jcesarmobile jcesarmobile left a comment

Choose a reason for hiding this comment

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

looks good to me

@imhoffd imhoffd merged commit 25505d2 into master Jul 7, 2020
@imhoffd imhoffd deleted the web-camera-file-input branch July 7, 2020 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants