Skip to content

Conversation

@ShivangMishra
Copy link

@ShivangMishra ShivangMishra commented Sep 2, 2025

In-App Screenshot showing scanned text from a QR code

image

Demo video

https://drive.google.com/file/d/1oDsS0v2u1Uro2liLUSEcog24UN7rTEoh/view?usp=sharing

Summary by CodeRabbit

  • New Features

    • In-form QR scanning: tap a QR field to open a camera scanner with on-screen guidance, processing state, and auto-close.
    • QR scanner used for search criteria and form inputs; scanned values display in Observations with a QR icon.
    • Camera-permission and unavailable-camera messaging with a Close action.
    • Added translations for QR prompts and messages.
  • Chores

    • Added camera/scan dependencies and build/config updates to enable the QR scanning feature.

@coderabbitai
Copy link

coderabbitai bot commented Sep 2, 2025

Walkthrough

Adds QR scanning and QR data-type support across the Android app: new QRFormElement, QRSearchFormElement, QRScanner components, Observations QR rendering, search criteria and action for QR, translations, dependencies (vision-camera, code-scanner, reanimated), Babel/app imports, a gradle property and a patch for the code-scanner module.

Changes

Cohort / File(s) Summary
Build config & patches
packages/openchs-android/android/gradle.properties, packages/openchs-android/patches/vision-camera-code-scanner+0.2.0.patch
Adds VisionCamera_enableCodeScanner=true and inserts Android namespace into the vision-camera-code-scanner patch.
Dependencies & runtime init
packages/openchs-android/package.json, packages/openchs-android/babel.config.js, packages/openchs-android/index.android.js
Adds react-native-vision-camera@2.16.2, vision-camera-code-scanner@0.2.0, react-native-reanimated@^3.5.4; registers reanimated plugin with { globals: ['__scanCodes'] } and imports react-native-reanimated at app entry.
Form element components
packages/openchs-android/src/views/form/formElement/QRFormElement.js, packages/openchs-android/src/views/form/formElement/QRScanner.js, packages/openchs-android/src/views/form/FormElementGroup.js
Adds QRFormElement (tappable QR input, modal scanner, notify/clear value), adds QRScanner (camera permission, back camera, barcode scanning, UI overlay, 30s auto-close, error state), and updates FormElementGroup to render QR form elements.
Search UI & logic
packages/openchs-android/src/views/individual/QRSearchFormElement.js, packages/openchs-android/src/views/individual/IndividualSearchView.js, packages/openchs-android/src/action/individual/IndividualSearchActions.js, packages/openchs-android/src/service/query/IndividualSearchCriteria.js
Adds QRSearchFormElement (search input + scanner modal), renders it in IndividualSearchView, introduces ENTER_QR_CRITERIA action and handler, and extends IndividualSearchCriteria with addQrCriteria and QR-based filter inclusion.
Observations UI
packages/openchs-android/src/views/common/Observations.js
Adds handling for Concept.dataType.QR and renderQRValue(qrValue) to display QR values with an icon in observation rows.
Translations
packages/openchs-android/translations/en.json
Adds keys: tapToScanQR, scanQRCode, pointCameraAtQR, processing, cameraNotAvailableOrPermissionNotGranted.
Minor code fix
packages/openchs-android/src/views/form/formElement/AbstractFormElement.js
Adds View to named imports from native-base to match JSX usage; trailing newline added.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Form as FormElementGroup
  participant QRF as QRFormElement / QRSearchFormElement
  participant QRS as QRScanner
  participant VC as VisionCamera
  participant Store as App Store

  User->>Form: Open form / search
  Form->>QRF: Render QR field
  User->>QRF: Tap QR field (open)
  QRF->>QRS: Open scanner modal

  alt permission & device OK
    QRS->>VC: Start camera & barcode reader
    VC-->>QRS: onBarcodeDetected(code)
    QRS-->>QRF: onRead(code)
    QRF->>Store: dispatchAction(PRIMITIVE_VALUE_CHANGE / ENTER_QR_CRITERIA, payload with value)
    QRF-->>User: Show scanned value
  else permission denied or device missing
    QRS-->>User: Show error UI
    User-->>QRS: Close
    QRS-->>QRF: onRead(null)
  end

  note over QRS: Auto-close after ~30s with null if no scan
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

I twitch my whiskers, chase the light,
A tiny square reveals the sight.
I hop, I scan, a code replies,
Forms fill quick beneath bright eyes.
Carrots ready, scanners bright—hop, scan, delight 🥕📷

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly and concisely summarizes the primary change — implementing an in-app QR scanner and adding QR data type support — which matches the changeset (new QR components, search/filter updates, dependencies, and translations). The issue reference (#1712) and "[DMP 2025]" tag provide context but do not obscure the main intent. Overall it is a specific, single-sentence summary that a reviewer scanning history would understand.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@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.

Actionable comments posted: 5

🧹 Nitpick comments (12)
packages/openchs-android/package.json (1)

116-132: Remove explicit @react-native-community/cli devDependency
React Native 0.72.8 includes its own CLI transitively. Keeping "@react-native-community/cli": "latest" risks unpredictable CI failures as it floats outside of your RN version. Remove this entry from devDependencies. If you do require a direct CLI install, pin to a known‐compatible 11.x release.

-    "@react-native-community/cli": "latest",
packages/openchs-android/src/views/form/formElement/QRScanner.js (6)

31-46: Prefer the built-in permission hook and store the timeout ref

The library provides useCameraPermission which simplifies permission flow, and storing the timeout id in a ref lets you clear it on scan. Status strings ('granted', etc.) are documented. (react-native-vision-camera.com)

Minimal change if keeping the current approach:

-        const timeout = setTimeout(() => {
-            props.onRead(null);
-        }, 30 * 1000);
-
-        return () => clearTimeout(timeout);
+        timeoutRef.current = setTimeout(() => props.onRead(null), 30_000);
+        return () => timeoutRef.current && clearTimeout(timeoutRef.current);

26-30: Remove unused “refresh” state

The refresh flip in this effect doesn’t affect rendering and just triggers an extra setState. Safe to delete.

-    const [refresh, setRefresh] = useState(false);
-    ...
-    useEffect(() => {
-        // exception case for device changes
-        setRefresh(!refresh);
-    }, [device, hasPermission]);
+    // Removed unused refresh state/effect

65-73: Activate Camera only when permission/device are ready

You already guard earlier, but extra safety: set isActive={hasPermission && !!device} to avoid rendering a live camera if state flips.

-            <Camera
+            <Camera
                 codeScanner={codeScanner}
                 style={StyleSheet.absoluteFill}
                 device={device}
-                isActive={true}
+                isActive={hasPermission && !!device}
             />

75-83: Safe areas and accessibility

  • Use SafeAreaView for the header/footer to avoid notches/overlaps on devices with cutouts.
  • Add accessibilityRole="button" and accessible labels to the back/close buttons.

Also applies to: 98-106, 219-231


20-21: Drop console logs in production

Console spam from the scanner callback can be noisy. Remove or guard behind a dev flag.


11-24: Add basic prop type validation

Ensure onRead is provided and a function. If this file isn’t TypeScript, add PropTypes.

+import PropTypes from "prop-types";
 ...
 const QRScanner = (props) => {
 ...
 };
+
+QRScanner.propTypes = {
+  onRead: PropTypes.func.isRequired
+};
packages/openchs-android/translations/en.json (1)

74-75: Add i18n for scanner UI; avoid hardcoded English in QRScanner

QRScanner currently hardcodes several strings (title, instruction, error, close). Add keys here and use them there.

@@
-    "chooseATime": "Choose a time",
-    "tapToScanQR": "Tap to scan QR code",
+    "chooseATime": "Choose a time",
+    "tapToScanQR": "Tap to scan QR code",
+    "scanQRCodeTitle": "Scan QR Code",
+    "pointCameraAtQR": "Point your camera at a QR code",
+    "cameraPermissionError": "Camera not available or permission not granted"
packages/openchs-android/src/views/common/Observations.js (1)

218-223: Prevent layout breakage for long QR payloads

QR values can be lengthy. Truncate in the middle to keep rows compact.

-            <Text style={this.styles.observationPhoneNumber}>{qrValue}</Text>
+            <Text
+              style={this.styles.observationPhoneNumber}
+              numberOfLines={1}
+              ellipsizeMode="middle"
+            >
+              {qrValue}
+            </Text>
packages/openchs-android/src/views/form/formElement/QRFormElement.js (3)

14-20: PropTypes: include optional parentElement and questionGroupIndex

These are dispatched in notifyChange; declare them to aid type checks.

     static propTypes = {
         element: PropTypes.object.isRequired,
         actionName: PropTypes.string.isRequired,
         value: PropTypes.object,
-        validationResult: PropTypes.object
+        validationResult: PropTypes.object,
+        parentElement: PropTypes.object,
+        questionGroupIndex: PropTypes.number
     };

82-101: Accessibility and UX: label tappable row and support long values

Add accessibilityLabel and truncate long values in the middle.

-                    <TouchableNativeFeedback onPress={() => this.openQRScanner()}
+                    <TouchableNativeFeedback
+                        onPress={() => this.openQRScanner()}
+                        accessibilityLabel={this.I18n.t('tapToScanQR')}
                          background={TouchableNativeFeedback.SelectableBackground()}>
                         <View style={styles.qrInputContainer}>
@@
-                            <Text style={[
+                            <Text
+                                numberOfLines={1}
+                                ellipsizeMode="middle"
+                                style={[
                                     styles.qrText,
                                     {
                                         color: _.isNil(this.props.validationResult) ? 
                                             (hasValue ? Colors.InputNormal : Colors.InputBorderNormal) : 
                                             Colors.ValidationError,
                                         fontStyle: hasValue ? 'normal' : 'italic'
                                     }
-                            ]}>
+                                ]}>
                                 {this.displayValue()}
                             </Text>

60-71: Minor: add a11y label to remove button

-                <TouchableNativeFeedback onPress={() => this.removeValue()}
+                <TouchableNativeFeedback
+                    onPress={() => this.removeValue()}
+                    accessibilityLabel={this.I18n.t('delete')}
                     background={TouchableNativeFeedback.SelectableBackgroundBorderless()}
                     useForeground>
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ee9774e and 171d731.

⛔ Files ignored due to path filters (1)
  • packages/openchs-android/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (7)
  • packages/openchs-android/android/gradle.properties (1 hunks)
  • packages/openchs-android/package.json (3 hunks)
  • packages/openchs-android/src/views/common/Observations.js (2 hunks)
  • packages/openchs-android/src/views/form/FormElementGroup.js (2 hunks)
  • packages/openchs-android/src/views/form/formElement/QRFormElement.js (1 hunks)
  • packages/openchs-android/src/views/form/formElement/QRScanner.js (1 hunks)
  • packages/openchs-android/translations/en.json (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/openchs-android/src/views/form/formElement/QRFormElement.js (1)
packages/openchs-android/src/views/form/formElement/QRScanner.js (2)
  • styles (111-232)
  • QRScanner (11-109)
packages/openchs-android/src/views/form/formElement/QRScanner.js (1)
packages/openchs-android/src/views/form/formElement/QRFormElement.js (1)
  • styles (115-136)
🔇 Additional comments (6)
packages/openchs-android/android/gradle.properties (2)

33-33: Correct flag for enabling VisionCamera’s Code Scanner on Android

Setting VisionCamera_enableCodeScanner=true is the right way to bundle MLKit’s barcode model on Android. (react-native-vision-camera.com)


20-34: No action needed: compileSdkVersion and AGP meet VisionCamera v4 requirements
compileSdkVersion = 34 and AGP 8.1.1 are already in use, so Android build toolchain is compatible with VisionCamera v4.

packages/openchs-android/src/views/form/formElement/QRScanner.js (1)

82-96: Confirm scope: do we intend to scan EAN-13 for a “QR” datatype?

If the form datatype is strictly QR, consider restricting to ["qr"] to reduce false positives. Keep EAN-13 only if business requirements need it. Docs show EAN-13 nuances across platforms. (react-native-vision-camera.com, react-native-vision-camera.com)

packages/openchs-android/src/views/common/Observations.js (1)

202-204: QR observation rendering path added — looks good

The new QR branch integrates cleanly with existing render paths.

packages/openchs-android/src/views/form/FormElementGroup.js (2)

45-45: Import for QRFormElement — okay


355-362: Verify support for QR datatype in model and reducer

  • Confirm that Concept.dataType.QR is defined in your openchs-models dependency (no QR values were found in the vendored code).
  • Ensure the generic PRIMITIVE_VALUE_CHANGE handler (ObservationsHolderActions.onPrimitiveObsUpdateValue) correctly processes QRFormElement values—add a test case for QRFormElement if one doesn’t exist.

"react-native-vector-icons": "9.2.0",
"react-native-video": "5.2.1",
"react-native-video-player": "0.12.0",
"react-native-vision-camera": "^4.7.1",
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Blocking: react-native-vision-camera@^4.7.1 is incompatible with react-native 0.72.8

VisionCamera 4.3.2+ requires React Native ≥ 0.73 due to React Native framework API changes. This repo is on RN 0.72.8, so 4.7.1 will not build. Options:

  • Upgrade RN to ≥ 0.73 (preferably 0.74+) and update Android build to compileSdk 34 (and AGP 8.x), or
  • Use a VisionCamera version that supports RN 0.72 (≤ 4.3.1, or v3.x), validating Code Scanner availability/perf on your target devices.

Release notes documenting the RN ≥ 0.73 requirement: “react-native-vision-camera 4.3.2 now requires react-native 0.73 or above.” (download.ddeec.com)

Apply one of these diffs:

A) If upgrading RN later, temporarily pin a compatible VC to unblock this PR’s feature work:

-    "react-native-vision-camera": "^4.7.1",
+    "react-native-vision-camera": "4.3.1",

B) If you plan to upgrade RN now, leave ^4.7.1 but also raise Android compileSdk to 34 and verify AGP/Kotlin; coordinate that change in a separate PR to keep scope clean.

📝 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
"react-native-vision-camera": "^4.7.1",
"react-native-vision-camera": "4.3.1",
🤖 Prompt for AI Agents
In packages/openchs-android/package.json around line 100,
react-native-vision-camera is pinned to ^4.7.1 which requires React Native ≥0.73
and will not build with the repo's RN 0.72.8; either (A) temporarily
downgrade/pin the dependency to a version compatible with RN 0.72 (e.g., 4.3.1
or a 3.x release) to unblock this PR, or (B) if you intend to upgrade RN now,
keep ^4.7.1 but perform the RN upgrade in a separate PR that also raises Android
compileSdk to 34 and updates AGP/Kotlin as needed, then verify build and scanner
functionality.

Comment on lines +39 to +44
onQRRead(qrValue) {
this.setState({ showQRScanner: false });
if (qrValue) {
this.notifyChange(qrValue);
}
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Normalize scanned value; avoid dropping valid “0” and whitespace-only scans

Handle strings safely and ignore empty-after-trim.

-    onQRRead(qrValue) {
-        this.setState({ showQRScanner: false });
-        if (qrValue) {
-            this.notifyChange(qrValue);
-        }
-    }
+    onQRRead(qrValue) {
+        this.setState({ showQRScanner: false });
+        const normalized = _.isString(qrValue) ? qrValue.trim() : qrValue;
+        if (!_.isNil(normalized) && normalized !== '') {
+            this.notifyChange(normalized);
+        }
+    }
📝 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
onQRRead(qrValue) {
this.setState({ showQRScanner: false });
if (qrValue) {
this.notifyChange(qrValue);
}
}
onQRRead(qrValue) {
this.setState({ showQRScanner: false });
const normalized = _.isString(qrValue) ? qrValue.trim() : qrValue;
if (!_.isNil(normalized) && normalized !== '') {
this.notifyChange(normalized);
}
}
🤖 Prompt for AI Agents
In packages/openchs-android/src/views/form/formElement/QRFormElement.js around
lines 39 to 44, the QR handler currently closes the scanner and directly passes
qrValue which can drop valid values like "0" or pass whitespace-only strings;
normalize by converting qrValue to a string, trimming whitespace, and only call
notifyChange when the trimmed string is non-empty (ensure you treat
null/undefined separately so "0" is preserved), then set showQRScanner false as
before.

Comment on lines +104 to +110
<Modal
visible={this.state.showQRScanner}
animationType="slide"
onRequestClose={() => this.onQRRead(null)}>
<QRScanner onRead={(qrValue) => this.onQRRead(qrValue)} />
</Modal>
</View>
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use i18n strings inside QRScanner

QRScanner hardcodes English (“Scan QR Code”, instruction, error, “Close”). Wire the new translation keys via props or context.

I can send a small patch to QRScanner.js replacing literals with this.I18n.t('scanQRCodeTitle' | 'pointCameraAtQR' | 'cameraPermissionError') and reuse existing “closeModal” key. Want me to draft it?

🤖 Prompt for AI Agents
In packages/openchs-android/src/views/form/formElement/QRFormElement.js around
lines 104 to 110, QRScanner is used but currently relies on hardcoded English
strings; pass localized strings into QRScanner via props (e.g., title,
instruction, errorMessage, closeLabel) by calling I18n.t with the existing keys
('scanQRCodeTitle', 'pointCameraAtQR', 'cameraPermissionError', and reuse
'closeModal'), and update the QRScanner invocation to supply those props so the
component renders translated text instead of literals.

Copy link

@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.

Actionable comments posted: 3

♻️ Duplicate comments (1)
packages/openchs-android/src/views/form/formElement/QRScanner.js (1)

35-55: Remove debug logs from UI and localize strings

  • Don’t stringify device lists in user-facing error text.
  • Localize “Camera not available or permission not granted” and “Close”.
-    console.log("QRScanner render - device:", !!device, "hasPermission:", hasPermission, "devices:", devices);
+    // consider gating logs under __DEV__ if needed
@@
-                <Text style={styles.errorText}>
-                    Camera not available or permission not granted
-                    {'\n'}Device: {device ? 'Found' : 'Not found'}
-                    {'\n'}Permission: {hasPermission ? 'Granted' : 'Not granted'}
-                    {'\n'}Devices: {JSON.stringify(Object.keys(devices || {}))}
-                </Text>
+                <Text style={styles.errorText}>
+                    {/* t('qr.cameraUnavailable') */}
+                </Text>
@@
-                    <Text style={styles.closeButtonText}>Close</Text>
+                    <Text style={styles.closeButtonText}>{/* t('common.close') */}</Text>

Also localize the header title and instructions (“Scan QR Code”, “Point your camera at a QR code”).

🧹 Nitpick comments (4)
packages/openchs-android/package.json (2)

100-100: Kotlin/AGP alignment to fix metadata mismatch

Your failure mentions kotlin-stdlib 2.0.0 vs project expecting 1.6. Bump Kotlin in android build to ≥1.8.x (1.9.24 recommended) and ensure AGP/Gradle are compatible with RN 0.72. Example (do this in Android files, not here):

  • In android/build.gradle (or gradle.properties): ext.kotlinVersion = "1.9.24"
  • Ensure AGP ~8.0.x and Gradle 8.x as per RN 0.72 template.

I can generate a patch for android/ to bump Kotlin cleanly if you want me to.


116-116: Avoid “latest” for CLI to prevent breakages

Pin @react-native-community/cli to the version RN 0.72 expects, or remove it and rely on RN’s bundled CLI.

-    "@react-native-community/cli": "latest",
+    "@react-native-community/cli": "11.3.6"

(Verify exact version used by RN 0.72 in your repo; or remove this line entirely.)

packages/openchs-android/src/views/form/formElement/AbstractFormElement.js (1)

6-6: Confirm native-base exports View in v3.4.9

Some native-base versions don’t export View; using RN’s View is safer here since you’re not using themed props.

-import {Text, View} from "native-base";
+import {Text} from "native-base";
+import {View} from "react-native";
packages/openchs-android/src/views/form/formElement/QRScanner.js (1)

13-16: Redundant refresh toggle

This effect flips a state that’s unused elsewhere; drop it.

-    useEffect(() => {
-        // exception case for device changes
-        setRefresh(!refresh);
-    }, [device, hasPermission]);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 171d731 and 46f59d4.

⛔ Files ignored due to path filters (1)
  • packages/openchs-android/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (4)
  • packages/openchs-android/package.json (3 hunks)
  • packages/openchs-android/src/views/form/formElement/AbstractFormElement.js (2 hunks)
  • packages/openchs-android/src/views/form/formElement/QRFormElement.js (1 hunks)
  • packages/openchs-android/src/views/form/formElement/QRScanner.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/openchs-android/src/views/form/formElement/QRFormElement.js
🧰 Additional context used
🧬 Code graph analysis (1)
packages/openchs-android/src/views/form/formElement/QRScanner.js (1)
packages/openchs-android/src/views/form/formElement/QRFormElement.js (1)
  • styles (115-136)

"react-native-vector-icons": "9.2.0",
"react-native-video": "5.2.1",
"react-native-video-player": "0.12.0",
"react-native-vision-camera": "2.16.2",
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Blocking: VisionCamera v2 API mismatch with your QRScanner implementation

QRScanner uses VisionCamera Code Scanner props (onReadCode, codeTypes) that are only available in v4. Your dependency is pinned to 2.16.2, so scanning won’t work. Options:

  • Pin to a RN 0.72–compatible v4 (≤ 4.3.1) and switch to the v4 API (useCodeScanner + codeScanner prop).
  • Or keep 2.x and re-implement scanning via frame processors + a barcode plugin (heavier; not recommended here).

Recommend: upgrade to 4.3.1 and adjust QRScanner accordingly.

Apply:

-    "react-native-vision-camera": "2.16.2",
+    "react-native-vision-camera": "4.3.1",

Follow-up: You’ll also need to change QRScanner to v4 API (see my comment in QRScanner.js). Also ensure VisionCamera_enableCodeScanner=true remains in gradle.properties.

📝 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
"react-native-vision-camera": "2.16.2",
"react-native-vision-camera": "4.3.1",
🤖 Prompt for AI Agents
In packages/openchs-android/package.json around line 100, the project pins
react-native-vision-camera to 2.16.2 which is incompatible with the QRScanner
implementation that expects the v4 Code Scanner API; update the dependency to
"react-native-vision-camera": "4.3.1" (RN 0.72-compatible), run your package
install to update lockfile, then update QRScanner to use the v4 API (replace
onReadCode/codeTypes usage with useCodeScanner and pass the resulting
codeScanner via the codeScanner prop as noted in the QRScanner.js review
comment), and ensure gradle.properties contains
VisionCamera_enableCodeScanner=true before rebuilding.

Comment on lines 57 to 70
const onReadCode = (event) => {
console.log(`QR Code scanned:`, event.nativeEvent.codeStringValue);
props.onRead(event.nativeEvent.codeStringValue);
};

return (
<View style={styles.container}>
<Camera
style={StyleSheet.absoluteFill}
device={device}
isActive={true}
onReadCode={onReadCode}
codeTypes={['qr', 'ean-13']}
/>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

VisionCamera API misuse (v2/v4): replace onReadCode/codeTypes with useCodeScanner + codeScanner

onReadCode is not a VisionCamera v4 API (and doesn’t exist in v2 either). With v4, use useCodeScanner() and pass it via the codeScanner prop. Also gate multiple emissions and clear the auto-close timer to avoid double-callbacks.

Apply:

-import { Camera, useCameraDevices } from "react-native-vision-camera";
+import { Camera, useCameraDevice, useCodeScanner } from "react-native-vision-camera";
@@
-    const devices = useCameraDevices();
-    const device = devices.back;
+    const device = useCameraDevice('back');
+    const scannedRef = React.useRef(false);
+    const timeoutRef = React.useRef(null);
@@
-    const onReadCode = (event) => {
-        console.log(`QR Code scanned:`, event.nativeEvent.codeStringValue);
-        props.onRead(event.nativeEvent.codeStringValue);
-    };
+    const codeScanner = useCodeScanner({
+        codeTypes: ['qr', 'ean-13'],
+        onCodeScanned: (codes) => {
+            const value = codes?.[0]?.value;
+            if (!value || scannedRef.current) return;
+            scannedRef.current = true;
+            if (timeoutRef.current) {
+                clearTimeout(timeoutRef.current);
+                timeoutRef.current = null;
+            }
+            props.onRead(value);
+        }
+    });
@@
-            <Camera
+            <Camera
                 style={StyleSheet.absoluteFill}
-                device={device}
-                isActive={true}
-                onReadCode={onReadCode}
-                codeTypes={['qr', 'ean-13']}
+                device={device}
+                isActive={true}
+                codeScanner={codeScanner}
             />

Also ensure package.json uses VisionCamera 4.3.1 (see my package.json comment).

📝 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
const onReadCode = (event) => {
console.log(`QR Code scanned:`, event.nativeEvent.codeStringValue);
props.onRead(event.nativeEvent.codeStringValue);
};
return (
<View style={styles.container}>
<Camera
style={StyleSheet.absoluteFill}
device={device}
isActive={true}
onReadCode={onReadCode}
codeTypes={['qr', 'ean-13']}
/>
// At top of file, update the import
import { Camera, useCameraDevice, useCodeScanner } from "react-native-vision-camera";
...
// Replace device hook and old onReadCode handler
-const devices = useCameraDevices();
const device = useCameraDevice('back');
const scannedRef = React.useRef(false);
const timeoutRef = React.useRef(null);
const codeScanner = useCodeScanner({
codeTypes: ['qr', 'ean-13'],
onCodeScanned: (codes) => {
const value = codes?.[0]?.value;
if (!value || scannedRef.current) return;
scannedRef.current = true;
if (timeoutRef.current) {
clearTimeout(timeoutRef.current);
timeoutRef.current = null;
}
props.onRead(value);
},
});
...
// In the JSX, swap out onReadCode/codeTypes for the new codeScanner prop
<Camera
style={StyleSheet.absoluteFill}
device={device}
isActive={true}
- onReadCode={onReadCode}
codeScanner={codeScanner}
/>

@ShivangMishra ShivangMishra changed the title Implement QR scanner and add support for QR data type #1712 | [DMP 2025] Implement QR scanner and add support for QR data type Sep 8, 2025
Copy link

@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.

Actionable comments posted: 1

♻️ Duplicate comments (1)
packages/openchs-android/src/views/form/formElement/QRScanner.js (1)

53-71: Localize user-facing strings

Replace hard-coded text with your i18n helper and add missing keys.

+import I18n from "i18n-js";
@@
-                    Camera not available or permission not granted
+                    {I18n.t('camera_not_available_or_permission_not_granted')}
@@
-                    <Text style={styles.closeButtonText}>Close</Text>
+                    <Text style={styles.closeButtonText}>{I18n.t('close')}</Text>
@@
-                <Text style={styles.headerTitle}>Scan QR Code</Text>
+                <Text style={styles.headerTitle}>{I18n.t('scan_qr_code')}</Text>
@@
-                    {isScanning ? "Point your camera at a QR code" : "Processing..."}
+                    {isScanning ? I18n.t('point_camera_at_qr') : I18n.t('processing')}
@@
-                    <Text style={styles.closeButtonText}>Close</Text>
+                    <Text style={styles.closeButtonText}>{I18n.t('close')}</Text>

Also applies to: 90-91, 101-103, 106-114

🧹 Nitpick comments (9)
packages/openchs-android/babel.config.js (2)

13-18: Reanimated plugin placement is correct; drop unused globals unless required

Keep the plugin last (good). If __scanCodes isn’t referenced from a Reanimated worklet, remove the globals option to avoid confusing future readers.

-        [
-            'react-native-reanimated/plugin',
-            {
-                globals: ['__scanCodes'],
-            },
-        ]
+        ['react-native-reanimated/plugin']

11-11: Redundant Babel plugin

@babel/plugin-proposal-object-rest-spread is already included by metro-react-native-babel-preset. Safe to remove.

-        "@babel/plugin-proposal-object-rest-spread",
packages/openchs-android/patches/vision-camera-code-scanner+0.2.0.patch (1)

8-12: Align compileSdk for the scanner module

The module defaults to compileSdkVersion 30. To reduce warnings/mismatches with your app (RN 0.72 typically compiles with 33), set this property to match your app in android/gradle.properties.

+# android/gradle.properties
+VisionCameraCodeScanner_compileSdkVersion=33
packages/openchs-android/package.json (2)

110-111: Pin code-scanner to an exact version to avoid accidental v0.3+ upgrades

Caret may pull a version requiring newer VisionCamera. Recommend exact pin for stability with VC 2.x.

-    "vision-camera-code-scanner": "^0.2.0"
+    "vision-camera-code-scanner": "0.2.0"

118-118: Avoid “latest” in CI-facing tools

@react-native-community/cli: "latest" is risky; pin to the major that matches RN 0.72 (CLI 11.x). Or remove if not needed.

-    "@react-native-community/cli": "latest",
+    "@react-native-community/cli": "11.3.6",
packages/openchs-android/src/views/form/formElement/QRScanner.js (4)

19-23: Remove unused “refresh” state/effect

Forcing a re-render via refresh is unnecessary; React already re-renders when device or hasPermission change.

-    const [refresh, setRefresh] = useState(false);
@@
-    useEffect(() => {
-        // exception case for device changes
-        setRefresh(!refresh);
-    }, [device, hasPermission]);

75-81: Reduce CPU by throttling and disabling frame processing after success

Throttle with frameProcessorFps and stop processing when isScanning is false.

-            <Camera
+            <Camera
                 style={StyleSheet.absoluteFill}
                 device={device}
-                isActive={true}
-                frameProcessor={frameProcessor}
+                isActive={isScanning}
+                frameProcessor={isScanning ? frameProcessor : undefined}
+                frameProcessorFps={5}
             />

15-17: Scope to QR codes only (unless EAN-13 is intentional)

Including EAN_13 may produce false positives for non-QR barcodes. If the feature is “QR data type”, limit to QR.

-    const [frameProcessor, barcodes] = useScanBarcodes([BarcodeFormat.QR_CODE, BarcodeFormat.EAN_13], {
+    const [frameProcessor, barcodes] = useScanBarcodes([BarcodeFormat.QR_CODE], {
         checkInverted: true,
     });

51-51: Remove noisy render-time log

This logs on every render and dumps device info. Keep behind a dev flag or remove.

-    console.log("QRScanner render - device:", !!device, "hasPermission:", hasPermission, "devices:", devices);
+    // console.debug("QRScanner render - device:", !!device, "hasPermission:", hasPermission);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 46f59d4 and 6a274a4.

⛔ Files ignored due to path filters (1)
  • packages/openchs-android/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (5)
  • packages/openchs-android/babel.config.js (1 hunks)
  • packages/openchs-android/index.android.js (1 hunks)
  • packages/openchs-android/package.json (4 hunks)
  • packages/openchs-android/patches/vision-camera-code-scanner+0.2.0.patch (1 hunks)
  • packages/openchs-android/src/views/form/formElement/QRScanner.js (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/openchs-android/src/views/form/formElement/QRScanner.js (1)
packages/openchs-android/src/views/form/formElement/QRFormElement.js (1)
  • styles (115-136)
🔇 Additional comments (5)
packages/openchs-android/patches/vision-camera-code-scanner+0.2.0.patch (1)

8-12: Namespace addition LGTM

Adding namespace "com.visioncameracodescanner" is required for AGP 7+/8 builds. 👍

packages/openchs-android/index.android.js (1)

1-1: Required Reanimated import in entrypoint — good

Importing Reanimated at the very top is correct and prevents obscure runtime errors.

packages/openchs-android/package.json (3)

92-92: Reanimated addition is consistent with Babel/index changes

Version and wiring look fine for RN 0.72.x.


133-133: No action

Line is fine as-is.


101-101: The Gradle inspection failed because JAVA_HOME isn’t pointing at a valid JDK. Please set JAVA_HOME to your Android JDK path and rerun:

cd packages/openchs-android/android

# 1) Who brings kotlin-stdlib?
./gradlew -q :app:dependencyInsight \
  --configuration debugCompileClasspath \
  --dependency org.jetbrains.kotlin:kotlin-stdlib

# 2) Vision Camera deps:
./gradlew -q :react-native-vision-camera:dependencies --configuration debugCompileClasspath

# 3) Reanimated deps:
./gradlew -q :react-native-reanimated:dependencies --configuration debugCompileClasspath

Share the outputs so we can pinpoint which library pulls in kotlin-stdlib 2.0.0.

Comment on lines 33 to 39
// Auto close after 30 seconds if no scan
const timeout = setTimeout(() => {
props.onRead(null);
}, 30 * 1000);

return () => clearTimeout(timeout);
}, []);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Auto-close timer can override a successful scan; gate and clear timer

If a code is scanned, the 30s timeout may still fire and call onRead(null). Also guard against duplicate callbacks.

-import React, { useState, useEffect } from "react";
+import React, { useState, useEffect, useRef } from "react";
@@
-    useEffect(() => {
+    const scannedRef = useRef(false);
+    const timeoutRef = useRef(null);
+
+    useEffect(() => {
         const requestCameraPermission = async () => {
             const permission = await Camera.requestCameraPermission();
             console.log("Camera permission:", permission);
             setHasPermission(permission === "granted" || permission === "authorized");
         };
 
         requestCameraPermission();
 
         // Auto close after 30 seconds if no scan
-        const timeout = setTimeout(() => {
-            props.onRead(null);
-        }, 30 * 1000);
-
-        return () => clearTimeout(timeout);
+        timeoutRef.current = setTimeout(() => {
+            if (!scannedRef.current) props.onRead(null);
+        }, 30 * 1000);
+
+        return () => {
+            if (timeoutRef.current) {
+                clearTimeout(timeoutRef.current);
+                timeoutRef.current = null;
+            }
+        };
     }, []);
@@
-        if (barcodes.length > 0 && isScanning) {
-            const scannedCode = barcodes[0].displayValue || barcodes[0].rawValue;
+        if (barcodes.length > 0 && isScanning) {
+            const scannedCode = barcodes[0].displayValue || barcodes[0].rawValue;
             console.log(`QR Code scanned:`, scannedCode);
-            setIsScanning(false); // Prevent multiple scans
-            props.onRead(scannedCode);
+            if (!scannedCode || scannedRef.current) return;
+            scannedRef.current = true;
+            setIsScanning(false); // Prevent multiple scans
+            if (timeoutRef.current) {
+                clearTimeout(timeoutRef.current);
+                timeoutRef.current = null;
+            }
+            props.onRead(scannedCode);
         }

Also applies to: 41-49

🤖 Prompt for AI Agents
In packages/openchs-android/src/views/form/formElement/QRScanner.js around lines
33-39 (and also apply same fix to 41-49), the 30s auto-close timeout can still
fire after a successful scan and cause props.onRead(null) or duplicate
callbacks; modify the component to track whether a read has already been handled
(e.g., a local handled flag or ref), clear the timeout immediately when a
successful scan occurs, and guard all calls to props.onRead so they only run
when not already handled; ensure the cleanup function clears the timeout and
resets/guards the handled flag to prevent duplicate callbacks.

Copy link

@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.

Actionable comments posted: 2

♻️ Duplicate comments (2)
packages/openchs-android/src/views/form/formElement/QRScanner.js (2)

3-3: Optional: Prefer useCameraDevice('back') with VisionCamera ≥ v3

Future-proof and simplify by using useCameraDevice('back') instead of useCameraDevices(). If you switch, drop the devices references in the error UI as noted above.

-import { Camera, useCameraDevices } from "react-native-vision-camera";
+import { Camera, useCameraDevice } from "react-native-vision-camera";
@@
-    const devices = useCameraDevices();
-    const device = devices.back;
+    const device = useCameraDevice('back');

Also applies to: 15-16


13-15: Guarantee single callback: centralize finish(), clear timer, gate duplicates

There are still paths where the 30s timeout or Close buttons can fire after a successful scan, causing double-callbacks. Centralize exit logic and guard with a ref.

@@
-    const [isScanning, setIsScanning] = useState(true);
-    const [hasScanned, setHasScanned] = useState(false);
+    const [isScanning, setIsScanning] = useState(true);
     const timeoutRef = useRef(null); // Store timeout reference
+    const handledRef = useRef(false); // gate multiple callbacks
@@
+    const finish = (value) => {
+        if (handledRef.current) return;
+        handledRef.current = true;
+        setIsScanning(false);
+        if (timeoutRef.current) {
+            clearTimeout(timeoutRef.current);
+            timeoutRef.current = null;
+        }
+        props.onRead(value ?? null);
+    };
@@
-        timeoutRef.current = setTimeout(() => {
-            if (!hasScanned) {
-                console.log("QRScanner: Auto-close timeout triggered");
-                props.onRead(null);
-            }
-        }, 30 * 1000);
+        timeoutRef.current = setTimeout(() => {
+            if (!handledRef.current) {
+                console.log("QRScanner: Auto-close timeout triggered");
+                finish(null);
+            }
+        }, 30 * 1000);
@@
-        if (barcodes.length > 0 && isScanning && !hasScanned) {
+        if (barcodes.length > 0 && isScanning && !handledRef.current) {
             const scannedCode = barcodes[0].displayValue || barcodes[0].rawValue;
             console.log(`QR Code scanned:`, scannedCode);
@@
-            setIsScanning(false); // Prevent multiple scans
-            setHasScanned(true); // Mark that we've successfully scanned
-            props.onRead(scannedCode);
+            finish(scannedCode?.trim?.() ?? scannedCode);
         }
-    }, [barcodes, isScanning, hasScanned]);
+    }, [barcodes, isScanning]);
@@
-                    onPress={() => props.onRead(null)}
+                    onPress={() => finish(null)}
@@
-                    onPress={() => props.onRead(null)}
+                    onPress={() => finish(null)}
@@
-                    onPress={() => props.onRead(null)}
+                    onPress={() => finish(null)}

Also applies to: 36-50, 52-68, 84-87, 105-110, 127-132

🧹 Nitpick comments (2)
packages/openchs-android/src/views/form/formElement/QRScanner.js (2)

92-100: Pause camera after scan and throttle frame processor FPS

Reduce CPU/battery by stopping capture once we have a result and lowering FP rate.

             <Camera
                 style={StyleSheet.absoluteFill}
                 device={device}
-                isActive={true}
+                isActive={isScanning}
                 frameProcessor={frameProcessor}
+                frameProcessorFps={2}
+                /* onError optional: surface camera errors
+                onError={(e) => { if (__DEV__) console.warn('Camera error', e); }}
+                */
             />

72-90: Don’t expose debug details in end-user error text

“Device/Permission/Devices” diagnostics should be dev-only.

-                <Text style={styles.errorText}>
-                    {I18n.t('cameraNotAvailableOrPermissionNotGranted')}
-                    {'\n'}Device: {device ? 'Found' : 'Not found'}
-                    {'\n'}Permission: {hasPermission ? 'Granted' : 'Not granted'}
-                    {'\n'}Devices: {JSON.stringify(Object.keys(devices || {}))}
-                </Text>
+                <Text style={styles.errorText}>
+                    {I18n.t('cameraNotAvailableOrPermissionNotGranted')}
+                    {__DEV__ ? `\nDevice: ${device ? 'Found' : 'Not found'}\nPermission: ${hasPermission ? 'Granted' : 'Not granted'}` : ''}
+                </Text>
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6a274a4 and 16a7a9b.

📒 Files selected for processing (3)
  • packages/openchs-android/package.json (4 hunks)
  • packages/openchs-android/src/views/form/formElement/QRScanner.js (1 hunks)
  • packages/openchs-android/translations/en.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/openchs-android/translations/en.json
  • packages/openchs-android/package.json
🧰 Additional context used
🧬 Code graph analysis (1)
packages/openchs-android/src/views/form/formElement/QRScanner.js (1)
packages/openchs-android/src/views/form/formElement/QRFormElement.js (1)
  • styles (115-136)
🔇 Additional comments (1)
packages/openchs-android/src/views/form/formElement/QRScanner.js (1)

86-87: Confirm i18n key for “Close”

Ensure the translation key exists (you used I18n.t('CLOSE')). If your keys are lowercased, switch to I18n.t('close').

Would you like me to scan the repo for existing “close/CLOSE” keys and align usage?

Also applies to: 131-132

@@ -0,0 +1,261 @@
import React, { useState, useEffect, useRef } from "react";
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Build unblock: align VisionCamera + scanner approach with Kotlin/AGP versions

Your Android failure (“kotlin.Unit… compiled with 1.9.0, project expects 1.6.0”) indicates dependency drift. Choose one path and align tooling:

  • Path A (recommended): VisionCamera v4 built‑in code scanner

    • Replace vision-camera-code-scanner with useCodeScanner from react-native-vision-camera.
    • Keep VisionCamera_enableCodeScanner=true.
    • Kotlin ≥ 1.9.x; AGP ≥ 8.1; Gradle ≥ 8.2.
  • Path B: Frame-processor plugin (vision-camera-code-scanner)

    • Keep current code; set VisionCamera_enableFrameProcessors=true (not CodeScanner).
    • Kotlin/AGP still need to satisfy the library (often 1.8+ / 1.9+).

Also ensure all kotlin stdlib versions are consistent across transitive deps.

Run to locate and review versions/toggles:


🏁 Script executed:

#!/bin/bash
# Find Kotlin/AGP/Gradle and VisionCamera settings
rg -nP "kotlin.*(version|stdlib)|org.jetbrains.kotlin|com.android.tools.build:gradle|gradle-([\d.]+)-bin|VisionCamera_enable|react-native-vision-camera|vision-camera-code-scanner" -C2

# Show package versions
jq -r '.dependencies, .devDependencies | to_entries[] | select(.key|test("vision-camera|code-scanner|reanimated")) | "\(.key): \(.value)"' package.json 2>/dev/null

# Check gradle.properties for toggles
fd gradle.properties -x sed -n '1,200p' {}

Length of output: 203767


🏁 Script executed:

#!/bin/bash
# Display top 50 lines of the Android root build.gradle to locate kotlinVersion definition
sed -n '1,50p' packages/openchs-android/android/build.gradle

Length of output: 1640


Align Kotlin version and VisionCamera toggles with chosen scanner path

  • If you continue using vision-camera-code-scanner (current setup), update android/build.gradle ext.kotlinVersion to ≥ 1.9.0 and in android/gradle.properties replace
    VisionCamera_enableCodeScanner=true
    with
    VisionCamera_enableFrameProcessors=true
  • To migrate to the built-in scanner (recommended), upgrade react-native-vision-camera to v4+, swap out vision-camera-code-scanner for the useCodeScanner hook, keep VisionCamera_enableCodeScanner=true, and ensure Kotlin ≥ 1.9.x, AGP ≥ 8.1, Gradle ≥ 8.2.
🤖 Prompt for AI Agents
In packages/openchs-android/src/views/form/formElement/QRScanner.js around line
1, the project-level Android config and VisionCamera feature flags are
misaligned with the chosen scanner implementation; if you keep
vision-camera-code-scanner, update android/build.gradle ext.kotlinVersion to at
least 1.9.0 and set android/gradle.properties
VisionCamera_enableFrameProcessors=true (replacing
VisionCamera_enableCodeScanner=true); alternatively, preferred: migrate to
VisionCamera built-in scanner by upgrading react-native-vision-camera to v4+,
remove vision-camera-code-scanner and adopt the useCodeScanner hook, keep
VisionCamera_enableCodeScanner=true, and ensure Android toolchain meets Kotlin
≥1.9.x, AGP ≥8.1 and Gradle ≥8.2.

Comment on lines +22 to +26
useEffect(() => {
// exception case for device changes
setRefresh(!refresh);
}, [device, hasPermission]);

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove refresh flip effect — causes unnecessary renders and can loop

Toggling local state on every device/permission change is redundant and risks a render loop if the device object identity changes frequently. Drop this effect (and the unused refresh state).

Apply:

-    const [refresh, setRefresh] = useState(false);
@@
-    useEffect(() => {
-        // exception case for device changes
-        setRefresh(!refresh);
-    }, [device, hasPermission]);
📝 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
useEffect(() => {
// exception case for device changes
setRefresh(!refresh);
}, [device, hasPermission]);
🤖 Prompt for AI Agents
In packages/openchs-android/src/views/form/formElement/QRScanner.js around lines
22 to 26, remove the useEffect that flips the local refresh state on
device/hasPermission changes and delete the unused refresh state variable (and
its setter) so the component no longer triggers unnecessary re-renders or
potential render loops; also remove any imports or references to refresh
elsewhere in this file and ensure no other logic depended on the refresh toggle.

@ombhardwajj
Copy link
Member

This closes avniproject/avni-product#1712

Copy link

@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.

Actionable comments posted: 0

🧹 Nitpick comments (7)
packages/openchs-android/src/service/query/IndividualSearchCriteria.js (1)

89-92: Normalize at write-time for consistency.

Trim on set to avoid persisting trailing spaces and to keep UI and analytics consistent.

-    addQrCriteria(qrValue) {
-        this.qrValue = qrValue;
-    }
+    addQrCriteria(qrValue) {
+        this.qrValue = _.isString(qrValue) ? qrValue.trim() : qrValue;
+    }
packages/openchs-android/src/views/individual/IndividualSearchView.js (1)

124-128: Gate the QR filter like other filter widgets to respect per-subject-type configurations.

All other inputs are conditionally rendered via filterTypePresent. Consider similar gating to avoid clutter where QR isn’t applicable.

-                            <QRSearchFormElement
-                                actionName={Actions.ENTER_QR_CRITERIA}
-                                value={this.state.searchCriteria.qrValue}
-                                style={Styles.simpleTextFormElement}
-                            />
+                            {(
+                              _.isEmpty(this.customFilterService.getSearchFilterBySubjectType(subjectTypeUUID)) ||
+                              this.customFilterService.filterTypePresent(filterScreenName, CustomFilter.type.SearchAll, subjectTypeUUID)
+                            ) && (
+                              <QRSearchFormElement
+                                actionName={Actions.ENTER_QR_CRITERIA}
+                                value={this.state.searchCriteria.qrValue}
+                                style={Styles.simpleTextFormElement}
+                              />
+                            )}

Do you want this gated behind a dedicated CustomFilter.type.QR if/when added?

packages/openchs-android/src/action/individual/IndividualSearchActions.js (2)

88-92: Trim QR value at the action layer to keep state clean.

Pre-trimming here reduces surprises in analytics and UI echoes.

-    static enterQrCriteria = function (state, action, beans) {
+    static enterQrCriteria = function (state, action, beans) {
         const newState = IndividualSearchActions.clone(state);
-        newState.searchCriteria.addQrCriteria(action.value);
+        const v = _.isString(action.value) ? action.value.trim() : action.value;
+        newState.searchCriteria.addQrCriteria(v);
         return newState;
     };

168-176: Analytics filter naming: confirm acceptability of qrValue.

getSearchFilterTypes will surface qrValue as the filter key. If you prefer user-facing terms, consider mapping to qr before logging.

packages/openchs-android/src/views/individual/QRSearchFormElement.js (3)

43-47: Trim before dispatch and avoid dispatching non-strings verbatim.

-    notifyChange(value) {
-        this.dispatchAction(this.props.actionName, {
-            value: value
-        });
-    }
+    notifyChange(value) {
+        const v = _.isString(value) ? value.trim() : value;
+        this.dispatchAction(this.props.actionName, { value: v });
+    }

57-63: Add accessibility labels to the clear button.

Improves screen-reader UX; aligns with existing a11y patterns.

-                <TouchableNativeFeedback onPress={() => this.removeValue()}
+                <TouchableNativeFeedback
+                    accessibilityRole="button"
+                    accessibilityLabel={this.I18n.t('clear')}
+                    onPress={() => this.removeValue()}
                     background={TouchableNativeFeedback.SelectableBackgroundBorderless()}
                     useForeground>

75-91: Long-value rendering: constrain to one line with ellipsis.

Prevents layout breaks when scanning lengthy QR payloads.

-                            <Text style={[
+                            <Text
+                                numberOfLines={1}
+                                ellipsizeMode="middle"
+                                style={[
                                 styles.qrText,
                                 {
                                     color: hasValue ? Colors.InputNormal : Colors.InputBorderNormal,
                                     fontStyle: hasValue ? 'normal' : 'italic'
                                 }
                             ]}>

Also consider accessibilityLabel on the scan trigger:

-                    <TouchableNativeFeedback onPress={() => this.openQRScanner()}
+                    <TouchableNativeFeedback
+                        accessibilityRole="button"
+                        accessibilityLabel={this.I18n.t('scanQRCode')}
+                        onPress={() => this.openQRScanner()}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 16a7a9b and e5a20f8.

📒 Files selected for processing (4)
  • packages/openchs-android/src/action/individual/IndividualSearchActions.js (3 hunks)
  • packages/openchs-android/src/service/query/IndividualSearchCriteria.js (3 hunks)
  • packages/openchs-android/src/views/individual/IndividualSearchView.js (2 hunks)
  • packages/openchs-android/src/views/individual/QRSearchFormElement.js (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/openchs-android/src/views/individual/QRSearchFormElement.js (1)
packages/openchs-android/src/views/form/formElement/QRScanner.js (1)
  • QRScanner (9-136)
🔇 Additional comments (8)
packages/openchs-android/src/service/query/IndividualSearchCriteria.js (2)

133-133: Clone completeness — good.

qrValue is preserved in clones; aligns with other criteria.


39-42: Escape user-provided QR value before interpolating into query

-        if (!_.isEmpty(this.qrValue)) {
-            let trimmedQrValue = this.qrValue.trim();
-            criteria.push(`(observations.valueJSON contains[c] "${trimmedQrValue}" AND observations.concept.datatype = "QR")`);
-        }
+        if (!_.isEmpty(this.qrValue)) {
+            const trimmedQrValue = this.qrValue.trim().replace(/(["\\])/g, '\\$1');
+            criteria.push(`(observations.valueJSON contains[c] "${trimmedQrValue}" AND observations.concept.datatype = "QR")`);
+        }
  • Verify the persisted datatype literal is exactly "QR" (case-sensitive). If enrolment observations may also store QR, add an OR for enrolments.observations.
packages/openchs-android/src/views/individual/IndividualSearchView.js (2)

31-31: Import looks correct and scoped to Android UI layer.


124-128: Android build failure — align Kotlin Gradle plugin and kotlin-stdlib versions

Automated scan couldn't find Android/Gradle files in the repository, so I couldn't verify versions. Verify and fix:

  • Check: root build.gradle(.kts), android/build.gradle(.kts), android/app/build.gradle(.kts), gradle.properties.
  • Ensure the Kotlin Gradle plugin and kotlin-stdlib use the same version (either upgrade the plugin to match stdlib — e.g. classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:2.0.0" — or pin stdlib to the plugin version).
  • Ensure AGP/JDK compatibility (AGP 8.x + JDK 17 is typical for recent RN + VisionCamera); set compileOptions { sourceCompatibility = 17; targetCompatibility = 17 } and kotlinOptions { jvmTarget = "17" }.
  • After changes run ./gradlew clean build --refresh-dependencies and clear Gradle caches if needed.
packages/openchs-android/src/action/individual/IndividualSearchActions.js (3)

208-208: Action name addition — good.


223-223: Reducer map wiring — good.


201-216: ENTER_QR_CRITERIA — action, handler and translations present.
UI references Actions.ENTER_QR_CRITERIA; constant defined and mapped to IndividualSearchActions.enterQrCriteria; en.json includes QR strings (tapToScanQR, scanQRCode, pointCameraAtQR, processing, cameraNotAvailableOrPermissionNotGranted).
Locations: packages/openchs-android/src/views/individual/IndividualSearchView.js:125; packages/openchs-android/src/action/individual/IndividualSearchActions.js:208,223; packages/openchs-android/translations/en.json:74–78.

packages/openchs-android/src/views/individual/QRSearchFormElement.js (1)

95-100: Modal close behavior — good.

onRequestClose funnels to onQRRead(null); scanner cleans up state appropriately.

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