-
Notifications
You must be signed in to change notification settings - Fork 5
feat: add re request perm logic #685
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a camera-permission manager, a permission dialog UI, and permission-aware flows across scan, passport, and selfie features; updates Tauri mobile capabilities and the app identifier. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as App UI
participant PM as CameraPermissionManager
participant OS as Mobile OS
participant Camera as Camera API
User->>UI: Trigger camera flow (Scan / Passport / Selfie)
UI->>PM: checkAndRequestPermission()
PM->>OS: checkPermissions / requestPermissions (barcode-scanner plugin)
alt OS grants
OS-->>PM: granted
PM-->>UI: true
UI->>Camera: getMainCameraStream / getFrontCameraStream
Camera-->>UI: stream
else OS denies
OS-->>PM: denied
PM-->>UI: false
UI->>UI: show CameraPermissionDialog
opt User opens settings
User->>UI: Open Settings
UI->>PM: openSettings()
PM->>OS: openAppSettings()
end
opt User retries
User->>UI: Retry
UI->>PM: retryPermission()
PM->>OS: checkPermissions
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
coodos
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sosweetham we need to address the same problem in the KYC Pages as well when a user is doing KYC they may reject the camera prompt
There was a problem hiding this 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
🤖 Fix all issues with AI agents
In
`@infrastructure/eid-wallet/src/lib/ui/CameraPermissionDialog/CameraPermissionDialog.svelte`:
- Around line 22-34: The handleSwipe function is currently always passed to
<Drawer> allowing local isOpen toggles even when dismissible is false or
onOpenChange is absent; either remove handleSwipe entirely or guard wiring so
swipe only changes state when dismissible is true and onOpenChange exists.
Update the component to conditionally pass handleSwipe (or make handleSwipe
no-op) based on the dismissible prop and presence of onOpenChange, ensuring you
reference the handleSwipe function, the isOpen variable, the onOpenChange
callback, and the Drawer component when making the change.
In `@infrastructure/eid-wallet/src/lib/utils/cameraPermission.ts`:
- Around line 66-74: The computed isDenied boolean is currently set to
!isGranted which incorrectly treats non-granted states like "prompt" as denied;
change the calculation to isDenied = permissions === "denied" and update the
permissionState.set call to use that value (keeping status, isGranted, isDenied,
isChecking fields) so the stored state accurately represents the denied state;
also update any type/signature for the permission state if necessary and
run/adjust any code or tests that reference permissionState.isDenied (e.g.,
retryPermission) to ensure behavior remains correct.
In `@infrastructure/eid-wallet/src/routes/`(auth)/verify/steps/selfie.svelte:
- Around line 49-51: Make handleOpenSettings() async and after awaiting
openSettings(), re-invoke requestCameraPermission() to re-check and update the
camera permission state when the user returns from system settings; update the
handleOpenSettings() functions in both selfie.svelte (handleOpenSettings) and
passport.svelte (handleOpenSettings) so they await openSettings() and then call
requestCameraPermission() to refresh the UI/permission dialog accordingly.
♻️ Duplicate comments (2)
infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte (2)
87-107: Same guard needed for permission-check errors as in the selfie flow.
111-113: Same missing retry/re-check after opening settings as in the selfie flow.Also applies to: 448-453
| function handleSwipe(value: boolean | undefined) { | ||
| if (value) { | ||
| isOpen = false; | ||
| onOpenChange?.(false); | ||
| } | ||
| } | ||
| </script> | ||
|
|
||
| <Drawer | ||
| isPaneOpen={isOpen} | ||
| handleSwipe={handleSwipe} | ||
| dismissible={false} | ||
| > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
rg -n "handleSwipe|dismissible" -C3 infrastructure/eid-wallet/src/lib/ui/Drawer/Drawer.svelteRepository: MetaState-Prototype-Project/prototype
Length of output: 1076
🏁 Script executed:
rg -n "CupertinoPane|events:" -A5 infrastructure/eid-wallet/src/lib/ui/Drawer/Drawer.svelteRepository: MetaState-Prototype-Project/prototype
Length of output: 167
🏁 Script executed:
rg "new CupertinoPane" -A20 infrastructure/eid-wallet/src/lib/ui/Drawer/Drawer.svelte | head -30Repository: MetaState-Prototype-Project/prototype
Length of output: 648
🌐 Web query:
CupertinoPane library bottomClose dismissible behavior swipe events
💡 Result:
Short summary — how bottomClose / dismissible and swipe events work in CupertinoPane:
- bottomClose: when true, dragging the pane down to the bottom breakpoint will close/dismiss it. [1]
- fastSwipeClose / fastSwipeSensivity: if enabled, a quick (fast) drag downward will close the pane; sensitivity controls how “fast” must be. [1]
- lowerThanBottom / upperThanTop / bottomOffset: control whether pane can be dragged past the bottom/top limits and the offset used for bottom-close behavior. [1]
- preventDismiss(preventFlag): programmatically prevent dismissal (useful together with onWillDismiss to cancel dismiss). [2]
- Drag/touch options that affect swipe behavior: touchAngle, dragBy, dragByCursor, freeMode, showDraggable, simulateTouch, passiveListeners. These affect which gestures trigger drag and how they’re interpreted. [1]
Events you can listen to for swipe/close flows:
- onDragStart, onDrag, onDragEnd — get updates during user drags (use these to track swipe progress). [2]
- onWillDismiss, onDidDismiss — fired before/after a dismiss. Use onWillDismiss together with preventDismiss to cancel. [2]
- onWillPresent, onDidPresent, onBackdropTap, onTransitionStart, onTransitionEnd — additional lifecycle hooks. [2]
Quick usage patterns:
- Enable bottom-close: new CupertinoPane('.pane', { bottomClose: true, fastSwipeClose: true, fastSwipeSensivity: 3 });
- Listen for dismiss/cancel: pane.on('onWillDismiss', (ev) => { /* call pane.preventDismiss(true) to cancel */ });
- Track swipe progress: pane.on('onDrag', (pos) => { /* pos = current pane position */ });
Sources: CupertinoPane docs (settings and options) and Events / Public Methods docs. [1][2]
References
[1] Cupertino Pane settings (bottomClose, fastSwipeClose, touchAngle, etc.).
[2] Cupertino Pane Events & Public Methods (onDragStart/onDrag/onDragEnd, onWillDismiss/onDidDismiss, preventDismiss).
🏁 Script executed:
rg -n "handleDrawerSwipe|addEventListener|on:" infrastructure/eid-wallet/src/lib/ui/Drawer/Drawer.svelteRepository: MetaState-Prototype-Project/prototype
Length of output: 206
🏁 Script executed:
sed -n '30,45p' infrastructure/eid-wallet/src/lib/ui/Drawer/Drawer.svelteRepository: MetaState-Prototype-Project/prototype
Length of output: 448
🏁 Script executed:
rg -n "use:swipe" infrastructure/eid-wallet/src/lib/ui/Drawer/Drawer.svelteRepository: MetaState-Prototype-Project/prototype
Length of output: 97
🏁 Script executed:
sed -n '88,98p' infrastructure/eid-wallet/src/lib/ui/Drawer/Drawer.svelteRepository: MetaState-Prototype-Project/prototype
Length of output: 255
🏁 Script executed:
cat -n infrastructure/eid-wallet/src/lib/ui/CameraPermissionDialog/CameraPermissionDialog.svelte | head -50Repository: MetaState-Prototype-Project/prototype
Length of output: 1896
Remove handleSwipe or guard it based on dismissible and onOpenChange availability.
The use:swipe directive in the Drawer component is unconditional—swipe events fire even when dismissible={false}. In CameraPermissionDialog, this means a user can swipe to set isOpen = false locally while the parent remains unaware (if onOpenChange is undefined), causing state desynchronization and bypassing the permission gate.
Suggested fix
<Drawer
isPaneOpen={isOpen}
- handleSwipe={handleSwipe}
+ handleSwipe={onOpenChange ? handleSwipe : undefined}
dismissible={false}
>Alternatively, only wire handleSwipe when the dialog is actually dismissible:
<Drawer
isPaneOpen={isOpen}
- handleSwipe={handleSwipe}
- dismissible={false}
+ handleSwipe={onOpenChange ? handleSwipe : undefined}
+ dismissible={Boolean(onOpenChange)}
>📝 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.
| function handleSwipe(value: boolean | undefined) { | |
| if (value) { | |
| isOpen = false; | |
| onOpenChange?.(false); | |
| } | |
| } | |
| </script> | |
| <Drawer | |
| isPaneOpen={isOpen} | |
| handleSwipe={handleSwipe} | |
| dismissible={false} | |
| > | |
| function handleSwipe(value: boolean | undefined) { | |
| if (value) { | |
| isOpen = false; | |
| onOpenChange?.(false); | |
| } | |
| } | |
| </script> | |
| <Drawer | |
| isPaneOpen={isOpen} | |
| handleSwipe={onOpenChange ? handleSwipe : undefined} | |
| dismissible={false} | |
| > |
🤖 Prompt for AI Agents
In
`@infrastructure/eid-wallet/src/lib/ui/CameraPermissionDialog/CameraPermissionDialog.svelte`
around lines 22 - 34, The handleSwipe function is currently always passed to
<Drawer> allowing local isOpen toggles even when dismissible is false or
onOpenChange is absent; either remove handleSwipe entirely or guard wiring so
swipe only changes state when dismissible is true and onOpenChange exists.
Update the component to conditionally pass handleSwipe (or make handleSwipe
no-op) based on the dismissible prop and presence of onOpenChange, ensuring you
reference the handleSwipe function, the isOpen variable, the onOpenChange
callback, and the Drawer component when making the change.
| const isGranted = permissions === "granted"; | ||
| const isDenied = !isGranted; | ||
|
|
||
| permissionState.set({ | ||
| status: permissions, | ||
| isDenied, | ||
| isGranted, | ||
| isChecking: false, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n infrastructure/eid-wallet/src/lib/utils/cameraPermission.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 4288
🏁 Script executed:
rg "isDenied" infrastructure/eid-wallet/src --type ts --type tsx -B 2 -A 2Repository: MetaState-Prototype-Project/prototype
Length of output: 167
🏁 Script executed:
rg "isDenied" infrastructure/eid-wallet/src --max-count 20Repository: MetaState-Prototype-Project/prototype
Length of output: 600
🏁 Script executed:
rg "permissionState" infrastructure/eid-wallet/src --max-count 30Repository: MetaState-Prototype-Project/prototype
Length of output: 914
🏁 Script executed:
cat -n infrastructure/eid-wallet/src/routes/\(auth\)/verify/steps/selfie.svelteRepository: MetaState-Prototype-Project/prototype
Length of output: 5982
🏁 Script executed:
cat -n infrastructure/eid-wallet/src/routes/\(auth\)/verify/steps/passport.svelteRepository: MetaState-Prototype-Project/prototype
Length of output: 20862
Fix isDenied to be semantically accurate.
The computed isDenied flag conflates "denied" with other non-granted states ("prompt", null/error). While this doesn't currently cause routing issues (since retryPermission() explicitly checks permissions === "denied"), the flag's inclusion in the state type is misleading. Update it to accurately reflect only the denied state.
Suggested fix
-const isGranted = permissions === "granted";
-const isDenied = !isGranted;
+const isGranted = permissions === "granted";
+const isDenied = permissions === "denied";📝 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.
| const isGranted = permissions === "granted"; | |
| const isDenied = !isGranted; | |
| permissionState.set({ | |
| status: permissions, | |
| isDenied, | |
| isGranted, | |
| isChecking: false, | |
| }); | |
| const isGranted = permissions === "granted"; | |
| const isDenied = permissions === "denied"; | |
| permissionState.set({ | |
| status: permissions, | |
| isDenied, | |
| isGranted, | |
| isChecking: false, | |
| }); |
🤖 Prompt for AI Agents
In `@infrastructure/eid-wallet/src/lib/utils/cameraPermission.ts` around lines 66
- 74, The computed isDenied boolean is currently set to !isGranted which
incorrectly treats non-granted states like "prompt" as denied; change the
calculation to isDenied = permissions === "denied" and update the
permissionState.set call to use that value (keeping status, isGranted, isDenied,
isChecking fields) so the stored state accurately represents the denied state;
also update any type/signature for the permission state if necessary and
run/adjust any code or tests that reference permissionState.isDenied (e.g.,
retryPermission) to ensure behavior remains correct.
| function handleOpenSettings() { | ||
| openSettings(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and read the selfie.svelte file
fd -e "selfie.svelte" infrastructure/Repository: MetaState-Prototype-Project/prototype
Length of output: 218
🏁 Script executed:
# Once found, read the specific lines and surrounding context
cat -n infrastructure/eid-wallet/src/routes/\(auth\)/verify/steps/selfie.svelte | head -160Repository: MetaState-Prototype-Project/prototype
Length of output: 5982
🏁 Script executed:
# Also search for requestCameraPermission to understand how it works
rg "requestCameraPermission" infrastructure/eid-wallet/src/routes/\(auth\)/verify/steps/Repository: MetaState-Prototype-Project/prototype
Length of output: 516
🏁 Script executed:
# Find the createCameraPermissionManager implementation
rg -l "createCameraPermissionManager" infrastructure/eid-wallet/Repository: MetaState-Prototype-Project/prototype
Length of output: 284
🏁 Script executed:
# Also check passport.svelte for the second location mentioned
fd -e "passport.svelte" infrastructure/eid-wallet/src/routes/Repository: MetaState-Prototype-Project/prototype
Length of output: 534
🏁 Script executed:
cat -n infrastructure/eid-wallet/src/lib/utils/cameraPermission.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 4288
🏁 Script executed:
cat -n infrastructure/eid-wallet/src/routes/\(auth\)/verify/steps/passport.svelte | head -160Repository: MetaState-Prototype-Project/prototype
Length of output: 6327
🏁 Script executed:
# Check if retryPermission is used anywhere in the codebase
rg "retryPermission" infrastructure/eid-wallet/Repository: MetaState-Prototype-Project/prototype
Length of output: 774
🏁 Script executed:
# Let's see what handleOpenSettings is supposed to do more carefully
# and check if there are other similar permission flows
rg -B 3 -A 3 "handleOpenSettings" infrastructure/eid-wallet/src/routes/Repository: MetaState-Prototype-Project/prototype
Length of output: 5839
Re-check camera permission after opening system settings.
After openSettings() completes and the user returns from system settings with permission granted, the UI will not update because requestCameraPermission() is never re-invoked. The permission dialog will persist even though the user granted access.
Make handleOpenSettings() async and call requestCameraPermission() after the settings are opened to validate the new permission state:
🛠️ Suggested fix
-function handleOpenSettings() {
- openSettings();
-}
+async function handleOpenSettings() {
+ await openSettings();
+ await requestCameraPermission();
+}This applies to both selfie.svelte (lines 49-51) and passport.svelte (lines 111-113).
📝 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.
| function handleOpenSettings() { | |
| openSettings(); | |
| } | |
| async function handleOpenSettings() { | |
| await openSettings(); | |
| await requestCameraPermission(); | |
| } |
🤖 Prompt for AI Agents
In `@infrastructure/eid-wallet/src/routes/`(auth)/verify/steps/selfie.svelte
around lines 49 - 51, Make handleOpenSettings() async and after awaiting
openSettings(), re-invoke requestCameraPermission() to re-check and update the
camera permission state when the user returns from system settings; update the
handleOpenSettings() functions in both selfie.svelte (handleOpenSettings) and
passport.svelte (handleOpenSettings) so they await openSettings() and then call
requestCameraPermission() to refresh the UI/permission dialog accordingly.
There was a problem hiding this 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
🤖 Fix all issues with AI agents
In `@infrastructure/eid-wallet/src/routes/`(auth)/verify/steps/passport.svelte:
- Around line 88-105: The call to checkAndRequestPermission() in
requestCameraPermission can throw and currently isn't caught; wrap the call in a
try/catch inside requestCameraPermission (before attempting getMainCameraStream)
so any thrown error will set permissionGranted.set(false), set
showPermissionDialog = true (and optionally log the error) and return,
preventing the function from rejecting and leaving the UI broken; keep the
existing try/catch around getMainCameraStream unchanged and ensure
requestCameraPermission handles both permission rejections and thrown errors
from checkAndRequestPermission().
| async function requestCameraPermission() { | ||
| // First check native permissions via Tauri | ||
| const hasPermission = await checkAndRequestPermission(); | ||
| if (!hasPermission) { | ||
| permissionGranted.set(false); | ||
| showPermissionDialog = true; | ||
| return; | ||
| } | ||
| // Now get the camera stream | ||
| try { | ||
| await ensureCameraPermission(); | ||
| stream = await getMainCameraStream(); | ||
| video.srcObject = stream; | ||
| video.play(); | ||
| permissionGranted.set(true); | ||
| showPermissionDialog = false; | ||
| } catch (err) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard permission checks against thrown errors (avoid unhandled rejection).
If checkAndRequestPermission() throws, requestCameraPermission() rejects and onMount won’t handle it, leaving the UI in a broken state. Catch and fall back to the dialog.
🛠️ Proposed fix
async function requestCameraPermission() {
// First check native permissions via Tauri
- const hasPermission = await checkAndRequestPermission();
+ let hasPermission = false;
+ try {
+ hasPermission = await checkAndRequestPermission();
+ } catch (err) {
+ console.error("Failed to check camera permission:", err);
+ permissionGranted.set(false);
+ showPermissionDialog = true;
+ return;
+ }
if (!hasPermission) {
permissionGranted.set(false);
showPermissionDialog = true;
return;🤖 Prompt for AI Agents
In `@infrastructure/eid-wallet/src/routes/`(auth)/verify/steps/passport.svelte
around lines 88 - 105, The call to checkAndRequestPermission() in
requestCameraPermission can throw and currently isn't caught; wrap the call in a
try/catch inside requestCameraPermission (before attempting getMainCameraStream)
so any thrown error will set permissionGranted.set(false), set
showPermissionDialog = true (and optionally log the error) and return,
preventing the function from rejecting and leaving the UI broken; keep the
existing try/catch around getMainCameraStream unchanged and ensure
requestCameraPermission handles both permission rejections and thrown errors
from checkAndRequestPermission().
…thub.com:MetaState-Prototype-Project/prototype into fix(eid-w)/reprompt-for-missing-camera-perms
Description of change
adds a go back button and a open settings button to open the page to allow camera permissions on scan page fallback which is used when the perm is not given
Issue Number
Closes #637
Type of change
How the change has been tested
Manual
Change checklist
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.