-
Notifications
You must be signed in to change notification settings - Fork 5
fix: eid-wallet-issues #678
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
Conversation
📝 WalkthroughWalkthroughThe verify page and several auth/app UIs were restructured: layout and spacing adjusted, verification steps consolidated into a single block with spacer, modal footer/buttons simplified to one adaptive action, status rendering cleaned, onboarding redirects added for key-init failures, plus minor presentational tweaks across passport, identity card, toast, settings, login, onboarding, and AuthDrawer components. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
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: 2
🤖 Fix all issues with AI agents
In `@infrastructure/eid-wallet/src/routes/`(auth)/login/+page.svelte:
- Around line 256-261: Update the full-screen overlay div (the element whose
class string is currently "fixed inset-0 flex flex-col items-center gap-3 py-8")
to include vertical centering and a semi-opaque background so the spinner is
centered and underlying UI is obscured; add "justify-center" to center
vertically and a background utility such as "bg-black/50" (and a high stacking
context like "z-50") to the class list to create the proper loading overlay.
In `@infrastructure/eid-wallet/src/routes/`(auth)/onboarding/+page.svelte:
- Around line 313-324: The two external anchor elements that open in a new tab
(the links with visible text "Terms & Conditions" and "Privacy Policy." in the
+page.svelte onboarding template) must include rel="noopener noreferrer" to
mitigate reverse-tabnabbing; update both <a> tags that use target="_blank" so
they include rel="noopener noreferrer" alongside the existing class and target
attributes.
🧹 Nitpick comments (2)
infrastructure/eid-wallet/src/routes/(auth)/login/+page.svelte (1)
86-148: Consider extracting shared post-authentication logic.The eVault health check, public key sync, and deep link processing logic is duplicated between the PIN login flow (lines 86-148) and the biometric login flow (lines 170-232). Extracting this into a shared helper function would reduce ~80 lines of duplication and make future maintenance easier.
Example extraction
async function handlePostAuthFlow(globalState: GlobalState): Promise<{ redirectPath: string | null; showDeletedModal: boolean }> { // Check eVault health try { const vault = await globalState.vaultController.vault; if (vault?.ename) { const healthCheck = await globalState.vaultController.checkHealth(vault.ename); if (!healthCheck.healthy && healthCheck.deleted) { return { redirectPath: null, showDeletedModal: true }; } // Sync public key try { await globalState.vaultController.syncPublicKey(vault.ename); } catch (error) { console.error("Error syncing public key:", error); } } } catch (error) { console.error("Error during eVault health check:", error); } // Check for pending deep link const pendingDeepLink = sessionStorage.getItem("pendingDeepLink"); if (pendingDeepLink) { try { JSON.parse(pendingDeepLink); // validate sessionStorage.setItem("deepLinkData", pendingDeepLink); sessionStorage.removeItem("pendingDeepLink"); return { redirectPath: "/scan-qr", showDeletedModal: false }; } catch (error) { console.error("Error processing pending deep link:", error); sessionStorage.removeItem("pendingDeepLink"); } } return { redirectPath: "/main", showDeletedModal: false }; }infrastructure/eid-wallet/src/lib/ui/Toast/Toast.svelte (1)
15-24: Pre-existing: Fade-out animation never plays.The
{#ifisVisible}block removes the element from the DOM the momentisVisiblebecomesfalse, so theopacity-0class is never applied—the toast disappears instantly rather than fading out.If a fade-out animation is desired, consider separating the rendering condition from the opacity state:
♻️ Suggested approach to enable fade-out
-let isVisible = $state(true); +let shouldRender = $state(true); +let isVisible = $state(true); onMount(() => { const timer = setTimeout(() => { isVisible = false; setTimeout(() => { + shouldRender = false; onClose?.(); }, 300); // Wait for fade out animation }, duration); return () => clearTimeout(timer); });Then change
{#ifisVisible}to{#ifshouldRender}on line 27. This allows the opacity transition to complete before removing the element.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
infrastructure/eid-wallet/src/lib/fragments/IdentityCard/IdentityCard.svelteinfrastructure/eid-wallet/src/lib/ui/Toast/Toast.svelteinfrastructure/eid-wallet/src/routes/(app)/settings/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/e-passport/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/login/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte
🧰 Additional context used
🧠 Learnings (2)
📚 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)/onboarding/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/e-passport/+page.svelteinfrastructure/eid-wallet/src/lib/ui/Toast/Toast.svelteinfrastructure/eid-wallet/src/routes/(auth)/login/+page.svelteinfrastructure/eid-wallet/src/lib/fragments/IdentityCard/IdentityCard.svelteinfrastructure/eid-wallet/src/routes/(app)/settings/+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)/onboarding/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/e-passport/+page.svelteinfrastructure/eid-wallet/src/lib/ui/Toast/Toast.svelteinfrastructure/eid-wallet/src/routes/(auth)/login/+page.svelteinfrastructure/eid-wallet/src/lib/fragments/IdentityCard/IdentityCard.svelteinfrastructure/eid-wallet/src/routes/(app)/settings/+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: build
- GitHub Check: lint
🔇 Additional comments (9)
infrastructure/eid-wallet/src/lib/fragments/IdentityCard/IdentityCard.svelte (3)
46-46: LGTM!The reduced
min-h-[100px]is a reasonable adjustment for card compactness. The conditional background color logic remains intact.
64-64: LGTM!Padding reduction from
p-5top-3is consistent with the UI density improvements across the wallet.
100-102: LGTM!The reduced margin (
mb-1) tightens the eVault header spacing appropriately.infrastructure/eid-wallet/src/routes/(auth)/login/+page.svelte (2)
283-286: LGTM!Minor formatting adjustment with no behavioral change.
293-297: LGTM!Formatting adjustment with no behavioral change.
infrastructure/eid-wallet/src/routes/(app)/settings/+page.svelte (1)
117-131: The component renders a plain HTML<a>tag and correctly supports external URLs. No changes needed.Likely an incorrect or invalid review comment.
infrastructure/eid-wallet/src/routes/(auth)/e-passport/+page.svelte (3)
25-25: Validate safe‑area/edge spacing after padding simplification.Switching to a uniform
p-4can reduce top/bottom breathing room vs. the prior svh/vw padding. Please confirm the layout doesn’t collide with system UI on small/notched screens.
59-61: ButtonAction formatting change looks good.
27-31: No action needed —Herocomponent properly supports thetitleClassesprop.The Hero component exports
titleClassesas an optional string prop in its interface and correctly applies it to the title element. The usage at lines 27-31 and 44 is valid.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
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)/login/+page.svelte:
- Around line 256-263: The loading overlay div (the element with class "fixed
inset-0 flex flex-col items-center justify-center gap-3 py-8") lacks a
background so underlying UI remains visible; update that div's class list to
include a semi-opaque background utility (for example "bg-black/50" or
"bg-white/80") and a high z-index (e.g., "z-50") so the spinner overlay fully
obscures and sits above the PIN input and other content during loading.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/AuthDrawer.svelteinfrastructure/eid-wallet/src/routes/(auth)/login/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (1)
- infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/AuthDrawer.svelte
🧰 Additional context used
🧠 Learnings (2)
📚 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)/onboarding/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/login/+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)/onboarding/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/login/+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 (3)
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte (1)
313-326: External policy links look good.
target="_blank"now includesrel="noopener noreferrer"and the URLs are updated.infrastructure/eid-wallet/src/routes/(auth)/login/+page.svelte (2)
285-286: LGTM!Formatting-only change with no functional impact.
295-297: LGTM!Formatting adjustment for readability; no functional change.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Description of change
eid wallet ui bugs fixed
Issue Number
closes #409
closes #668
Type of change
How the change has been tested
Manual
Change checklist
Summary by CodeRabbit
Bug Fixes
Style
Content
✏️ Tip: You can customize this high-level summary in your review settings.