gruhn / vue-qrcode-reader

A set of Vue.js components for detecting and decoding QR codes.
https://gruhn.github.io/vue-qrcode-reader
MIT License
2.09k stars 335 forks source link

feat: add formats prop #357

Closed samydoesit closed 1 year ago

samydoesit commented 1 year ago

Add formats prop

356

netlify[bot] commented 1 year ago

Deploy Preview for vue-qrcode-reader ready!

Name Link
Latest commit 435d42f9e877913f60acce351d3f1f0b2b1199cd
Latest deploy log https://app.netlify.com/sites/vue-qrcode-reader/deploys/64dcfcb7ac936b000903388c
Deploy Preview https://deploy-preview-357--vue-qrcode-reader.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

gruhn commented 1 year ago

Amazing, thanks! Could you also add a demo that shows the feature in action? Maybe on QrcodeStream and with a (multi-)select field, where users can select the barcode formats to be used. The Visual Tracking demo has a similar setup. Maybe it's also useful to turn on tracking for the new demo so we can see how tracking behaves on the different barcode formats.

samydoesit commented 1 year ago

Duplicated the "Visual Tracking" Page added "Visual Tracking with Formats" (though the naming might not be ideal).

Regarding the component props reactivity I attempted to add props.formats to the cameraSettings computed property in QrcodeStream, so that the cameraSettings watcher also responds to changes when the formats change. Additionally, I set cameraActive.value to false and then, after a nextTick, set it to true. However, this caused the barcode detection to stop working. So, I've omitted this change. Any suggestions to make this more reactive?

image
const cameraSettings = computed(() => {
  return {
    torch: props.torch,
+    formats: props.formats,
    constraints: props.constraints,
    shouldStream: isMounted.value && !props.paused
  }
})

+watch(cameraSettings, async (cameraSettings, prevCameraSettings) => {
+  if(cameraSettings.formats !== prevCameraSettings.formats) {
+    cameraActive.value = false
+    await nextTick()
+    cameraActive.value = true
+  }
  if (cameraSettings.shouldStream) { // start camera
gruhn commented 1 year ago

Regarding the component props reactivity ...

Exactly, this is the tricky part.

I attempted to add props.formats to the cameraSettings computed property in QrcodeStream, so that the cameraSettings watcher also responds to changes when the formats change.

I think it's not necessary to reload the camera when formats changes. It should be possible to let the camera stream running but just switch the BarcodeDetector instance "on-the-fly". Maybe we can just export a setFormats function or something from scanner.ts so the BarcodeDetector instance can be replaced during scanning. But I'm not sure if that's the most elegant approach or if there might be problems with that.

Incidentally, I think this check:

if(cameraSettings.formats !== prevCameraSettings.formats) {

might be true too often, since it checks equality by reference. For example: ["qr_code"] !== ["qr_code"] is true because the objects on both sides have different references.

samydoesit commented 1 year ago

I tried to avoid introducing a side effect, but replacing the BarcodeDetector instance seemed to work well when I tested it.

gruhn commented 1 year ago

I tried to avoid introducing a side effect

I also try to avoid that most the time and rely on the reactive system as much as possible. But as I said, not sure what would be nicer here.

seemed to work well when I tested it.

Same here good job 👍

gruhn commented 1 year ago

fyi: here are examples images for each barcode types: https://github.com/gruhn/barcode-detector/tree/master/example_codes

gruhn commented 1 year ago

Alright, looks good to me :slightly_smiling_face:

github-actions[bot] commented 1 year ago

:tada: This PR is included in version 5.3.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: