Post-commit review follow-ups for the batch-3 feed work.
Frontend — VirtualFeed: the guarded setOptions keyed only on (count, scrollMargin),
so a rotation (or the first-paint 0->real container width) changed colWidth / card
heights without invalidating the cached measurements, leaving getTotalSize and the
scrollbar stale until each row scrolled back through the window. Track appliedWidth
and call virtualizer.measure() on a width delta so heights re-estimate at the new
width (rendered rows re-measure immediately via their ResizeObserver). Covers list +
grid and the first-paint 120px-estimate case.
Backend — social.rs: the fresh like/comment count SELECT used `.await?`, so a
transient DB failure would 500 the request after the like/comment had already
committed. Make the count + broadcast best-effort (if let Ok { ... }); the mutation
succeeds regardless and the next event / pull-to-refresh reconciles the count.
Docs — FOLLOWUPS: record the review findings left as-is (grid-prepend tile reflow,
filtered-grid auto-load which is pre-existing, comment-delete stale count, and the
last-write-wins count ordering note).
Verified: svelte-check 0 errors, cargo build clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
9.1 KiB
Follow-ups
Tracked work that was deferred during the multi-round UI/UX review pass. Each item has a clear acceptance criterion so a future pass can land it without re-deriving the context.
A11y — assistive-tech containment inside modals
Problem. Open modals (LightboxModal, ConfirmSheet, Modal, OnboardingGuide,
join PIN modal, account data-mode sheet, export HTML guide) trap keyboard Tab
via focusTrap, but VoiceOver rotor / TalkBack arrow-key navigation can still
escape into the page content behind the dialog. Screen-reader users hear the
wrong context.
Why deferred. A naive sibling-walk that sets inert on direct children of
the modal's parent silences the global <Toaster> (aria-live="polite" region
mounted in +layout.svelte) and the <BottomNav> while a modal is open —
breaking toast announcements and the visible nav state. SvelteKit has no
built-in portal mechanism, so dialogs render inside the route tree alongside
the Toaster.
Acceptance criterion. With any modal open:
- VoiceOver rotor (iOS Safari) and TalkBack swipe navigation (Android Chrome) cannot leave the dialog subtree.
- Toasts that fire while a modal is open are still announced.
- Nested modals (e.g. ConfirmSheet opened from inside ContextSheet) maintain correct containment when the inner closes.
Sketch of an approach. One of:
- Portal pattern. Render dialogs into a dedicated
<div id="modal-root">that's a sibling of the main app root inapp.html.focusTrapthen setsinerton the main root, leaving the modal root and the toast region (also moved to its own portal root) untouched. - Opt-out marker. Walk siblings and inert them, but skip any node carrying
a
data-modal-passthroughattribute. Mark<Toaster>with it. Document the contract. - Stack-aware containment. Maintain a module-level stack of open dialog nodes; the topmost owns the inert state, popped dialogs restore the previous layer. Avoids the nested-modal restoration bug.
Approach 1 is the cleanest long-term but the highest blast radius. Approach 2 is the smallest patch.
Files to touch.
- frontend/src/lib/actions/focus-trap.ts — add inert logic
- frontend/src/lib/components/Toaster.svelte — add passthrough marker (if approach 2) or move to a portal (if approach 1)
- frontend/src/app.html — add
<div id="modal-root">(if approach 1)
Feed — DOM-windowing virtualization (IMPLEMENTED — residual validation owed)
Status. Implemented in frontend/src/lib/components/VirtualFeed.svelte
using @tanstack/svelte-virtual's createWindowVirtualizer. Both the list
view (dynamic measureElement heights, keyed by upload id with anchorTo:'start'
so an SSE prepend doesn't yank a scrolled-down reader) and the grid view
(three measured square tiles per row) now keep only the on-screen window (+overscan)
in the DOM instead of one node per upload. The window virtualizer scrolls the
document, so the sticky header, pull-to-refresh, the infinite-scroll sentinel and
the bottom nav are untouched. The earlier content-visibility band-aid was removed
from FeedListCard (it interferes with real-height measurement), and the old
FeedGrid.svelte was deleted (its sole consumer migrated to VirtualFeed).
Verified. svelte-check 0 errors, production build clean. The integration
follows the library's documented window-virtualizer contract (confirmed against
virtual-core source: item start includes scrollMargin, getTotalSize()
excludes it; the SSR path is guarded by getScrollElement() returning null).
Residual validation owed (needs the running app — could not be done headless).
- Manual scroll testing on a ~1000-item event: confirm no jank, correct scrollbar
size, and that an SSE
new-upload/feed-deltaprepend while scrolled down does not jump the viewport (theanchorTo:'start'+ id-key path). scrollMarginre-measure when grid filter chips change the header height (handled reactively via theuploads-length-driven effect, but unverified visually).- A new e2e spec that scrolls far down, likes an item via SSE, and asserts scroll position is retained — the existing suite only asserts a single card is visible, so it cannot catch a scroll regression.
Files.
- frontend/src/lib/components/VirtualFeed.svelte — new windowing component (list + grid)
- frontend/src/routes/feed/+page.svelte — renders
VirtualFeedfor both views - frontend/src/lib/components/FeedListCard.svelte —
content-visibilityremoved
Known limitations (surfaced in the post-commit review, left as-is — low impact).
- Grid prepend reflows tiles. Grid rows are index-keyed and pack 3 tiles each, so
an SSE
new-uploadshifts every tile by one position;anchorTo:'start'can only anchor a scrolled grid reader when the prepend crosses a 3-item boundary (it adds a row). List view is unaffected (one id-keyed row per upload). A fix would key rows by the first tile's id and accept partial-row churn; not worth it for the rarer "new upload while browsing the grid" case. - Filtered grid can auto-load the whole feed. When a grid filter matches few items
the
VirtualFeedis short, so the infinite-scroll sentinel sits in-viewport andloadMore()fires untilnextCursoris null — pulling all pages to widen the client-side search. This is pre-existing (the oldFeedGridhad the same shape, and the empty-filter copy even says "scrolle weiter"), not a virtualization regression. If undesired, gate auto-load to list view or to actual user scroll.
Feed — comment deletion leaves a stale live count
Problem. add_comment now broadcasts a fresh comment_count so feed clients patch
the card in place, but delete_comment and host_delete_comment
(backend/src/handlers/social.rs,
host.rs) soft-delete without broadcasting any
count/event. So a deletion leaves the count too high on every client until a full
refetch (pull-to-refresh or an unrelated upload-processed merge). toggle_like
already broadcasts on both add and remove, so likes are fine — the gap is
comments-on-delete. Pre-existing, but the in-place-patch scheme makes it observable.
Fix. Emit a new-comment (or a comment-deleted) event carrying the refreshed
comment_count from both delete paths, the same best-effort way add_comment does;
the frontend patchCount(..., 'comment_count') handler already consumes it.
Note — count ordering. The broadcast count is read just after the (auto-committed) mutation, not inside it, so under concurrent likes/comments on one upload the SSE messages are last-write-wins. The frontend replaces (not increments) the count, so steady state is correct and self-healing; only document this if strict per-event ordering is ever required (then compute the count in-tx with a monotonic sequence).
Feed — per-image exact CLS reservation
Problem. FeedListCard now reserves a default aspect-[4/5] box for photos so
the card doesn't collapse to height 0 and reflow as images stream in (matching
Skeleton). But no image dimensions are stored anywhere (not on FeedUpload, the
upload table, or any migration), so the box is a uniform guess that crops to fit —
the original is one tap away in the lightbox.
Acceptance criterion. Extract image width/height during the compression worker,
store them on upload, expose them on FeedUpload, and have the card reserve the
true aspect ratio (no crop, zero shift).
Files to touch.
- backend:
services/compression.rs,models/upload.rs,handlers/feed.rs, a migration - frontend/src/lib/types.ts, FeedListCard.svelte
Smaller nits, optional
- Auto-submit on retried 4th digit. recover/+page.svelte, join/+page.svelte — after a wrong PIN, deleting one digit and retyping triggers an immediate submit. Backend's 3-attempts/15-min lockout makes this safe; could feel hair-trigger after a typo. Consider gating the second auto-submit per input session behind an explicit button press.
- Onboarding pip tap target on the vertical axis. OnboardingGuide.svelte — current
p-2.5yields ~26 px height, meets WCAG 2.2 AA (≥24 px) but below iOS HIG / Material's 44 / 48 dp recommendation. Bumping top-3is the easy improvement; further increases start crowding the row. - Migrate bespoke focus-trapped dialogs to
<Modal>. Join PIN modal, OnboardingGuide, LightboxModal, HTML guide, account data-mode sheet — all currently roll their own shell withfocusTrap. They're correct, just not using the canonical primitive. Migrate when<Modal>gains features (e.g. the inert work above) you'd want everywhere.