From e696e105ff1573e538427cc81b3c4de4e78dc3cf Mon Sep 17 00:00:00 2001 From: Hamdi LAADHARI Date: Wed, 2 Sep 2026 20:17:09 +0200 Subject: [PATCH] refactor(sound): extract playActionSound from FileViewer into src/sound.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FileViewer.tsx had a ~105-line inline Web Audio synthesis function (oscillator/gain/filter setup for the delete "whoosh" and keep "chime" sounds), untestable in jsdom and creating a brand-new AudioContext on every call. Move it to src/sound.ts, unchanged in what it plays: - playActionSound(action): void — no enabled parameter; FileViewer keeps the settings.soundEffects check itself, since it owns "when to play" - a lazily-created, module-level singleton AudioContext, reused across calls instead of created fresh each time - if construction fails once, that's cached so later calls skip retrying and re-logging (an environment-support gap, not transient) — scoped only to construction, not to a synthesis-time failure Also promotes the "delete" | "keep" union (independently duplicated across sound.ts, types.ts, FileViewer.tsx, and App.tsx) to one exported FileAction type in types.ts, per code review. Adds src/sound.test.ts: smoke-level coverage (constructs on delete/keep, reuses the same context across calls, doesn't throw when AudioContext is unavailable) — no assertions on exact frequencies/envelopes, since those are aesthetic choices, not correctness-critical. Co-Authored-By: Claude Sonnet 5 --- src/App.tsx | 3 +- src/components/FileViewer.tsx | 128 ++++------------------------------ src/sound.test.ts | 78 +++++++++++++++++++++ src/sound.ts | 125 +++++++++++++++++++++++++++++++++ src/types.ts | 4 +- 5 files changed, 223 insertions(+), 115 deletions(-) create mode 100644 src/sound.test.ts create mode 100644 src/sound.ts diff --git a/src/App.tsx b/src/App.tsx index 5e50e3a..191e751 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -4,6 +4,7 @@ import FileViewer from "./components/FileViewer"; import UpdateNotifier from "./components/UpdateNotifier"; import WelcomeScreen from "./components/WelcomeScreen"; import { useFileSession } from "./hooks/useFileSession"; +import type { FileAction } from "./types"; import "./electron.d.ts"; type AppState = "welcome" | "viewing" | "completed"; @@ -38,7 +39,7 @@ function App() { } }; - const handleFileAction = async (action: "delete" | "keep") => { + const handleFileAction = async (action: FileAction) => { if (action === "delete") { await deleteFile(); } else { diff --git a/src/components/FileViewer.tsx b/src/components/FileViewer.tsx index f18d38f..6cc384a 100644 --- a/src/components/FileViewer.tsx +++ b/src/components/FileViewer.tsx @@ -1,13 +1,20 @@ import { ArrowLeft, CheckCircle, Eye, RotateCcw, Settings, XCircle } from "lucide-react"; import type React from "react"; import { useCallback, useEffect, useState } from "react"; -import { type AppSettings, formatDate, formatFileSize, type SessionState } from "../types"; +import { playActionSound } from "../sound"; +import { + type AppSettings, + type FileAction, + formatDate, + formatFileSize, + type SessionState, +} from "../types"; import FilePreview from "./FilePreview"; import SettingsComponent from "./Settings"; interface FileViewerProps { sessionState: SessionState; - onFileAction: (action: "delete" | "keep") => Promise; + onFileAction: (action: FileAction) => Promise; onUndo: () => void; onBack: () => void; } @@ -25,7 +32,7 @@ const FileViewer: React.FC = ({ sessionState, onFileAction, onU }); const [actionFeedback, setActionFeedback] = useState<{ show: boolean; - type: "delete" | "keep"; + type: FileAction; }>({ show: false, type: "delete" }); // Load settings on component mount @@ -50,115 +57,8 @@ const FileViewer: React.FC = ({ sessionState, onFileAction, onU } }, []); - const playActionSound = useCallback( - (action: "delete" | "keep") => { - if (!settings.soundEffects) return; - - // Create audio context for sound feedback - try { - const audioContext = new ( - window.AudioContext || - (window as unknown as { webkitAudioContext: typeof AudioContext }).webkitAudioContext - )(); - - if (action === "delete") { - // Delete sound: Descending "whoosh" effect (like something being thrown away) - const oscillator1 = audioContext.createOscillator(); - const oscillator2 = audioContext.createOscillator(); - const gainNode = audioContext.createGain(); - const filter = audioContext.createBiquadFilter(); - - // Connect: oscillators -> filter -> gain -> destination - oscillator1.connect(filter); - oscillator2.connect(filter); - filter.connect(gainNode); - gainNode.connect(audioContext.destination); - - // Two oscillators for richer sound - oscillator1.frequency.setValueAtTime(300, audioContext.currentTime); - oscillator1.frequency.exponentialRampToValueAtTime(50, audioContext.currentTime + 0.3); - oscillator1.type = "sawtooth"; - - oscillator2.frequency.setValueAtTime(200, audioContext.currentTime); - oscillator2.frequency.exponentialRampToValueAtTime(30, audioContext.currentTime + 0.3); - oscillator2.type = "triangle"; - - // Low-pass filter for "whoosh" effect - filter.type = "lowpass"; - filter.frequency.setValueAtTime(800, audioContext.currentTime); - filter.frequency.exponentialRampToValueAtTime(200, audioContext.currentTime + 0.3); - - // Volume envelope - gainNode.gain.setValueAtTime(0.15, audioContext.currentTime); - gainNode.gain.exponentialRampToValueAtTime(0.01, audioContext.currentTime + 0.3); - - oscillator1.start(audioContext.currentTime); - oscillator1.stop(audioContext.currentTime + 0.3); - oscillator2.start(audioContext.currentTime); - oscillator2.stop(audioContext.currentTime + 0.3); - } else { - // Keep sound: Ascending "chime" effect (like a positive confirmation) - const oscillator1 = audioContext.createOscillator(); - const oscillator2 = audioContext.createOscillator(); - const oscillator3 = audioContext.createOscillator(); - const gainNode = audioContext.createGain(); - const filter = audioContext.createBiquadFilter(); - - // Connect: oscillators -> filter -> gain -> destination - oscillator1.connect(filter); - oscillator2.connect(filter); - oscillator3.connect(filter); - filter.connect(gainNode); - gainNode.connect(audioContext.destination); - - // Three oscillators for a pleasant chord - oscillator1.frequency.setValueAtTime(523.25, audioContext.currentTime); // C5 - oscillator1.frequency.exponentialRampToValueAtTime( - 659.25, - audioContext.currentTime + 0.2 - ); // E5 - oscillator1.type = "sine"; - - oscillator2.frequency.setValueAtTime(659.25, audioContext.currentTime); // E5 - oscillator2.frequency.exponentialRampToValueAtTime( - 783.99, - audioContext.currentTime + 0.2 - ); // G5 - oscillator2.type = "sine"; - - oscillator3.frequency.setValueAtTime(783.99, audioContext.currentTime); // G5 - oscillator3.frequency.exponentialRampToValueAtTime( - 1046.5, - audioContext.currentTime + 0.2 - ); // C6 - oscillator3.type = "sine"; - - // High-pass filter for bright, clear sound - filter.type = "highpass"; - filter.frequency.setValueAtTime(400, audioContext.currentTime); - - // Volume envelope with quick attack and decay - gainNode.gain.setValueAtTime(0, audioContext.currentTime); - gainNode.gain.linearRampToValueAtTime(0.12, audioContext.currentTime + 0.05); - gainNode.gain.exponentialRampToValueAtTime(0.01, audioContext.currentTime + 0.25); - - oscillator1.start(audioContext.currentTime); - oscillator1.stop(audioContext.currentTime + 0.25); - oscillator2.start(audioContext.currentTime); - oscillator2.stop(audioContext.currentTime + 0.25); - oscillator3.start(audioContext.currentTime); - oscillator3.stop(audioContext.currentTime + 0.25); - } - } catch (_error) { - // Silently fail if audio context is not available - console.log("Audio feedback not available"); - } - }, - [settings.soundEffects] - ); - const performAction = useCallback( - async (action: "delete" | "keep") => { + async (action: FileAction) => { // Check if we need confirmation for delete if (action === "delete" && settings.confirmDelete) { const confirmed = window.confirm(`Are you sure you want to delete "${currentFile.name}"?`); @@ -169,7 +69,9 @@ const FileViewer: React.FC = ({ sessionState, onFileAction, onU setActionFeedback({ show: true, type: action }); // Play audio feedback - playActionSound(action); + if (settings.soundEffects) { + playActionSound(action); + } // Trigger action after brief delay for feedback setTimeout(async () => { @@ -184,7 +86,7 @@ const FileViewer: React.FC = ({ sessionState, onFileAction, onU setActionFeedback({ show: false, type: action }); }, 200); }, - [onFileAction, playActionSound, settings.confirmDelete, currentFile.name] + [onFileAction, settings.confirmDelete, settings.soundEffects, currentFile.name] ); // Keyboard shortcuts diff --git a/src/sound.test.ts b/src/sound.test.ts new file mode 100644 index 0000000..4f3de0b --- /dev/null +++ b/src/sound.test.ts @@ -0,0 +1,78 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +// A fake AudioContext: enough of the graph (oscillator/gain/filter nodes, +// their scheduling methods) for sound.ts to run without throwing, so tests +// can assert on construction rather than on exact frequencies/envelopes. +function mockAudioContext() { + const node = () => ({ + connect: vi.fn(), + frequency: { setValueAtTime: vi.fn(), exponentialRampToValueAtTime: vi.fn() }, + gain: { + setValueAtTime: vi.fn(), + exponentialRampToValueAtTime: vi.fn(), + linearRampToValueAtTime: vi.fn(), + }, + type: "", + start: vi.fn(), + stop: vi.fn(), + }); + + return vi.fn(() => ({ + currentTime: 0, + destination: {}, + createOscillator: vi.fn(node), + createGain: vi.fn(node), + createBiquadFilter: vi.fn(node), + })); +} + +describe("playActionSound", () => { + beforeEach(() => { + // sound.ts caches its AudioContext in module state, so each test needs a + // fresh module instance to observe construction in isolation. + vi.resetModules(); + }); + + afterEach(() => { + // @ts-expect-error test-only cleanup of a global this suite sets up + window.AudioContext = undefined; + }); + + it("does not throw when AudioContext is unavailable", async () => { + const { playActionSound } = await import("./sound"); + + expect(() => playActionSound("delete")).not.toThrow(); + expect(() => playActionSound("keep")).not.toThrow(); + }); + + it("constructs an AudioContext to play a delete sound", async () => { + const ctor = mockAudioContext(); + window.AudioContext = ctor as unknown as typeof AudioContext; + const { playActionSound } = await import("./sound"); + + playActionSound("delete"); + + expect(ctor).toHaveBeenCalledOnce(); + }); + + it("constructs an AudioContext to play a keep sound", async () => { + const ctor = mockAudioContext(); + window.AudioContext = ctor as unknown as typeof AudioContext; + const { playActionSound } = await import("./sound"); + + playActionSound("keep"); + + expect(ctor).toHaveBeenCalledOnce(); + }); + + it("reuses the same AudioContext across multiple calls", async () => { + const ctor = mockAudioContext(); + window.AudioContext = ctor as unknown as typeof AudioContext; + const { playActionSound } = await import("./sound"); + + playActionSound("delete"); + playActionSound("keep"); + + expect(ctor).toHaveBeenCalledOnce(); + }); +}); diff --git a/src/sound.ts b/src/sound.ts new file mode 100644 index 0000000..a33c867 --- /dev/null +++ b/src/sound.ts @@ -0,0 +1,125 @@ +import type { FileAction } from "./types"; + +// Lazily created and reused across calls instead of a fresh AudioContext +// per call. undefined = not yet attempted, null = attempted and +// unavailable (a deterministic environment-support gap, not something +// that resolves mid-session, so we don't retry construction on every call). +let audioContext: AudioContext | null | undefined; + +function getAudioContext(): AudioContext | null { + if (audioContext !== undefined) return audioContext; + + try { + const Ctor = + window.AudioContext || + (window as unknown as { webkitAudioContext: typeof AudioContext }).webkitAudioContext; + audioContext = new Ctor(); + } catch (_error) { + // Silently fail if audio context is not available + console.log("Audio feedback not available"); + audioContext = null; + } + + return audioContext; +} + +// Delete sound: Descending "whoosh" effect (like something being thrown away) +function playDeleteSound(ctx: AudioContext) { + const oscillator1 = ctx.createOscillator(); + const oscillator2 = ctx.createOscillator(); + const gainNode = ctx.createGain(); + const filter = ctx.createBiquadFilter(); + + // Connect: oscillators -> filter -> gain -> destination + oscillator1.connect(filter); + oscillator2.connect(filter); + filter.connect(gainNode); + gainNode.connect(ctx.destination); + + // Two oscillators for richer sound + oscillator1.frequency.setValueAtTime(300, ctx.currentTime); + oscillator1.frequency.exponentialRampToValueAtTime(50, ctx.currentTime + 0.3); + oscillator1.type = "sawtooth"; + + oscillator2.frequency.setValueAtTime(200, ctx.currentTime); + oscillator2.frequency.exponentialRampToValueAtTime(30, ctx.currentTime + 0.3); + oscillator2.type = "triangle"; + + // Low-pass filter for "whoosh" effect + filter.type = "lowpass"; + filter.frequency.setValueAtTime(800, ctx.currentTime); + filter.frequency.exponentialRampToValueAtTime(200, ctx.currentTime + 0.3); + + // Volume envelope + gainNode.gain.setValueAtTime(0.15, ctx.currentTime); + gainNode.gain.exponentialRampToValueAtTime(0.01, ctx.currentTime + 0.3); + + oscillator1.start(ctx.currentTime); + oscillator1.stop(ctx.currentTime + 0.3); + oscillator2.start(ctx.currentTime); + oscillator2.stop(ctx.currentTime + 0.3); +} + +// Keep sound: Ascending "chime" effect (like a positive confirmation) +function playKeepSound(ctx: AudioContext) { + const oscillator1 = ctx.createOscillator(); + const oscillator2 = ctx.createOscillator(); + const oscillator3 = ctx.createOscillator(); + const gainNode = ctx.createGain(); + const filter = ctx.createBiquadFilter(); + + // Connect: oscillators -> filter -> gain -> destination + oscillator1.connect(filter); + oscillator2.connect(filter); + oscillator3.connect(filter); + filter.connect(gainNode); + gainNode.connect(ctx.destination); + + // Three oscillators for a pleasant chord + oscillator1.frequency.setValueAtTime(523.25, ctx.currentTime); // C5 + oscillator1.frequency.exponentialRampToValueAtTime(659.25, ctx.currentTime + 0.2); // E5 + oscillator1.type = "sine"; + + oscillator2.frequency.setValueAtTime(659.25, ctx.currentTime); // E5 + oscillator2.frequency.exponentialRampToValueAtTime(783.99, ctx.currentTime + 0.2); // G5 + oscillator2.type = "sine"; + + oscillator3.frequency.setValueAtTime(783.99, ctx.currentTime); // G5 + oscillator3.frequency.exponentialRampToValueAtTime(1046.5, ctx.currentTime + 0.2); // C6 + oscillator3.type = "sine"; + + // High-pass filter for bright, clear sound + filter.type = "highpass"; + filter.frequency.setValueAtTime(400, ctx.currentTime); + + // Volume envelope with quick attack and decay + gainNode.gain.setValueAtTime(0, ctx.currentTime); + gainNode.gain.linearRampToValueAtTime(0.12, ctx.currentTime + 0.05); + gainNode.gain.exponentialRampToValueAtTime(0.01, ctx.currentTime + 0.25); + + oscillator1.start(ctx.currentTime); + oscillator1.stop(ctx.currentTime + 0.25); + oscillator2.start(ctx.currentTime); + oscillator2.stop(ctx.currentTime + 0.25); + oscillator3.start(ctx.currentTime); + oscillator3.stop(ctx.currentTime + 0.25); +} + +// Plays a short Web Audio sound for a keep/delete action. Always plays — +// callers decide whether sound is wanted (e.g. a settings toggle) before +// calling this. +export function playActionSound(action: FileAction): void { + const ctx = getAudioContext(); + if (!ctx) return; + + try { + if (action === "delete") { + playDeleteSound(ctx); + } else { + playKeepSound(ctx); + } + } catch (_error) { + // Silently fail if playback itself throws + console.log("Audio feedback not available"); + } +} diff --git a/src/types.ts b/src/types.ts index 0276d27..58104af 100644 --- a/src/types.ts +++ b/src/types.ts @@ -4,8 +4,10 @@ import type { FileItem } from "../electron/ipc-types"; export type { AppSettings, FileItem } from "../electron/ipc-types"; +export type FileAction = "delete" | "keep"; + export interface UndoAction { - action: "delete" | "keep"; + action: FileAction; fileIndex: number; file: FileItem; }