-
Notifications
You must be signed in to change notification settings - Fork 5
fix: adjust bottom button height #754
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
📝 WalkthroughWalkthroughUI-only adjustments across the eID Wallet Svelte routes: bottom safe-area and padding updated, Scan QR button repositioned and resized, and several pages' bottom padding increased; no exported API or backend logic changes detected. Changes
Sequence Diagram(s)(omitted — changes are presentational UI adjustments and do not introduce new multi-component control flow requiring a sequence diagram) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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: 7
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/(app)/settings/pin/+page.svelte (1)
64-64:⚠️ Potential issue | 🟡 MinorTypo in user-facing text.
"Enter you current PIN" should be "Enter your current PIN".
✏️ Proposed fix
- <p class="mb-[1svh]">Enter you current PIN</p> + <p class="mb-[1svh]">Enter your current PIN</p>
🤖 Fix all issues with AI agents
In `@infrastructure/eid-wallet/src/routes/`(app)/main/+page.svelte:
- Around line 60-74: Move the getContext invocation into the onMount callback
and guard against null so globalState is only accessed after the root layout
initializes: remove the top-level const globalState = getContext<() =>
GlobalState>("globalState")();, call getContext<() =>
GlobalState>("globalState")() inside onMount before using globalState, and add a
null/undefined check before accessing globalState.userController,
globalState.vaultController and assigning profileCreationStatus so you avoid
runtime errors when GlobalState.create() hasn’t run yet.
In `@infrastructure/eid-wallet/src/routes/`(app)/settings/pin/+page.svelte:
- Around line 23-27: The function handleChangePIN has inconsistent indentation
for the if block which likely breaks formatting checks; open the handleChangePIN
function and re-indent so the if (newPin.length < 4 || repeatPin.length < 4 ||
currentPin.length < 4) { ... } block is aligned with the rest of the function
body, ensuring lines that set isError = true; and return; are indented one level
inside that if; check surrounding braces in handleChangePIN to confirm
consistent spacing and run the formatter to validate (references: function
handleChangePIN, variables newPin, repeatPin, currentPin, isError).
In `@infrastructure/eid-wallet/src/routes/`(auth)/register/+page.svelte:
- Around line 51-63: The mis-indentation around the early-return and try block
causes Biome format failure; adjust the indentation so the console.error line
inside the if (!globalState) block and the await line inside the try block align
with their surrounding block statements. Locate the if (!globalState) block and
ensure console.error("Global state not available; cannot set onboarding PIN.");,
isError = true;, currentStep = "CREATE";, pin = "";, repeatPin = ""; remain
uniformly indented, and inside the try block make await
globalState?.securityController.setOnboardingPin(pin, repeatPin); line match the
indentation level of isError = false; and currentStep = "PIN_DONE"; to restore
consistent formatting.
- Around line 93-96: In onMount, don't call the getContext factory directly;
first retrieve the factory via getContext<() => GlobalState>("globalState") into
a local variable (e.g., globalStateFactory), check that this factory is
defined/non-null, then invoke it to set globalState; update the code paths that
use globalState accordingly so you only call the factory when it exists
(matching the parent auth layout pattern) and handle the undefined case
defensively (e.g., skip initialization or set a safe default).
In `@infrastructure/eid-wallet/src/routes/`(auth)/review/+page.svelte:
- Around line 1-10: The file fails CI due to formatting/indentation issues in
the <script> block (imports and variable declarations like getContext, onMount,
globalState and ename) — run the project's formatter (pnpm run format) or apply
the project's Biome formatting rules to the file so imports, indentation and
spacing inside the <script lang="ts"> block match the repo style and resolve the
Biome errors.
In `@infrastructure/eid-wallet/src/routes/`(auth)/verify/+page.svelte:
- Around line 161-169: The onmessage handler for eventSource currently logs and
stores the entire websocket payload (variables: data, websocketData, person,
document) which can leak PII; remove the full-payload console.log calls and stop
persisting the raw payload unredacted. Instead only log minimal fields like
data.status and data.reason (or wrap the logs behind an explicit debug flag),
keep assigning status.set(data.status) and reason.set(data.reason) as before,
and if you must keep person/document in memory ensure they are never logged or
stored in websocketData without redaction.
In `@infrastructure/eid-wallet/src/routes/`+layout.svelte:
- Around line 95-126: The globalDeepLinkHandler currently re-dispatches a new
CustomEvent("deepLinkReceived") when window.location.pathname === "/scan-qr",
causing synchronous re-entry and infinite recursion; instead, stop
creating/dispatching a new event and simply return so the original event can
propagate to local listeners on the scan page (i.e., remove the dispatchEvent
call in globalDeepLinkHandler and exit early when on "/scan-qr"). Ensure you
reference globalDeepLinkHandler and the "deepLinkReceived" event name so the
scan page's existing listener receives the original event without a new
synchronous dispatch.
🧹 Nitpick comments (4)
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte (1)
104-104: Useenv(safe-area-inset-bottom)for safe-area-aware bottom padding.
pb-[8svh]works as a quick fix, but combining it withenv(safe-area-inset-bottom)adapts to devices with notches or dynamic navigation bars, improving the experience on modern mobile devices.📱 Example safe-area-aware padding
- class="h-full pt-[5.2svh] px-[5vw] pb-[8svh] flex flex-col justify-between" + class="h-full pt-[5.2svh] px-[5vw] pb-[calc(8svh_+_env(safe-area-inset-bottom))] flex flex-col justify-between"This syntax is supported in Tailwind CSS v4 arbitrary values (note: spaces in
calc()expressions use underscores).infrastructure/eid-wallet/src/routes/(app)/main/+page.svelte (1)
201-209: Consider safe‑area‑aware bottom positioning.A fixed
bottom-12can still overlap system UI on devices with larger insets. You already expose--safe-bottom; combining it with the offset is more robust across devices.♻️ Suggested tweak
- class="fixed bottom-12 left-1/2 -translate-x-1/2" + class="fixed left-1/2 -translate-x-1/2 bottom-[calc(3rem+var(--safe-bottom))]"infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte (1)
43-68: Avoid creating a new hardware‑test key on every attempt.Using a timestamped key ID will leave orphaned keys across retries. Reuse a fixed test key ID (or delete it after the check if the API supports it).
♻️ Suggested tweak
- const testKeyId = `hardware-test-${Date.now()}`; + const testKeyId = "hardware-test";infrastructure/eid-wallet/src/routes/(auth)/login/+page.svelte (1)
71-152: Extract the shared post‑auth flow to avoid drift.The PIN and biometric branches duplicate the same health‑check + pending deep‑link routing. A shared helper would reduce maintenance risk and keep behavior aligned.
♻️ Suggested structure
+ async function handlePostAuth(): Promise<void> { + // health check + public key sync + // pending deep link handling + // navigation to /scan-qr or /main + } + handlePinInput = async (pin: string) => { ... - // health check + pending deep link + navigation + await handlePostAuth(); return; }; ... if ((await globalState.securityController.biometricSupport) && (await checkStatus()).isAvailable) { try { await authenticate(...); isPostAuthLoading = true; - // health check + pending deep link + navigation + await handlePostAuth(); } catch (e) { ... } }Also applies to: 163-241
infrastructure/eid-wallet/src/routes/(app)/settings/pin/+page.svelte
Outdated
Show resolved
Hide resolved
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
Outdated
Show resolved
Hide resolved
| onMount(async () => { | ||
| globalState = getContext<() => GlobalState>("globalState")(); | ||
| try { | ||
| isBiometricsAvailable = (await checkStatus()).isAvailable; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for setContext with globalState
rg -nP '\bsetContext\s*\(\s*["\']globalState["\']' --type svelteRepository: MetaState-Prototype-Project/prototype
Length of output: 149
🏁 Script executed:
#!/bin/bash
# Also search for any setContext usage without type filter to be thorough
rg -n 'setContext.*globalState'Repository: MetaState-Prototype-Project/prototype
Length of output: 287
🏁 Script executed:
#!/bin/bash
# Look at the specific file mentioned in the review
head -120 infrastructure/eid-wallet/src/routes/\(auth\)/register/\+page.svelte | tail -40Repository: MetaState-Prototype-Project/prototype
Length of output: 1474
🏁 Script executed:
#!/bin/bash
# Check the route structure to understand the hierarchy
find infrastructure/eid-wallet/src/routes -type f -name '+layout.svelte' | sortRepository: MetaState-Prototype-Project/prototype
Length of output: 316
🏁 Script executed:
#!/bin/bash
# Get the full context of the getContext usage in the register page
cat -n infrastructure/eid-wallet/src/routes/\(auth\)/register/\+page.svelte | head -110 | tail -30Repository: MetaState-Prototype-Project/prototype
Length of output: 1227
🏁 Script executed:
#!/bin/bash
# Check if there are other layout files in the (auth) directory
find infrastructure/eid-wallet/src/routes/\(auth\) -type f -name '+layout.svelte'Repository: MetaState-Prototype-Project/prototype
Length of output: 139
🏁 Script executed:
#!/bin/bash
# Check what's in the +layout.svelte where setContext is called
cat -n infrastructure/eid-wallet/src/routes/\+layout.svelte | head -40Repository: MetaState-Prototype-Project/prototype
Length of output: 1803
🏁 Script executed:
#!/bin/bash
# Check the (auth) layout to see if it maintains context
cat -n infrastructure/eid-wallet/src/routes/\(auth\)/\+layout.svelteRepository: MetaState-Prototype-Project/prototype
Length of output: 2602
🏁 Script executed:
#!/bin/bash
# Check if globalState is ever undefined in context or if there's conditional rendering
rg -n 'globalState' infrastructure/eid-wallet/src/routes/\(auth\)/register/\+page.svelte -B2 -A2Repository: MetaState-Prototype-Project/prototype
Length of output: 1283
Add defensive guard before invoking getContext factory.
The context provider exists at the root level, but the getContext<() => GlobalState>("globalState") returns a factory function that could return undefined. The parent (auth) layout demonstrates this pattern—it retrieves the factory without immediately invoking it, then calls and checks it inside onMount. Follow the same pattern for consistency and robustness:
onMount(async () => {
+ const globalStateFactory = getContext<() => GlobalState>("globalState");
+ globalState = globalStateFactory?.();
+ if (!globalState) {
+ console.error("Global state factory returned undefined");
+ return;
+ }
try {
isBiometricsAvailable = (await checkStatus()).isAvailable;🤖 Prompt for AI Agents
In `@infrastructure/eid-wallet/src/routes/`(auth)/register/+page.svelte around
lines 93 - 96, In onMount, don't call the getContext factory directly; first
retrieve the factory via getContext<() => GlobalState>("globalState") into a
local variable (e.g., globalStateFactory), check that this factory is
defined/non-null, then invoke it to set globalState; update the code paths that
use globalState accordingly so you only call the factory when it exists
(matching the parent auth layout pattern) and handle the undefined case
defensively (e.g., skip initialization or set a safe default).
infrastructure/eid-wallet/src/routes/(auth)/review/+page.svelte
Outdated
Show resolved
Hide resolved
| 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 |
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.
Remove PII‑heavy logging from the verification stream.
data includes personal/document fields; logging the full payload can leak sensitive data in production logs. Log only status/reason or guard behind a debug flag.
🔧 Suggested fix
- if (!data.status) console.log(data);
- console.log("STATUS", data);
+ if (!data.status) console.warn("Verification update missing status");
+ console.log("Verification status:", data.status, data.reason);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 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 | |
| eventSource.onmessage = (e) => { | |
| const data = JSON.parse(e.data as string); | |
| if (!data.status) console.warn("Verification update missing status"); | |
| console.log("Verification status:", data.status, data.reason); | |
| status.set(data.status); | |
| reason.set(data.reason); | |
| person = data.person; | |
| document = data.document; | |
| websocketData = data; // Store the full websocket data |
🤖 Prompt for AI Agents
In `@infrastructure/eid-wallet/src/routes/`(auth)/verify/+page.svelte around lines
161 - 169, The onmessage handler for eventSource currently logs and stores the
entire websocket payload (variables: data, websocketData, person, document)
which can leak PII; remove the full-payload console.log calls and stop
persisting the raw payload unredacted. Instead only log minimal fields like
data.status and data.reason (or wrap the logs behind an explicit debug flag),
keep assigning status.set(data.status) and reason.set(data.reason) as before,
and if you must keep person/document in memory ensure they are never logged or
stored in websocketData without redaction.
Description of change
Adjusted bottom buttons padding to avoid being behind mobile nav bar
I was trying to find a miracle solution for devices, but apparently this is the most reliable way.
Issue Number
Closes #741
Type of change
How the change has been tested
Tested on Redmo Note 14, and on virtual Pixel 7
Change checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.