Conversation
Add password field to Session type so the backup modal can show credentials later. The password is generated in doSignUp() and was previously discarded after the API call. Old sessions are backfilled with an empty string — the password is lost but the token still works. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the bookmark link in the header with a user icon that opens a dropdown menu with "My Saved" and "Log out". Uses svelte-outclick for outside click handling (same as NavDropdownDesktop). Only visible when the user has a session. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add test for password backfill when loading old sessions without the password field (mirrors existing savedAreas backfill test) - Add aria-haspopup="true" to UserMenu trigger button for screen readers (matches NavDropdownDesktop pattern) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
UserMenu is now always visible: - Logged out: outline account icon, dropdown shows "Log in" - Logged in: filled account icon, dropdown shows "My Saved" + "Log out" Links to /login which will be built in the next step. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Every user gets an auto-generated account on first save. "Log out" made no sense — clicking Save again would just create another throwaway. Account switching will be handled by the login flow which replaces the current session. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- POST /api/session/login server route proxying token creation - /login page with username + password form - session.login() method to replace current session - After login, fetches saved places/areas from server to populate store - Redirects to /saved on success - Error toast on invalid credentials or server error Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Since every user always has an account (auto-generated on first save), they need a way to switch to a different account without a logout step. Links to /login which replaces the current session. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Hovering the user icon now shows the account username (e.g. "glossy-wealth-1878") via the title attribute. Shows "Account" when not logged in. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When a user clicks Save for the first time and the throwaway account is silently created, show a success toast: "Account created — your saved places are stored on this device." Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Theme toggle is h-10 w-10. User menu trigger was unsized, causing visual misalignment. Match the dimensions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
session.init() is a synchronous localStorage read but was deferred to onMount, causing a visible 2-second delay before the user icon rendered. Move it to the module level with a browser guard so it runs during script evaluation, before the first render. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This reverts commit e62dfd8.
- "Switch account" now shows a confirmation dialog warning that the current saved places will be replaced - Remove unused nav.logout i18n key - Update security comment in session.ts to acknowledge password storage in localStorage alongside the token Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- BackupModal shows username + password with copy buttons and show/hide toggle. Accessible from "Back up account" in UserMenu. - Old accounts without a stored password show "Not available" - Remove switch account confirmation — if the user knows their credentials (just logged in), switching is safe - Remove dead switchAccountConfirm i18n key Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add autoGenerated flag to Session type. Set true in signUp(), false in login(). UserMenu only shows "Back up account" when the account was auto-generated — users who logged in with known credentials don't need to back them up. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When logging in with known credentials, the user already has their password. No reason to keep it in localStorage where an XSS could exfiltrate it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Redirecting anonymous users to /map was confusing — they had no idea why they landed there. Now /saved just shows the empty state message regardless of session status. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
If the user visits /saved without an account, redirect to /login where they can log in with existing credentials or learn they need to save something first. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reject oversized username (>100) or password (>200) before forwarding to the upstream API. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Users who want a custom username can create an account on developer.btcmap.org, then log in on the main site. Added "Don't have an account? Create one here" below the login form. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Close BackupModal on Escape key press
- Remove document.execCommand("copy") fallback — navigator.clipboard
is the standard API and works on all modern browsers over HTTPS
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for btcmap ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Users start with no account, so "Switch account" was confusing — it implies multiple accounts. "Log out" is clearer: it means "forget this session." If they want a different account, they log out first, then log in. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Header renders UserMenu twice (desktop + mobile), producing duplicate DOM IDs. Use a random suffix per instance so each trigger has a unique ID and the OutClick exclusion works correctly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Wrap clipboard.writeText in try/catch to prevent unhandled rejections (console.error only, no error toast — failure is extremely unlikely on HTTPS) - Replace hardcoded "copied" string with i18n key backup.copied Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The security comment said both token and password are stored, but manual logins intentionally store an empty password. Clarify that password persistence only applies to auto-generated sessions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Stop trimming password input — spaces may be valid characters in credentials. Username is still trimmed (no valid username has leading/trailing spaces). - Sanitize console.error in login server route and client page to only log the HTTP status code, not the full error object which may contain request headers with credentials. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Prevents unintended form submits if the component is ever rendered inside a form element. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tton Use role=presentation on the backdrop overlay instead of suppressing a11y warnings. The backdrop is decorative — keyboard interaction is handled via svelte:window Escape handler. Add type=button to the close button. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace Math.random() trigger ID with a prop-based ID to avoid SSR/client hydration mismatches. Header passes distinct IDs for desktop and mobile instances. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Group authenticated user pages under /user/ to establish a clear URL pattern for future user features (profile, feed, settings). Login stays at /login since it's a public entry point. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Switch from filled bookmark (ic:baseline-bookmark) to bookmark with checkmark (ic:baseline-bookmark-added) for the saved state. Aligns with the Android app's icon pattern — the checkmark is a more explicit signal than fill alone. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/auth/BackupModal.svelte (1)
7-8:⚠️ Potential issue | 🟡 MinorShow a user-facing error when clipboard copy fails.
copyToClipboardlogs failures but gives no feedback to users. This leaves copy actions looking broken when permissions/contexts reject clipboard access.Suggested patch
-import { successToast } from "$lib/utils"; +import { errToast, successToast } from "$lib/utils"; @@ async function copyToClipboard(text: string) { try { await navigator.clipboard.writeText(text); successToast($_("backup.copied")); - } catch (err) { - console.error("Clipboard write failed", err); + } catch { + errToast($_("backup.copyFailed")); } }Also applies to: 13-19
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/auth/BackupModal.svelte` around lines 7 - 8, The copyToClipboard call in BackupModal.svelte currently only logs failures and never notifies the user; update the clipboard handler (where copyToClipboard is invoked inside the BackupModal component) to catch errors/rejected promises and show a user-facing toast on failure in addition to the existing successToast on success. Import or use the existing error/failure toast utility (e.g., errorToast from "$lib/utils") or create a visible fallback notification, and ensure the catch block passes the error message/context to the toast so users see why the copy failed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/auth/BackupModal.svelte`:
- Around line 30-34: The modal container in BackupModal.svelte needs proper
dialog semantics so assistive tech treats it as a modal: update the outer modal
div (the element that currently has classes "mx-4 w-full max-w-sm rounded-xl
bg-white p-6 shadow-xl dark:bg-dark") to include role="dialog" and
aria-modal="true", add a unique id to the heading (e.g., id="backup-title" on
the h2 rendered by {$_("backup.title")}), and add aria-labelledby="backup-title"
plus tabindex="-1" on the dialog container so it can receive programmatic focus
when the modal opens; ensure any focus trapping or initial focus logic targets
that container or a focusable child.
---
Duplicate comments:
In `@src/components/auth/BackupModal.svelte`:
- Around line 7-8: The copyToClipboard call in BackupModal.svelte currently only
logs failures and never notifies the user; update the clipboard handler (where
copyToClipboard is invoked inside the BackupModal component) to catch
errors/rejected promises and show a user-facing toast on failure in addition to
the existing successToast on success. Import or use the existing error/failure
toast utility (e.g., errorToast from "$lib/utils") or create a visible fallback
notification, and ensure the catch block passes the error message/context to the
toast so users see why the copy failed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 74710807-7440-4273-88b3-4a218ff93950
📒 Files selected for processing (6)
src/components/Icon.sveltesrc/components/auth/BackupModal.sveltesrc/components/layout/Header.sveltesrc/components/layout/UserMenu.sveltesrc/routes/login/+page.sveltesrc/routes/user/saved/+page.svelte
✅ Files skipped from review due to trivial changes (1)
- src/routes/user/saved/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/Icon.svelte
- src/components/layout/Header.svelte
* feat(save): prompt login or create account on logged-out save Previously, clicking Save while logged out silently created a new throwaway account. For users who already had an account (on this device, a new device, or from the dev portal) this produced unrecoverable orphans — the v4 API has no password recovery or username-exists lookup. Now the click opens a modal offering "Log in" or "Create account". Creation still auto-generates credentials but immediately shows the backup view so the user sees username + password at least once. Login uses an inline form (shared with /login via a new LoginForm component) and merges the new id into the server's saved list. - Add SaveAuthPrompt modal (Modal wrapper; choice/login/backup views) - Extract LoginForm.svelte; /login composes it - Extract savedItems.ts helpers (put/set/get/toggle) reused by SaveButton and SaveAuthPrompt - Add save.prompt.* i18n keys Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(save): show backup view before initial save can fail If the post-signup save throws, the catch block closed the modal and the user never saw the credentials for the account that was just created server-side. Commit view="backup" right after signUp() succeeds and swallow save errors (performInitialSave already toasts). * style(imports): separate type imports from value imports CLAUDE.md forbids mixing types and values in a single import. Splits the SavedItemType, Session, and session imports across the four files that violated the rule. * refactor(auth): extract BackupCredentials shared component SaveAuthPrompt's backup view and BackupModal rendered near-identical credential blocks (~80 lines each). Extract BackupCredentials.svelte with an idPrefix prop and use it from both. Also fixes the missing unavailable-password fallback in SaveAuthPrompt. * refactor(auth): reuse shared Modal in BackupModal BackupModal rolled its own backdrop, escape handler, and close button, missing the focus trap/restoration that $components/Modal.svelte provides and with inner buttons that lacked type="button". Wrap the shared Modal and switch UserMenu to bind:open. * refactor(saved): extract hydrateSavedFromServer helper SaveAuthPrompt and /login both ran the same Promise.allSettled block fetching /api/session/saved-places and /api/session/saved-areas to populate the session store. Move it next to putSavedList so there is one place to update the hydration contract. * fix(login): cap input length to match server limits Server rejects username > 100 chars and password > 200 chars. Add maxlength attributes so the UI fails fast instead of sending oversized payloads that will 400. * refactor(auth): use get(session) and extract titleFor helper Three cleanups to SaveAuthPrompt and LoginForm: - Replace subscribe/assign/unsub idiom with get(session) for clarity. - Extract titleFor(view) helper; the nested ternary was hard to scan. - Reset creating when the modal closes so a mid-signup close doesn't leave the create button disabled on reopen. * style(imports): separate type Session from value in savedItems Missed in the earlier import-separation pass. Flagged by qodo and CodeRabbit on PR #912. * fix(backup): show unavailable message for empty-string password ?? only falls back on null/undefined, so a session with password="" (the known-account backfill path) rendered a blank input instead of the "unavailable" text. Use || so empty strings fall through too. * fix(save): keep title reactive on locale changes Svelte 4 tracks reactive-statement deps lexically. Wrapping the title selection in a helper hid the $_ store access from the tracker, so switching locale did not retitle the modal until the view changed. Inline the ternary instead. * fix(backup): toast on clipboard write failure Successful copies toasted but failures only logged, leaving users with no signal. Surface an error toast pointing them to manual copy. * refactor(login): extract status cast in catch block Same (err as { response?: { status?: number } })?.response?.status appeared twice. Pull the status once up front. * fix(save): use area-aware copy in save auth prompt SaveAuthPrompt is rendered from both SaveButton type="place" (merchant page) and type="area" (area page). The title, description, and account-created toast hard-coded "place" wording. Split each string into titlePlace/titleArea, descriptionPlace/descriptionArea, accountCreatedPlace/accountCreatedArea and select by the type prop. * fix(save): don't overwrite server saved list when hydrate fails hydrateSavedFromServer used allSettled so a failed fetch left the session holding pre-login (empty) lists. handleLoginSuccess then PUT that empty list plus the pending id, wiping the user's real server state. Return a per-type success flag from hydrateSavedFromServer and bail the initial save with a toast when the relevant type failed. * fix(save): bail create-account flow if modal closed during signUp If the user dismissed the prompt (Escape/outclick) while signUp was pending, the continuation still set view="backup" and toasted, leaving stale state in the next open. Bail out when open=false after the await. The account persists locally and can be backed up via the UserMenu. --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/savedItems.ts (1)
72-79: Consider validating extracted IDs for defensive robustness.The
.map((p: { id: number }) => p.id)extracts IDs without runtime validation. If the API response contains malformed objects (missing or non-numericid), this could silently populate the session withundefinedorNaNvalues.Since these are trusted internal proxy endpoints, this is low risk, but adding a filter would provide extra safety:
🛡️ Optional defensive validation
if (placeOk) { - const ids = placesRes.value.data.map((p: { id: number }) => p.id); + const ids = placesRes.value.data + .map((p: { id: number }) => p.id) + .filter((id): id is number => typeof id === "number"); session.setSavedPlaces(ids); } if (areaOk) { - const ids = areasRes.value.data.map((a: { id: number }) => a.id); + const ids = areasRes.value.data + .map((a: { id: number }) => a.id) + .filter((id): id is number => typeof id === "number"); session.setSavedAreas(ids); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/savedItems.ts` around lines 72 - 79, The extraction of IDs from placesRes and areasRes is not validating values, so update the blocks guarded by placeOk and areaOk (where you call placesRes.value.data.map(...) and areasRes.value.data.map(...)) to produce only valid numeric IDs before calling session.setSavedPlaces and session.setSavedAreas; extract each item's id, coerce/check it (e.g., Number(id) and Number.isFinite/Number.isInteger) and filter out undefined/NaN/non-numeric values so the arrays passed to session.setSavedPlaces and session.setSavedAreas contain only valid numbers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/auth/BackupCredentials.svelte`:
- Around line 40-45: These icon-only buttons (e.g., the copy button with
on:click={() => copyToClipboard(username)} and the other icon-only action
buttons in BackupCredentials.svelte) rely only on title text which isn't
reliably announced by screen readers; add aria-label attributes to each
icon-only <button> using the same localized strings used for title (e.g.,
$_("backup.copy")), and for the visibility toggle button also add aria-pressed
bound to the visibility state (e.g., aria-pressed={showPassword}) so assistive
tech can discover and convey the control state.
---
Nitpick comments:
In `@src/lib/savedItems.ts`:
- Around line 72-79: The extraction of IDs from placesRes and areasRes is not
validating values, so update the blocks guarded by placeOk and areaOk (where you
call placesRes.value.data.map(...) and areasRes.value.data.map(...)) to produce
only valid numeric IDs before calling session.setSavedPlaces and
session.setSavedAreas; extract each item's id, coerce/check it (e.g., Number(id)
and Number.isFinite/Number.isInteger) and filter out undefined/NaN/non-numeric
values so the arrays passed to session.setSavedPlaces and session.setSavedAreas
contain only valid numbers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8ec6b35e-acb8-4e79-ace6-dfa7e452a9b8
📒 Files selected for processing (9)
src/components/SaveButton.sveltesrc/components/auth/BackupCredentials.sveltesrc/components/auth/BackupModal.sveltesrc/components/auth/LoginForm.sveltesrc/components/auth/SaveAuthPrompt.sveltesrc/components/layout/UserMenu.sveltesrc/lib/i18n/locales/en.jsonsrc/lib/savedItems.tssrc/routes/login/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (4)
- src/routes/login/+page.svelte
- src/components/SaveButton.svelte
- src/components/layout/UserMenu.svelte
- src/lib/i18n/locales/en.json
| <button | ||
| type="button" | ||
| on:click={() => copyToClipboard(username)} | ||
| class="shrink-0 rounded-lg border border-gray-300 p-2 transition-colors hover:bg-gray-100 dark:border-white/20 dark:hover:bg-white/10" | ||
| title={$_("backup.copy")} | ||
| > |
There was a problem hiding this comment.
Add accessible names to icon-only action buttons.
On Line 40, Line 71, and Line 86 button controls are icon-only and currently rely on title. Screen readers may not consistently announce title as the button name. Add aria-label (and aria-pressed for the visibility toggle) to make these controls reliably discoverable.
Suggested patch
<button
type="button"
on:click={() => copyToClipboard(username)}
class="shrink-0 rounded-lg border border-gray-300 p-2 transition-colors hover:bg-gray-100 dark:border-white/20 dark:hover:bg-white/10"
title={$_("backup.copy")}
+ aria-label={$_("backup.copy")}
>
@@
<button
type="button"
on:click={() => (showPassword = !showPassword)}
class="shrink-0 rounded-lg border border-gray-300 p-2 transition-colors hover:bg-gray-100 dark:border-white/20 dark:hover:bg-white/10"
title={showPassword ? $_("backup.hide") : $_("backup.show")}
+ aria-label={showPassword ? $_("backup.hide") : $_("backup.show")}
+ aria-pressed={showPassword}
>
@@
<button
type="button"
on:click={() => copyToClipboard(password ?? "")}
class="shrink-0 rounded-lg border border-gray-300 p-2 transition-colors hover:bg-gray-100 dark:border-white/20 dark:hover:bg-white/10"
title={$_("backup.copy")}
+ aria-label={$_("backup.copy")}
>Also applies to: 71-76, 86-91
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/auth/BackupCredentials.svelte` around lines 40 - 45, These
icon-only buttons (e.g., the copy button with on:click={() =>
copyToClipboard(username)} and the other icon-only action buttons in
BackupCredentials.svelte) rely only on title text which isn't reliably announced
by screen readers; add aria-label attributes to each icon-only <button> using
the same localized strings used for title (e.g., $_("backup.copy")), and for the
visibility toggle button also add aria-pressed bound to the visibility state
(e.g., aria-pressed={showPassword}) so assistive tech can discover and convey
the control state.

Related: #894
What's new
Next steps
Screenshots
Logged in with known account
First time opened
After saved is pressed without login first
Backup modal
Login (link to developers.btcmap.org)
Summary by CodeRabbit
New Features
UI
Localization