fix(scan): keep the scanner escapable when the camera is refused (MO-1016) - #1550
Open
HashEngineering wants to merge 3 commits into
Open
fix(scan): keep the scanner escapable when the camera is refused (MO-1016)#1550HashEngineering wants to merge 3 commits into
HashEngineering wants to merge 3 commits into
Conversation
… (MO-1016) When ScanActivity is opened with a circular-reveal transition — every entry point except the main screen and the lock screen's Scan to Send — onCreate sets the content view to alpha 0 over a transparent window and leaves it that way until maybeTriggerSceneTransition() runs. That call sits on the successful camera-open path, so denying the camera (once or permanently), or a device that refuses to hand the camera over, leaves the scanner fully invisible: an all-black screen with no close button behind the warning dialog, and after dismissing it no way out but restarting the wallet. It is only noticeable on devices without a navigation bar, where there is no on-screen Back button to escape with. Both warning dialogs now reveal the content over an opaque background first, and tint the close icon white so it is legible there (it is nearly black, which only works over the camera preview). The transition setup is skipped if the camera was already refused before the first draw, so it cannot hide the content again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
master does not compile as of 74bd90a: - MenuItem.kt imports android.content.res.Configuration twice, which Kotlin rejects as an ambiguous import, so :common fails to build. - MayaAddressInputScreen passes subtitleMaxLines = 1 to ActionItem, which has no such parameter, so :integrations:maya fails to build. ActionItem already renders its subtitle at maxLines = 1, and both call sites also pass subtitleMiddleEllipsis = true (single-line by construction), so the argument was redundant — dropping it preserves the rendering. Unrelated to MO-1016; carried here only because nothing builds without it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The warning dialog was a dead end: one Dismiss button that dropped the user
back onto a scanner that still could not scan. Worse, on a permanent refusal
the system never prompts again, so there was no in-app route back at all —
the user had to know to go find App info themselves.
- Dismissing either warning now leaves the scanner. It has one job and
cannot do it, and every caller offers another way in (typing an address,
pasting one, picking a contact), so hand the user back instead of parking
them on a dead preview. The close button stays as the safety net.
- The permission dialog gained a primary action. After a first refusal that
is Allow, which re-asks in-app; once shouldShowRequestPermissionRationale
reports the system will not prompt again it becomes Settings, opening App
info via the existing openAppSettings() helper. onResume() already opens
the camera on the way back, so granting it there lands the user in a
working scanner with no extra code. This mirrors the location-permission
flow in exploredash.
- The message follows the same split, since telling someone to grant a
permission they can no longer be asked for is a dead end of its own.
- The camera-problem dialog was titled "Camera permission", which it is not.
It now uses scan_camera_problem_dialog_title ("Sorry"), a string that was
already present and unused.
Verified on an API 34 emulator: refuse once -> Allow re-prompts; refuse
again -> copy switches and Settings opens App info; granting there returns
straight to a live preview; Not now closes the scanner.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (2)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe scanner now handles unavailable cameras and denied permissions with fallback UI, permission recovery actions, settings access, and canceled exit behavior. New strings support these dialogs. Unrelated UI cleanup removes an import and allows address subtitles to wrap. ChangesScanner availability flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change restores an accessible exit and permission-recovery flow when camera access is unavailable, while preserving normal scan results and app boundaries; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant ScanActivity
participant AndroidPermissionSystem
participant AppSettings
ScanActivity->>AndroidPermissionSystem: Request camera permission
AndroidPermissionSystem-->>ScanActivity: Permission refused or camera unavailable
ScanActivity->>ScanActivity: Show unavailable-camera UI and warning dialog
ScanActivity->>AndroidPermissionSystem: Request permission again
AndroidPermissionSystem-->>ScanActivity: Permission remains denied
ScanActivity->>AppSettings: Open application settings
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes MO-1016 — no X button on some devices when scan QR code with disabled Camera.
The bug
ScanActivitysupports a circular-reveal open animation. When it is launched withgetTransitionIntent,onCreatesetscontentView.alpha = 0fover a transparent window and leaves it that way untilmaybeTriggerSceneTransition()runs — and that call lives insideopenRunnable, the successful camera-open path. Refuse the camera and it never runs, so the whole scanner stays invisible: an all-black screen with no close button behind the warning dialog, and after dismissing it no way out but restarting the wallet.The main screen was never affected because
WalletFragmentuses the plaingetIntent(changed incidentally in #1385), and neither is the lock screen's Scan to Send. That is what QA meant by "already fixed on the main screen". The five remaining call sites all use the transition and all carry the bug:SendCoinsQrActivityPaymentsPayFragmentAddressInputFragmentMayaAddressInputFragmentDEXRefundAddressFragmentIt is only noticeable on devices without a navigation bar, where there is no on-screen Back button to escape with — hence the report.
So the fix belongs in
ScanActivity, not at the call sites.The changes
1.
fix(scan): keep the close button reachable when the camera is refusedBoth warning dialogs now reveal the content over an opaque background first, and tint the close icon white so it is legible there —
ic_nav_bar_closeis nearly black, which works over the camera preview but not over the empty background that replaces it. The transition setup is skipped if the camera was already refused before the first draw, so it cannot hide the content again.2.
fix: repair the build broken by #1542— unrelated, see note below3.
fix(scan): give the camera-permission dialog somewhere to goThe X is an escape hatch, not a resolution. The dialog was a dead end — one Dismiss button that dropped the user back onto a scanner that still could not scan — and on a permanent refusal the system never prompts again, so there was no in-app route back at all.
shouldShowRequestPermissionRationale: after a first refusal it is Allow, which re-asks in-app; once the system will no longer prompt it becomes Settings, opening App info through the existingContext.openAppSettings().onResume()already opens the camera on the way back, so granting it there lands the user in a working scanner with no extra code. This mirrors the location-permission flow inexploredash.scan_camera_problem_dialog_title("Sorry"), a string already present and unused.Three new English strings:
button_not_now,button_settings,scan_camera_permission_denied_dialog_message.Please look at this separately
masterdoes not compile as of 74bd90a (#1542) —MenuItem.ktimportsConfigurationtwice (ambiguous import,:commonfails), andMayaAddressInputScreenpassessubtitleMaxLines = 1toActionItem, which has no such parameter (:integrations:mayafails). Nothing builds without the repair, so it is carried here as its own commit.ActionItemalready renders its subtitle atmaxLines = 1and both call sites also passsubtitleMiddleEllipsis = true, so the argument was redundant and dropping it preserves the rendering. Worth landing on its own, ahead of this PR.Testing
Verified on an API 34 emulator with gesture navigation (no navigation bar), driving
ScanActivitydirectly with the transition extras.Reproduced the original bug on
masterfirst: after Don't allow → Dismiss, a fully black screen with no X,ScanActivitystill the resumed activity.With the fix:
Left alone
The camera-problem copy still reads "The camera has a problem. You probably need to restart the device.", which sits oddly on a single button that now closes the screen. Rewording needs a translated string and a product opinion, so I left it.
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
UI Improvements