From 0201e2eff838384c31aafa534ddd7a11b9fcdcf6 Mon Sep 17 00:00:00 2001 From: Ross Cadogan Date: Thu, 2 Jul 2026 22:34:11 +0100 Subject: [PATCH] fix(voices): apply settings-page voice changes to host menus immediately (#475) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Picking a voice in the settings catalog switched TTS at once (the preference cache updates on the storage write) but the host page's selector kept showing the old voice until its next repopulate. PreferenceModule already relays the storage change into the tab as a userPreferenceChanged event carrying voicePreferences — the selectors just never listened. VoiceSelector now subscribes and applies the re-read stored voice in place: Claude updates its trigger label + checkmarks (and pins a shortlist-hidden voice for the next populate); the Pi button grids re-mark the selected row. No repopulate, so an open menu is never torn down. Closes #475 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012DZR4Vp9B1RiKVrkfYiM1o --- src/chatbots/ClaudeVoiceMenu.ts | 23 +++ src/tts/VoiceMenu.ts | 61 +++++++ .../VoiceMenu-preference-sync.spec.ts | 171 ++++++++++++++++++ 3 files changed, 255 insertions(+) create mode 100644 test/chatbots/VoiceMenu-preference-sync.spec.ts diff --git a/src/chatbots/ClaudeVoiceMenu.ts b/src/chatbots/ClaudeVoiceMenu.ts index 3f23402cb3..ddd351ae27 100644 --- a/src/chatbots/ClaudeVoiceMenu.ts +++ b/src/chatbots/ClaudeVoiceMenu.ts @@ -643,6 +643,29 @@ export class ClaudeVoiceMenu extends VoiceSelector { delete voiceSelector.dataset.clickListenerAdded; } + /** + * The Claude selector is a dropdown, not a button grid: reflect an + * externally-changed voice on the trigger button + row checkmarks. If the + * shortlist is hiding the new voice, pin it for the next populate — the + * menu rebuilds from the (now-correct) button label on every open, so the + * row appears then. Never tears the menu down, so safe while it is open. + */ + protected override applySelectedVoice( + voice: SpeechSynthesisVoiceRemote | null + ): void { + this.updateSelectedVoice(voice); + if (!voice) { + this.pinnedCustomVoiceId = null; + return; + } + const hasRow = Array.from( + this.menuContent.querySelectorAll("[role='menuitem']") + ).some((item) => item.dataset.voiceName === voice.name); + if (!hasRow) { + this.pinnedCustomVoiceId = voice.id; + } + } + private updateSelectedVoice( selectedVoice: SpeechSynthesisVoiceRemote | null ): void { diff --git a/src/tts/VoiceMenu.ts b/src/tts/VoiceMenu.ts index f8bf4ce3c3..6978597d1f 100644 --- a/src/tts/VoiceMenu.ts +++ b/src/tts/VoiceMenu.ts @@ -52,6 +52,7 @@ export abstract class VoiceSelector { this.userPreferences = userPreferences; this.element = element; this.registerAuthenticationChangeHandler(); + this.registerVoicePreferenceChangeHandler(); } abstract getId(): string; @@ -73,6 +74,66 @@ export abstract class VoiceSelector { }); } + /** + * Registers a listener for voice-preference changes that originate outside + * this menu. Picking a voice in the settings-page catalog persists to + * chrome.storage, which PreferenceModule relays into the host tab as a + * "userPreferenceChanged" event carrying `voicePreferences`. TTS switches + * voices the moment that write lands, so the selector must follow in the + * same beat rather than waiting for its next repopulate (#475). + */ + protected registerVoicePreferenceChangeHandler(): void { + EventBus.on( + "userPreferenceChanged", + (detail: { + voicePreferences?: unknown; + voiceChatbotId?: string | null; + }) => { + if (!detail || detail.voicePreferences === undefined) { + return; // not a voice-preference event + } + if ( + detail.voiceChatbotId && + detail.voiceChatbotId !== this.chatbot.getID() + ) { + return; // another host's selection + } + // Re-read through the preference module (its cache is already + // updated) so we apply a resolved voice object, not a bare id. + this.userPreferences.getVoice(this.chatbot).then((voice) => { + this.applySelectedVoice(voice ?? null); + }); + } + ); + } + + /** + * Reflect an externally-changed stored voice on this surface without a + * repopulate (and therefore without disturbing an open menu). The base + * covers button-grid surfaces (Pi's menus); dropdown-style selectors + * override (ClaudeVoiceMenu). + */ + protected applySelectedVoice(voice: SpeechSynthesisVoiceRemote | null): void { + if (!voice) { + this.element + .querySelectorAll("button.saypi-custom-voice") + .forEach((button) => this.unmarkButtonAsSelectedVoice(button)); + return; + } + const target = this.element.querySelector( + `button[data-voice-id="${voice.id}"]` + ); + if (!target) { + // Hidden by the shortlist cap: pin it so the next populate shows it. + this.pinnedCustomVoiceId = voice.id; + return; + } + this.element.querySelectorAll("button").forEach((button) => { + this.unmarkButtonAsSelectedVoice(button as HTMLButtonElement); + }); + this.markButtonAsSelectedVoice(target); + } + /** * Text-to-speech via Say, Pi requires authentication. When the user is signed * out AND there are no voices to offer (the /voices request returns [] on a diff --git a/test/chatbots/VoiceMenu-preference-sync.spec.ts b/test/chatbots/VoiceMenu-preference-sync.spec.ts new file mode 100644 index 0000000000..c38a110eba --- /dev/null +++ b/test/chatbots/VoiceMenu-preference-sync.spec.ts @@ -0,0 +1,171 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +// ConfigModule reads injected env at import time; stub it (mirrors other specs). +vi.mock("../../src/ConfigModule", () => ({ + config: { + appServerUrl: "https://app.example.com", + apiServerUrl: "https://api.saypi.ai", + GA_MEASUREMENT_ID: "x", + GA_API_SECRET: "x", + GA_ENDPOINT: "x", + }, +})); + +vi.mock("../../src/JwtManager", () => ({ + getJwtManagerSync: () => ({ + isAuthenticated: () => true, + getClaims: () => ({ ttsQuotaRemaining: 1000 }), + }), +})); + +const openSettingsMock = vi.fn(); +vi.mock("../../src/popup/popupopener", () => ({ + openSettings: (...args: unknown[]) => openSettingsMock(...args), +})); + +import { ClaudeVoiceMenu } from "../../src/chatbots/ClaudeVoiceMenu"; +import { PiVoiceMenu } from "../../src/chatbots/PiVoiceMenu"; +import EventBus from "../../src/events/EventBus"; +import { claudeMockVoices } from "../data/Voices"; +import { ElevenLabsVoice } from "../data/Voices"; +import { SpeechSynthesisVoiceRemote } from "../../src/tts/SpeechModel"; + +// Selecting a voice in the settings catalog reaches the host tab as a +// chrome.storage.onChanged → EventBus "userPreferenceChanged" event carrying +// {voicePreferences, voiceId, voiceChatbotId} (PreferenceModule). The voice +// selectors must apply it immediately — the founder-reported defect (#475) was +// the Claude button label lagging while TTS had already switched voices. + +function flushAsync(): Promise { + return new Promise((resolve) => setTimeout(resolve, 0)); +} + +function emitVoiceChange(voiceId: string | null, chatbotId: string): void { + EventBus.emit("userPreferenceChanged", { + voicePreferences: voiceId ? { [chatbotId]: voiceId } : {}, + voiceId, + voiceChatbotId: chatbotId, + }); +} + +function makeClaudeMenu(stored: () => SpeechSynthesisVoiceRemote | null): any { + const menu = Object.create(ClaudeVoiceMenu.prototype); + menu.chatbot = { getID: () => "claude" } as any; + menu.userPreferences = { + getVoice: vi.fn(async () => stored()), + setVoice: vi.fn(async () => {}), + unsetVoice: vi.fn(async () => {}), + }; + menu.element = document.createElement("div"); + menu.element.id = "claude-voice-selector"; + document.body.appendChild(menu.element); + menu.menuButton = document.createElement("button"); + menu.menuContent = document.createElement("div"); + menu.toggleMenu = vi.fn(); + (menu as any).registerVoicePreferenceChangeHandler(); + return menu; +} + +const piVoices = [ + new ElevenLabsVoice("ig1TeITnnNlsJtfHxJlW", "Paola"), + new ElevenLabsVoice("bWJPewAagbymiJXZcxnh", "Joey"), +]; + +function makePiMenu(stored: () => SpeechSynthesisVoiceRemote | null): any { + const menu = Object.create(PiVoiceMenu.prototype); + menu.chatbot = { getID: () => "pi" } as any; + menu.userPreferences = { + getVoice: vi.fn(async () => stored()), + setVoice: vi.fn(async () => {}), + unsetVoice: vi.fn(async () => {}), + }; + menu.element = document.createElement("div"); + document.body.appendChild(menu.element); + (menu as any).registerVoicePreferenceChangeHandler(); + return menu; +} + +beforeEach(() => { + document.body.innerHTML = ""; +}); + +describe("ClaudeVoiceMenu reacts to settings-page voice changes (#475)", () => { + it("updates the button label and checkmark as soon as the preference event arrives", async () => { + const cassidy = claudeMockVoices.find((v) => v.name === "Cassidy")!; + const jarnathan = claudeMockVoices.find((v) => v.name === "Jarnathan")!; + let stored: SpeechSynthesisVoiceRemote | null = cassidy; + const menu = makeClaudeMenu(() => stored); + menu.populateVoices(claudeMockVoices, menu.element); + await flushAsync(); + expect( + menu.menuButton.querySelector(".voice-name")?.textContent + ).toBe("Cassidy"); + + stored = jarnathan; // the settings page persisted a new voice... + emitVoiceChange(jarnathan.id, "claude"); // ...and storage.onChanged relayed it + await flushAsync(); + + expect( + menu.menuButton.querySelector(".voice-name")?.textContent + ).toBe("Jarnathan"); + const row = menu.menuContent.querySelector( + "[data-voice-name='Jarnathan'] .checkmark-container" + ) as HTMLElement; + expect(row.innerHTML).not.toBe(""); + }); + + it("ignores voice changes addressed to another chatbot", async () => { + const cassidy = claudeMockVoices.find((v) => v.name === "Cassidy")!; + let stored: SpeechSynthesisVoiceRemote | null = cassidy; + const menu = makeClaudeMenu(() => stored); + menu.populateVoices(claudeMockVoices, menu.element); + await flushAsync(); + + emitVoiceChange("some-pi-voice", "pi"); + await flushAsync(); + + expect( + menu.menuButton.querySelector(".voice-name")?.textContent + ).toBe("Cassidy"); + }); + + it("shows Voice off when the preference is cleared elsewhere", async () => { + const cassidy = claudeMockVoices.find((v) => v.name === "Cassidy")!; + let stored: SpeechSynthesisVoiceRemote | null = cassidy; + const menu = makeClaudeMenu(() => stored); + menu.populateVoices(claudeMockVoices, menu.element); + await flushAsync(); + + stored = null; + emitVoiceChange(null, "claude"); + await flushAsync(); + + expect( + menu.menuButton.querySelector(".voice-name")?.textContent + ).not.toBe("Cassidy"); + }); +}); + +describe("PiVoiceMenu reacts to settings-page voice changes (#475)", () => { + it("marks the newly stored voice's button selected on the preference event", async () => { + const paola = piVoices[0]; + const joey = piVoices[1]; + let stored: SpeechSynthesisVoiceRemote | null = paola; + const menu = makePiMenu(() => stored); + menu.populateVoices(piVoices, menu.element); + await flushAsync(); + + stored = joey; + emitVoiceChange(joey.id, "pi"); + await flushAsync(); + + const joeyButton = menu.element.querySelector( + `button[data-voice-id="${joey.id}"]` + ) as HTMLButtonElement; + const paolaButton = menu.element.querySelector( + `button[data-voice-id="${paola.id}"]` + ) as HTMLButtonElement; + expect(joeyButton.classList.contains("selected")).toBe(true); + expect(paolaButton.classList.contains("selected")).toBe(false); + }); +});