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

20334 implement desktop camera upload feature #335

Merged
merged 8 commits into from
Mar 25, 2024

Conversation

madassdev
Copy link
Collaborator

@madassdev madassdev commented Mar 6, 2024

Summary by CodeRabbit

  • New Features
    • Improved camera upload feature with conditional rendering based on user settings.
    • Added a button to open the webcam for camera uploads.
    • Updated layout and styling of camera upload component for better user experience.
    • Introduced a checkbox in forms to enable camera uploads for image fields.

@madassdev madassdev requested a review from JhumanJ March 6, 2024 11:43
Copy link
Contributor

coderabbitai bot commented Mar 6, 2024

Walkthrough

The recent update enhances user experience by enabling direct image uploads from webcams in specific forms. A new cameraUpload feature governs webcam upload visibility and functionality, with UI improvements for image capture, processing, and permission management. Additionally, a new setting in form fields allows users to toggle camera uploads.

Changes

File Path Change Summary
client/components/forms/FileInput.vue Added webcam upload functionality with a cameraUpload prop.
client/components/forms/OpenFormField.vue Integrated cameraUpload into form fields based on field.camera_upload.
client/components/forms/fields/components/FieldOptions.vue Added checkbox for enabling camera uploads in form settings.
client/lib/forms/form-themes.js Modified cameraInput style property in fileInput object across themes for input element appearance.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit-tests for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 7

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 30587fb and 6d79d2c.
Files ignored due to path filters (1)
  • client/package.json is excluded by: !**/*.json
Files selected for processing (4)
  • client/components/forms/FileInput.vue (6 hunks)
  • client/components/forms/components/CameraUpload.vue (1 hunks)
  • client/components/open/forms/OpenFormField.vue (1 hunks)
  • client/components/open/forms/fields/components/FieldOptions.vue (1 hunks)
Additional comments: 11
client/components/forms/FileInput.vue (4)
  • 8-10: The implementation of the camera-upload component within a conditional rendering block based on cameraUpload and isInWebcam flags is correctly done. However, ensure that the CameraUpload component emits the uploadImage and stopWebcam events as expected for seamless integration.
  • 92-92: Correctly importing the CameraUpload component ensures that it can be used within this file. This is a necessary step for integrating the new camera upload functionality.
  • 102-102: Introducing a cameraUpload prop with a default value of false is a good practice. It allows for the feature to be optionally enabled, maintaining backward compatibility and flexibility in usage.
  • 118-118: The addition of isInWebcam to the component's data is essential for controlling the visibility of the webcam UI. This state management approach is clear and effective.
client/components/forms/components/CameraUpload.vue (5)
  • 1-4: The setup for the video and canvas elements to display the webcam feed and captured images is correctly implemented. Using conditional classes to hide elements based on state (isCapturing and capturedImage) is a clean approach.
  • 48-62: Handling the case where camera permission is blocked by displaying instructions to the user is a thoughtful addition. This enhances the user experience by guiding them on how to resolve the issue. The implementation appears correct and user-friendly.
  • 90-91: Importing the webcam-easy library for handling webcam functionality simplifies the implementation. This choice leverages an existing solution to manage webcam access and image capture, which is a good practice.
  • 92-98: Setting initial data properties such as webcam, isCapturing, capturedImage, and cameraPermissionStatus is essential for managing the component's state. The initial values are appropriately chosen for the expected behavior upon component initialization.
  • 132-137: The takeSnapshot method demonstrates a clear and concise approach to capturing an image from the webcam feed. Stopping the webcam after capturing the image is a good practice to conserve resources.
client/components/open/forms/OpenFormField.vue (1)
  • 306-306: The addition of inputProperties.cameraUpload based on the presence of field.camera_upload is a key part of implementing the desktop camera upload feature. This change correctly integrates the new feature with the existing form field properties, allowing for dynamic enabling of camera uploads based on form configuration. Ensure that the rest of the application properly handles this new property, especially in components that consume inputProperties.
client/components/open/forms/fields/components/FieldOptions.vue (1)
  • 59-63: The addition of the "Allow Camera uploads" checkbox is correctly implemented and integrates well with the existing structure of the component. Ensure that the camera_upload property is properly initialized and that any necessary backend changes to support this feature have been made.

Comment on lines 303 to 309
}
} else if (field.type === 'files' || (field.type === 'url' && field.file_upload)) {
inputProperties.multiple = (field.multiple !== undefined && field.multiple)
inputProperties.cameraUpload = (field.camera_upload !== undefined && field.camera_upload)
inputProperties.mbLimit = this.form?.max_file_size ?? this.currentWorkspace?.max_file_size
inputProperties.accept = (this.form.is_pro && field.allowed_file_types) ? field.allowed_file_types : ''
} else if (field.type === 'number' && field.is_rating) {
Copy link
Contributor

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [57-59]

Dynamically rendering arbitrary HTML on your website can be very dangerous because it can easily lead to XSS vulnerabilities. Only use HTML interpolation (v-html) on trusted content and never on user-provided content. If field.content is user-provided, consider sanitizing it or using alternative methods to display the content safely.


📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [61-63]

Similar to the previous comment, dynamically rendering arbitrary HTML with v-html can lead to XSS vulnerabilities. Ensure that field.content is from a trusted source or properly sanitized before rendering it in the DOM.

Comment on lines 67 to 73
<button type="button" class="text-white cursor-pointer py-2 p-1 px-2 rounded-full bg-blue-900 text-xs" @click.stop="openWebcam" v-if="cameraUpload">
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor" class="w-6 h-6">
<path stroke-linecap="round" stroke-linejoin="round" d="M6.827 6.175A2.31 2.31 0 0 1 5.186 7.23c-.38.054-.757.112-1.134.175C2.999 7.58 2.25 8.507 2.25 9.574V18a2.25 2.25 0 0 0 2.25 2.25h15A2.25 2.25 0 0 0 21.75 18V9.574c0-1.067-.75-1.994-1.802-2.169a47.865 47.865 0 0 0-1.134-.175 2.31 2.31 0 0 1-1.64-1.055l-.822-1.316a2.192 2.192 0 0 0-1.736-1.039 48.774 48.774 0 0 0-5.232 0 2.192 2.192 0 0 0-1.736 1.039l-.821 1.316Z" />
<path stroke-linecap="round" stroke-linejoin="round" d="M16.5 12.75a4.5 4.5 0 1 1-9 0 4.5 4.5 0 0 1 9 0ZM18.75 10.5h.008v.008h-.008V10.5Z" />
</svg>
</button>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding a button to open the webcam is a user-friendly feature. However, consider adding an aria-label attribute to the button for accessibility purposes, ensuring that screen readers can accurately describe the button's function to users who rely on assistive technologies.

<button type="button" class="text-white cursor-pointer py-2 p-1 px-2 rounded-full bg-blue-900 text-xs" @click.stop="openWebcam" v-if="cameraUpload" aria-label="Open webcam">

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
<button type="button" class="text-white cursor-pointer py-2 p-1 px-2 rounded-full bg-blue-900 text-xs" @click.stop="openWebcam" v-if="cameraUpload">
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor" class="w-6 h-6">
<path stroke-linecap="round" stroke-linejoin="round" d="M6.827 6.175A2.31 2.31 0 0 1 5.186 7.23c-.38.054-.757.112-1.134.175C2.999 7.58 2.25 8.507 2.25 9.574V18a2.25 2.25 0 0 0 2.25 2.25h15A2.25 2.25 0 0 0 21.75 18V9.574c0-1.067-.75-1.994-1.802-2.169a47.865 47.865 0 0 0-1.134-.175 2.31 2.31 0 0 1-1.64-1.055l-.822-1.316a2.192 2.192 0 0 0-1.736-1.039 48.774 48.774 0 0 0-5.232 0 2.192 2.192 0 0 0-1.736 1.039l-.821 1.316Z" />
<path stroke-linecap="round" stroke-linejoin="round" d="M16.5 12.75a4.5 4.5 0 1 1-9 0 4.5 4.5 0 0 1 9 0ZM18.75 10.5h.008v.008h-.008V10.5Z" />
</svg>
</button>
</div>
<button type="button" class="text-white cursor-pointer py-2 p-1 px-2 rounded-full bg-blue-900 text-xs" @click.stop="openWebcam" v-if="cameraUpload" aria-label="Open webcam">
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor" class="w-6 h-6">
<path stroke-linecap="round" stroke-linejoin="round" d="M6.827 6.175A2.31 2.31 0 0 1 5.186 7.23c-.38.054-.757.112-1.134.175C2.999 7.58 2.25 8.507 2.25 9.574V18a2.25 2.25 0 0 0 2.25 2.25h15A2.25 2.25 0 0 0 21.75 18V9.574c0-1.067-.75-1.994-1.802-2.169a47.865 47.865 0 0 0-1.134-.175 2.31 2.31 0 0 1-1.64-1.055l-.822-1.316a2.192 2.192 0 0 0-1.736-1.039 48.774 48.774 0 0 0-5.232 0 2.192 2.192 0 0 0-1.736 1.039l-.821 1.316Z" />
<path stroke-linecap="round" stroke-linejoin="round" d="M16.5 12.75a4.5 4.5 0 1 1-9 0 4.5 4.5 0 0 1 9 0ZM18.75 10.5h.008v.008h-.008V10.5Z" />
</svg>
</button>
</div>

Comment on lines +200 to +210
openWebcam(){
if(!this.cameraUpload){
return;
}
this.isInWebcam = true;
},
cameraFileUpload(file){
this.isInWebcam = false
this.isUploading = false;
this.uploadFileToServer(file)
},
Copy link
Contributor

Choose a reason for hiding this comment

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

The openWebcam and cameraFileUpload methods are crucial for handling webcam interactions. Ensure that error handling is considered in cameraFileUpload to manage any issues during the upload process, enhancing the robustness of the feature.

cameraFileUpload(file){
  this.isInWebcam = false
  this.isUploading = false;
+ try {
    this.uploadFileToServer(file)
+ } catch (error) {
+   console.error("Error uploading file from camera:", error);
+   // Consider notifying the user of the upload failure
+ }
}

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
openWebcam(){
if(!this.cameraUpload){
return;
}
this.isInWebcam = true;
},
cameraFileUpload(file){
this.isInWebcam = false
this.isUploading = false;
this.uploadFileToServer(file)
},
openWebcam(){
if(!this.cameraUpload){
return;
}
this.isInWebcam = true;
},
cameraFileUpload(file){
this.isInWebcam = false
this.isUploading = false;
try {
this.uploadFileToServer(file)
} catch (error) {
console.error("Error uploading file from camera:", error);
// Consider notifying the user of the upload failure
}
},

Comment on lines 20 to 46
<div class="bg-black rounded-full p-1 px-2 flex items-center justify-center text-xs space-x-4"
v-if="capturedImage">
<span class="text-amsber-600 cursor-pointer" @click="openCameraUpload">
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5"
stroke="currentColor" class="w-4 h-4">
<path stroke-linecap="round" stroke-linejoin="round"
d="M16.023 9.348h4.992v-.001M2.985 19.644v-4.992m0 0h4.992m-4.993 0 3.181 3.183a8.25 8.25 0 0 0 13.803-3.7M4.031 9.865a8.25 8.25 0 0 1 13.803-3.7l3.181 3.182m0-4.991v4.99" />
</svg>

</span>


<span class="text-lime-600 cursor-pointer w-8 h-8 rounded-full bg-gray-100 grid place-content-center"
@click="processCapturedImage">
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5"
stroke="currentColor" class="w-6 h-6">
<path stroke-linecap="round" stroke-linejoin="round" d="m4.5 12.75 6 6 9-13.5" />
</svg>
</span>

<span class="text-resd-600 cursor-pointer" @click="cancelCamera">
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5"
stroke="currentColor" class="w-4 h-4">
<path stroke-linecap="round" stroke-linejoin="round" d="M6 18 18 6M6 6l12 12" />
</svg>
</span>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

The UI for handling the captured image, including options to retake the image or confirm the capture, is well-designed. However, there's a typo in the class name text-amsber-600 which should likely be text-amber-600. Correcting this typo will ensure the intended styling is applied.

- <span class="text-amsber-600 cursor-pointer" @click="openCameraUpload">
+ <span class="text-amber-600 cursor-pointer" @click="openCameraUpload">

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
<div class="bg-black rounded-full p-1 px-2 flex items-center justify-center text-xs space-x-4"
v-if="capturedImage">
<span class="text-amsber-600 cursor-pointer" @click="openCameraUpload">
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5"
stroke="currentColor" class="w-4 h-4">
<path stroke-linecap="round" stroke-linejoin="round"
d="M16.023 9.348h4.992v-.001M2.985 19.644v-4.992m0 0h4.992m-4.993 0 3.181 3.183a8.25 8.25 0 0 0 13.803-3.7M4.031 9.865a8.25 8.25 0 0 1 13.803-3.7l3.181 3.182m0-4.991v4.99" />
</svg>
</span>
<span class="text-lime-600 cursor-pointer w-8 h-8 rounded-full bg-gray-100 grid place-content-center"
@click="processCapturedImage">
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5"
stroke="currentColor" class="w-6 h-6">
<path stroke-linecap="round" stroke-linejoin="round" d="m4.5 12.75 6 6 9-13.5" />
</svg>
</span>
<span class="text-resd-600 cursor-pointer" @click="cancelCamera">
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5"
stroke="currentColor" class="w-4 h-4">
<path stroke-linecap="round" stroke-linejoin="round" d="M6 18 18 6M6 6l12 12" />
</svg>
</span>
</div>
<div class="bg-black rounded-full p-1 px-2 flex items-center justify-center text-xs space-x-4"
v-if="capturedImage">
<span class="text-amber-600 cursor-pointer" @click="openCameraUpload">
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5"
stroke="currentColor" class="w-4 h-4">
<path stroke-linecap="round" stroke-linejoin="round"
d="M16.023 9.348h4.992v-.001M2.985 19.644v-4.992m0 0h4.992m-4.993 0 3.181 3.183a8.25 8.25 0 0 0 13.803-3.7M4.031 9.865a8.25 8.25 0 0 1 13.803-3.7l3.181 3.182m0-4.991v4.99" />
</svg>
</span>
<span class="text-lime-600 cursor-pointer w-8 h-8 rounded-full bg-gray-100 grid place-content-center"
@click="processCapturedImage">
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5"
stroke="currentColor" class="w-6 h-6">
<path stroke-linecap="round" stroke-linejoin="round" d="m4.5 12.75 6 6 9-13.5" />
</svg>
</span>
<span class="text-resd-600 cursor-pointer" @click="cancelCamera">
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5"
stroke="currentColor" class="w-4 h-4">
<path stroke-linecap="round" stroke-linejoin="round" d="M6 18 18 6M6 6l12 12" />
</svg>
</span>
</div>

Comment on lines 115 to 129
methods: {
openCameraUpload() {
this.isCapturing = true;
this.capturedImage = null;
this.webcam.start()
.then(result => {
this.cameraPermissionStatus = 'allowed';
})
.catch(err => {
if(err.toString() === 'NotAllowedError: Permission denied'){
this.cameraPermissionStatus = 'blocked';
return;
}
this.cameraPermissionStatus = 'unknown';
});
Copy link
Contributor

Choose a reason for hiding this comment

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

The openCameraUpload method correctly handles starting the webcam and setting the camera permission status based on the outcome. However, the error handling could be improved by logging the error or providing more specific feedback to the user for errors other than NotAllowedError.

.catch(err => {
  if(err.toString()  === 'NotAllowedError: Permission denied'){
    this.cameraPermissionStatus = 'blocked';
    return;
  }
+ console.error("Error starting webcam:", err);
  this.cameraPermissionStatus = 'unknown';
});

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
methods: {
openCameraUpload() {
this.isCapturing = true;
this.capturedImage = null;
this.webcam.start()
.then(result => {
this.cameraPermissionStatus = 'allowed';
})
.catch(err => {
if(err.toString() === 'NotAllowedError: Permission denied'){
this.cameraPermissionStatus = 'blocked';
return;
}
this.cameraPermissionStatus = 'unknown';
});
methods: {
openCameraUpload() {
this.isCapturing = true;
this.capturedImage = null;
this.webcam.start()
.then(result => {
this.cameraPermissionStatus = 'allowed';
})
.catch(err => {
if(err.toString() === 'NotAllowedError: Permission denied'){
this.cameraPermissionStatus = 'blocked';
return;
}
console.error("Error starting webcam:", err);
this.cameraPermissionStatus = 'unknown';
});

Comment on lines 144 to 164
processCapturedImage() {
const byteCharacters = atob(this.capturedImage.split(',')[1]);
const byteArrays = [];
for (let offset = 0; offset < byteCharacters.length; offset += 512) {
const slice = byteCharacters.slice(offset, offset + 512);

const byteNumbers = new Array(slice.length);
for (let i = 0; i < slice.length; i++) {
byteNumbers[i] = slice.charCodeAt(i);
}

const byteArray = new Uint8Array(byteNumbers);
byteArrays.push(byteArray);
}

// Create Blob from binary data
const blob = new Blob(byteArrays, { type: 'image/png' });
const filename = Date.now()
// Create a File object from the Blob
const file = new File([blob], `${filename}.png`, { type: 'image/png' });
this.$emit('uploadImage', file)
Copy link
Contributor

Choose a reason for hiding this comment

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

The processCapturedImage method effectively converts the captured image to a File object, ready for upload. This method is well-implemented, but consider adding error handling to manage potential issues during the conversion process.

processCapturedImage() {
+ try {
    const byteCharacters = atob(this.capturedImage.split(',')[1]);
    // Existing implementation...
    this.$emit('uploadImage', file)
+ } catch (error) {
+   console.error("Error processing captured image:", error);
+   // Consider notifying the user of the failure
+ }
}

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
processCapturedImage() {
const byteCharacters = atob(this.capturedImage.split(',')[1]);
const byteArrays = [];
for (let offset = 0; offset < byteCharacters.length; offset += 512) {
const slice = byteCharacters.slice(offset, offset + 512);
const byteNumbers = new Array(slice.length);
for (let i = 0; i < slice.length; i++) {
byteNumbers[i] = slice.charCodeAt(i);
}
const byteArray = new Uint8Array(byteNumbers);
byteArrays.push(byteArray);
}
// Create Blob from binary data
const blob = new Blob(byteArrays, { type: 'image/png' });
const filename = Date.now()
// Create a File object from the Blob
const file = new File([blob], `${filename}.png`, { type: 'image/png' });
this.$emit('uploadImage', file)
processCapturedImage() {
try {
const byteCharacters = atob(this.capturedImage.split(',')[1]);
const byteArrays = [];
for (let offset = 0; offset < byteCharacters.length; offset += 512) {
const slice = byteCharacters.slice(offset, offset + 512);
const byteNumbers = new Array(slice.length);
for (let i = 0; i < slice.length; i++) {
byteNumbers[i] = slice.charCodeAt(i);
}
const byteArray = new Uint8Array(byteNumbers);
byteArrays.push(byteArray);
}
// Create Blob from binary data
const blob = new Blob(byteArrays, { type: 'image/png' });
const filename = Date.now()
// Create a File object from the Blob
const file = new File([blob], `${filename}.png`, { type: 'image/png' });
this.$emit('uploadImage', file)
} catch (error) {
console.error("Error processing captured image:", error);
// Consider notifying the user of the failure
}

Copy link
Owner

@JhumanJ JhumanJ left a comment

Choose a reason for hiding this comment

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

Great work! Some feedback:
CleanShot 2024-03-12 at 18 39 51@2x

  1. The snap button should have the same background color (and same text color) than the submit button (essentially use the form-color). The icon color should be computed based on the form color (like it's done for the submit button

  2. Form Theme Styling should be respected for the camera upload (missing shadow, not rounded etc.). Feel free to add a new entry in form-themes.js for the fileInput
    CleanShot 2024-03-12 at 18 41 48@2x

  3. Currently the process is like this: 1. click the camera button, 2. confirm, 3 see uploaded image. I think to simplify we should get rid of step 2 and go directly from 1 to 3

CleanShot 2024-03-12 at 18 43 43@2x

  1. Multiple file upload isn't working with the camera upload feature. I have the multiple file option enabled, but I can't take more pictures (although I still can upload files)
    CleanShot 2024-03-12 at 18 46 17@2x

  2. it seems like the picture taken has the size of the CameraUpload component size, is it possible to take larger picture? Full size ideally?

  3. I think we should disable this on mobile since they have direct access to the camera. Make to keep it simple we can hide by default (with css hidden) and show it on larger screens (sm:block)?

Thank you!

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 6d79d2c and 0d34200.
Files selected for processing (3)
  • client/components/forms/FileInput.vue (6 hunks)
  • client/components/forms/components/CameraUpload.vue (1 hunks)
  • client/lib/forms/form-themes.js (3 hunks)
Additional comments: 8
client/components/forms/components/CameraUpload.vue (4)
  • 3-4: The use of :class to conditionally hide or show the <video> and <canvas> elements based on isCapturing and capturedImage is a clean implementation for dynamic UI updates.
  • 20-34: The conditional rendering based on cameraPermissionStatus provides a clear user interface for when camera access is blocked. It's good practice to guide users on how to enable permissions. However, consider adding an action or link that directly opens the browser settings for camera permissions, if possible, to enhance user experience.
  • 44-55: The error handling UI for when the camera device cannot be started is well implemented. It's user-friendly to provide a clear message and an action to go back. This approach helps in maintaining a good user experience even in error scenarios.
  • 91-105: The error handling in openCameraUpload method has been improved based on previous comments, which is great. Logging the error for cases other than NotAllowedError is a good practice for debugging purposes. Ensure that this error logging is consistent across all error handling in the application for maintainability.
client/lib/forms/form-themes.js (3)
  • 42-42: The addition of rounded-lg to the cameraInput style in the default theme enhances the visual consistency with other input elements. This is a good practice for maintaining a cohesive UI design.
  • 87-87: For the simple theme, the cameraInput style has been adjusted without specifying rounded-lg. Ensure that the absence of rounded-lg is intentional and consistent with the design language of the simple theme.
  • 132-132: The addition of rounded to the cameraInput style in the notion theme aligns with the theme's design principles. Consistency in UI elements across different themes is crucial for a seamless user experience.
client/components/forms/FileInput.vue (1)
  • 8-10: The conditional rendering for the camera upload component is well-implemented. It ensures that the camera upload UI is only available when the cameraUpload prop is true and the user is in the webcam mode. This is a good practice for feature toggling and user experience.

Comment on lines 71 to 78
<div class="w-full items-center justify-center mt-2 hidden sm:flex">
<open-form-button native-typ="buttom" :loading="loading" :theme="theme" :color="color" class="py-2 p-1 px-2" @click.stop="openWebcam" v-if="cameraUpload">
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor" class="w-6 h-6">
<path stroke-linecap="round" stroke-linejoin="round" d="M6.827 6.175A2.31 2.31 0 0 1 5.186 7.23c-.38.054-.757.112-1.134.175C2.999 7.58 2.25 8.507 2.25 9.574V18a2.25 2.25 0 0 0 2.25 2.25h15A2.25 2.25 0 0 0 21.75 18V9.574c0-1.067-.75-1.994-1.802-2.169a47.865 47.865 0 0 0-1.134-.175 2.31 2.31 0 0 1-1.64-1.055l-.822-1.316a2.192 2.192 0 0 0-1.736-1.039 48.774 48.774 0 0 0-5.232 0 2.192 2.192 0 0 0-1.736 1.039l-.821 1.316Z" />
<path stroke-linecap="round" stroke-linejoin="round" d="M16.5 12.75a4.5 4.5 0 1 1-9 0 4.5 4.5 0 0 1 9 0ZM18.75 10.5h.008v.008h-.008V10.5Z" />
</svg>
</open-form-button>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

The button to open the webcam is a nice addition for user interaction. However, ensure that the native-typ="buttom" attribute is a typo and should be corrected to native-type="button" to avoid potential HTML validation issues.

- <open-form-button native-typ="buttom" :loading="loading" :theme="theme" :color="color" class="py-2 p-1 px-2" @click.stop="openWebcam" v-if="cameraUpload">
+ <open-form-button native-type="button" :loading="loading" :theme="theme" :color="color" class="py-2 p-1 px-2" @click.stop="openWebcam" v-if="cameraUpload">

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
<div class="w-full items-center justify-center mt-2 hidden sm:flex">
<open-form-button native-typ="buttom" :loading="loading" :theme="theme" :color="color" class="py-2 p-1 px-2" @click.stop="openWebcam" v-if="cameraUpload">
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor" class="w-6 h-6">
<path stroke-linecap="round" stroke-linejoin="round" d="M6.827 6.175A2.31 2.31 0 0 1 5.186 7.23c-.38.054-.757.112-1.134.175C2.999 7.58 2.25 8.507 2.25 9.574V18a2.25 2.25 0 0 0 2.25 2.25h15A2.25 2.25 0 0 0 21.75 18V9.574c0-1.067-.75-1.994-1.802-2.169a47.865 47.865 0 0 0-1.134-.175 2.31 2.31 0 0 1-1.64-1.055l-.822-1.316a2.192 2.192 0 0 0-1.736-1.039 48.774 48.774 0 0 0-5.232 0 2.192 2.192 0 0 0-1.736 1.039l-.821 1.316Z" />
<path stroke-linecap="round" stroke-linejoin="round" d="M16.5 12.75a4.5 4.5 0 1 1-9 0 4.5 4.5 0 0 1 9 0ZM18.75 10.5h.008v.008h-.008V10.5Z" />
</svg>
</open-form-button>
</div>
<div class="w-full items-center justify-center mt-2 hidden sm:flex">
<open-form-button native-type="button" :loading="loading" :theme="theme" :color="color" class="py-2 p-1 px-2" @click.stop="openWebcam" v-if="cameraUpload">
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor" class="w-6 h-6">
<path stroke-linecap="round" stroke-linejoin="round" d="M6.827 6.175A2.31 2.31 0 0 1 5.186 7.23c-.38.054-.757.112-1.134.175C2.999 7.58 2.25 8.507 2.25 9.574V18a2.25 2.25 0 0 0 2.25 2.25h15A2.25 2.25 0 0 0 21.75 18V9.574c0-1.067-.75-1.994-1.802-2.169a47.865 47.865 0 0 0-1.134-.175 2.31 2.31 0 0 1-1.64-1.055l-.822-1.316a2.192 2.192 0 0 0-1.736-1.039 48.774 48.774 0 0 0-5.232 0 2.192 2.192 0 0 0-1.736 1.039l-.821 1.316Z" />
<path stroke-linecap="round" stroke-linejoin="round" d="M16.5 12.75a4.5 4.5 0 1 1-9 0 4.5 4.5 0 0 1 9 0ZM18.75 10.5h.008v.008h-.008V10.5Z" />
</svg>
</open-form-button>
</div>

Comment on lines +209 to +213
cameraFileUpload(file){
this.isInWebcam = false
this.isUploading = false;
this.uploadFileToServer(file)
},
Copy link
Contributor

Choose a reason for hiding this comment

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

The cameraFileUpload method correctly handles the transition from webcam to file upload state. However, as previously suggested, consider adding error handling around this.uploadFileToServer(file) to manage any issues during the upload process, enhancing the robustness of the feature.

cameraFileUpload(file){
  this.isInWebcam = false
  this.isUploading = false;
+ try {
    this.uploadFileToServer(file)
+ } catch (error) {
+   console.error("Error uploading file from camera:", error);
+   // Consider notifying the user of the upload failure
+ }
}

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
cameraFileUpload(file){
this.isInWebcam = false
this.isUploading = false;
this.uploadFileToServer(file)
},
cameraFileUpload(file){
this.isInWebcam = false
this.isUploading = false;
try {
this.uploadFileToServer(file)
} catch (error) {
console.error("Error uploading file from camera:", error);
// Consider notifying the user of the upload failure
}
},

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 0d34200 and 79ad843.
Files selected for processing (1)
  • client/components/forms/components/CameraUpload.vue (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • client/components/forms/components/CameraUpload.vue

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 79ad843 and 50c1f29.
Files selected for processing (1)
  • client/components/forms/FileInput.vue (6 hunks)
Files skipped from review as they are similar to previous changes (1)
  • client/components/forms/FileInput.vue

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 50c1f29 and da3a964.
Files ignored due to path filters (2)
  • client/package-lock.json is excluded by: !**/*.json
  • client/package.json is excluded by: !**/*.json
Files selected for processing (3)
  • client/components/forms/FileInput.vue (5 hunks)
  • client/components/open/forms/OpenFormField.vue (1 hunks)
  • client/components/open/forms/fields/components/FieldOptions.vue (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • client/components/forms/FileInput.vue
Additional comments: 2
client/components/open/forms/OpenFormField.vue (1)
  • 275-275: The implementation of the cameraUpload property in inputProperties method correctly enables camera uploads based on the form field configuration. This is a key part of enabling the new feature.
client/components/open/forms/fields/components/FieldOptions.vue (1)
  • 49-53: The addition of the checkbox for enabling camera uploads in the template is correctly implemented and bound to the field.camera_upload property. This allows for easy configuration of camera uploads on a per-field basis.

@JhumanJ JhumanJ merged commit 659dc50 into main Mar 25, 2024
5 checks passed
@JhumanJ JhumanJ deleted the 20334-implement-desktop-camera-upload-feature branch March 25, 2024 10:00
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.

2 participants