Feat/mobile responsiveness #292
Conversation
WalkthroughConverts QR-only login views into device-aware flows. Both pages add mobile detection to choose between rendering a deep-link “Login with eID Wallet” button on mobile or a QR code on desktop. Layouts, headers, and copy are restructured; router navigation is removed in favor of window.location redirects on eVoting. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Page as Auth Page
participant Device as Device Checker
participant API as /api/auth/offer
participant Wallet as eID Wallet
User->>Page: Open login page
Page->>Device: Detect isMobile
Page->>API: Fetch QR/deep-link data
API-->>Page: qrData (offer URL)
alt isMobile
Page-->>User: Show "Login with eID Wallet" button
User->>Wallet: Open deep link (qrData)
Wallet-->>Page: Redirect callback (authenticated)
Page->>User: Navigate to "/"
else Desktop
Page-->>User: Render QR code (qrData)
User->>Wallet: Scan QR with eID Wallet
Wallet-->>Page: Event stream indicates success
Page->>User: Navigate to "/"
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
platforms/pictique/src/routes/(auth)/auth/+page.svelte (1)
14-16: Consider using a more robust mobile detection approach.The current implementation only checks viewport width, which doesn't account for actual device type. This could lead to inconsistent behavior when desktop browsers are resized to mobile widths.
Consider using a more comprehensive mobile detection similar to the eVoting implementation:
function checkMobile() { - isMobile = window.innerWidth <= 640; // Tailwind's `sm` breakpoint + isMobile = /Android|webOS|iPhone|iPad|iPod|BlackBerry|IEMobile|Opera Mini/i.test(navigator.userAgent) || + window.innerWidth <= 640; }platforms/eVoting/src/app/(auth)/login/page.tsx (2)
37-40: Improve error handling with specific error message.The catch block discards the error details, making debugging difficult. Consider preserving the error information.
-} catch { - setError("Failed to load QR code"); +} catch (err) { + console.error("Failed to fetch QR code:", err); + setError(err instanceof Error ? err.message : "Failed to load QR code"); setIsLoading(false); }
58-58: Consider using Next.js router for navigation.Using
window.location.hreffor navigation causes a full page reload, losing React state and potentially affecting user experience. The original implementation used Next.js router which is more appropriate for a Next.js application.Consider using Next.js router for client-side navigation:
+"use client"; +import { useRouter } from "next/navigation"; import { useState, useEffect } from "react"; export default function LoginPage() { + const router = useRouter(); const { login } = useAuth(); // ... in the eventSource.onmessage handler: - window.location.href = "/"; + router.push("/");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
platforms/eVoting/src/app/(auth)/login/page.tsx(4 hunks)platforms/pictique/src/routes/(auth)/auth/+page.svelte(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
platforms/eVoting/src/app/(auth)/login/page.tsx (1)
platforms/eVoting/src/lib/utils/mobile-detection.ts (2)
isMobileDevice(1-6)getDeepLinkUrl(8-10)
🔇 Additional comments (1)
platforms/pictique/src/routes/(auth)/auth/+page.svelte (1)
65-70: Verify mobile deep link format for eID Wallet loginIt looks like we’re using
qrData(returned asdata.uri) directly in the<a href={qrData}>tag, but there’s no deep-link helper in Pictique (unlike other projects that use agetDeepLinkUrlfunction). Please confirm that your/api/auth/offerendpoint already returns a correctly formatted deep link (e.g.eidwallet://…or a universal link). If it doesn’t, you’ll need to transform the URI on the client (or add a helper) to ensure mobile wallets open the app as expected.• File: platforms/pictique/src/routes/(auth)/auth/+page.svelte
• Lines: 65–70 (<a href={qrData}…>Login with eID Wallet</a>)
| useEffect(() => { | ||
| setIsMobile(isMobileDevice()); | ||
| }, []); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding window resize listener for responsive behavior.
Unlike the Pictique implementation, this component doesn't listen for window resize events. Users who rotate their device or resize their browser window won't see the UI update accordingly.
Add a resize event listener to update mobile state dynamically:
useEffect(() => {
setIsMobile(isMobileDevice());
+
+ const handleResize = () => {
+ setIsMobile(isMobileDevice());
+ };
+
+ window.addEventListener('resize', handleResize);
+ return () => window.removeEventListener('resize', handleResize);
}, []);🤖 Prompt for AI Agents
In platforms/eVoting/src/app/(auth)/login/page.tsx around lines 17 to 19, the
useEffect only sets isMobile once and doesn't respond to window resizes; add a
window resize listener inside the same useEffect that recalculates and calls
setIsMobile(isMobileDevice()) on resize, remove the listener in the effect
cleanup to avoid leaks, and optionally debounce the handler to reduce frequent
state updates.
| {isMobile ? ( | ||
| <div className="flex flex-col gap-4 items-center"> | ||
| <a | ||
| href={getDeepLinkUrl(qrData)} |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify getDeepLinkUrl implementation.
The getDeepLinkUrl function currently just returns the input without transformation. This may not provide the correct deep link format for mobile wallets.
🌐 Web query:
What is the standard deep link URL format for opening authentication URIs in mobile wallet applications?
💡 Result:
Short answer: there isn’t one universal URL format — mobile wallets use either (A) an app-specific custom URI scheme or (B) an HTTPS universal link that the wallet claims — and the common pattern is:
- ://wc?uri=<URL‑ENCODED_AUTH_URI>
or - https:///wc?uri=<URL‑ENCODED_AUTH_URI>
Where the embedded auth/connection URI (e.g. a WalletConnect "wc:..." URI or an OAuth redirect) must be percent-encoded. Wallets (and dapps) commonly use that pattern for WalletConnect-style session/auth requests; iOS prefers claimed HTTPS (universal) links for better UX/security. (alpha-docs.walletconnect.com, reddit.com)
Examples
- WalletConnect deep link (custom scheme):
metamask://wc?uri=wc%3A0123abcd...%401%3Fbridge%3D... (the wc: URI is percent‑encoded). (alpha-docs.walletconnect.com) - WalletConnect universal link:
https://example.wallet/wc?uri=wc%3A0123abcd...%401%3FsymKey%3D... . (alpha-docs.walletconnect.com) - OAuth native-app redirect (recommended patterns): use an app‑claimed HTTPS redirect like
https://app.example.com/oauth2redirect/my‑app
or a private-use scheme like com.example.app:/oauth2redirect/example-provider per RFC 8252. (datatracker.ietf.org) - Phantom (Solana) deeplink example:
https://phantom.app/ul/v1/connect?app_url=...&dapp_encryption_public_key=...&redirect_link=... (or phantom://v1/connect...). (docs.phantom.com)
Best practices
- Prefer universal (https) links when possible on iOS/Android for security and UX. (alpha-docs.walletconnect.com, datatracker.ietf.org)
- Always percent-encode an embedded URI passed as a query parameter (many wallets require it). (reddit.com)
- Follow the wallet vendor’s docs (Metamask, Phantom, WalletConnect, etc.) because each wallet defines exact path, query names and supported behaviors. (alpha-docs.walletconnect.com, docs.phantom.com)
If you tell me which wallet/protocol you’re targeting (MetaMask, WalletConnect v1/v2, Phantom, Rainbow, OAuth, Solana, Ethereum EIP‑681, etc.), I can give an exact example URL formatted for that wallet and note any encoding/redirect parameters required.
Citations:
- 1: https://alpha-docs.walletconnect.com/web3wallet/mobileLinking?utm_source=chatgpt.com
- 2: https://www.reddit.com/r/ethdev/comments/racwz8?utm_source=chatgpt.com
- 3: https://alpha-docs.walletconnect.com/web3wallet/mobileLinking?utm_source=chatgpt.com
- 4: https://alpha-docs.walletconnect.com/web3wallet/mobileLinking?utm_source=chatgpt.com
- 5: https://datatracker.ietf.org/doc/html/rfc8252?utm_source=chatgpt.com
- 6: https://docs.phantom.com/phantom-deeplinks?utm_source=chatgpt.com
- 7: https://alpha-docs.walletconnect.com/web3wallet/mobileLinking?utm_source=chatgpt.com
- 8: https://datatracker.ietf.org/doc/html/rfc8252?utm_source=chatgpt.com
- 9: https://www.reddit.com/r/ethdev/comments/racwz8?utm_source=chatgpt.com
- 10: https://alpha-docs.walletconnect.com/web3wallet/mobileLinking?utm_source=chatgpt.com
- 11: https://docs.phantom.com/phantom-deeplinks?utm_source=chatgpt.com
Implement proper deep-link formatting in getDeepLinkUrl
The helper currently returns the raw qrData URI, but mobile wallets expect a percent-encoded URI wrapped in either a custom scheme or a claimed HTTPS link. Please update getDeepLinkUrl to:
- Percent-encode the embedded auth URI:
const encoded = encodeURIComponent(qrData) - Prefix with the wallet’s scheme or universal link:
• Custom scheme (e.g. MetaMask/WalletConnect):
metamask://wc?uri=${encoded}
• HTTPS universal link (recommended on iOS/Android):
https://example.wallet/wc?uri=${encoded} - Match the exact path and query param names from your target wallet’s docs (MetaMask, WalletConnect v2, Phantom, etc.)
Action items:
- In
platforms/eVoting/src/app/(auth)/login/page.tsx(or wherevergetDeepLinkUrlis defined), replace the stub with logic that builds and returns a properly formatted deep link. - If you need to support multiple wallets, consider adding a
walletparameter or separate helpers for each format. - Add or update unit tests to verify the output URL for your chosen wallet protocols.
🤖 Prompt for AI Agents
In platforms/eVoting/src/app/(auth)/login/page.tsx around line 115, replace the
current stub that returns raw qrData with a helper that percent-encodes the
embedded auth URI (const encoded = encodeURIComponent(qrData)) and then prefixes
it with the chosen wallet deep-link format (e.g. for MetaMask/WalletConnect use
metamask://wc?uri=${encoded} or use an HTTPS universal link like
https://example.wallet/wc?uri=${encoded}); ensure the path and query parameter
names exactly match the target wallet docs, add an optional wallet parameter or
separate helpers if you must support multiple wallets, and add/update unit tests
to assert the expected deep-link outputs for each supported wallet format.
| onDestroy(() => { | ||
| window.removeEventListener('resize', checkMobile); | ||
| }); |
There was a problem hiding this comment.
Incorrect placement of onDestroy call.
The onDestroy call is nested inside onMount, which is incorrect. Svelte lifecycle hooks should be called at the top level of the component.
Move the onDestroy call outside of onMount:
watchEventStream(new URL(qrData).searchParams.get('session') as string);
-
- onDestroy(() => {
- window.removeEventListener('resize', checkMobile);
- });
});
+
+onDestroy(() => {
+ window.removeEventListener('resize', checkMobile);
+});
</script>🤖 Prompt for AI Agents
In platforms/pictique/src/routes/(auth)/auth/+page.svelte around lines 45 to 47,
the onDestroy call is incorrectly nested inside onMount; move the onDestroy(()
=> window.removeEventListener('resize', checkMobile')) call out of the onMount
block so Svelte lifecycle hooks are invoked at the component top level, keeping
the addEventListener in onMount and the corresponding removeEventListener in a
sibling onDestroy to ensure proper cleanup.
Description of change
mobile responsiveness
Issue Number
n/a
Type of change
How the change has been tested
manual
Change checklist
Summary by CodeRabbit