Update browser plugin #435

GedasGa · 2019-03-29T10:14:14Z

Platforms affected

Browser and Electron

Motivation and Context

Using the camera plugin with the Browser platform you can not specify the location, where the camera opens. Once you call navigator.camera.getPicture, for example, from your index.js with sourceType: Camera.PictureSourceType.CAMERA option, it creates a container and opens the Camera in it.

The problem is, the container with the video is always appended to the document.body and the user has no control of it, except the width and height. The only way to customize its style is by selecting cordova-camera-capture class and applying your styles. Besides, this makes close impossible to open the camera inside a modal, unless you're willing to do crazy DOM manipulation after you call navigator.camera.getPicture method. Same applies for the capture button.

Moreover, I'm missing the Cancel/Close button because once you open the camera, now there's no way to close it unless you take a picture.

Similarly when you call navigator.camera.getPicture, for example, from your index.js with sourceType: Camera.PictureSourceType.PHOTOLIBRARY or sourceType: Camera.PictureSourceType. SAVEDPHOTOALBUM option, it creates the input field.This field is also appended to the document.body and the user has no control of it.

I've felt, the need to allow the user to choose where he wants to open the camera, which capture/ cancel button or which input field he wants to use. For example, the user could create a container element and provide us with the ID of the element, where he wants to open the camera. This would give the user more control of the camera plugin and let easier modify its container. This would allow the user to easily open the Camera in a modal or where ever he wants.

Description

Allow users to set 4 additional options:
- customCameraContainer (id of custom camera's container element)
- customCaptureButton (id of custom camera's capture button)
- customCancelButton (id of custom camera's cancel button)
- customSourceInput (id of the custom source input element, when using different mode than the default sourceType)

The default values of these options are null. If the user does not provide the ID of a custom element, the *default element will be used.

Besides, these changes, I've refactored the code to use const and let instead of var. Also, I've broken down big function to functions smaller.

Testing

npm t

I've already tested the code manually.

Also tested the following cases manually.

Calling navigator.camera.getPicture with no options:
Results:
Once function is called, it create the following container. Streams a video form the Camera.
- If Capture is clicked, it returns the image data as base64, stops video stream and deletes the <div> element.
- If Cancelis clicked, it stops video stream and deletes the <div> element.

<div class="cordova-camera-capture" style="position: relative; z-index: 2147483647;">
    <video width="320" height="240" src="blob:file:///90647c34-f521-4c8d-9df4-1224f60c981e"></video>
    <button>Capture</button>
    <button>Cancel</button>
</div>

Calling navigator.camera.getPicture with options:

    sourceType: Camera.PictureSourceType.PHOTOLIBRARY

Results:
Once function is called, it create the following input field.
- If Input is clicked, it open default file manager and allows to upload 1 image. Return the image data as a callback.

<input class="cordova-camera-select" type="file" name="files[]" style="position: relative; z-index: 2147483647;">

Calling navigator.camera.getPicture with options:

    sourceType: Camera.PictureSourceType.PHOTOLIBRARY,
    customCancelButton: 'cordova-source-input'

Results:
Once function is called, it create the following input field.
- If Input is clicked, it open default file manager and allows to upload 1 image. Return the image data as a callback.

<input class="cordova-source-input" type="file" name="files[]">

Calling navigator.camera.getPicture with options:

    customCameraContainer: 'cordova-camera-preview',

Results:
Once function is called, it create the following container. Streams a video form the Camera.
- If Capture is clicked, it returns the image data as base64, stops video stream and deletes everything inside <div> element.
- If Cancelis clicked, it stops video stream and deletes the <div> everything inside element.

<div id="cordova-camera-preview">
    <video width="320" height="240" src="blob:file:///90647c34-f521-4c8d-9df4-1224f60c981e"></video>
    <button>Capture</button>
    <button>Cancel</button>
</div>

Calling navigator.camera.getPicture with options:

    customCameraContainer: 'cordova-camera-preview',
    customCaptureButton: 'cordova-camera-capture',
    customCancelButton: 'cordova-camera-cancel'

Results:
Once function is called, it create the following container. Streams a video form the Camera.
- If Capture is clicked, it returns the image data as base64, stops video stream and deletes the <video> element.
- If Cancelis clicked, it stops video stream and deletes the <video> element.

<div id="cordova-camera-preview">
    <video width="320" height="240" src="blob:file:///90647c34-f521-4c8d-9df4-1224f60c981e"></video>
    <button id="cordova-camera-capture">Capture</button>
    <button  id="cordova-camera-cancel">Cancel</button>
</div>

Checklist

I've run the tests to see all new and existing tests pass
I added automated test coverage as appropriate for this change
Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
I've updated the documentation if necessary

- Add ability to specify camera container, buttons and source input - Add cancel button, that can stop video stream - Refactor code

erisu

Overall, everything looks good.

As I posted within the review,

IMO, I think it would be better if these new options accept either a query string or DOMElement.

Query string would give more flexibility in how the element is discovered vs an ID only. In this case, we would need to use the document.querySelector method instead of the getElementById.

DOMElement could also be a viable option.

Even though an ID is the most preferred way, users might not use an ID. In this case for this PR, either or is acceptable by me.

I want to see if anyone else, for example @dpogue, has any opinions on this.

erisu · 2019-04-09T03:17:15Z

README.md

@@ -266,6 +266,10 @@ Optional parameters to customize the camera settings.
 | saveToPhotoAlbum | <code>Boolean</code> |  | Save the image to the photo album on the device after capture. |
 | popoverOptions | <code>[CameraPopoverOptions](#module_CameraPopoverOptions)</code> |  | iOS-only options that specify popover location in iPad. |
 | cameraDirection | <code>[Direction](#module_Camera.Direction)</code> | <code>BACK</code> | Choose the camera to use (front- or back-facing). |
+| customCameraContainer | <code>string</code> |  | Browser-only option, specify the id of custom camera's container element.  |


[1] IMO, I think it would be better if these new options accept either a query string or DOMElement.

Query string would give more flexibility in how the element is discovered vs an ID only. In this case, we would need to use the document.querySelector method instead of the getElementById.

DOMElement could also be a viable option.

customCameraContainer sounds like is something you need a specific DOMElement, so allowing flexibility of querySelector I think promotes bad coding practice.

If you need a specific element, the element should be tagged by an ID as to guarantee that you will receive the expected DOMElement. Using querySelector makes code brittle by picking the first occurrence of the selected term, which may not be the node you want.

The YouTube JS API[1] for example does something similar where it needs to insert HTML in a specific node of your choice. Their API signature accepts either an id, or the HTMLElement itself.

So I would prefer to keep customCameraContainer accepting an id rather than a query string. For flexibility, it could also optionally accept a DOM node and it would be up to the developer to find that reference as they see fit (which they could use querySelector if they wanted to, or it could be a fresh node that isn't even in the DOM tree yet (via document.createElement), etc.

[1] https://developers.google.com/youtube/iframe_api_reference#Loading_a_Video_Player

I believe this PR is no longer valid and can be closed.

There was a discussion after the PR creation and a decision that users should maybe use WebRTC camera functionality instead of this. After that discussion, the progression of this PR halted.

If memory serves, iOS has limitation/problems with WebRTC camera functionality so the plugin will still be needed for iOS. It can be conditionally wrapped to decided when to use WebRTC over Plugin.

The PR author created this blog post: "Cross-Platform development with Cordova and Electron" which uses the WebRTC camera functionality for the sample application.

Must be this limitation:

Does not work in standalone running ("installed") PWAs, getUserMedia returns no video input devices in UIWebView or WKWebView, but only directly in Safari.

https://caniuse.com/#search=getUserMedia

erisu · 2019-04-09T03:17:30Z

README.md

@@ -266,6 +266,10 @@ Optional parameters to customize the camera settings.
 | saveToPhotoAlbum | <code>Boolean</code> |  | Save the image to the photo album on the device after capture. |
 | popoverOptions | <code>[CameraPopoverOptions](#module_CameraPopoverOptions)</code> |  | iOS-only options that specify popover location in iPad. |
 | cameraDirection | <code>[Direction](#module_Camera.Direction)</code> | <code>BACK</code> | Choose the camera to use (front- or back-facing). |
+| customCameraContainer | <code>string</code> |  | Browser-only option, specify the id of custom camera's container element.  |
+| customCaptureButton | <code>string</code> |  | Browser-only option, specify the id of custom camera's capture button. |


Same as above [1].

erisu · 2019-04-09T03:17:34Z

README.md

@@ -266,6 +266,10 @@ Optional parameters to customize the camera settings.
 | saveToPhotoAlbum | <code>Boolean</code> |  | Save the image to the photo album on the device after capture. |
 | popoverOptions | <code>[CameraPopoverOptions](#module_CameraPopoverOptions)</code> |  | iOS-only options that specify popover location in iPad. |
 | cameraDirection | <code>[Direction](#module_Camera.Direction)</code> | <code>BACK</code> | Choose the camera to use (front- or back-facing). |
+| customCameraContainer | <code>string</code> |  | Browser-only option, specify the id of custom camera's container element.  |
+| customCaptureButton | <code>string</code> |  | Browser-only option, specify the id of custom camera's capture button. |
+| customCancelButton | <code>string</code> |  | Browser-only option, specify the id of custom camera's cancel button.  |


Same as above [1].

erisu · 2019-04-09T03:17:39Z

README.md

+| customCameraContainer | <code>string</code> |  | Browser-only option, specify the id of custom camera's container element.  |
+| customCaptureButton | <code>string</code> |  | Browser-only option, specify the id of custom camera's capture button. |
+| customCancelButton | <code>string</code> |  | Browser-only option, specify the id of custom camera's cancel button.  |
+| customSourceInput | <code>string</code> |  | Browser-only option, specify the id of custom  source input element, when using diffrerent mode than the default `sourceType`. |


Same as above [1].

erisu · 2019-04-09T03:22:18Z

src/browser/CameraProxy.js

+}
+
+function createSourceInput () {
+    let input = document.createElement('input');


Suggested change

let input = document.createElement('input');

const input = document.createElement('input');

erisu · 2019-04-09T03:28:36Z

src/browser/CameraProxy.js

-    var targetWidth = opts[3];
-    var targetHeight = opts[4];
+function createCameraContainer () {
+    let parent = document.createElement('div');


Suggested change

let parent = document.createElement('div');

const parent = document.createElement('div');

erisu · 2019-04-09T03:28:54Z

src/browser/CameraProxy.js

-    parent.style.position = 'relative';
-    parent.style.zIndex = HIGHEST_POSSIBLE_Z_INDEX;
-    parent.className = 'cordova-camera-capture';
+    let video = document.createElement('video');


Suggested change

let video = document.createElement('video');

const video = document.createElement('video');

erisu · 2019-04-09T03:29:07Z

src/browser/CameraProxy.js

+}
+
+function createButton (parent, innerText) {
+    let button = document.createElement('button');


Suggested change

let button = document.createElement('button');

const button = document.createElement('button');

erisu · 2019-04-09T03:29:57Z

src/browser/CameraProxy.js

-        canvas.width = targetWidth;
-        canvas.height = targetHeight;
-        canvas.getContext('2d').drawImage(video, 0, 0, targetWidth, targetHeight);
+        let canvas = document.createElement('canvas');


Suggested change

let canvas = document.createElement('canvas');

const canvas = document.createElement('canvas');

erisu · 2019-04-09T03:30:52Z

src/browser/CameraProxy.js

+}
+
+function removeAppendedCameraElements (video, customElements) {
+


Suggested change

baibhavprdn · 2019-10-22T05:43:13Z

Any idea when this feature will be included? This is exactly the thing I've been needing in my app.

timbru31 · 2019-10-28T13:30:10Z

@baibhavprdn - No. As you can see, there are still unresolved discussions that need to be worked on first. Generally: No ETA on features, bugs and new releases.

GedasGa added 3 commits April 8, 2019 23:16

Update browser plugin

b05b77a

- Add ability to specify camera container, buttons and source input - Add cancel button, that can stop video stream - Refactor code

Update README.md

5a8765b

Update appended elements cleanup

9b030a1

erisu force-pushed the update-browser-plugin branch from 5ae2b1d to 9b030a1 Compare April 8, 2019 14:16

erisu requested changes Apr 9, 2019

View reviewed changes

janpio added enhancement platform: browser labels Jun 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update browser plugin #435

Update browser plugin #435

GedasGa commented Mar 29, 2019

erisu left a comment •

edited

Loading

erisu Apr 9, 2019

breautek Oct 23, 2019

erisu Oct 24, 2019 •

edited

Loading

breautek Oct 24, 2019

erisu Apr 9, 2019

erisu Apr 9, 2019

erisu Apr 9, 2019

erisu Apr 9, 2019

erisu Apr 9, 2019

erisu Apr 9, 2019

erisu Apr 9, 2019

erisu Apr 9, 2019

erisu Apr 9, 2019

baibhavprdn commented Oct 22, 2019

timbru31 commented Oct 28, 2019

	let input = document.createElement('input');
	const input = document.createElement('input');

	let parent = document.createElement('div');
	const parent = document.createElement('div');

	let video = document.createElement('video');
	const video = document.createElement('video');

	let button = document.createElement('button');
	const button = document.createElement('button');

	let canvas = document.createElement('canvas');
	const canvas = document.createElement('canvas');

		}

		function removeAppendedCameraElements (video, customElements) {

Update browser plugin #435

Are you sure you want to change the base?

Update browser plugin #435

Conversation

GedasGa commented Mar 29, 2019

Platforms affected

Motivation and Context

Description

Testing

Checklist

erisu left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erisu Oct 24, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

baibhavprdn commented Oct 22, 2019

timbru31 commented Oct 28, 2019

erisu left a comment •

edited

Loading

erisu Oct 24, 2019 •

edited

Loading