diff --git a/changelog.d/1546-chat-provider-switch.md b/changelog.d/1546-chat-provider-switch.md index 1c3210330..69cdbc939 100644 --- a/changelog.d/1546-chat-provider-switch.md +++ b/changelog.d/1546-chat-provider-switch.md @@ -1,5 +1,5 @@ - Keep typed chat's view when switching AI Provider: Settings opened from - chat, or chat itself, stay as they were for the new provider instead of - falling back to voice and widening the dock. Opening chat or closing the + chat stay chat's Settings for the new provider instead of falling back to + voice and widening the dock. Opening chat or closing the voice panel now also clears a voice error left on the dock (Manjunath, @manjunath22466, #1546). diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index fa2287567..c01693e3d 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -767,8 +767,8 @@ starts. Settings opened from chat keep chat's frame and show the shared AI provider, the quick-commands option and then the voice preferences (Voice Preview, inactivity timeout and model tier), so they stay in reach while Preview is off, with Back to chat; voice Settings show Back to voice. Switching -AI Provider keeps that view: chat's Settings, or chat itself, stay as they were -for the new provider. Escape steps back from either Settings view, closes Voice +AI Provider keeps that view: Settings opened from chat stay chat's Settings, +in chat's frame, for the new provider. Escape steps back from either Settings view, closes Voice Preview, and does nothing in chat. Closing chat ends a typed session, and the voice panel's close turns voice off. Opening chat or closing the voice panel also clears a voice error left on the dock. Asking the assistant from chat to open diff --git a/src/voice/chatControl.js b/src/voice/chatControl.js index d366181a7..0525b9bc5 100644 --- a/src/voice/chatControl.js +++ b/src/voice/chatControl.js @@ -1,6 +1,7 @@ import { bindChatAttachments } from './chatAttachments.js'; import { bindChatPreferences } from './chatPreferences.js'; import { readStoredCloudVoiceAuthMode } from './cloudVoiceAuth.js'; +import { isStoppable } from './session.js'; import { t } from '../i18n/index.js'; /** @@ -96,8 +97,7 @@ export function bindChatControl({ ui, session, onOpen, dataManager }) { leaveChatSettings(); // Chat replaces voice, a stale voice error included, so the dock stops // showing it. - if (!isTyped() && (session.isActive() || session.state === 'error')) - session.stop(); + if (!isTyped() && isStoppable(session)) session.stop(); onOpen?.(); ui.update('chat', { open: true, minimized: false, rows: 2, authNote }); sync(); @@ -392,15 +392,7 @@ export function bindChatControl({ ui, session, onOpen, dataManager }) { } const frame = ui.element('chatPanel')?.getBoundingClientRect?.(); close(); - ui.update('voicePanel', { - context: 'chat', - ...(frame?.width && { - frameWidth: frame.width, - frameHeight: frame.height, - }), - }); - ui.update('chat', { backVisible: true }); - sync(); + showChatSettings(frame); void ui.focus(['chatBack']); }), // The panel clears its context as it closes, so chat tracks its own. @@ -416,32 +408,30 @@ export function bindChatControl({ ui, session, onOpen, dataManager }) { }), ]; sync(); - // What a provider switch carries to the next mount: chat's Settings or - // chat itself, never the conversation. + // Settings shown from chat, in chat's frame when it has one. + function showChatSettings(frame) { + ui.update('voicePanel', { + context: 'chat', + ...(frame?.width && { + frameWidth: frame.width, + frameHeight: frame.height, + }), + }); + ui.update('chat', { backVisible: true }); + sync(); + } + // What a provider switch carries to the next mount: chat's Settings, never + // the conversation. The provider choice lives only in Settings, so chat + // itself is never open during a switch. function presentation() { - if (inChatSettings()) { - const { frameWidth, frameHeight } = ui.state.voicePanel; - return { view: 'settings', frameWidth, frameHeight }; - } - if (chat().open) return { view: chat().minimized ? 'minimized' : 'open' }; - return null; + if (!inChatSettings()) return null; + const { frameWidth: width, frameHeight: height } = ui.state.voicePanel; + return { view: 'settings', frame: width ? { width, height } : null }; } // Restored without moving focus: the switch keeps it on the provider choice. function restorePresentation(state) { - if (disposed || !state) return; - if (state.view === 'settings') { - ui.update('voicePanel', { - context: 'chat', - ...(state.frameWidth && { - frameWidth: state.frameWidth, - frameHeight: state.frameHeight, - }), - }); - ui.update('chat', { backVisible: true }); - return sync(); - } - open({ focus: false }); - if (state.view === 'minimized') minimize({ focus: false }); + if (disposed || state?.view !== 'settings') return; + showChatSettings(state.frame); } return { close, diff --git a/src/voice/chatControlView.test.mjs b/src/voice/chatControlView.test.mjs index e329a2dc3..c0175456c 100644 --- a/src/voice/chatControlView.test.mjs +++ b/src/voice/chatControlView.test.mjs @@ -365,7 +365,7 @@ test('the chat gear is named for the settings it opens', async (t) => { ); }); -test('a provider switch carries chat Settings or chat itself to the next mount', async (t) => { +test('a provider switch carries chat Settings to the next mount', async (t) => { const mount = () => { const ui = createTestVoiceUi({ panel: true }); const chat = bindChatControl({ ui, session: typedSession() }); @@ -377,21 +377,22 @@ test('a provider switch carries chat Settings or chat itself to the next mount', before.ui.emit('chat-button'); before.ui.update('voicePanel', { hidden: false }); before.ui.emit('settings-button'); + before.ui.update('voicePanel', { frameWidth: 320, frameHeight: 480 }); const settings = before.chat.presentation(); - assert.equal(settings.view, 'settings'); + assert.deepEqual(settings, { + view: 'settings', + frame: { width: 320, height: 480 }, + }); const after = mount(); after.chat.restorePresentation(settings); assert.equal(after.ui.state.voicePanel.context, 'chat'); + assert.equal(after.ui.state.voicePanel.frameWidth, 320); + assert.equal(after.ui.state.voicePanel.frameHeight, 480); assert.equal(after.ui.state.chat.backVisible, true); assert.equal(after.ui.state.root.assistantMode, 'chat'); - // Open and minimized chat come back as they were, without taking focus. - for (const view of ['open', 'minimized']) { - const next = mount(); - next.chat.restorePresentation({ view }); - assert.equal(next.ui.state.chat.open, true); - assert.equal(next.ui.state.chat.minimized, view === 'minimized'); - assert.equal(next.chat.presentation().view, view); - } - // Idle chat carries nothing. + // Chat itself, or no chat at all, carries nothing. + const open = mount(); + open.ui.emit('chat-button'); + assert.equal(open.chat.presentation(), null); assert.equal(mount().chat.presentation(), null); }); diff --git a/src/voice/panel/host.test.mjs b/src/voice/panel/host.test.mjs index ae5cd1994..57b512ac0 100644 --- a/src/voice/panel/host.test.mjs +++ b/src/voice/panel/host.test.mjs @@ -1319,6 +1319,56 @@ test('opening chat or closing the voice panel clears a stale voice error', async } }); +test('switching AI Provider from chat Settings keeps chat Settings on the new mount', async (t) => { + const view = mountVoiceView(t); + const host = createVoicePanelHost({ storage: null }); + const instances = []; + const commands = createProviderCommands( + { + runner: async () => ({}), + dataManager: { layers: new Map() }, + supportedProviders: ['openai', 'gemini'], + }, + { + bind: (options) => + bind({ + ...options, + panelHost: host, + createControl: (control) => + createTestVoiceUi({ ...control, pinia: view.pinia }), + }), + factories: { openai: factory(instances), gemini: factory(instances) }, + storage: null, + }, + ); + t.after(() => { + commands.destroy(); + host.destroy(); + }); + await view.settle(); + $('#gev-chat-button').click(); + await view.settle(); + $('#gev-chat-panel').getBoundingClientRect = () => ({ + width: 320, + height: 480, + }); + $('#gev-voice-settings-button').click(); + await view.settle(); + const old = commands.ui; + assert.equal(old.state.voicePanel.context, 'chat'); + old.emit('provider-change', 'gemini'); + await view.settle(); + const { ui } = commands; + assert.notEqual(ui, old); + assert.equal(ui.state.voicePanel.context, 'chat'); + assert.equal(ui.state.voicePanel.frameWidth, 320); + assert.equal(ui.state.voicePanel.frameHeight, 480); + assert.equal(ui.state.chat.backVisible, true); + assert.equal(ui.state.root.assistantMode, 'chat'); + assert.equal($('#gev-chat-back').hidden, false); + assert.equal(active(), 'gev-voice-provider'); +}); + test('the voice panel close turns voice off', async (t) => { const f = mounted(t, { view: true }); await f.commands.session.start(); diff --git a/src/voice/providerCommands.js b/src/voice/providerCommands.js index 756215c7b..5d77ebef6 100644 --- a/src/voice/providerCommands.js +++ b/src/voice/providerCommands.js @@ -64,7 +64,7 @@ export function createProviderCommands(options, { bind, factories, storage }) { const textOnly = Boolean( current.session.adapter.getStartOptions?.().textOnly, ); - // The panel stays where the user is: chat's Settings or chat itself. + // Settings opened from chat stay chat's Settings. const presentation = current.chatPresentation?.() ?? null; const generation = ++mountGeneration; voiceEnabled = false; diff --git a/src/voice/providerCommands.test.mjs b/src/voice/providerCommands.test.mjs index dfcf77052..a9acd2804 100644 --- a/src/voice/providerCommands.test.mjs +++ b/src/voice/providerCommands.test.mjs @@ -857,13 +857,13 @@ for (const action of ['stop', 'destroy']) { } } -test('a provider switch hands chat Settings or chat to the new mount', async (t) => { +test('a provider switch hands chat Settings to the new mount', async (t) => { const f = compositionFixture(t); const first = f.facade.provider; const next = first === 'openai' ? 'gemini' : 'openai'; // Without chat open there is nothing to carry. assert.equal(f.mounts[0].controls.chatPresentation(), null); - const shown = { view: 'settings', frameWidth: 320, frameHeight: 480 }; + const shown = { view: 'settings', frame: { width: 320, height: 480 } }; f.mounts[0].controls.chatPresentation = () => shown; f.select(next); await settle(); diff --git a/src/voice/session.js b/src/voice/session.js index 21400f04c..7c31f17d2 100644 --- a/src/voice/session.js +++ b/src/voice/session.js @@ -185,6 +185,10 @@ export function createVoiceSession({ createAdapter, runner, signal }) { }; } +// Live, or left in error: stopping either returns the session to idle. +export const isStoppable = (session) => + session.isActive() || session.state === 'error'; + function abortError() { return new DOMException('Voice action cancelled', 'AbortError'); } diff --git a/src/voice/sessionCommands.js b/src/voice/sessionCommands.js index 5a8a7e90c..242a2126b 100644 --- a/src/voice/sessionCommands.js +++ b/src/voice/sessionCommands.js @@ -3,7 +3,7 @@ import { bindAssistantPanelLayout, bindLayoutDock } from './panelLayout.js'; import { bindChatControl } from './chatControl.js'; import { bindVoiceMiniPreview } from './voiceMiniPreview.js'; import { createVoiceControl, voiceStatusLabel } from './control.js'; -import { createVoiceSession } from './session.js'; +import { createVoiceSession, isStoppable } from './session.js'; import { VoiceCardControls } from './voiceCard.js'; import { PointerReticle } from './pointerReticle.js'; import { VOICE_PROVIDERS } from './providerCapabilities.js'; @@ -40,7 +40,7 @@ export function createVoiceCommands({ createReticle = createDefaultReticle, panelHost = null, nativePanelProvider = null, - // Chat's Settings or chat itself, carried across a provider switch. + // Chat's Settings, carried across a provider switch. presentation = null, }) { if (resetExisting) window.__gevVoiceCommands?.stop?.({ removeUi: true }); @@ -289,11 +289,7 @@ export function createVoiceCommands({ // session belongs to chat, which handles its own close. const offClose = ui.on('settings-close', () => { // A voice error the user closes is over too; stopping returns to idle. - if ( - (!session.isActive() && session.state !== 'error') || - adapter.getStartOptions?.().textOnly - ) - return; + if (!isStoppable(session) || adapter.getStartOptions?.().textOnly) return; const ticket = panelMount?.stopTicket(); session.stop(); panelMount?.closeAfterStop(ticket);