fix(feed): width-change measurement invalidation + best-effort SSE counts
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>
This commit is contained in:
35
FOLLOWUPS.md
35
FOLLOWUPS.md
@@ -79,6 +79,41 @@ excludes it; the SSR path is guarded by `getScrollElement()` returning null).
|
|||||||
- [frontend/src/routes/feed/+page.svelte](frontend/src/routes/feed/+page.svelte) — renders `VirtualFeed` for both views
|
- [frontend/src/routes/feed/+page.svelte](frontend/src/routes/feed/+page.svelte) — renders `VirtualFeed` for both views
|
||||||
- [frontend/src/lib/components/FeedListCard.svelte](frontend/src/lib/components/FeedListCard.svelte) — `content-visibility` removed
|
- [frontend/src/lib/components/FeedListCard.svelte](frontend/src/lib/components/FeedListCard.svelte) — `content-visibility` removed
|
||||||
|
|
||||||
|
**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-upload` shifts 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 `VirtualFeed` is short, so the infinite-scroll sentinel sits in-viewport and
|
||||||
|
`loadMore()` fires until `nextCursor` is null — pulling all pages to widen the
|
||||||
|
client-side search. This is pre-existing (the old `FeedGrid` had 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](backend/src/handlers/social.rs),
|
||||||
|
[host.rs](backend/src/handlers/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
|
## Feed — per-image exact CLS reservation
|
||||||
|
|
||||||
**Problem.** `FeedListCard` now reserves a default `aspect-[4/5]` box for photos so
|
**Problem.** `FeedListCard` now reserves a default `aspect-[4/5]` box for photos so
|
||||||
|
|||||||
@@ -44,18 +44,22 @@ pub async fn toggle_like(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Fresh count so feed clients can patch the single card in place instead of
|
// Fresh count so feed clients can patch the single card in place instead of
|
||||||
// refetching page 1 (mirrors v_feed.like_count = COUNT(DISTINCT user_id)).
|
// refetching page 1 (mirrors v_feed.like_count = COUNT(DISTINCT user_id)). The
|
||||||
let like_count: i64 =
|
// count + broadcast are a UI optimisation — the like itself is already committed,
|
||||||
sqlx::query_scalar("SELECT COUNT(DISTINCT user_id) FROM \"like\" WHERE upload_id = $1")
|
// so a failure here must not fail the request. Swallow the error and skip the
|
||||||
|
// broadcast; the next event or a pull-to-refresh reconciles the count.
|
||||||
|
if let Ok(like_count) = sqlx::query_scalar::<_, i64>(
|
||||||
|
"SELECT COUNT(DISTINCT user_id) FROM \"like\" WHERE upload_id = $1",
|
||||||
|
)
|
||||||
.bind(upload_id)
|
.bind(upload_id)
|
||||||
.fetch_one(&state.pool)
|
.fetch_one(&state.pool)
|
||||||
.await?;
|
.await
|
||||||
|
{
|
||||||
// Broadcast SSE
|
|
||||||
let _ = state.sse_tx.send(crate::state::SseEvent {
|
let _ = state.sse_tx.send(crate::state::SseEvent {
|
||||||
event_type: "like-update".to_string(),
|
event_type: "like-update".to_string(),
|
||||||
data: serde_json::json!({ "upload_id": upload_id, "like_count": like_count }).to_string(),
|
data: serde_json::json!({ "upload_id": upload_id, "like_count": like_count }).to_string(),
|
||||||
});
|
});
|
||||||
|
}
|
||||||
|
|
||||||
Ok(StatusCode::NO_CONTENT)
|
Ok(StatusCode::NO_CONTENT)
|
||||||
}
|
}
|
||||||
@@ -123,20 +127,23 @@ pub async fn add_comment(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Fresh count so feed clients can patch the single card in place instead of
|
// Fresh count so feed clients can patch the single card in place instead of
|
||||||
// refetching page 1 (mirrors v_feed.comment_count = COUNT(DISTINCT c.id)).
|
// refetching page 1 (mirrors v_feed.comment_count = COUNT(DISTINCT c.id); COUNT(*)
|
||||||
let comment_count: i64 = sqlx::query_scalar(
|
// over the same deleted_at filter is identical since comment.id is the PK). The
|
||||||
|
// count + broadcast are a UI optimisation — the comment is already committed, so a
|
||||||
|
// failure here must not fail the request. Swallow the error and skip the broadcast.
|
||||||
|
if let Ok(comment_count) = sqlx::query_scalar::<_, i64>(
|
||||||
"SELECT COUNT(*) FROM comment WHERE upload_id = $1 AND deleted_at IS NULL",
|
"SELECT COUNT(*) FROM comment WHERE upload_id = $1 AND deleted_at IS NULL",
|
||||||
)
|
)
|
||||||
.bind(upload_id)
|
.bind(upload_id)
|
||||||
.fetch_one(&state.pool)
|
.fetch_one(&state.pool)
|
||||||
.await?;
|
.await
|
||||||
|
{
|
||||||
// Broadcast SSE
|
|
||||||
let _ = state.sse_tx.send(crate::state::SseEvent {
|
let _ = state.sse_tx.send(crate::state::SseEvent {
|
||||||
event_type: "new-comment".to_string(),
|
event_type: "new-comment".to_string(),
|
||||||
data: serde_json::json!({ "upload_id": upload_id, "comment_count": comment_count })
|
data: serde_json::json!({ "upload_id": upload_id, "comment_count": comment_count })
|
||||||
.to_string(),
|
.to_string(),
|
||||||
});
|
});
|
||||||
|
}
|
||||||
|
|
||||||
let dto = CommentDto {
|
let dto = CommentDto {
|
||||||
id: comment.id,
|
id: comment.id,
|
||||||
|
|||||||
@@ -106,23 +106,33 @@
|
|||||||
// this effect (that would loop).
|
// this effect (that would loop).
|
||||||
let appliedCount = -1;
|
let appliedCount = -1;
|
||||||
let appliedMargin = Number.NaN;
|
let appliedMargin = Number.NaN;
|
||||||
|
let appliedWidth = Number.NaN;
|
||||||
|
|
||||||
function applyOptions() {
|
function applyOptions() {
|
||||||
const count = rowCount;
|
const count = rowCount;
|
||||||
|
const width = containerWidth;
|
||||||
const margin = listEl ? listEl.getBoundingClientRect().top + window.scrollY : 0;
|
const margin = listEl ? listEl.getBoundingClientRect().top + window.scrollY : 0;
|
||||||
scrollMargin = margin; // drives the template transforms
|
scrollMargin = margin; // drives the template transforms
|
||||||
if (count === appliedCount && Math.abs(margin - appliedMargin) < 0.5) return;
|
// A width change (rotation, or the first 0→real clientWidth landing) changes
|
||||||
|
// every card's / tile-row's real height, so the heights cached from the old
|
||||||
|
// width are stale even when count and margin are unchanged. Invalidate the
|
||||||
|
// measurement cache so getTotalSize + positions recompute (rendered rows then
|
||||||
|
// re-measure immediately via their ResizeObserver) instead of trusting them.
|
||||||
|
const widthChanged = width > 0 && Math.abs(width - appliedWidth) > 0.5;
|
||||||
|
if (count === appliedCount && Math.abs(margin - appliedMargin) < 0.5 && !widthChanged) return;
|
||||||
appliedCount = count;
|
appliedCount = count;
|
||||||
appliedMargin = margin;
|
appliedMargin = margin;
|
||||||
|
appliedWidth = width;
|
||||||
get(virtualizer).setOptions({ count, scrollMargin: margin });
|
get(virtualizer).setOptions({ count, scrollMargin: margin });
|
||||||
|
if (widthChanged) get(virtualizer).measure();
|
||||||
}
|
}
|
||||||
|
|
||||||
// Re-apply when the row count or column width changes (load-more, prepend,
|
// Re-apply when the row count or container width changes (load-more, prepend,
|
||||||
// delete, filter, rotate). `colWidth`/`rowCount` are the only tracked reads, so a
|
// delete, filter, rotate). `containerWidth`/`rowCount` are the only tracked
|
||||||
// length-stable like/comment patch doesn't re-run this.
|
// reads, so a length-stable like/comment patch doesn't re-run this.
|
||||||
$effect(() => {
|
$effect(() => {
|
||||||
void rowCount;
|
void rowCount;
|
||||||
void colWidth;
|
void containerWidth;
|
||||||
applyOptions();
|
applyOptions();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user