Skip to content

refactor(qr-scanner): improve camera permission handling in AnimatedQRScannerModal#25936

Merged
Akaryatrh merged 1 commit intomainfrom
fix/20442-QR-error-before-enabling-camera
Feb 13, 2026
Merged

refactor(qr-scanner): improve camera permission handling in AnimatedQRScannerModal#25936
Akaryatrh merged 1 commit intomainfrom
fix/20442-QR-error-before-enabling-camera

Conversation

@dawnseeker8
Copy link
Copy Markdown
Contributor

@dawnseeker8 dawnseeker8 commented Feb 11, 2026 •
edited by cursor bot
Loading

This PR fix #20442

This PR has improved camera permission handling in AnimatedQRScannerModal

  • Updated the camera permission request logic to ensure onScanError is only called after the permission request resolves with denial.
  • Added tests to verify that onScanError is not called when permission is granted or already available.
  • Enhanced cleanup logic in the effect to prevent state updates on unmounted components.

This change improves the reliability of camera permission handling and enhances the user experience by preventing unnecessary error calls.

Description

Changelog

CHANGELOG entry: Improved camera permission handling in AnimatedQRScannerModal

Related issues

Fixes: #20442

Manual testing steps

Feature: my feature name

  Scenario: user [verb for user action]
    Given [describe expected initial app state]

    When user [verb for user action]
    Then [describe expected outcome]

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Low Risk
Small, localized change to permission-request side effects plus test updates; low risk aside from potential behavior change in when/if permission errors are surfaced.

Overview
Fixes camera permission error handling in AnimatedQRScannerModal so onScanError is triggered only after requestPermission() resolves with a denial, and adds effect cleanup to avoid firing after unmount.

Updates unit tests to assert requestPermission is called only when the modal is visible, and to cover the granted/already-granted paths where onScanError must not fire.

Written by Cursor Bugbot for commit 6dcc997. This will update automatically on new commits. Configure here.

@dawnseeker8
refactor(qr-scanner): improve camera permission handling in AnimatedQ…
6dcc997 
…RScannerModal

- Updated the camera permission request logic to ensure `onScanError` is only called after the permission request resolves with denial.
- Added tests to verify that `onScanError` is not called when permission is granted or already available.
- Enhanced cleanup logic in the effect to prevent state updates on unmounted components.

This change improves the reliability of camera permission handling and enhances the user experience by preventing unnecessary error calls.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Feb 11, 2026

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot metamaskbot added the team-accounts-framework Accounts team label Feb 11, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Feb 11, 2026

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeAccounts
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: low
  • AI Confidence: 85%
click to see 🤖 AI reasoning details

E2E Test Selection:
The PR fixes a race condition in the AnimatedQRScanner component's camera permission handling. Previously, there were two separate useEffect hooks - one requesting permission and another immediately checking hasPermission and calling onScanError. This caused premature error triggering before the permission request could complete.

The fix consolidates the logic into a single useEffect that properly awaits the requestPermission() promise and only calls onScanError if permission is actually denied.

The AnimatedQRScannerModal component is used in:

  1. ConnectQRHardware - For adding QR-based hardware wallet accounts
  2. QRSigningDetails - For signing with QR hardware wallets
  3. qr-info.tsx (confirmations) - For QR signing in confirmations

SmokeAccounts is selected because it covers "adding QR-based hardware wallet accounts" which directly uses this component via ConnectQRHardware.

While the component is also used in confirmations (QRSigningDetails, qr-info.tsx), the SmokeConfirmations tests focus on standard transaction/signature flows rather than QR hardware wallet signing specifically. The primary user-facing impact is in the QR hardware wallet connection flow covered by SmokeAccounts.

This is a low-risk bug fix with good test coverage updates. The change is isolated to camera permission handling logic and doesn't modify the core QR scanning or signing functionality.

Performance Test Selection:
This change is a bug fix for camera permission handling in the QR scanner component. It does not affect UI rendering performance, data loading, state management, or any critical performance paths. The change is purely about fixing a race condition in permission request logic and has no performance implications.

View GitHub Actions results

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Feb 11, 2026

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@github-project-automation github-project-automation bot moved this to Needs dev review in PR review queue Feb 12, 2026
@nikolastoimenovski-consensys
Copy link
Copy Markdown

nikolastoimenovski-consensys commented Feb 13, 2026

Tested on Android and iOS, the original issue is fixed. Adding the "qa passed" label.

@nikolastoimenovski-consensys nikolastoimenovski-consensys added the QA Passed QA testing has been completed and passed label Feb 13, 2026
@github-project-automation github-project-automation bot moved this from Needs dev review to Review finalised - Ready to be merged in PR review queue Feb 13, 2026
@Akaryatrh Akaryatrh added this pull request to the merge queue Feb 13, 2026
Merged via the queue into main with commit ccaa928 Feb 13, 2026
110 checks passed
@Akaryatrh Akaryatrh deleted the fix/20442-QR-error-before-enabling-camera branch February 13, 2026 09:57
@github-actions github-actions bot locked and limited conversation to collaborators Feb 13, 2026
@metamaskbot metamaskbot added the release-7.67.0 Issue or pull request that will be included in release 7.67.0 label Feb 13, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Akaryatrh Akaryatrh Akaryatrh approved these changes

Assignees

No one assigned

Labels

QA Passed QA testing has been completed and passed release-7.67.0 Issue or pull request that will be included in release 7.67.0 size-S team-accounts-framework Accounts team

Projects

PR review queue
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

(mob) [Bug] QR error thrown for permissions before enabling camera permission

4 participants

@dawnseeker8 @nikolastoimenovski-consensys @Akaryatrh @metamaskbot