From 4ec3bb297056ad2331551f6a3a93151ff7c630d0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 13:23:44 +0000 Subject: [PATCH] desktop: only toast for actions that replace the queue Undo entries now carry replacesQueue (play album/tracks, clear queue, restore saved queue) and the core's Undid/Redid toasts carry the entry id they report on. The desktop reducer announces only queue-replacing mutations and their undo/redo, and drops its own success toasts (diagnostics copied, config exported, filter exported). Error toasts and the deep-link prompt still show; every change stays undoable. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CMp2CmvGbYrTBopB2qgTsY --- .../src/test/resources/json-fixtures.json | 3 +- crates/hocket-android/src/fixtures.rs | 1 + crates/hocket-core/src/api.rs | 7 ++++ crates/hocket-core/src/core/actor.rs | 11 ++++++ .../hocket-core/src/core/handlers/library.rs | 2 +- .../hocket-core/src/core/handlers/session.rs | 6 ++- crates/hocket-core/src/undo/mod.rs | 9 +++++ desktop/e2e/a11y.spec.ts | 7 ++-- desktop/e2e/backup.spec.ts | 4 +- desktop/e2e/motion-contrast.spec.ts | 4 +- desktop/e2e/native.spec.ts | 8 ++-- desktop/e2e/undo.spec.ts | 18 ++++++--- desktop/src/main/fake-core/index.ts | 37 ++++++++++--------- desktop/src/renderer/store/actions.ts | 5 +-- desktop/src/renderer/store/reducer.test.ts | 33 ++++++++++++----- desktop/src/renderer/store/reducer.ts | 19 +++++++--- desktop/src/renderer/views/FilterBuilder.tsx | 6 --- desktop/src/renderer/views/Settings.tsx | 5 +-- desktop/src/shared/strings.ts | 3 -- 19 files changed, 119 insertions(+), 69 deletions(-) diff --git a/android/core/src/test/resources/json-fixtures.json b/android/core/src/test/resources/json-fixtures.json index abf5503..64fc005 100644 --- a/android/core/src/test/resources/json-fixtures.json +++ b/android/core/src/test/resources/json-fixtures.json @@ -640,7 +640,8 @@ "message": "Removed 3 tracks", "actionLabel": "Undo", "actionCommand": "{\"type\":\"undo\"}", - "durationMs": 5000 + "durationMs": 5000, + "undoEntryId": null } } }, diff --git a/crates/hocket-android/src/fixtures.rs b/crates/hocket-android/src/fixtures.rs index 92bcfa7..cd0602c 100644 --- a/crates/hocket-android/src/fixtures.rs +++ b/crates/hocket-android/src/fixtures.rs @@ -461,6 +461,7 @@ mod tests { action_label: Some("Undo".into()), action_command: Some(serde_json::to_string(&Command::Undo).unwrap()), duration_ms: 5000, + undo_entry_id: None, } }), ); diff --git a/crates/hocket-core/src/api.rs b/crates/hocket-core/src/api.rs index 1ef2724..3fc40e0 100644 --- a/crates/hocket-core/src/api.rs +++ b/crates/hocket-core/src/api.rs @@ -1279,6 +1279,10 @@ pub struct UndoEntry { pub at: EpochMs, /// e.g. "undid 487 of 500, 13 changed elsewhere" pub note: Option, + /// The action threw away the queue (played a new context, cleared it, + /// restored a saved queue), so undoing it is worth offering loudly. + #[serde(default)] + pub replaces_queue: bool, } #[typeshare] @@ -1292,6 +1296,9 @@ pub struct Toast { /// Command to dispatch when the button is pressed, JSON-encoded [`Command`]. pub action_command: Option, pub duration_ms: Ms, + /// Set on "Undid …" / "Redid …" toasts: the [`UndoEntry`] they report on. + #[serde(default)] + pub undo_entry_id: Option, } // --------------------------------------------------------------------------- diff --git a/crates/hocket-core/src/core/actor.rs b/crates/hocket-core/src/core/actor.rs index cea727d..1b53950 100644 --- a/crates/hocket-core/src/core/actor.rs +++ b/crates/hocket-core/src/core/actor.rs @@ -1078,6 +1078,16 @@ impl Actor { } pub(crate) fn toast(&mut self, message: impl Into, action: Option<(String, Command)>) { + self.toast_for(message, action, None); + } + + /// A toast reporting on an undo entry (the "Undid …" / "Redid …" pair). + pub(crate) fn toast_for( + &mut self, + message: impl Into, + action: Option<(String, Command)>, + undo_entry_id: Option, + ) { let (action_label, action_command) = match action { Some((l, c)) => (Some(l), serde_json::to_string(&c).ok()), None => (None, None), @@ -1089,6 +1099,7 @@ impl Actor { action_label, action_command, duration_ms: 5_000, + undo_entry_id, }, }); } diff --git a/crates/hocket-core/src/core/handlers/library.rs b/crates/hocket-core/src/core/handlers/library.rs index d452c4f..a47ccdb 100644 --- a/crates/hocket-core/src/core/handlers/library.rs +++ b/crates/hocket-core/src/core/handlers/library.rs @@ -744,7 +744,7 @@ impl Actor { } else { ("Redo".to_string(), Command::Redo) }; - self.toast(message, Some(action)); + self.toast_for(message, Some(action), Some(entry_id.to_string())); self.emit(Event::UndoChanged { state: self.undo.state(), }); diff --git a/crates/hocket-core/src/core/handlers/session.rs b/crates/hocket-core/src/core/handlers/session.rs index 0a2fce1..b1aeb92 100644 --- a/crates/hocket-core/src/core/handlers/session.rs +++ b/crates/hocket-core/src/core/handlers/session.rs @@ -940,7 +940,11 @@ impl Actor { ("Redo".to_string(), Command::Redo) }; if result.tier == UndoTier::SessionState { - self.toast(format!("{verb} {label}"), Some(action)); + self.toast_for( + format!("{verb} {label}"), + Some(action), + Some(result.entry_id.clone()), + ); } self.emit(Event::UndoChanged { state: self.undo.state(), diff --git a/crates/hocket-core/src/undo/mod.rs b/crates/hocket-core/src/undo/mod.rs index 6cb9474..866e2a6 100644 --- a/crates/hocket-core/src/undo/mod.rs +++ b/crates/hocket-core/src/undo/mod.rs @@ -429,10 +429,19 @@ impl Entry { device_id: self.device_id.clone(), at: self.at, note: self.note.clone(), + replaces_queue: replaces_queue(&self.kind), } } } +/// Undo kinds whose action replaces or empties the whole queue. +pub fn replaces_queue(kind: &str) -> bool { + matches!( + kind, + "playContext" | "playTracks" | "clearQueue" | "restoreSavedQueue" + ) +} + /// What the actor performs after `undo` / `redo`. #[derive(Debug, Clone, PartialEq)] pub struct UndoResult { diff --git a/desktop/e2e/a11y.spec.ts b/desktop/e2e/a11y.spec.ts index 907543c..8b5e435 100644 --- a/desktop/e2e/a11y.spec.ts +++ b/desktop/e2e/a11y.spec.ts @@ -175,11 +175,12 @@ test.describe("axe: no violations anywhere", () => { await expect(page.getByTestId("dialog-trackInfo").locator("dl")).toBeVisible(); await expectNoViolations(page, `${theme} track info dialog`); await page.keyboard.press("Escape"); - // A toast with an Undo action. - await page.getByTestId("shuffle").click(); + // A toast with an Undo action (only queue-replacing actions get one). + await page.evaluate(() => window.hocket.dispatch({ type: "clearQueue" })); await expect(page.getByTestId("toast").first()).toBeVisible(); await expectNoViolations(page, `${theme} toast`); - await page.getByTestId("shuffle").click(); + await page.evaluate(() => window.hocket.dispatch({ type: "undo" })); + await expect(page.getByTestId("queue-row-current")).toHaveCount(1); } }); diff --git a/desktop/e2e/backup.spec.ts b/desktop/e2e/backup.spec.ts index 0096f56..bcbd213 100644 --- a/desktop/e2e/backup.spec.ts +++ b/desktop/e2e/backup.spec.ts @@ -1,6 +1,6 @@ // Config backup round trip: an export with passwords resolved by main is // enough to restore a second, fresh install straight from the setup screen. -import { mkdirSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { completeSetup, expect, launchFake, stubDialogs, test } from "./fixtures"; @@ -24,7 +24,7 @@ test.describe("config backup", () => { await stubDialogs(app, file); await page.getByTestId("export-secrets").check(); await page.getByTestId("export-config").click(); - await expect(page.getByTestId("toast").last()).toContainText("Configuration exported"); + await expect.poll(() => existsSync(file)).toBe(true); const doc = JSON.parse(readFileSync(file, "utf8")) as { secrets?: Record; servers: { id: string; url: string }[] }; expect(doc.servers[0]?.url).toBe("https://music.example.org"); // Main resolved the keystore reference to the password entered at setup. diff --git a/desktop/e2e/motion-contrast.spec.ts b/desktop/e2e/motion-contrast.spec.ts index f55055a..6cf3e3c 100644 --- a/desktop/e2e/motion-contrast.spec.ts +++ b/desktop/e2e/motion-contrast.spec.ts @@ -192,8 +192,8 @@ test.describe("motion, contrast and zoom", () => { await page.keyboard.press("ArrowDown"); await expectNoViolations(page, `${theme} accent ${accent}: menu`); await page.keyboard.press("Escape"); - // A toast's action is the accent on the inverse surface. - await page.getByTestId("shuffle").click(); + // A toast's action is the accent on the inverse surface (replaying the album offers Undo). + await page.getByTestId("album-play").click(); await expect(page.getByTestId("toast-action").first()).toBeVisible(); await expectNoViolations(page, `${theme} accent ${accent}: toast`); await page.getByTestId("shuffle").click(); diff --git a/desktop/e2e/native.spec.ts b/desktop/e2e/native.spec.ts index 2d3023f..a9c9581 100644 --- a/desktop/e2e/native.spec.ts +++ b/desktop/e2e/native.spec.ts @@ -90,13 +90,13 @@ test.describe("real core against a fake Navidrome", () => { // Rate the current track from the album table; the outbox reaches the server; undo reverts. const row = page.getByTestId("track-row").first(); await row.locator(".stars .star").nth(3).click(); - const toast = page.getByTestId("toast").last(); - await expect(toast.getByTestId("toast-action")).toHaveText("Undo", { timeout: 10_000 }); await expect(row.locator(".stars .star.on")).toHaveCount(4, { timeout: 10_000 }); await expect.poll(() => server.callsTo("setRating").length, { timeout: 15_000 }).toBeGreaterThan(0); - await toast.getByTestId("toast-action").click(); + // Rating leaves the queue alone, so no toast: undo from the keyboard. + await expect(page.getByTestId("toast")).toHaveCount(0); + await page.getByTestId("content").click(); + await page.keyboard.press("Control+z"); await expect(row.locator(".stars .star.on")).toHaveCount(0, { timeout: 10_000 }); - await expect(page.getByTestId("toast").last()).toContainText("Undid"); // Lyrics from getLyricsBySongId render (line tier) in the right panel. await expect(page.getByTestId("lyrics-view")).toBeVisible({ timeout: 20_000 }); diff --git a/desktop/e2e/undo.spec.ts b/desktop/e2e/undo.spec.ts index ca2ca29..d3ee419 100644 --- a/desktop/e2e/undo.spec.ts +++ b/desktop/e2e/undo.spec.ts @@ -1,21 +1,27 @@ import { completeSetup, expect, playFirstAlbum, test } from "./fixtures"; test.describe("undo", () => { - test("a queue mutation shows a toast with the single Undo action and Ctrl+Z undoes it", async ({ hocket }) => { + test("replacing the queue shows a toast with the single Undo action; other mutations stay quiet", async ({ hocket }) => { const { page } = hocket; await completeSetup(page); await playFirstAlbum(page); await expect(page.getByTestId("queue-row-current")).toHaveCount(1); - await page.getByTestId("shuffle").click(); const toast = page.getByTestId("toast").last(); - await expect(toast).toContainText("Shuffle on"); + await expect(toast).toContainText("Play"); await expect(toast.getByTestId("toast-action")).toHaveText("Undo"); - await expect(page.getByTestId("shuffle")).toHaveAttribute("aria-pressed", "true"); await toast.getByTestId("toast-action").click(); - await expect(page.getByTestId("shuffle")).toHaveAttribute("aria-pressed", "false"); + await expect(page.getByTestId("queue-row-current")).toHaveCount(0); await expect(page.getByTestId("toast").last()).toContainText("Undone"); - // Keyboard redo/undo. + await page.getByTestId("toast-action").last().click(); + await expect(page.getByTestId("queue-row-current")).toHaveCount(1); + // Shuffle leaves the queue's contents alone: no toast, but still undoable. + await page.getByTestId("shuffle").click(); + await expect(page.getByTestId("shuffle")).toHaveAttribute("aria-pressed", "true"); + await expect(page.getByTestId("toasts")).not.toContainText("Shuffle"); + // Keyboard undo/redo. await page.getByTestId("content").click(); + await page.keyboard.press("Control+z"); + await expect(page.getByTestId("shuffle")).toHaveAttribute("aria-pressed", "false"); await page.keyboard.press("Control+Shift+z"); await expect(page.getByTestId("shuffle")).toHaveAttribute("aria-pressed", "true"); await page.keyboard.press("Control+z"); diff --git a/desktop/src/main/fake-core/index.ts b/desktop/src/main/fake-core/index.ts index 1987f29..a26a0b5 100644 --- a/desktop/src/main/fake-core/index.ts +++ b/desktop/src/main/fake-core/index.ts @@ -67,6 +67,8 @@ interface UndoRecord { id: string; label: string; at: number; + /** Like the core's `replacesQueue`: the action threw the queue away. */ + replacesQueue: boolean; undo: () => string | undefined; redo: () => void; selection?: ActionTarget; @@ -341,7 +343,7 @@ export class FakeCore implements CoreHandle { this.withUndo("Clear queue", () => { this.session = { ...this.session, context: undefined, order: [], cursor: 0, current: undefined, insertions: [], history: [] }; this.stop(); - }); + }, true); return; case "clearInsertions": this.withUndo("Clear playing next", () => { @@ -766,7 +768,7 @@ export class FakeCore implements CoreHandle { if (!silent && username.toLowerCase() === "wrong") { // Like the core: a failed probe is Error + toast only; no ServersChanged, no job, no problem. this.later(600, () => { - this.toast("Couldn't reach the server: authentication failed: Wrong username or password", false); + this.toast("Couldn't reach the server: authentication failed: Wrong username or password"); this.emit({ type: "error", data: { kind: "auth", message: "server probe failed", detail: "authentication failed: Wrong username or password" } }); }); return; @@ -900,7 +902,7 @@ export class FakeCore implements CoreHandle { this.restoreSession(before); this.seek(outgoingPos); return undefined; - }, () => this.playContext(resolved, startIndex, shuffle, false)); + }, () => this.playContext(resolved, startIndex, shuffle, false), undefined, true); this.emitAll(); } @@ -919,7 +921,6 @@ export class FakeCore implements CoreHandle { } } }); - this.toast(next ? "Playing next" : "Added to queue", true); } private jumpTo(key: string): void { @@ -1356,7 +1357,7 @@ export class FakeCore implements CoreHandle { this.pushUndo(`Restore ${q.label}`, () => { this.restoreSession(before); return undefined; - }, () => this.restoreSavedQueue(id)); + }, () => this.restoreSavedQueue(id), undefined, true); this.emitAll(); } @@ -1384,7 +1385,7 @@ export class FakeCore implements CoreHandle { this.emitAll(); } - private withUndo(label: string, mutate: () => void): void { + private withUndo(label: string, mutate: () => void, replacesQueue = false): void { const before = this.captureSession(); const selection = clone(this.selection); mutate(); @@ -1393,19 +1394,19 @@ export class FakeCore implements CoreHandle { this.pushUndo(label, () => { this.restoreSession(before); return undefined; - }, () => this.restoreSession(after), selection); + }, () => this.restoreSession(after), selection, replacesQueue); this.emitAll(); } - private pushUndo(label: string, undo: () => string | undefined, redo: () => void, selection?: ActionTarget): void { + private pushUndo(label: string, undo: () => string | undefined, redo: () => void, selection?: ActionTarget, replacesQueue = false): void { const now = Date.now(); const top = this.undoStack[this.undoStack.length - 1]; // Coalesce rapid repeats of the same label (dragging a rating, holding a key). - if (top && top.label === label && now - top.at < 800) { + if (top && top.label === label && top.replacesQueue === replacesQueue && now - top.at < 800) { top.redo = redo; top.at = now; } else { - this.undoStack.push({ id: newId("undo"), label, at: now, undo, redo, selection }); + this.undoStack.push({ id: newId("undo"), label, at: now, replacesQueue, undo, redo, selection }); if (this.undoStack.length > 200) this.undoStack.shift(); } this.redoStack = []; @@ -1419,7 +1420,7 @@ export class FakeCore implements CoreHandle { if (rec.selection) this.selection = rec.selection; this.redoStack.push(rec); this.emitUndo(); - this.emit({ type: "toast", data: { toast: { id: newId("toast"), message: note ? `Undone: ${rec.label} (${note})` : `Undone: ${rec.label}`, actionLabel: "Redo", actionCommand: JSON.stringify({ type: "redo" } satisfies Command), durationMs: 5000 } } }); + this.emit({ type: "toast", data: { toast: { id: newId("toast"), message: note ? `Undone: ${rec.label} (${note})` : `Undone: ${rec.label}`, actionLabel: "Redo", actionCommand: JSON.stringify({ type: "redo" } satisfies Command), durationMs: 5000, undoEntryId: rec.id } } }); } private redo(): void { @@ -1428,18 +1429,18 @@ export class FakeCore implements CoreHandle { rec.redo(); this.undoStack.push(rec); this.emitUndo(); - this.emit({ type: "toast", data: { toast: { id: newId("toast"), message: `Redone: ${rec.label}`, actionLabel: "Undo", actionCommand: JSON.stringify({ type: "undo" } satisfies Command), durationMs: 5000 } } }); + this.emit({ type: "toast", data: { toast: { id: newId("toast"), message: `Redone: ${rec.label}`, actionLabel: "Undo", actionCommand: JSON.stringify({ type: "undo" } satisfies Command), durationMs: 5000, undoEntryId: rec.id } } }); } private undoState(): UndoState { const top = this.undoStack[this.undoStack.length - 1]; const rtop = this.redoStack[this.redoStack.length - 1]; - const history: UndoEntry[] = [...this.undoStack].reverse().slice(0, 20).map((u) => ({ id: u.id, label: u.label, deviceId: this.config.deviceId, at: u.at, note: undefined })); + const history: UndoEntry[] = [...this.undoStack].reverse().slice(0, 20).map((u) => ({ id: u.id, label: u.label, deviceId: this.config.deviceId, at: u.at, note: undefined, replacesQueue: u.replacesQueue })); return { canUndo: !!top, undoLabel: top?.label, canRedo: !!rtop, redoLabel: rtop?.label, history }; } - private toast(message: string, undoable: boolean): void { - const toast: Toast = { id: newId("toast"), message, actionLabel: undoable ? "Undo" : undefined, actionCommand: undoable ? JSON.stringify({ type: "undo" } satisfies Command) : undefined, durationMs: 4000 }; + private toast(message: string): void { + const toast: Toast = { id: newId("toast"), message, actionLabel: undefined, actionCommand: undefined, durationMs: 4000 }; this.emit({ type: "toast", data: { toast } }); } @@ -1538,7 +1539,7 @@ export class FakeCore implements CoreHandle { trackIds: [...trackIds], }); this.libraryChanged("playlists", [id]); - this.toast(`Created playlist ${name}`, false); + this.toast(`Created playlist ${name}`); } private playlistAdd(playlistId: string, trackIds: string[], atIndex: number | undefined): void { @@ -1880,7 +1881,7 @@ export class FakeCore implements CoreHandle { this.emit({ type: "filtersChanged", data: { filters: this.filters } }); this.emit({ type: "shortcutsChanged", data: { shortcuts: this.shortcuts() } }); this.emit({ type: "audioSettingsChanged", data: { settings: this.audio } }); - this.toast("Configuration imported", false); + this.toast("Configuration imported"); } catch (err) { this.emit({ type: "error", data: { kind: "storage", message: "Couldn't import the configuration document", detail: String(err) } }); } @@ -1897,7 +1898,7 @@ export class FakeCore implements CoreHandle { this.emit({ type: "devicesChanged", data: { devices: this.devices } }); this.emitTransport(); this.emitMediaSession(); - this.toast(`Playing on ${target.name}`, false); + this.toast(`Playing on ${target.name}`); } private resumeHere(): void { diff --git a/desktop/src/renderer/store/actions.ts b/desktop/src/renderer/store/actions.ts index 5a67df2..364d274 100644 --- a/desktop/src/renderer/store/actions.ts +++ b/desktop/src/renderer/store/actions.ts @@ -101,10 +101,7 @@ export async function executeAction(rawId: string, target: ActionTarget = { type return; case "copyDiagnostics": { const r = await b.query({ type: "diagnostics" }); - if (r.type === "text") { - b.clipboard.writeText(r.data); - app.applyEvent({ type: "toast", data: { toast: { id: `diag-${Date.now()}`, message: t("settings.diagnosticsCopied"), actionLabel: undefined, actionCommand: undefined, durationMs: 3000 } } }); - } + if (r.type === "text") b.clipboard.writeText(r.data); return; } case "goToAlbum": { diff --git a/desktop/src/renderer/store/reducer.test.ts b/desktop/src/renderer/store/reducer.test.ts index b5ed565..bf20d8a 100644 --- a/desktop/src/renderer/store/reducer.test.ts +++ b/desktop/src/renderer/store/reducer.test.ts @@ -63,22 +63,37 @@ describe("reducer", () => { expect(s.network).toBeUndefined(); }); - it("announces a fresh undo entry with one Undo toast, never twice, and not on undo/redo", () => { - const entry = (id: string, label: string) => ({ id, label, deviceId: "me", at: 1, note: undefined }); - let s = reduce(initialCoreState, { type: "undoChanged", data: { state: { canUndo: true, undoLabel: "Rate ★★★★", canRedo: false, redoLabel: undefined, history: [entry("u1", "Rate ★★★★")] } } }); - expect(s.toasts.map((t) => [t.message, t.actionLabel])).toEqual([["Rate ★★★★", "Undo"]]); + it("announces a fresh queue-replacing undo entry with one Undo toast, never twice, and not on undo/redo", () => { + const entry = (id: string, label: string, replacesQueue = true) => ({ id, label, deviceId: "me", at: 1, note: undefined, replacesQueue }); + let s = reduce(initialCoreState, { type: "undoChanged", data: { state: { canUndo: true, undoLabel: "Play Album", canRedo: false, redoLabel: undefined, history: [entry("u1", "Play Album")] } } }); + expect(s.toasts.map((t) => [t.message, t.actionLabel])).toEqual([["Play Album", "Undo"]]); // Undo pops it: no new toast (the core sends its own "Undid …"). - s = reduce(s, { type: "undoChanged", data: { state: { canUndo: false, undoLabel: undefined, canRedo: true, redoLabel: "Rate ★★★★", history: [] } } }); + s = reduce(s, { type: "undoChanged", data: { state: { canUndo: false, undoLabel: undefined, canRedo: true, redoLabel: "Play Album", history: [] } } }); expect(s.toasts).toHaveLength(1); // Redo pushes the same id back: already announced, no toast. - s = reduce(s, { type: "undoChanged", data: { state: { canUndo: true, undoLabel: "Rate ★★★★", canRedo: false, redoLabel: undefined, history: [entry("u1", "Rate ★★★★")] } } }); + s = reduce(s, { type: "undoChanged", data: { state: { canUndo: true, undoLabel: "Play Album", canRedo: false, redoLabel: undefined, history: [entry("u1", "Play Album")] } } }); expect(s.toasts).toHaveLength(1); - s = reduce(s, { type: "undoChanged", data: { state: { canUndo: true, undoLabel: "Shuffle on", canRedo: false, redoLabel: undefined, history: [entry("u2", "Shuffle on"), entry("u1", "Rate ★★★★")] } } }); - expect(s.toasts.map((t) => t.message)).toEqual(["Rate ★★★★", "Shuffle on"]); + s = reduce(s, { type: "undoChanged", data: { state: { canUndo: true, undoLabel: "Clear queue", canRedo: false, redoLabel: undefined, history: [entry("u2", "Clear queue"), entry("u1", "Play Album")] } } }); + expect(s.toasts.map((t) => t.message)).toEqual(["Play Album", "Clear queue"]); + }); + + it("stays quiet for undo entries that leave the queue alone, and for their Undid/Redid toasts", () => { + const entry = (id: string, label: string, replacesQueue: boolean) => ({ id, label, deviceId: "me", at: 1, note: undefined, replacesQueue }); + const undid = (id: string, undoEntryId: string, message: string): Event => ({ type: "toast", data: { toast: { id, message, actionLabel: "Redo", actionCommand: JSON.stringify({ type: "redo" }), durationMs: 5000, undoEntryId } } }); + let s = reduce(initialCoreState, { type: "undoChanged", data: { state: { canUndo: true, undoLabel: "Shuffle on", canRedo: false, redoLabel: undefined, history: [entry("u1", "Shuffle on", false)] } } }); + expect(s.toasts).toEqual([]); + s = reduce(s, undid("t1", "u1", "Undid Shuffle on")); + expect(s.toasts).toEqual([]); + s = reduce(s, { type: "undoChanged", data: { state: { canUndo: true, undoLabel: "Play Album", canRedo: false, redoLabel: undefined, history: [entry("u2", "Play Album", true)] } } }); + s = reduce(s, undid("t2", "u2", "Undid Play Album")); + expect(s.toasts.map((t) => t.message)).toEqual(["Play Album", "Undid Play Album"]); + // A toast that isn't about an undo entry (an error) still shows. + s = reduce(s, { type: "toast", data: { toast: { id: "t3", message: "Nothing to play", actionLabel: undefined, actionCommand: undefined, durationMs: 5000, undoEntryId: undefined } } }); + expect(s.toasts.at(-1)?.message).toBe("Nothing to play"); }); it("keeps announcing fresh undo entries once the core caps the history it sends", () => { - const entry = (id: string) => ({ id, label: `Mutation ${id}`, deviceId: "me", at: 1, note: undefined }); + const entry = (id: string) => ({ id, label: `Mutation ${id}`, deviceId: "me", at: 1, note: undefined, replacesQueue: true }); const cap = 50; let s = initialCoreState; let history: ReturnType[] = []; diff --git a/desktop/src/renderer/store/reducer.ts b/desktop/src/renderer/store/reducer.ts index 9b7de18..9e3d18b 100644 --- a/desktop/src/renderer/store/reducer.ts +++ b/desktop/src/renderer/store/reducer.ts @@ -47,6 +47,8 @@ export interface CoreState { lastError: { kind: string; message: string; detail?: string; at: number } | undefined; /** Undo entry ids already announced with a toast (the core only emits UndoChanged for a fresh mutation). */ announcedUndo: string[]; + /** Undo entry ids whose action replaced or cleared the queue: the only ones that get toasts. */ + queueReplacingUndo: string[]; exported: { kind: "nsp" | "config"; document: string; path?: string; at: number } | undefined; } @@ -91,6 +93,7 @@ export const initialCoreState: CoreState = { lastError: undefined, exported: undefined, announcedUndo: [], + queueReplacingUndo: [], }; @@ -150,19 +153,25 @@ export function reduce(state: CoreState, e: Event): CoreState { case "savedQueuesChanged": return { ...state, savedQueues: e.data.queues }; case "undoChanged": { - // design.md "Global undo": a fresh mutation gets one toast with the single - // Undo action. The core emits only UndoChanged for it (its own toasts are - // "Undid …/Redid …"), so announce the newest entry here, once per id. + // design.md "Global undo": a fresh mutation that throws the queue away gets + // one toast with the single Undo action; everything else stays quiet (the + // undo button and history still cover it). The core emits only UndoChanged + // for it (its own toasts are "Undid …/Redid …"), so announce the newest + // entry here, once per id. // Freshness is keyed on the top entry's id, never on the history length: // the core caps the history it sends (HISTORY_SHEET_LIMIT, byte budget), // so the length stops growing long before the user stops mutating. const top = e.data.state.history[0]; const fresh = top && e.data.state.canUndo && !state.announcedUndo.includes(top.id) && top.id !== state.undo.history[0]?.id; - const toasts = fresh ? [...state.toasts, { id: `undo-${top.id}`, message: top.label, actionLabel: "Undo", actionCommand: JSON.stringify({ type: "undo" }), durationMs: 5000 }].slice(-4) : state.toasts; + const toasts = fresh && top.replacesQueue ? [...state.toasts, { id: `undo-${top.id}`, message: top.label, actionLabel: "Undo", actionCommand: JSON.stringify({ type: "undo" }), durationMs: 5000 }].slice(-4) : state.toasts; const announced = fresh ? [...state.announcedUndo, top.id].slice(-500) : state.announcedUndo; - return { ...state, undo: e.data.state, toasts, announcedUndo: announced }; + const replacing = e.data.state.history.filter((h) => h.replacesQueue && !state.queueReplacingUndo.includes(h.id)).map((h) => h.id); + const queueReplacingUndo = replacing.length ? [...state.queueReplacingUndo, ...replacing].slice(-500) : state.queueReplacingUndo; + return { ...state, undo: e.data.state, toasts, announcedUndo: announced, queueReplacingUndo }; } case "toast": + // "Undid …/Redid …" only for an entry that replaced the queue. + if (e.data.toast.undoEntryId && !state.queueReplacingUndo.includes(e.data.toast.undoEntryId)) return state; return { ...state, toasts: [...state.toasts.filter((t) => t.id !== e.data.toast.id), e.data.toast].slice(-4) }; case "playerNotice": return { ...state, playerNotice: e.data.message || e.data.code ? { message: e.data.message ?? undefined, code: e.data.code ?? undefined, detail: e.data.detail ?? undefined } : undefined }; diff --git a/desktop/src/renderer/views/FilterBuilder.tsx b/desktop/src/renderer/views/FilterBuilder.tsx index 9f026cd..5831224 100644 --- a/desktop/src/renderer/views/FilterBuilder.tsx +++ b/desktop/src/renderer/views/FilterBuilder.tsx @@ -51,7 +51,6 @@ export function FilterBuilder({ id }: { id: string }) { const navigate = useApp((s) => s.navigate); const serverId = useApp((s) => s.servers[0]?.id ?? ""); const nativeApi = useApp((s) => s.servers[0]?.capabilities.nativeApi ?? false); - const exported = useApp((s) => s.exported); const existing = filters.find((f) => f.id === id); const [filter, setFilter] = useState(() => existing ?? { id: `flt-${Date.now().toString(36)}`, name: "", root: { type: "all", data: [{ type: "rule", data: defaultRule() }] }, sort: "title", descending: false, limit: undefined }); const [preview, setPreview] = useState(undefined); @@ -62,11 +61,6 @@ export function FilterBuilder({ id }: { id: string }) { }, 150); return () => clearTimeout(h); }, [filter]); - useEffect(() => { - if (exported?.kind === "nsp" && Date.now() - exported.at < 2000 && exported.path) { - useApp.getState().applyEvent({ type: "toast", data: { toast: { id: `nsp-${exported.at}`, message: t("filters.exported", { path: exported.path }), actionLabel: undefined, actionCommand: undefined, durationMs: 4000 } } }); - } - }, [exported]); const valid = filter.name.trim().length > 0; const save = () => bridge().dispatch({ type: "saveFilter", data: { filter: { ...filter, name: filter.name.trim() } } }); diff --git a/desktop/src/renderer/views/Settings.tsx b/desktop/src/renderer/views/Settings.tsx index a1aa9e9..7c3e95a 100644 --- a/desktop/src/renderer/views/Settings.tsx +++ b/desktop/src/renderer/views/Settings.tsx @@ -630,10 +630,7 @@ function Backup() { setPending(undefined); // Main resolves the keystore references (with a plain-text warning) and // writes the file to a path the user picks; the page never sees a path. - void bridge().config.export(doc).then((path) => { - if (!path) return; - useApp.getState().applyEvent({ type: "toast", data: { toast: { id: `cfg-${Date.now()}`, message: t("settings.configExported"), actionLabel: undefined, actionCommand: undefined, durationMs: 3000 } } }); - }).catch((err: unknown) => console.error("config export failed", err)); + void bridge().config.export(doc).catch((err: unknown) => console.error("config export failed", err)); }, [exported, pending]); // Main picks the file, confirms, adds any servers whose passwords the file carries, then imports. const doImport = () => bridge().config.import().catch((err: unknown) => console.error("config import failed", err)); diff --git a/desktop/src/shared/strings.ts b/desktop/src/shared/strings.ts index a6113fa..919c9e8 100644 --- a/desktop/src/shared/strings.ts +++ b/desktop/src/shared/strings.ts @@ -272,7 +272,6 @@ const STRINGS = { "filters.descending": "Descending", "filters.limit": "Limit", "filters.delete": "Delete filter", - "filters.exported": "Exported to {path}", "filters.smartUnavailable": "Needs the native API (behind a capability flag)", "filters.sample": "Sample", @@ -364,8 +363,6 @@ const STRINGS = { "settings.importConfig": "Import configuration…", "settings.importConfirm": "Importing replaces your settings, filters and shortcuts on this device. Continue?", "settings.copyDiagnostics": "Copy diagnostics", - "settings.diagnosticsCopied": "Diagnostics copied to the clipboard", - "settings.configExported": "Configuration exported", "settings.credentialsVolatile": "No usable OS keyring was found, so server passwords are kept in memory only: you'll be asked to sign in again after restarting Hocket.", "config.exportSecretsTitle": "Export server passwords in plain text?", "config.exportSecretsDetail": "{n} server password(s) will be written into the file unencrypted. Anyone who can read the file can sign in as you. Keep it private and delete it after importing.",