-
Notifications
You must be signed in to change notification settings - Fork 344
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
Conversation
WalkthroughThe recent update enhances user experience by enabling direct image uploads from webcams in specific forms. A new Changes
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? TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this 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
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 oncameraUpload
andisInWebcam
flags is correctly done. However, ensure that theCameraUpload
component emits theuploadImage
andstopWebcam
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 offalse
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
andcanvas
elements to display the webcam feed and captured images is correctly implemented. Using conditional classes to hide elements based on state (isCapturing
andcapturedImage
) 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
, andcameraPermissionStatus
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 offield.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 consumeinputProperties
.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.
} | ||
} 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) { |
There was a problem hiding this comment.
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.
<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> |
There was a problem hiding this comment.
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.
<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> |
openWebcam(){ | ||
if(!this.cameraUpload){ | ||
return; | ||
} | ||
this.isInWebcam = true; | ||
}, | ||
cameraFileUpload(file){ | ||
this.isInWebcam = false | ||
this.isUploading = false; | ||
this.uploadFileToServer(file) | ||
}, |
There was a problem hiding this comment.
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.
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 | |
} | |
}, |
<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> |
There was a problem hiding this comment.
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.
<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> |
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'; | ||
}); |
There was a problem hiding this comment.
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.
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'; | |
}); |
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) |
There was a problem hiding this comment.
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.
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 | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
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
-
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 thefileInput
-
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
-
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)
-
it seems like the picture taken has the size of the CameraUpload component size, is it possible to take larger picture? Full size ideally?
-
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!
There was a problem hiding this 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
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 onisCapturing
andcapturedImage
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 thanNotAllowedError
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 thecameraInput
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 specifyingrounded-lg
. Ensure that the absence ofrounded-lg
is intentional and consistent with the design language of the simple theme.- 132-132: The addition of
rounded
to thecameraInput
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.
<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> |
There was a problem hiding this comment.
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.
<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> |
cameraFileUpload(file){ | ||
this.isInWebcam = false | ||
this.isUploading = false; | ||
this.uploadFileToServer(file) | ||
}, |
There was a problem hiding this comment.
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.
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 | |
} | |
}, |
There was a problem hiding this 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
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
There was a problem hiding this 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
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
There was a problem hiding this 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
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 ininputProperties
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.
Summary by CodeRabbit