fix/drawers-replaced-with-component #651
Conversation
|
Warning Rate limit exceeded@grv-saini-20 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 4 minutes and 39 seconds before requesting another review. ⌛ 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)
📝 WalkthroughWalkthroughConverts multiple Drawer-based modals in the scan-QR page to inline full-screen fixed panels with improved styling and accessibility. Updates scan lifecycle management with proper cleanup functions. Adds global body background styling and removes redundant teardown logic. Minor configuration cleanup across the codebase. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte:
- Around line 90-92: Remove the async onDestroy block and instead invoke
cancelScan() from the existing onMount cleanup function (the function returned
in the onMount block around the current lines 84–87); locate the onDestroy call
and delete it, then add a call to cancelScan() inside the onMount return cleanup
so cleanup is centralized (call cancelScan() directly from the cleanup function
and do not rely on an awaited async onDestroy callback).
🧹 Nitpick comments (3)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/AuthDrawer.svelte (1)
6-13: Consider migrating to Svelte 5$props()rune for consistency.The parent page (
+page.svelte) uses Svelte 5 runes like$effect(), but this component still uses legacyexport letsyntax. While backward compatible, using$props()would be more consistent with the codebase direction.🔎 Proposed refactor using Svelte 5 $props()
- export let platform: string | null | undefined; - export let hostname: string | null | undefined; - export let scannedContent: string | undefined; - export let isSigningRequest: boolean; - export let authError: string | null | undefined; - export let authLoading: boolean | undefined; - export let onConfirm: () => void; - export let onDecline: () => void; + let { + platform, + hostname, + scannedContent, + isSigningRequest, + authError, + authLoading = false, + onConfirm, + onDecline, + }: { + platform: string | null | undefined; + hostname: string | null | undefined; + scannedContent: string | undefined; + isSigningRequest: boolean; + authError: string | null | undefined; + authLoading: boolean; + onConfirm: () => void; + onDecline: () => void; + } = $props();infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte (2)
94-99: Consider removing or guarding debug logging.This
$effectlogs every change toselectedBlindVoteOption. Debug logging should typically be removed before release or wrapped in a development-mode check.🔎 Suggested guard
$effect(() => { + if (import.meta.env.DEV) { console.log( "🔍 DEBUG: selectedBlindVoteOption changed to:", $selectedBlindVoteOption, ); + } });
113-115:handleAuthDrawerOpenChangeappears unused.This function is defined but never passed to
AuthDrawerin the template (lines 192-201). The refactoredAuthDrawercomponent no longer acceptsonOpenChangeas noted in the AI summary. Consider removing this dead code.🔎 Proposed removal
- function handleAuthDrawerOpenChange(value: boolean) { - setCodeScannedDrawerOpen(value); - }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/AuthDrawer.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 (7)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/AuthDrawer.svelte (3)
16-38: LGTM!The container structure, icon integration, and header text are well-organized. The decorative diagonal lines create a nice visual effect for the QR code icon.
40-76: LGTM!Good use of fallback values for platform name and URL display. The
break-allclass on the URL ensures long URLs don't break the layout. Error display is well-structured with appropriate visual hierarchy.
78-114: LGTM!Button logic correctly handles both error and normal states. The loading state properly disables interaction and provides clear feedback. The conditional helper text for non-signing requests is a good UX touch.
infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte (4)
2-66: LGTM!Imports are well-organized, context retrieval follows standard patterns, and the destructuring of stores and actions provides clean access to the scan logic API.
117-158: LGTM!Handler implementations are consistent:
handleLoggedInDrawerConfirmproperly sequences navigation before restarting scan.handleSigningDrawerDeclinemirrors the auth decline pattern with error branching.handleSigningDrawerOpenChangeappropriately restarts scanning when the drawer closes without success.
161-186: LGTM!The viewfinder SVG and instruction text provide clear visual guidance for the QR scanning interaction.
188-203: Approve with accessibility consideration.The fixed overlay approach works, but unlike the other drawers that use
onOpenChange, this overlay has no escape key handler or backdrop click-to-dismiss functionality. Users must interact with the buttons to close it. If this is intentional for this flow, it's fine, but consider adding keyboard accessibility (e.g., Escape key to trigger decline) for users who expect standard modal behavior.Verify if the escape key behavior should be added for consistency with standard modal/drawer UX patterns.
infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:
- Around line 51-52: The LoggedInDrawer.svelte view renders the platform prop
directly but platform is typed as string | null | undefined; update the
rendering logic around the <p> that uses platform (the "You're now connected to
{platform}" text) to provide a safe fallback when platform is null/undefined
(e.g., use a default label like "an unknown platform" or "your platform") or
conditionally render the whole sentence only when platform is truthy; adjust the
expression that references platform accordingly so the UI never shows a trailing
empty phrase.
🧹 Nitpick comments (3)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.svelte (1)
226-235: Consider usingButton.Actionfor consistency.The blind vote submit button uses a native
<button>element with custom Tailwind classes, while other actions in this component useButton.Action. This creates visual and behavioral inconsistency.Suggested consistency fix
- <button - onclick={onSubmitBlindVote} - disabled={selectedBlindVoteOption === null || - isSubmittingBlindVote} - class="w-full bg-blue-600 text-white py-3 rounded-xl font-bold disabled:bg-gray-300 transition-colors" - > - {isSubmittingBlindVote - ? "Submitting..." - : "Submit Blind Vote"} - </button> + <Button.Action + variant="solid" + class="w-full" + callback={onSubmitBlindVote} + disabled={selectedBlindVoteOption === null || + isSubmittingBlindVote} + > + {isSubmittingBlindVote + ? "Submitting..." + : "Submit Blind Vote"} + </Button.Action>infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte (2)
5-5: Remove unusedonDestroyimport.The
onDestroylifecycle hook is imported but no longer used in this file, as the cleanup logic has been moved into theonMountteardown function.Suggested fix
-import { getContext, onDestroy, onMount } from "svelte"; +import { getContext, onMount } from "svelte";
91-96: Remove debug logging before merging.This
$effectblock appears to be debug logging that should be removed before production.Suggested fix
-$effect(() => { - console.log( - "🔍 DEBUG: selectedBlindVoteOption changed to:", - $selectedBlindVoteOption, - ); -});
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/AuthDrawer.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/RevealDrawer.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.svelte
🧰 Additional context used
🧠 Learnings (1)
📚 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/(app)/scan-qr/components/SigningDrawer.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/RevealDrawer.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/AuthDrawer.svelte
🔇 Additional comments (6)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte (1)
25-76: Clean implementation of the full-page modal pattern.The component correctly replaces the Drawer wrapper with a fixed, full-screen overlay. The internal state synchronization pattern is consistent with other components in this PR, and the structure aligns well with the PR objectives of preventing the camera from remaining visible.
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/AuthDrawer.svelte (1)
30-174: Well-structured modal with proper state handling.The component correctly implements the full-page modal pattern with:
- Proper fallbacks for
platformandhostname- Clear error state display with styled error panel
- Conditional button rendering based on
authErrorstate- Appropriate disabled states tied to
authLoadingThe explicit
isSigningRequest === falsecheck on line 162 correctly distinguishes betweenfalseandundefined/null.infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.svelte (1)
40-268: Complex but well-organized modal with multiple states.The component handles multiple scenarios (signing success, blind voting, regular signing) with clear conditional rendering. The state synchronization pattern is consistent with other drawer components in this PR.
infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte (2)
84-88: Good addition ofcancelScan()to cleanup.Adding
cancelScan()to the teardown function ensures the media stream is properly stopped when navigating away, addressing the issue where the camera remained visible/frozen in the background.
224-228: Verify:has()selector browser support for your target audience.The
:has()CSS pseudo-class is used here to apply a global body background when the logged-in drawer is present. Browser support is now comprehensive: Chrome/Edge (M105+), Safari (15.4+), and Firefox (121+) all support it. If your target audience uses older browsers, consider a fallback approach.infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/RevealDrawer.svelte (1)
30-176: Clean implementation matching the unified modal pattern.The component follows the same structure as other drawer components in this PR:
- Consistent state synchronization pattern
- Proper conditional rendering for success vs. request states
- Good fallback handling for
revealPollId- Action buttons correctly reflect the current state
The success state properly guards against null
revealedVoteDatabefore accessing its properties.
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte (1)
12-22: Consider simplifying the reactive statement pattern.The current approach with
lastReportedOpentracking works correctly, but could be simplified. The pattern is functional and safe, so this is an optional improvement.♻️ Alternative approach using a single reactive statement
- let internalOpen = isOpen; - let lastReportedOpen = internalOpen; + let internalOpen = isOpen; - $: if (isOpen !== internalOpen) { - internalOpen = isOpen; - } - - $: if (internalOpen !== lastReportedOpen) { - lastReportedOpen = internalOpen; - onOpenChange?.(internalOpen); - } + $: { + if (isOpen !== internalOpen) { + internalOpen = isOpen; + } + } + + $: onOpenChange?.(internalOpen);Note: This assumes
onOpenChangeis idempotent. If not, the current pattern is safer.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte
🧰 Additional context used
🧠 Learnings (1)
📚 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/(app)/scan-qr/components/LoggedInDrawer.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 (2)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte (2)
2-10: LGTM! Props and imports are well-structured.The component props are properly typed and the imports are appropriate for the new full-page modal approach.
25-28: Body scroll lock is properly implemented. The scroll prevention is applied via thecustom-global-styleclass on the<body>element in the app layout (+layout.svelte), which appliesoverflow-y: hiddenwhenever the page is on the scan-qr route. This approach prevents background scrolling for all modal drawers on this page, including LoggedInDrawer.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
platforms/pictique/src/routes/(protected)/messages/+page.svelte (1)
189-199: Bug: Group name only includes first member, not all selected members.The filter condition
m.id === selectedMembers[0]only matches the first selected member. For a group chat with multiple members,groupMemberswill contain at most one element, resulting in an incorrect group name.🐛 Proposed fix
} else { // Create group chat - const groupMembers = allMembers.filter((m) => m.id === selectedMembers[0]); + const groupMembers = allMembers.filter((m) => selectedMembers.includes(m.id)); const groupName = groupMembers.map((m) => m.name ?? m.handle ?? m.ename).join(', '); // Create group chat via API await apiClient.post('/api/chats', { name: groupName, participantIds: selectedMembers, isGroup: true }); }
🤖 Fix all issues with AI agents
In @platforms/pictique/src/routes/(protected)/messages/+page.svelte:
- Around line 569-587: The button variants are reversed: in the group creation
footer the Cancel Button uses variant="primary" and the Create Group Button uses
variant="secondary"; update the Button props so Cancel uses the secondary
variant and Create Group uses the primary variant (the Create Group button is
the one invoking createGroup(), disabled when selectedMembers.length === 0 ||
!groupName.trim(), and the Cancel button closes the modal by setting
openNewGroupModal = false) to match the New Chat Modal and standard UX.
- Line 276: The unread prop is being passed inverted (unread={!message.unread})
so the component receives the opposite of the message state; change the prop to
unread={message.unread} wherever it is passed (the prop named unread and the
message object with field message.unread) so the component shows the unread
indicator when message.unread is true.
🧹 Nitpick comments (7)
platforms/pictique/src/routes/(protected)/messages/+page.svelte (6)
7-7: Unused import:clickOutside.The
clickOutsideutility is imported but not used anywhere in this component. Consider removing it to keep the imports clean, or apply it to the modals for better UX (closing on outside click).
45-45: Remove debug console.log statement.Debug logging should be removed before merging to production.
🧹 Proposed fix
- console.log('Raw chat data from API:', data.chats);
42-43: Duplicate API call to/api/users.This endpoint is called here and again in
onMount(line 111). Consider fetching user data once and passing it as a parameter, or caching the result to avoid redundant network requests.
238-241: Consider removing redundant navigation.Since this component is already at
/messages, callinggoto('/messages')beforeloadMessagesmay be unnecessary. If the intent is to refresh the list, just callingloadMessages(1, false)should suffice. If the page needs a full reload, consider usinginvalidateAll()instead.♻️ Proposed simplification
// Navigate to messages and refresh the feed - goto('/messages'); // Refresh the messages to show the newly created group await loadMessages(1, false);
365-368: Unnecessaryreturn;statement.The
return;aftertoggleMemberSelection(member.id)serves no purpose. This pattern also appears at lines 506-509.🧹 Proposed cleanup
onchange={(e: Event) => { toggleMemberSelection(member.id); - return; }}
312-318: Consider improving modal accessibility.The modals lack keyboard handling (Escape to close) and focus trapping. For better accessibility:
- Add
onkeydownhandler to close on Escape key- Consider using the imported
clickOutsideaction to close on backdrop click- Trap focus within the modal while open
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte (1)
33-49: Consider a more maintainable icon decoration approach.The rotated white bars create a nice visual effect, but the implementation relies on hardcoded dimensions (
w-[200px],h-[16px]) and magic numbers that could break if the container size changes. For better maintainability, consider using an SVG mask or a custom icon component.Alternative approach using CSS clip-path
<div - class="flex justify-center mb-4 relative items-center overflow-hidden bg-gray rounded-xl p-4 h-[72px] w-[72px]" + class="flex justify-center mb-4 relative items-center bg-gray rounded-xl p-4 h-[72px] w-[72px] [clip-path:polygon(0_20%,100%_0,100%_80%,0_100%)]" > - <div - class="bg-white h-[16px] w-[200px] -rotate-45 absolute top-1" - ></div> - <div - class="bg-white h-[16px] w-[200px] -rotate-45 absolute bottom-1" - ></div> <HugeiconsIcon size={40} - className="z-10" icon={QrCodeIcon} strokeWidth={1.5} color="var(--color-primary)" /> </div>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelteplatforms/pictique/src/routes/(protected)/group/[id]/+page.svelteplatforms/pictique/src/routes/(protected)/messages/+page.svelte
✅ Files skipped from review due to trivial changes (1)
- platforms/pictique/src/routes/(protected)/group/[id]/+page.svelte
🧰 Additional context used
🧠 Learnings (1)
📚 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/(app)/scan-qr/components/LoggedInDrawer.svelteplatforms/pictique/src/routes/(protected)/messages/+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 (5)
platforms/pictique/src/routes/(protected)/messages/+page.svelte (2)
95-104: LGTM!The pagination guard logic correctly prevents duplicate requests and handles the loading state properly.
165-177: LGTM!The debounced search implementation correctly clears results when input is empty and waits 300ms before triggering the search.
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte (3)
12-22: LGTM: Reactive state management pattern is sound.The two-way binding pattern with
internalOpenandlastReportedOpencorrectly prevents infinite reactive loops while synchronizing external and internal state. The optional chaining ononOpenChange?.(internalOpen)is good defensive programming.
51-64: Content and messaging are clear.The heading, connection confirmation, and conditional redirect message effectively communicate the login status. The fallback text "the platform" (line 53) is acceptable, though you might consider a slightly more specific fallback like "this service" if platform is ever unexpectedly null.
67-75: LGTM: Action button is properly implemented.The button is correctly positioned, uses the appropriate variant, and properly wires the
onConfirmcallback. The simple "Ok" label is clear and appropriate for this confirmation dialog.
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte
Show resolved
Hide resolved
grv-saini-20
force-pushed
the
fix/drawers-replaced-with-component
branch
from
99498f7 to
ed19c8e
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
@infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:
- Line 52: Replace the invalid Tailwind class on the paragraph in the
LoggedInDrawer.svelte component: change the <p> with class "text-black-700" to
use a valid utility such as "text-black" or a gray shade like "text-gray-700"
depending on the intended color; update the class on the <p> element in
LoggedInDrawer.svelte accordingly.
In
@infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.svelte:
- Line 93: Replace the invalid Tailwind utility `text-black-700` in the
SigningDrawer.svelte component with a valid class such as `text-gray-700` (or
`text-gray-900` if you need darker text); update every occurrence of
`text-black-700` in SigningDrawer.svelte to the chosen valid Tailwind class so
the styles compile correctly.
- Around line 48-50: In SigningDrawer.svelte, update the conditional class
string that uses showSigningSuccess to replace the invalid Tailwind utility
'bg-gray' with a valid shade like 'bg-gray-100' so the ternary becomes
'bg-green-100' when showSigningSuccess is true and 'bg-gray-100' when false;
locate the class attribute on the element containing "flex justify-center mb-4
relative items-center overflow-hidden {showSigningSuccess ? 'bg-green-100' :
'bg-gray'} rounded-xl p-4 h-[72px] w-[72px]" and change the fallback value to
'bg-gray-100'.
In @platforms/group-charter-manager/next-env.d.ts:
- Line 3: This auto-generated Next.js file is tracked but should be ignored;
remove it from version control using git rm --cached <file> (e.g., git rm
--cached platforms/group-charter-manager/next-env.d.ts), commit the change, and
ensure the file (and the .next/ directory) remain listed in .gitignore so it
stays untracked going forward.
🧹 Nitpick comments (2)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte (1)
25-78: Consider adding accessibility features for the modal overlay.The blocking modal overlay successfully replaces the drawer per PR objectives, but lacks standard accessibility features:
- ARIA attributes: Add
role="dialog"andaria-modal="true"to the outer div, plusaria-labelledbyreferencing the heading- Focus management: Trap focus within the modal and set initial focus to a logical element (e.g., the "Ok" button or close control)
- Keyboard navigation: Handle the Escape key to close the modal (via
onOpenChange(false))- Screen reader announcements: Ensure the title is properly associated with the dialog
These improvements would align with WCAG 2.1 guidelines for modal dialogs.
Example accessibility enhancements
{#if internalOpen} <div - class="loggedin-drawer fixed inset-0 z-50 bg-white p-4 overflow-y-auto" + class="loggedin-drawer fixed inset-0 z-50 bg-white p-4 overflow-y-auto" + role="dialog" + aria-modal="true" + aria-labelledby="dialog-title" + onkeydown={(e) => { if (e.key === 'Escape') onOpenChange(false); }} > <div class="flex flex-col justify-between min-h-full w-full max-w-md mx-auto" > <div class="flex flex-col items-start pt-2"> <!-- QR icon section --> - <h4 class="text-xl font-bold">You're logged in!</h4> + <h4 id="dialog-title" class="text-xl font-bold">You're logged in!</h4>Consider using a Svelte action or library like
svelte-focus-trapfor proper focus management.infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.svelte (1)
41-269: Consider adding accessibility features for this complex modal.This modal contains form controls and multiple interactive states, making accessibility even more critical than in LoggedInDrawer. Consider adding:
- ARIA attributes:
role="dialog",aria-modal="true",aria-labelledbyreferencing the heading, andaria-describedbyfor the description text- Focus management: Trap focus within the modal, set initial focus appropriately (to first interactive element or to error messages when present)
- Keyboard navigation: Handle Escape key to decline/close
- Radio button group: Add
aria-labelor<legend>to the fieldset for screen reader context- Loading states: Consider
aria-liveannouncements for async operations ("Submitting...", "Signing...")- Error announcements: Use
aria-live="polite"on error containers to announce errors to screen readersExample accessibility enhancements
{#if internalOpen} - <div class="fixed inset-0 z-50 bg-white p-4 overflow-y-auto"> + <div + class="fixed inset-0 z-50 bg-white p-4 overflow-y-auto" + role="dialog" + aria-modal="true" + aria-labelledby="signing-dialog-title" + aria-describedby="signing-dialog-description" + onkeydown={(e) => { if (e.key === 'Escape' && !showSigningSuccess) onDecline(); }} + > <!-- content --> - <h4 + <h4 + id="signing-dialog-title" class="text-xl font-bold {showSigningSuccess ? 'text-green-800' : ''}" > - <p class="text-black-700 mt-1"> + <p id="signing-dialog-description" class="text-gray-700 mt-1"> - <fieldset class="space-y-2"> + <fieldset class="space-y-2" aria-label="Select your vote option">Consider using a focus trap library or Svelte action for comprehensive focus management.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.svelteplatforms/group-charter-manager/next-env.d.ts
🧰 Additional context used
🧠 Learnings (1)
📚 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/(app)/scan-qr/components/LoggedInDrawer.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.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 (7)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte (2)
2-10: LGTM! Clean component API.The imports are well-organized, and the exported props provide a clear, type-safe interface for the component.
12-22: LGTM! State synchronization logic is sound.The bidirectional sync pattern correctly handles external prop changes and notifies parent components via
onOpenChange. ThelastReportedOpenguard prevents redundant callbacks.infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.svelte (5)
2-21: LGTM! Well-structured component interface.The imports are clean, and the extensive prop API provides fine-grained control over the signing flow with proper TypeScript types.
23-38: LGTM! State management is correctly implemented.The synchronization pattern matches LoggedInDrawer and properly handles bidirectional state updates. The
hasPollDetailscomputed property provides a clean way to check for poll-related data.
73-172: LGTM! Complex conditional rendering is well-structured.The title logic correctly handles all combinations of success/request states and voting types. The table structure with conditional rows properly displays relevant signing data based on the request type, with appropriate optional chaining to prevent errors.
174-208: LGTM! Radio button form controls are correctly implemented.The blind voting UI properly implements controlled radio buttons with Svelte 5 syntax, includes error messaging, and maintains accessible form structure with semantic fieldset grouping.
219-266: LGTM! Button state management handles all signing flow states correctly.The conditional button rendering properly handles success, blind voting, and signing states with appropriate loading indicators and disabled states. The logic correctly prevents submission when no option is selected or when already processing.
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte
Outdated
Show resolved
Hide resolved
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.svelte
Outdated
Show resolved
Hide resolved
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @infrastructure/control-panel/src/lib/ui/Table/Table.svelte:
- Line 377: The snippet parameter lost its explicit typing—change the BodyCell
snippet signature back to use a properly parameterized Record so type safety is
preserved (e.g., BodyCell(data: Record<string, TableCell>, field: string, i:
number)); update the snippet declaration that currently uses BodyCell(data:
Record, ...) to the explicit Record<string, TableCell> (or the exact generic
types used by your TableCell definition) so TypeScript enforces the expected
cell shape.
In
@infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:
- Around line 25-78: The modal lacks accessibility: when internalOpen is true,
add role="dialog" and aria-modal="true" to the overlay container, give the
heading element (h4 "You're logged in!") a unique id and set aria-labelledby on
the dialog to that id, implement an Escape key handler (e.g., svelte:window
on:keydown or container on:keydown) that calls the existing onConfirm or close
handler to dismiss the drawer, and add a focus trap so Tab cycles within the
modal and set initial focus to the Ok Button.Action (callback={onConfirm}) when
opened and restore focus when closed (use svelte-focus-trap or manual focus
management).
In
@infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.svelte:
- Around line 173-207: The fieldset in SigningDrawer.svelte wrapping the radio
group lacks a semantic legend; move or duplicate the visible "Select Option"
text into a <legend> inside the fieldset (keeping the same visual styling or
using a visually-hidden class if you want the existing heading to remain
visible) so screen readers get the programmatic group label, and ensure the
legend sits immediately inside the fieldset that contains the loop over
signingData?.pollDetails?.options and the inputs tied to selectedBlindVoteOption
and onBlindVoteOptionChange; no other structural changes to blindVoteError
handling are needed.
🧹 Nitpick comments (2)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte (1)
25-28: Consider preventing body scroll when overlay is open.While the overlay covers the background, users can still scroll the underlying page content. Locking body scroll provides better modal UX and prevents confusion.
🔒 Suggested body scroll lock
Add an effect to toggle body overflow:
let internalOpen = isOpen; let lastReportedOpen = internalOpen; +$effect(() => { + if (internalOpen) { + document.body.style.overflow = 'hidden'; + return () => { + document.body.style.overflow = ''; + }; + } +}); + $: if (isOpen !== internalOpen) { internalOpen = isOpen; }infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.svelte (1)
72-90: Consider extracting title logic to improve readability.The nested ternaries determining the title text work correctly but require careful parsing. Extracting to a derived value would improve maintainability.
♻️ Suggested refactor
Add a derived title value in the script block:
let hasPollDetails = false; $: hasPollDetails = signingData?.pollId !== undefined && signingData?.pollDetails !== undefined; + +$: title = (() => { + if (showSigningSuccess) { + if (isBlindVotingRequest) return "Blind Vote Submitted!"; + if (signingData?.pollId) return "Vote Signed!"; + return "Message Signed!"; + } + if (isBlindVotingRequest) return "Blind Vote Request"; + if (signingData?.pollId) return "Sign Vote Request"; + return "Sign Message Request"; +})();Then simplify the template:
<h4 class="text-xl font-bold {showSigningSuccess ? 'text-green-800' : ''}" > - {#if showSigningSuccess} - {isBlindVotingRequest - ? "Blind Vote Submitted!" - : signingData?.pollId - ? "Vote Signed!" - : "Message Signed!"} - {:else} - {isBlindVotingRequest - ? "Blind Vote Request" - : signingData?.pollId - ? "Sign Vote Request" - : "Sign Message Request"} - {/if} + {title} </h4>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
infrastructure/control-panel/src/lib/ui/Table/Table.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.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/control-panel/src/lib/ui/Table/Table.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.svelte
📚 Learning: 2025-08-08T04:15:35.098Z
Learnt from: sosweetham
Repo: MetaState-Prototype-Project/prototype PR: 279
File: infrastructure/control-panel/src/app.css:0-0
Timestamp: 2025-08-08T04:15:35.098Z
Learning: The MetaState-Prototype-Project uses Tailwind CSS v3, where the theme directive is valid and supported. The theme block should not be changed to :root or layer base in this project.
Applied to files:
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.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/(app)/scan-qr/components/SigningDrawer.svelte (1)
218-265: Button state logic looks good.The conditional action button rendering correctly handles all flow states:
- Success confirmation
- Blind vote submission with proper disabled state
- Error acknowledgment
- Sign/Decline options with loading indicators
The implementation properly coordinates button variants, labels, and callbacks across the different signing scenarios.
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.svelte
Show resolved
Hide resolved
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.svelte
Show resolved
Hide resolved
…State-Prototype-Project/prototype into fix/drawers-replaced-with-component
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.svelte:
- Around line 179-218: Remove the redundant visual label by deleting the
standalone <p> element that renders "Select Option" inside the block gated by
showSigningSuccess/isBlindVotingRequest/hasPollDetails; keep the semantic
<legend> inside the fieldset so the radio group (rendered from
signingData?.pollDetails?.options) remains properly labeled for accessibility,
ensuring the rest of the behavior (selectedBlindVoteOption, onchange calling
onBlindVoteOptionChange, and blindVoteError display) is unchanged.
🧹 Nitpick comments (2)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.svelte (1)
106-177: Consider using description list for semantic HTML.The current table structure works functionally, but since you're displaying key-value pairs (Poll ID → value, Message → value) rather than true tabular data, a description list (
<dl>,<dt>,<dd>) would be more semantically appropriate. This would better convey the content structure to assistive technologies.Example structure:
<dl class="w-full mt-6 border border-gray-100 rounded-2xl overflow-hidden bg-gray-50 divide-y divide-gray-200"> {#if signingData?.pollId} <div class="py-3 px-4"> <dt class="text-xs font-semibold text-gray-500 uppercase tracking-wider"> Poll ID </dt> <dd class="text-sm text-gray-700 font-medium break-all mt-1"> {signingData.pollId} </dd> </div> {/if} <!-- ... other fields ... --> </dl>infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte (1)
58-68: Consider consistent null handling for platform.Line 58 uses a fallback when
platformis null/undefined ({platform ?? "the platform"}), but lines 61-68 only show redirect messaging whenplatformis truthy. This creates inconsistent behavior:
- If
platformis null butredirectexists: description shows "the platform" but no redirect instructions appear- The user experience could be confusing
Consider either:
- Also showing redirect instructions with a fallback platform name, or
- Using the fallback only when redirect is not present
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.svelteplatforms/group-charter-manager/next-env.d.ts
💤 Files with no reviewable changes (1)
- platforms/group-charter-manager/next-env.d.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-08T04:15:35.098Z
Learnt from: sosweetham
Repo: MetaState-Prototype-Project/prototype PR: 279
File: infrastructure/control-panel/src/app.css:0-0
Timestamp: 2025-08-08T04:15:35.098Z
Learning: The MetaState-Prototype-Project uses Tailwind CSS v3, where the theme directive is valid and supported. The theme block should not be changed to :root or layer base in this project.
Applied to files:
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.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/(app)/scan-qr/components/LoggedInDrawer.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.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 (5)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.svelte (2)
40-46: LGTM! Excellent accessibility improvements.The conversion from Drawer to a fixed overlay with proper dialog semantics (
role="dialog",aria-modal="true",aria-labelledby) aligns well with the PR objective to create blocking pages instead of drawer overlays. The ARIA attributes ensure screen reader compatibility.
229-277: LGTM! Button logic and states are well-structured.The conditional button rendering correctly handles all three flows:
- Success: single dismissal action
- Blind voting: proper disabled state when no option selected
- Normal signing: appropriate Decline/Sign actions with loading states
The disabled conditions and dynamic button text ensure good UX throughout the signing workflow.
infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte (3)
1-23: LGTM: Clean state management.The internal state synchronization pattern correctly handles both external prop updates and internal state changes, ensuring
onOpenChangeis called appropriately.
27-30: LGTM: Proper dialog accessibility semantics.The dialog correctly implements ARIA attributes with
role="dialog",aria-modal="true", andaria-labelledbylinkage to the title.
37-37: Remove this comment —bg-grayis a valid custom color.The project uses Tailwind CSS v4 with a custom theme defined in
app.css. The--color-gray: #f5f5f5token in the@themeblock automatically generates thebg-grayutility class. This is intentional and renders correctly. The proposed fix tobg-gray-100would actually break this sincebg-gray-100is not defined in the theme.
Description of change
Login drawers in eid wallet are converted into simple components.
Issue Number
close #427
Type of change
How the change has been tested
Manual
Change checklist
Summary by CodeRabbit
Release Notes
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.