fix: onboarding-flow-drawers #662
Open
Conversation
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
Contributor
📝 WalkthroughWalkthroughReplaces Drawer-based onboarding, registration, and verification UIs with full-screen, state-controlled overlays/modals; moves conditional branches (loading, preVerified, verification steps, biometric/PIN flows, hardware checks) into these overlays; adds SSE lifecycle handling for verification. Exported signatures unchanged. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Modal UI
participant Key as KeyManager
participant SSE as Verification SSE (Server)
participant VerifSvc as Verification Service
User->>UI: Open verification modal
UI->>Key: initialize / prepare key (async)
Key-->>UI: ready
UI->>VerifSvc: start verification (submit doc/selfie)
VerifSvc-->>SSE: open event stream
SSE-->>UI: onopen / stream established
SSE-->>UI: onmessage(status updates)
UI->>User: update status / step / show success or error
User->>UI: close modal
UI->>SSE: closeEventStream
SSE-->>SSE: stream closed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte (1)
45-75: Clean up the test hardware key after verification.The test key created at line 45 (
hardware-test-${Date.now()}) is never deleted after verification completes. Over time, this could accumulate stale test keys in storage.🧹 Suggested fix: Clean up test key after verification
Add cleanup logic after the hardware check succeeds:
// Hardware works! Clean up test key and proceed console.log("Hardware keys are working"); + // Clean up the test key + try { + await globalState.keyService.deleteKey(testKeyId, "onboarding"); + console.log("Test key cleaned up"); + } catch (cleanupError) { + console.warn("Failed to clean up test key:", cleanupError); + } checkingHardware = false;
🤖 Fix all issues with AI agents
In @infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte:
- Around line 341-477: The overlay lacks a way to dismiss it; add a visible
back/close button that calls the component's existing pane-close logic (toggle
the isPaneOpen state or call the same handler used to open/close the pane) so
users can exit onboarding without clicking Next/Continue. Import HugeiconsIcon
from @hugeicons/svelte and render a small icon button in the overlay header
(above the img or in the footer area) that triggers a new onClose function or
directly sets isPaneOpen = false; ensure the button is keyboard accessible and
placed alongside existing actions (handleNext, handlePreVerified,
handleFinalSubmit) so it cleanly closes the overlay and returns users to the
main onboarding page.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: grv-saini-20
Repo: MetaState-Prototype-Project/prototype PR: 651
File: infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:25-83
Timestamp: 2026-01-08T07:01:50.189Z
Learning: The EID wallet (`infrastructure/eid-wallet`) is a mobile application, so keyboard event handlers (like ESC key handlers) are not needed for dialog/modal components.
📚 Learning: 2025-11-14T17:54:47.719Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte
📚 Learning: 2026-01-08T07:01:50.189Z
Learnt from: grv-saini-20
Repo: MetaState-Prototype-Project/prototype PR: 651
File: infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:25-83
Timestamp: 2026-01-08T07:01:50.189Z
Learning: In the EID wallet module (infrastructure/eid-wallet), disable or omit ESC-key handlers for dialog/modal components in mobile contexts. Specifically, for LoggedInDrawer.svelte and similar modals, avoid keyboard dismissal logic unless the platform requires accessibility-compliant behavior; ensure that any modal can still be dismissed via touch/tap interactions or appropriate mobile-native controls.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (2)
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte (2)
158-164: LGTM!The button attribute restructuring maintains the same functionality while improving code formatting and clarity. The
variantandcallbackattributes are properly configured.Also applies to: 185-189
194-312: LGTM!The full-screen overlay structure properly replaces the Drawer component with appropriate dismissal mechanisms. The back buttons (lines 264-266, 297-299) correctly close the overlay by setting
showDrawer = false, and all conditional branches (PIN confirmation, biometrics setup, completion) have proper navigation controls.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte (3)
5-6: Remove unused imports.
DrawerandButtonare imported but not used in this file. The drawer-based UI has been replaced with the custom overlay.🧹 Suggested cleanup
-import { ButtonAction, Drawer, InputPin } from "$lib/ui"; -import * as Button from "$lib/ui/Button"; +import { ButtonAction, InputPin } from "$lib/ui";
199-205: Consider resetting state when closing the overlay.The back button closes the overlay without resetting
isBiometricScreenorisBiometricsAdded. If the overlay is reopened (e.g., if the user re-enters PINs), it would display stale state.♻️ Suggested fix to reset state on close
<button - onclick={() => (showDrawer = false)} + onclick={() => { + showDrawer = false; + isBiometricScreen = false; + isBiometricsAdded = false; + }} class="flex items-center gap-2 text-black-500 mb-6 py-2" >
268-279: Apply consistent state reset for all "Back" buttons in the overlay.The footer Back buttons (lines 272-274 and 305-307) should also reset the overlay state for consistency with the suggested fix above.
♻️ Suggested fix
Consider extracting a helper function to ensure consistent behavior:
<script lang="ts"> // Add near other handlers const closeOverlay = () => { showDrawer = false; isBiometricScreen = false; isBiometricsAdded = false; }; </script>Then use
callback={closeOverlay}for all overlay Back buttons.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (1)
- infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
📚 Learning: 2025-11-14T17:54:47.719Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
📚 Learning: 2026-01-08T07:01:50.189Z
Learnt from: grv-saini-20
Repo: MetaState-Prototype-Project/prototype PR: 651
File: infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:25-83
Timestamp: 2026-01-08T07:01:50.189Z
Learning: In the EID wallet module (infrastructure/eid-wallet), disable or omit ESC-key handlers for dialog/modal components in mobile contexts. Specifically, for LoggedInDrawer.svelte and similar modals, avoid keyboard dismissal logic unless the platform requires accessibility-compliant behavior; ensure that any modal can still be dismissed via touch/tap interactions or appropriate mobile-native controls.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte (4)
134-166: LGTM!The first step PIN entry UI is well-structured with appropriate error handling and navigation.
179-184: Consider whether "Back" should return to the first step.The Back button navigates to
/onboardinginstead of returning to the first PIN entry step. If the intended UX is to let users correct their initial PIN entry, consider resetting tofirstStep = trueinstead.
294-298: Good UX: Biometrics unavailable feedback.Clear messaging when biometrics are unavailable, and the "Set up" button is appropriately disabled.
194-319: Overlay implementation aligns with PR objectives.The conversion from drawer-based UI to full-screen overlay is well-executed. The overlay structure with fixed positioning, scrollable content area, and sticky footer provides a good mobile experience. The conditional rendering for different flow states (PIN success → biometrics setup → completion) is clear.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte:
- Line 402: When closing the Veriff modal (where onclick currently sets
showVeriffModal = false) also reset the verification state by setting verifStep
= 0, clearing status (e.g., status = null or ''), and resetting any document
upload state variables and flags (clear uploaded files, front/back image
variables, progress/error flags, etc.). Replace the inline onclick with or call
a dedicated close handler (e.g., closeVeriffModal) that performs showVeriffModal
= false and the above resets so reopening the modal always starts fresh.
🧹 Nitpick comments (3)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (3)
13-13: Remove unused Drawer import.The
Drawercomponent is no longer used after converting to a full-screen modal. Remove this import to keep the code clean.♻️ Proposed fix
-import Drawer from "$lib/ui/Drawer/Drawer.svelte";
399-407: Add ARIA attributes for modal accessibility.The modal lacks semantic accessibility attributes. Consider adding:
role="dialog"andaria-modal="true"on the container (line 399)aria-label="Close verification"on the back button (line 401)aria-labelledbypointing to a heading element to describe the modal purposeThese attributes help screen readers properly announce the modal and its controls.
♻️ Proposed accessibility improvements
- <div class="fixed inset-0 z-50 bg-white flex flex-col h-full"> + <div + role="dialog" + aria-modal="true" + aria-labelledby="verification-title" + class="fixed inset-0 z-50 bg-white flex flex-col h-full" + > <div class="flex-none px-[5vw] pt-[4svh]"> <button onclick={() => (showVeriffModal = false)} + aria-label="Close verification" class="flex items-center gap-2 text-black-500 py-2" >
399-399: Consider preventing body scroll when modal is open.When the full-screen modal is displayed, the background page can still be scrolled on some devices, which may cause a confusing user experience. Consider adding a body scroll lock when
showVeriffModalis true.Example implementation using Svelte effects:
$effect(() => { if (showVeriffModal) { document.body.style.overflow = 'hidden'; return () => { document.body.style.overflow = ''; }; } });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
📚 Learning: 2026-01-08T07:01:50.189Z
Learnt from: grv-saini-20
Repo: MetaState-Prototype-Project/prototype PR: 651
File: infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:25-83
Timestamp: 2026-01-08T07:01:50.189Z
Learning: In the EID wallet module (infrastructure/eid-wallet), disable or omit ESC-key handlers for dialog/modal components in mobile contexts. Specifically, for LoggedInDrawer.svelte and similar modals, avoid keyboard dismissal logic unless the platform requires accessibility-compliant behavior; ensure that any modal can still be dismissed via touch/tap interactions or appropriate mobile-native controls.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: build
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte:
- Around line 401-406: The back button is icon-only (uses HugeiconsIcon with
ArrowLeft01Icon and toggles showVeriffModal) and lacks accessible labeling; add
an explicit accessible name by adding an aria-label (e.g., aria-label="Back" or
aria-label="Close verification modal") to the button element or include
visually-hidden text inside the button (e.g., a span with class "sr-only") so
screen readers can announce its purpose while preserving the icon UI.
- Around line 398-476: The modal opened when showVeriffModal is true is missing
ARIA and focus management: add role="dialog" and aria-modal="true" to the modal
container div, add aria-labelledby pointing to a unique id (e.g.,
id="verification-title") on the visible <h3> title(s) inside the content
branches for $status, implement a focus trap on the modal container (use a
Svelte action like use:focusTrap or svelte-focus-trap attached to the same
container) to keep keyboard focus inside while open, and restore focus to the
element that opened the modal when showVeriffModal becomes false (store the
opener element before opening and call .focus() on close); ensure any references
use the existing symbols showVeriffModal, the modal container div, the <h3>
title elements, and the close/back handlers (onclick and handleContinue) so the
changes integrate with the current component.
🧹 Nitpick comments (1)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (1)
13-13: Remove unused Drawer import.The Drawer component is no longer used after the refactor to a full-screen modal. Remove this import to clean up the code.
♻️ Suggested fix
-import Drawer from "$lib/ui/Drawer/Drawer.svelte";
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
📚 Learning: 2026-01-08T07:01:50.189Z
Learnt from: grv-saini-20
Repo: MetaState-Prototype-Project/prototype PR: 651
File: infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:25-83
Timestamp: 2026-01-08T07:01:50.189Z
Learning: In the EID wallet module (infrastructure/eid-wallet), disable or omit ESC-key handlers for dialog/modal components in mobile contexts. Specifically, for LoggedInDrawer.svelte and similar modals, avoid keyboard dismissal logic unless the platform requires accessibility-compliant behavior; ensure that any modal can still be dismissed via touch/tap interactions or appropriate mobile-native controls.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (1)
402-402: > Likely an incorrect or invalid review comment.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte:
- Around line 314-336: The duplicate branch trusts websocketData?.w3id without
robust validation; update the check around websocketData and existingW3id to
verify websocketData came from the expected EventSource and that existingW3id is
a non-empty, well-formed identifier before using it, returning a clear error or
retrying if invalid; if valid, proceed to call axios.get with
PUBLIC_REGISTRY_URL and existingW3id and only then update
globalState.vaultController.profileCreationStatus and
globalState.vaultController.vault (ensure response.data.uri exists before
assigning).
- Line 257: handleContinue is currently a no-op until its real implementation is
assigned inside onMount, which can allow a race if the button is clicked early;
change initialization to either set handleContinue to a guarded wrapper that
awaits a ready flag or throws if called before assignment, or add an
"initialized" reactive boolean set true at the end of onMount and bind the
button's disabled state to it (or conditionally render the button) so
handleContinue cannot be invoked until onMount completes; update references to
handleContinue and the button click handler accordingly.
- Around line 150-177: The EventSource opened in watchEventStream is never
closed, leaking resources; modify watchEventStream to save the EventSource
instance to a module-level variable (e.g., currentEventSource), ensure any
existing currentEventSource is closed before creating a new one, add a
closeEventStream() function that calls currentEventSource.close() and nulls it,
and invoke closeEventStream() both when the modal is closed (replace the current
modal-close handler) and inside Svelte's onDestroy to guarantee cleanup on
navigation/unmount.
- Around line 284-365: The hardcoded setTimeout in handleContinue (setTimeout(()
=> { goto("/register"); }, 10_000)) forces a 10s wait; replace it by observing
globalState.vaultController.profileCreationStatus (and/or websocketData) and
call goto("/register") immediately when profileCreationStatus becomes "success"
(or when the resolved w3id/uri is available), optionally implementing a short
timeout fallback/poll with visible user feedback and an explicit "Continue
anyway" action so users can leave early; remove the fixed 10_000ms delay and
ensure navigation only happens once the condition is met.
🧹 Nitpick comments (2)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (2)
112-148: Consider notifying users of verification failures.When verification fails (line 134), the error is only logged to console. Users redirected to onboarding (lines 122, 145) receive no explanation of what went wrong, which could create confusion.
🔔 Proposed enhancement with user notification
Consider adding a toast/notification before redirecting:
} catch (error) { console.error("Failed to start verification:", error); + // Show user-friendly error message + globalState?.notificationService?.showError( + "Verification failed to start. Redirecting to setup..." + ); // If verification fails due to key issues or any initialization error, go back to onboarding const errorMessage =
472-478: Clarify the "Back" button label.The button labeled "Back" (line 477) navigates to
/onboarding(line 475), which is not going back in history but rather abandoning the current verification session. This could confuse users who expect it to return to the previous step.📝 Suggested clarification
- Back + CancelOr if you want to be more explicit:
- Back + Exit Verification
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
📚 Learning: 2026-01-08T07:01:50.189Z
Learnt from: grv-saini-20
Repo: MetaState-Prototype-Project/prototype PR: 651
File: infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:25-83
Timestamp: 2026-01-08T07:01:50.189Z
Learning: In the EID wallet module (infrastructure/eid-wallet), disable or omit ESC-key handlers for dialog/modal components in mobile contexts. Specifically, for LoggedInDrawer.svelte and similar modals, avoid keyboard dismissal logic unless the platform requires accessibility-compliant behavior; ensure that any modal can still be dismissed via touch/tap interactions or appropriate mobile-native controls.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (2)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (2)
381-404: Good progressive disclosure of hardware check status.The template appropriately handles three states: checking hardware (spinner), hardware not supported (error message), and hardware supported (action button). This provides clear feedback to users throughout the initialization process.
406-494: Modal structure successfully replaces drawer UI.The full-screen modal implementation addresses the original issue (#649) by replacing the drawer-based UI that had zoom-related layout problems. The modal uses proper semantic HTML with
role="dialog"andaria-modal, maintains the same verification flow logic, and provides a clean full-screen experience.
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
Outdated
Show resolved
Hide resolved
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
Outdated
Show resolved
Hide resolved
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
Outdated
Show resolved
Hide resolved
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (1)
161-180: Wrap JSON.parse in try/catch to prevent unhandled exceptions.If the server sends malformed data,
JSON.parse(e.data)will throw and crash the message handler. Also, closing the EventSource on any error prevents the browser's built-in reconnection mechanism from working for transient network issues.🐛 Proposed fix
eventSource.onmessage = (e) => { - const data = JSON.parse(e.data as string); - if (!data.status) console.log(data); - console.log("STATUS", data); - status.set(data.status); - reason.set(data.reason); - person = data.person; - document = data.document; - websocketData = data; // Store the full websocket data - if (data.status === "resubmission_requested") { - DocFront.set(null); - DocBack.set(null); - SelfiePic.set(null); + try { + const data = JSON.parse(e.data as string); + if (!data.status) console.log(data); + console.log("STATUS", data); + status.set(data.status); + reason.set(data.reason); + person = data.person; + document = data.document; + websocketData = data; + if (data.status === "resubmission_requested") { + DocFront.set(null); + DocBack.set(null); + SelfiePic.set(null); + } + verifStep.set(3); + } catch (parseError) { + console.error("Failed to parse SSE message:", parseError); } - verifStep.set(3); }; + eventSource.onerror = (error) => { console.error("EventSource error:", error); - eventSource?.close(); + // Only close on terminal errors; browser auto-reconnects for transient failures + if (eventSource?.readyState === EventSource.CLOSED) { + eventSource = null; + } };
🤖 Fix all issues with AI agents
In @infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte:
- Around line 466-472: Replace the awkward heading in the resubmission branch by
changing the text in the {:else if $status === "resubmission_requested"} block
to a grammatically correct sentence like "Your verification failed for the
following reason:" and keep displaying {$reason}; also ensure the UI still
provides navigation when status is "declined" by removing or adjusting the
conditional that currently checks `$status !== "declined"` (so action buttons or
a navigation link are rendered for the "declined" case) — update the template
branches around {$status}, "resubmission_requested", "declined", and {$reason}
to show a clear message and include navigation/actions for declined users.
🧹 Nitpick comments (1)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (1)
411-429: Consider adding focus management for improved accessibility.The modal has good semantic attributes (
role="dialog",aria-modal,aria-label). For better accessibility, consider moving focus to the modal when it opens and returning focus when closed. Since this is a mobile app context (per project learnings, ESC handlers are intentionally omitted), focus trapping may be less critical but programmatic focus management improves screen reader experience.♻️ Optional: Focus management example
<script> let modalRef: HTMLDivElement; $effect(() => { if (showVeriffModal && modalRef) { modalRef.focus(); } }); </script> <!-- In the modal div, add tabindex and bind: --> <div bind:this={modalRef} tabindex="-1" role="dialog" ... >
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
📚 Learning: 2025-04-16T18:04:28.983Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 99
File: infrastructure/w3id/tests/w3id.test.ts:48-57
Timestamp: 2025-04-16T18:04:28.983Z
Learning: In test files, suggestions to add defensive coding patterns (like extra null checks) are often unnecessary, as tests are designed to fail explicitly when expectations aren't met. The test in `infrastructure/w3id/tests/w3id.test.ts` validates that `id.logs` is an instance of `IDLogManager` before accessing its repository property.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
📚 Learning: 2025-06-07T04:59:24.520Z
Learnt from: Sahil2004
Repo: MetaState-Prototype-Project/prototype PR: 193
File: platforms/metagram/src/lib/store/store.svelte.ts:0-0
Timestamp: 2025-06-07T04:59:24.520Z
Learning: In the MetaState prototype project, prefer using centralized type definitions from `$lib/types` over importing interfaces from component files for better type organization and to avoid circular dependencies.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
📚 Learning: 2026-01-08T07:01:50.189Z
Learnt from: grv-saini-20
Repo: MetaState-Prototype-Project/prototype PR: 651
File: infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:25-83
Timestamp: 2026-01-08T07:01:50.189Z
Learning: In the EID wallet module (infrastructure/eid-wallet), disable or omit ESC-key handlers for dialog/modal components in mobile contexts. Specifically, for LoggedInDrawer.svelte and similar modals, avoid keyboard dismissal logic unless the platform requires accessibility-compliant behavior; ensure that any modal can still be dismissed via touch/tap interactions or appropriate mobile-native controls.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (4)
14-16: LGTM!The new imports are correctly added for the modal's back icon and SSE cleanup lifecycle hook.
183-188: LGTM!The cleanup helper properly closes the EventSource and nulls the reference to prevent stale access.
368-371: LGTM!Good addition of
onDestroyto ensure the EventSource connection is properly cleaned up when the component unmounts, preventing resource leaks.
438-444: LGTM!The loading state display provides appropriate feedback during eVault generation, and the subsequent navigation handles the state transition.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description of change
Onboarding UI converted to full page UI.
Issue Number
close #649
Type of change
How the change has been tested
Manual
Change checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.