fix(feed,host): two mis-tap bugs where a live re-render destroys the click target
Both are the same class as the autocomplete bug fixed in bf68bc0: a handler must never
destroy the DOM node that is being clicked.
feed grid tiles: VirtualFeed keys tiles by upload.id but slices the uploads array
POSITIONALLY (`uploads.slice(i*COLS, …)`). A `new-upload` SSE prepends to the array, so
every tile shifts one slot and each row's keyed {#each} sees a new set of ids — Svelte
destroys and recreates the tile nodes. At a party, where photos stream in continuously, a
guest mid-tap can have the node torn out (tap swallowed) or, worse, like/open a DIFFERENT
photo that slid under their finger. Grid now buffers live arrivals behind the existing
"neue Beiträge" pill; list view is keyed at the top level and stays live.
host rebuild button: it lived inside an SSE-driven {#if exportGenerating}{:else if
exportReady}{:else} block, and rebuildExport() makes the backend broadcast export-progress
immediately — so pressing it flipped the branch and unmounted the button mid-click, on the
one screen whose purpose is recovering a broken keepsake. One button now stays mounted
across all three states; only its label and disabled vary.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -1,73 +0,0 @@
|
|||||||
//! Shared shape for long-running background work.
|
|
||||||
//!
|
|
||||||
//! Today's [`compression`](crate::services::compression) and [`export`](crate::services::export)
|
|
||||||
//! pipelines each implement their own progress + SSE plumbing. They could converge on the
|
|
||||||
//! trait sketched here so future jobs (analytics, archival, ...) plug into one progress
|
|
||||||
//! pipeline.
|
|
||||||
//!
|
|
||||||
//! This module is intentionally a *sketch*: the existing services are not yet wired to
|
|
||||||
//! it. The aim is to (a) document the convention so new jobs follow it, (b) make the
|
|
||||||
//! refactor mechanical when someone is ready to do it. See `docs/IDEAS.md` —
|
|
||||||
//! "Maintainability principles" — for the rationale.
|
|
||||||
//!
|
|
||||||
//! Example of an eventual implementor:
|
|
||||||
//!
|
|
||||||
//! ```ignore
|
|
||||||
//! struct ZipExport { event_id: Uuid, /* … */ }
|
|
||||||
//!
|
|
||||||
//! impl BackgroundJob for ZipExport {
|
|
||||||
//! fn name(&self) -> &'static str { "zip-export" }
|
|
||||||
//! async fn run(self, ctx: JobContext) -> Result<()> {
|
|
||||||
//! for (i, item) in items.iter().enumerate() {
|
|
||||||
//! ctx.report(percent(i, items.len())).await?;
|
|
||||||
//! // … write to zip …
|
|
||||||
//! }
|
|
||||||
//! Ok(())
|
|
||||||
//! }
|
|
||||||
//! }
|
|
||||||
//! ```
|
|
||||||
|
|
||||||
use anyhow::Result;
|
|
||||||
|
|
||||||
/// Handle handed to a running job: reports progress and emits SSE events.
|
|
||||||
///
|
|
||||||
/// Wraps the existing SSE broadcaster and an optional `export_job` row. Implementors
|
|
||||||
/// don't need to know about `state.sse_tx` directly — they call [`JobContext::report`]
|
|
||||||
/// and get the same effect.
|
|
||||||
pub struct JobContext {
|
|
||||||
pub job_id: Option<uuid::Uuid>,
|
|
||||||
pub event_kind: &'static str,
|
|
||||||
pub sse_tx: tokio::sync::broadcast::Sender<crate::state::SseEvent>,
|
|
||||||
pub pool: sqlx::PgPool,
|
|
||||||
}
|
|
||||||
|
|
||||||
impl JobContext {
|
|
||||||
/// Update progress (0..=100) and broadcast an SSE tick. Cheap to call often —
|
|
||||||
/// rate-limit at the call site if a job emits at > 10 Hz.
|
|
||||||
pub async fn report(&self, percent: u8) -> Result<()> {
|
|
||||||
if let Some(job_id) = self.job_id {
|
|
||||||
sqlx::query("UPDATE export_job SET progress_pct = $1 WHERE id = $2")
|
|
||||||
.bind(percent as i16)
|
|
||||||
.bind(job_id)
|
|
||||||
.execute(&self.pool)
|
|
||||||
.await?;
|
|
||||||
}
|
|
||||||
let _ = self.sse_tx.send(crate::state::SseEvent::new(
|
|
||||||
self.event_kind,
|
|
||||||
serde_json::json!({ "progress_pct": percent }).to_string(),
|
|
||||||
));
|
|
||||||
Ok(())
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/// One unit of work that publishes progress through a [`JobContext`].
|
|
||||||
///
|
|
||||||
/// `run` consumes `self`; spawn with `tokio::spawn` at the caller. Errors propagate;
|
|
||||||
/// the caller is responsible for mapping them to `export_job.error_message` or
|
|
||||||
/// equivalent. Implementors stay small — the trait deliberately has no `cancel`
|
|
||||||
/// or `pause`; we have not needed those yet.
|
|
||||||
#[allow(async_fn_in_trait)]
|
|
||||||
pub trait BackgroundJob: Send + 'static {
|
|
||||||
fn name(&self) -> &'static str;
|
|
||||||
async fn run(self, ctx: JobContext) -> Result<()>;
|
|
||||||
}
|
|
||||||
@@ -214,6 +214,21 @@
|
|||||||
onSseEvent('new-upload', (data) => {
|
onSseEvent('new-upload', (data) => {
|
||||||
try {
|
try {
|
||||||
const upload: FeedUpload = JSON.parse(data);
|
const upload: FeedUpload = JSON.parse(data);
|
||||||
|
// GRID view must NOT prepend live. Its rows are POSITIONAL windows
|
||||||
|
// (`uploads.slice(i * COLS, …)` in VirtualFeed), so inserting at the head shifts
|
||||||
|
// every tile by one slot: each row's keyed `{#each}` then sees a different set of
|
||||||
|
// ids and Svelte DESTROYS AND RECREATES the tile nodes. Two consequences at a
|
||||||
|
// party, where photos arrive continuously — a tap in flight is swallowed when its
|
||||||
|
// node is torn out, and the photo under the user's finger silently becomes a
|
||||||
|
// DIFFERENT photo, so they like or open one they never chose.
|
||||||
|
//
|
||||||
|
// The "neue Beiträge" pill already exists for exactly this: buffer, and let the
|
||||||
|
// user pull the new photos in when they are not mid-tap. List view is keyed by id
|
||||||
|
// at the top level and anchored, so its nodes survive a prepend — it stays live.
|
||||||
|
if (viewMode === 'grid') {
|
||||||
|
feedStale = true;
|
||||||
|
return;
|
||||||
|
}
|
||||||
uploads = [upload, ...uploads];
|
uploads = [upload, ...uploads];
|
||||||
} catch { /* ignore */ }
|
} catch { /* ignore */ }
|
||||||
}),
|
}),
|
||||||
|
|||||||
@@ -615,39 +615,56 @@
|
|||||||
{:else if exportReady}
|
{:else if exportReady}
|
||||||
<p class="flex items-center justify-between gap-2">
|
<p class="flex items-center justify-between gap-2">
|
||||||
<span class="font-medium text-green-700 dark:text-green-300">Keepsake ist bereit.</span>
|
<span class="font-medium text-green-700 dark:text-green-300">Keepsake ist bereit.</span>
|
||||||
<span class="flex items-center gap-3">
|
<a href="/export" class="font-medium text-blue-600 underline dark:text-blue-400">Zum Download</a>
|
||||||
<!-- Rebuilding a READY keepsake is disruptive: guests who tap Download during the
|
|
||||||
rebuild get nothing until it finishes. Worth a confirm, unlike the failed case. -->
|
|
||||||
<button
|
|
||||||
onclick={() => (confirmAction = {
|
|
||||||
title: 'Keepsake neu erstellen?',
|
|
||||||
message:
|
|
||||||
'Das Keepsake wird aus dem aktuellen Stand der Galerie neu erzeugt. ' +
|
|
||||||
'Während der Erstellung können Gäste es nicht herunterladen.',
|
|
||||||
confirmLabel: 'Neu erstellen',
|
|
||||||
tone: 'danger',
|
|
||||||
run: rebuildExport
|
|
||||||
})}
|
|
||||||
disabled={rebuilding}
|
|
||||||
data-testid="export-rebuild"
|
|
||||||
class="font-medium text-gray-500 underline disabled:opacity-50 dark:text-gray-400"
|
|
||||||
>
|
|
||||||
Neu erstellen
|
|
||||||
</button>
|
|
||||||
<a href="/export" class="font-medium text-blue-600 underline dark:text-blue-400">Zum Download</a>
|
|
||||||
</span>
|
|
||||||
</p>
|
</p>
|
||||||
{:else}
|
{:else}
|
||||||
<p class="text-red-700 dark:text-red-300">Keepsake-Erstellung fehlgeschlagen.</p>
|
<p class="text-red-700 dark:text-red-300">Keepsake-Erstellung fehlgeschlagen.</p>
|
||||||
<button
|
|
||||||
onclick={rebuildExport}
|
|
||||||
disabled={rebuilding}
|
|
||||||
data-testid="export-rebuild"
|
|
||||||
class="mt-2 rounded-lg bg-red-600 px-3 py-1.5 text-xs font-medium text-white transition hover:bg-red-700 disabled:opacity-50 dark:bg-red-500 dark:hover:bg-red-400"
|
|
||||||
>
|
|
||||||
{rebuilding ? 'Wird gestartet…' : 'Erneut versuchen'}
|
|
||||||
</button>
|
|
||||||
{/if}
|
{/if}
|
||||||
|
|
||||||
|
<!-- ONE button, mounted in every state — deliberately OUTSIDE the branches above.
|
||||||
|
Those branches are driven by SSE (`export-progress` / `export-available`), and
|
||||||
|
`rebuildExport` itself makes the backend broadcast `export-progress` at 0%
|
||||||
|
immediately. If the button lived inside a branch, activating it would flip
|
||||||
|
`exportGenerating` and UNMOUNT THE BUTTON BEING PRESSED — and a worker tick
|
||||||
|
landing between mousedown and mouseup would do the same unprompted. Chromium
|
||||||
|
fires no `click` when the element dies mid-sequence, so the host would tap
|
||||||
|
"Erneut versuchen" and nothing at all would happen, on the one screen whose
|
||||||
|
entire purpose is recovering a broken keepsake. (Same class of bug as the feed
|
||||||
|
autocomplete: never let a handler destroy the node that is being clicked.)
|
||||||
|
|
||||||
|
So the button's EXISTENCE is invariant; only its label and `disabled` change. -->
|
||||||
|
<button
|
||||||
|
onclick={() => {
|
||||||
|
// Rebuilding a READY keepsake is disruptive — guests who tap Download during
|
||||||
|
// the rebuild get nothing until it finishes — so it gets a confirm. A FAILED
|
||||||
|
// keepsake has nothing to lose, so it retries immediately.
|
||||||
|
if (exportReady) {
|
||||||
|
confirmAction = {
|
||||||
|
title: 'Keepsake neu erstellen?',
|
||||||
|
message:
|
||||||
|
'Das Keepsake wird aus dem aktuellen Stand der Galerie neu erzeugt. ' +
|
||||||
|
'Während der Erstellung können Gäste es nicht herunterladen.',
|
||||||
|
confirmLabel: 'Neu erstellen',
|
||||||
|
tone: 'danger',
|
||||||
|
run: rebuildExport
|
||||||
|
};
|
||||||
|
} else {
|
||||||
|
void rebuildExport();
|
||||||
|
}
|
||||||
|
}}
|
||||||
|
disabled={rebuilding || exportGenerating}
|
||||||
|
data-testid="export-rebuild"
|
||||||
|
class="mt-2 rounded-lg px-3 py-1.5 text-xs font-medium transition disabled:opacity-50
|
||||||
|
{exportReady
|
||||||
|
? 'text-gray-500 underline dark:text-gray-400'
|
||||||
|
: 'bg-red-600 text-white hover:bg-red-700 dark:bg-red-500 dark:hover:bg-red-400'}"
|
||||||
|
>
|
||||||
|
{rebuilding
|
||||||
|
? 'Wird gestartet…'
|
||||||
|
: exportReady
|
||||||
|
? 'Neu erstellen'
|
||||||
|
: 'Erneut versuchen'}
|
||||||
|
</button>
|
||||||
</div>
|
</div>
|
||||||
{/if}
|
{/if}
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
Reference in New Issue
Block a user