feat: Add forbiddenBarcodeTypes property #727
Conversation
IlyaPasternakAmitech
changed the title
|
Hi @IlyaPasternakAmitech I really like this idea, however I think the best use case would be the reverse - an allow list. So the suggestion is simply to flip the behavior, so instead of: The benefit is that it's also easier to achieve faster QR code scanning, as you can simply state which codes you want supported, and then it only spends time looking for those. What do you think? |
|
Hi @scarlac, thanks for the feedback! I understand your point about using an allow list approach (allowedBarcodeTypes) — it makes sense in many cases and could help avoid issues when new barcode types are added in the future. However, in my use case, I find the blocklist approach (forbiddenBarcodeTypes) more practical. Here's why: 🔍 Use Case: Filter Out Only a Few Specific Barcode Types Using a blacklist allows us to simply block those unwanted QR codes, while still detecting any other barcode type that may appear — without having to explicitly allow each one. With an allow list , we would have to specify every single barcode type we support (e.g., EAN13, UPC_A, etc.), which is less flexible and harder to maintain. 🔄 Proposed Solution: Mutual Exclusivity Between Both Props This way: Developers can choose whichever fits their use case better. // Option 1: Block specific barcode types
<BarcodeScanner forbiddenBarcodeTypes={['qr']} />
// Option 2: Allow only specific barcode types
<BarcodeScanner allowedBarcodeTypes={['code-128', 'code-39', 'code-93', 'codabar', 'ean-13', 'ean-8', 'itf', 'upc-a', 'upc-e', 'pdf-417', 'aztec', 'data-matrix']} />If both props are provided, we can throw a runtime error or ignore one of them. |
|
Hi @scarlac, I’ve updated the PR based on your suggestion — I changed the logic and replaced forbiddenBarcodeTypes with allowedBarcodeTypes. The new prop works as we discussed: if provided, the scanner will only recognize the specified barcode types, making scanning more precise and future-proof. Also, I’d like to remind you of my previous proposal to support both approaches using two mutually exclusive props — either allowedBarcodeTypes or forbiddenBarcodeTypes. If you think that would be useful, I can implement it in a separate PR. If these current changes look good to you, feel free to merge them. I'd love to hear your thoughts! Thanks! |
| hasCameraBeenSetup = true | ||
| #if targetEnvironment(macCatalyst) | ||
| // Force front camera on Mac Catalyst during initial setup | ||
| camera.setup(cameraType: .front, supportedBarcodeType: scanBarcode && onReadCode != nil ? supportedBarcodeType : []) |
There was a problem hiding this comment.
do we still need supportedBarcodeType class variable?
| } | ||
| private func setupCamera() { | ||
| if hasPropBeenSetup && hasPermissionBeenGranted && !hasCameraBeenSetup { | ||
| let allowedBarcodeTypes = convertAllowedBarcodeTypes() |
There was a problem hiding this comment.
not sure it's a good thing to hide allowedBarcodeTypes class variable by using the same name
it can be source of tricky error
| private var frameColor = Color.GREEN | ||
| private var laserColor = Color.RED | ||
| private var barcodeFrameSize: Size? = null | ||
| private var allowedBarcodeTypes: ReadableArray? = null |
There was a problem hiding this comment.
Why not make it more meaningful with an Array<CodeFormat>
| @@ -143,6 +143,11 @@ class CKCameraManager : SimpleViewManager<CKCamera>(), CKCameraManagerInterface< | |||
| view.setShutterPhotoSound(enabled); | |||
| } | |||
|
|
|||
| @ReactProp(name = "allowedBarcodeTypes") | |||
| override fun setAllowedBarcodeTypes(view: CKCamera, types: ReadableArray?) { | |||
| view.setAllowedBarcodeTypes(types) | |||
There was a problem hiding this comment.
We might want to do the conversion to Array<CodeFormat> straight here, then forward result to the view
- it gets rid of incorrect values right away
- it avoids doing it on every single barcode scanned
| val filteredByType = if (allowedTypes.isEmpty()) { | ||
| barcodes | ||
| } else { | ||
| barcodes.filter { barcode -> |
There was a problem hiding this comment.
could we use Kotlin union?
https://kotlinlang.org/api/core/kotlin-stdlib/kotlin.collections/union.html
DavidBertet
left a comment
DavidBertet
left a comment
There was a problem hiding this comment.
Nice contribution! I wanted to do it for quite some time!
Don't forget to update the doc, so everyone is aware of it. I think it's been requested in the past!
…nd prevent fallback errors
# Conflicts: # android/src/main/java/com/rncamerakit/CKCamera.kt # android/src/newarch/java/com/rncamerakit/CKCameraManager.kt # android/src/paper/java/com/facebook/react/viewmanagers/CKCameraManagerDelegate.java # android/src/paper/java/com/facebook/react/viewmanagers/CKCameraManagerInterface.java
|
@scarlac and @DavidBertet, I would like to express my gratitude for your valuable feedback and suggestions. Thank you, @VishaKsu, for updating the pull request. Thank you all for your patience and valuable contributions. |
4 participants
🎯 Summary
This PR introduces a new prop forbiddenBarcodeTypes, allowing developers to specify an array of barcode types that should be ignored during scanning.
🐛 Problem
In scenarios where multiple barcodes are present in the camera's field of view (e.g., a QR code with advertising data and a product barcode), only the first detected barcode is passed to the onBarcodeRead callback. This often results in missing the desired product barcode, which appears later in the detection sequence.
✅ Solution
The proposed solution adds a new optional prop:
When provided, this prop filters out any detected barcode whose type matches one of the values in the array before invoking the onBarcodeRead callback. This allows developers to skip irrelevant or unwanted barcode types (like advertising QR codes) and wait for the correct barcode to be scanned.
📚 Documentation
If this change is accepted and considered valuable, I'm happy to update the documentation with:
Just let me know!