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

Support for Barcode reader #650

Merged
merged 5 commits into from
Jan 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
171 changes: 171 additions & 0 deletions client/components/forms/BarcodeInput.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
<template>
<input-wrapper v-bind="inputWrapperProps">
<template #label>
<slot name="label" />
</template>

<div
v-if="isScanning"
class="relative w-full"
>
<CameraUpload
:is-barcode-mode="true"
:decoders="decoders"
@stop-webcam="stopScanning"
@barcode-detected="handleBarcodeDetected"
/>
</div>

<div
v-else-if="scannedValue"
class="flex items-center justify-between border border-gray-300 dark:border-gray-600 w-full bg-white text-gray-700 dark:bg-notion-dark-light dark:text-gray-300 rounded-lg px-4 py-2"
>
<div class="flex-1 break-all">
{{ scannedValue }}
</div>
Comment on lines +23 to +25
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add XSS protection for scanned values

The scanned value is directly rendered in the template without sanitization, which could be a security risk if malicious QR codes are scanned.

-<div class="flex-1 break-all">
-  {{ scannedValue }}
-</div>
+<div class="flex-1 break-all">
+  {{ sanitizeBarcode(scannedValue) }}
+</div>

Add this method to the component:

methods: {
  sanitizeBarcode(value) {
    return value ? value.replace(/[<>]/g, '') : '';
  }
}

<button
v-if="!disabled"
type="button"
class="pt-1 text-gray-400 hover:text-gray-600"
@click="clearValue"
>
<Icon
name="i-heroicons-x-mark-20-solid"
class="h-5 w-5"
/>
</button>
</div>

<div
v-else
:style="inputStyle"
class="flex flex-col w-full items-center justify-center transition-colors duration-40"
:class="[
{'!cursor-not-allowed':disabled, 'cursor-pointer':!disabled},
theme.fileInput.input,
theme.fileInput.borderRadius,
theme.fileInput.spacing.horizontal,
theme.fileInput.spacing.vertical,
theme.fileInput.fontSize,
theme.fileInput.minHeight,
{'border-red-500 border-2':hasError},
'focus:outline-none focus:ring-2'
]"
tabindex="0"
role="button"
aria-label="Click to open a camera"
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance accessibility with dynamic aria-labels

The aria-label is static and doesn't reflect the component's current state (scanning/scanned/empty).

-aria-label="Click to open a camera"
+:aria-label="isScanning ? 'Camera is active for barcode scanning' : scannedValue ? 'Scanned barcode value: ' + scannedValue : 'Click to open camera for barcode scanning'"

Committable suggestion skipped: line range outside the PR's diff.

@click="startScanning"
@keydown.enter.prevent="startScanning"
>
<div class="flex w-full items-center justify-center">
<div class="text-center">
<template v-if="!scannedValue && !isScanning">
<div class="text-gray-500 w-full flex justify-center">
<svg
xmlns="http://www.w3.org/2000/svg"
fill="none"
viewBox="0 0 24 24"
stroke-width="1.5"
stroke="currentColor"
class="w-5 h-5"
>
<path
stroke-linecap="round"
stroke-linejoin="round"
d="M3 16.5v2.25A2.25 2.25 0 005.25 21h13.5A2.25 2.25 0 0021 18.75V16.5m-13.5-9L12 3m0 0l4.5 4.5M12 3v13.5"
/>
</svg>
</div>

<p class="mt-2 text-sm text-gray-500 font-medium select-none">
{{ $t('forms.barcodeInput.clickToOpenCamera') }}
</p>
<div class="w-full items-center justify-center mt-2 hidden sm:flex">
<UButton
icon="i-heroicons-camera"
color="white"
class="px-2"
@click.stop="startScanning"
/>
</div>
</template>
</div>
</div>
</div>

<template #error>
<slot name="error" />
</template>
</input-wrapper>
</template>

<script>
import { inputProps, useFormInput } from './useFormInput.js'
import InputWrapper from './components/InputWrapper.vue'
import CameraUpload from './components/CameraUpload.vue'

export default {
name: 'BarcodeInput',
components: { InputWrapper, CameraUpload },

props: {
...inputProps,
decoders: {
type: Array,
default: () => []
}
Comment on lines +113 to +116
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add validation and documentation for decoders prop

The decoders prop lacks validation for supported Quagga decoder types and documentation for API users.

 decoders: {
   type: Array,
   default: () => [],
+  validator: (value) => {
+    const supportedDecoders = ['code_128_reader', 'ean_reader', 'ean_8_reader', 'code_39_reader', 'upc_reader'];
+    return value.every(decoder => supportedDecoders.includes(decoder));
+  },
+  description: 'Array of Quagga decoder types to enable. Supported values: code_128_reader, ean_reader, ean_8_reader, code_39_reader, upc_reader'
 }
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
decoders: {
type: Array,
default: () => []
}
decoders: {
type: Array,
default: () => [],
validator: (value) => {
const supportedDecoders = ['code_128_reader', 'ean_reader', 'ean_8_reader', 'code_39_reader', 'upc_reader'];
return value.every(decoder => supportedDecoders.includes(decoder));
},
description: 'Array of Quagga decoder types to enable. Supported values: code_128_reader, ean_reader, ean_8_reader, code_39_reader, upc_reader'
}

},

setup(props, context) {
return {
...useFormInput(props, context),
}
},

data: () => ({
isScanning: false,
scannedValue: null
}),

watch: {
scannedValue: {
handler(value) {
this.compVal = value
},
immediate: true
}
},

beforeUnmount() {
this.stopScanning()
},

methods: {
startScanning() {
if (this.disabled) return
this.isScanning = true
},
Comment on lines +144 to +147
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling for camera initialization

The startScanning method should handle potential camera access errors and provide user feedback.

 startScanning() {
   if (this.disabled) return
+  try {
     this.isScanning = true
+    this.$emit('scan-start')
+  } catch (error) {
+    this.handleScanError(error)
+  }
 }

Add these methods:

methods: {
  handleScanError(error) {
    this.isScanning = false;
    this.$emit('scan-error', error);
    // Provide user feedback through UI
    console.error('Camera initialization failed:', error);
  }
}


stopScanning() {
this.isScanning = false
},

handleBarcodeDetected(code) {
this.scannedValue = code
this.stopScanning()
},

clearValue() {
this.scannedValue = null
}
}
}
</script>

<style scoped>
video {
/* Ensure the video displays properly */
max-width: 100%;
height: auto;
}
</style>
55 changes: 53 additions & 2 deletions client/components/forms/components/CameraUpload.vue
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
class="p-2 px-4 flex items-center justify-center text-xs space-x-2"
>
<span
v-if="!isBarcodeMode"
class="cursor-pointer rounded-full w-14 h-14 border-2 grid place-content-center"
@click="processCapturedImage"
>
Expand Down Expand Up @@ -98,9 +99,10 @@
<script>
import Webcam from "webcam-easy"
import CachedDefaultTheme from "~/lib/forms/themes/CachedDefaultTheme.js"
import Quagga from 'quagga'

export default {
name: "FileInput",
name: "CameraUpload",
props: {
theme: {
type: Object, default: () => {
Expand All @@ -111,13 +113,22 @@ export default {
return CachedDefaultTheme.getInstance()
}
},
isBarcodeMode: {
type: Boolean,
default: false
},
decoders: {
type: Array,
default: () => []
}
},
emits: ['stopWebcam', 'uploadImage'],
emits: ['stopWebcam', 'uploadImage', 'barcodeDetected'],
data: () => ({
webcam: null,
isCapturing: false,
capturedImage: null,
cameraPermissionStatus: "loading",
quaggaInitialized: false
}),
computed: {
videoDisplay() {
Expand All @@ -142,6 +153,9 @@ export default {
.start()
.then(() => {
this.cameraPermissionStatus = "allowed"
if (this.isBarcodeMode) {
this.initQuagga()
}
Comment on lines +156 to +158
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling for barcode initialization

While camera initialization errors are handled, there's no error handling if the camera succeeds but barcode initialization fails. This could leave users waiting indefinitely.

Add error handling:

    if (this.isBarcodeMode) {
-     this.initQuagga()
+     this.initQuagga().catch(err => {
+       console.error('Failed to initialize barcode scanner:', err);
+       this.cameraPermissionStatus = 'unknown';
+       this.cancelCamera();
+     });
    }

Committable suggestion skipped: line range outside the PR's diff.

})
.catch((err) => {
console.error(err)
Expand All @@ -152,9 +166,46 @@ export default {
this.cameraPermissionStatus = "unknown"
})
},
initQuagga() {
if (!this.quaggaInitialized) {
Quagga.init({
inputStream: {
name: "Live",
type: "LiveStream",
target: document.getElementById("webcam"),
constraints: {
facingMode: "environment"
},
},
decoder: {
readers: this.decoders || []
},
locate: true
}, (err) => {
if (err) {
console.error('Quagga initialization failed:', err)
return
}
Comment on lines +186 to +188
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Provide user feedback on Quagga initialization failure

In the initQuagga method, when Quagga initialization fails, the error is logged to the console but no feedback is given to the user. Consider updating cameraPermissionStatus or displaying an error message to inform the user about the failure.

Apply this diff to handle initialization errors:

            if (err) {
              console.error('Quagga initialization failed:', err)
+             this.cameraPermissionStatus = 'error'
+             this.isCapturing = false
+             this.$emit('stopWebcam')
              return
            }
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console.error('Quagga initialization failed:', err)
return
}
console.error('Quagga initialization failed:', err)
this.cameraPermissionStatus = 'error'
this.isCapturing = false
this.$emit('stopWebcam')
return
}


this.quaggaInitialized = true
Quagga.start()

Quagga.onDetected((result) => {
if (result.codeResult) {
this.$emit('barcodeDetected', result.codeResult.code)
this.cancelCamera()
}
})
Comment on lines +193 to +198
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add cleanup for Quagga event listeners

The onDetected event listener is not properly cleaned up, which could lead to memory leaks or duplicate event handling if the component is reused.

Add cleanup in the cancelCamera method:

    if (this.quaggaInitialized) {
+     Quagga.offDetected();
      Quagga.stop();
      this.quaggaInitialized = false;
    }
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Quagga.onDetected((result) => {
if (result.codeResult) {
this.$emit('barcodeDetected', result.codeResult.code)
this.cancelCamera()
}
})
if (this.quaggaInitialized) {
Quagga.offDetected();
Quagga.stop();
this.quaggaInitialized = false;
}

})
}
},
cancelCamera() {
this.isCapturing = false
this.capturedImage = null
if (this.quaggaInitialized) {
Quagga.stop()
this.quaggaInitialized = false
}
this.webcam.stop()
this.$emit("stopWebcam")
},
Expand Down
10 changes: 7 additions & 3 deletions client/components/forms/components/VSelect.vue
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@
<div
class="flex items-center"
:class="[
theme.SelectInput.minHeight
theme.SelectInput.minHeight,
'ltr:pr-8 rtl:pl-8'
]"
>
<transition
Expand Down Expand Up @@ -76,9 +77,12 @@
</div>
</transition>
</div>
<span class="absolute inset-y-0 ltr:right-0 rtl:left-0 rtl:!right-auto flex items-center ltr:pr-2 rtl:pl-2 rtl:!pr-0 pointer-events-none">
<div
class="absolute inset-y-0 ltr:right-6 rtl:left-6 w-10 pointer-events-none bg-gradient-to-r from-transparent to-white dark:to-notion-dark-light"
/>
<span class="absolute inset-y-0 ltr:right-0 rtl:left-0 rtl:!right-auto flex items-center ltr:pr-2 rtl:pl-2 rtl:!pr-0 pointer-events-none bg-white">
<Icon
name="heroicons:chevron-up-down-16-solid"
name="heroicons:chevron-up-down-16-solid"
class="h-5 w-5 text-gray-500"
/>
</span>
Expand Down
7 changes: 6 additions & 1 deletion client/components/open/forms/OpenFormField.vue
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,8 @@ export default {
url: 'TextInput',
email: 'TextInput',
phone_number: 'TextInput',
matrix: 'MatrixInput'
matrix: 'MatrixInput',
barcode: 'BarcodeInput'
}[field.type]
},
isPublicFormPage() {
Expand Down Expand Up @@ -338,6 +339,10 @@ export default {
inputProperties.columns = field.columns
}

if (field.type === 'barcode') {
inputProperties.decoders = field.decoders
}

if (['select','multi_select'].includes(field.type) && !this.isFieldRequired) {
inputProperties.clearable = true
}
Expand Down
34 changes: 33 additions & 1 deletion client/components/open/forms/fields/components/FieldOptions.vue
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,27 @@
/>
</div>

<!-- Barcode Reader -->
<div
v-if="field.type === 'barcode'"
class="px-4"
>
<EditorSectionHeader
icon="i-material-symbols-barcode-scanner-rounded"
title="Barcode Reader"
/>
<select-input
name="decoders"
class="mt-4"
:form="field"
:options="barcodeDecodersOptions"
label="Decoders"
:searchable="true"
:multiple="true"
help="Select the decoders you want to use"
/>
</div>

<div
v-if="field.type === 'rating'"
class="px-4"
Expand Down Expand Up @@ -611,7 +632,15 @@ export default {
editorToolbarCustom: [
['bold', 'italic', 'underline', 'link']
],
allCountries: countryCodes
allCountries: countryCodes,
barcodeDecodersOptions: [
{ name: 'EAN-13 (European Article Number)', value: 'ean_reader' },
{ name: 'EAN-8 (European Article Number)', value: 'ean_8_reader' },
{ name: 'UPC-A (Universal Product Code)', value: 'upc_reader' },
{ name: 'UPC-E (Universal Product Code)', value: 'upc_e_reader' },
{ name: 'Code 128', value: 'code_128_reader' },
{ name: 'Code 39', value: 'code_39_reader' },
]
}
},

Expand Down Expand Up @@ -828,6 +857,9 @@ export default {
selection_data:{
'Row 1': null
}
},
barcode: {
decoders: ['ean_reader', 'upc_reader']
}
}
if (this.field.type in defaultFieldValues) {
Expand Down
9 changes: 9 additions & 0 deletions client/data/blocks_types.json
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,15 @@
"text_class": "text-pink-900",
"is_input": true
},
"barcode": {
"name": "barcode",
"title": "Barcode Reader",
"icon": "i-material-symbols-barcode-scanner-rounded",
"default_block_name": "Scan Barcode",
"bg_class": "bg-pink-100",
"text_class": "text-pink-900",
"is_input": true
},
"nf-text": {
"name": "nf-text",
"title": "Text",
Expand Down
Loading
Loading