Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions console/web/src/components/chat/ChatView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
isHarnessAvailable,
} from '@/hooks/use-harness-status'
import { useLiveAnnouncer } from '@/hooks/use-live-announcer'
import { DESKTOP_POINTER_QUERY, useMediaQuery } from '@/hooks/use-media-query'
import { useWorktreeBinding } from '@/hooks/use-worktree-binding'
import { useWorktreeEvents } from '@/hooks/use-worktree-events'
import { expandAttachments, hasExpandableAttachments } from '@/lib/attachments'
Expand Down Expand Up @@ -212,6 +213,9 @@ export function ChatView({
: false
const harnessBlockedRef = useRef(harnessBlocked)
harnessBlockedRef.current = harnessBlocked
// This view is keyed by conversation, so mounting IS opening a session:
// the caret belongs in the composer, on the devices where that is free.
const focusComposerOnOpen = useMediaQuery(DESKTOP_POINTER_QUERY)

/* What the model on the other end can do with a picture, read at send time
rather than closed over: the send and edit-queued callbacks are built
Expand Down Expand Up @@ -2082,6 +2086,7 @@ export function ChatView({
isStreaming={streamingIndicator}
queueWhileStreaming={!!backend.queueMessage}
blocked={harnessBlocked}
autoFocus={focusComposerOnOpen && !harnessBlocked}
blockedPlaceholder={
conversationsCtx
? harnessComposerPlaceholder(conversationsCtx.harnessStatus)
Expand Down
8 changes: 8 additions & 0 deletions console/web/src/components/chat/Composer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ interface ComposerProps {
blocked?: boolean
/** Placeholder while `blocked` is true. */
blockedPlaceholder?: string
/**
* Put the caret in the editor on mount. The caller decides, because only it
* knows whether focus is welcome: on a touch device it raises the on-screen
* keyboard over the conversation, which is worse than aiming once.
*/
autoFocus?: boolean
/** Initial editor content (applied once on mount). */
initialContent?: (editor: LexicalEditor) => void
/**
Expand Down Expand Up @@ -177,6 +183,7 @@ export function Composer({
queueWhileStreaming,
blocked,
blockedPlaceholder = 'chat unavailable…',
autoFocus,
initialContent,
initialText,
onTextChange,
Expand Down Expand Up @@ -426,6 +433,7 @@ export function Composer({
: 'send a message…'
}
disabled={inputDisabled}
autoFocus={autoFocus}
initialContent={resolvedInitialContent}
functionEntries={functionEntries}
workingDir={workingDir}
Expand Down
25 changes: 24 additions & 1 deletion console/web/src/components/chat/LexicalShell.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { AutoFocusPlugin } from '@lexical/react/LexicalAutoFocusPlugin'
import { ClearEditorPlugin } from '@lexical/react/LexicalClearEditorPlugin'
import { LexicalComposer } from '@lexical/react/LexicalComposer'
import { useLexicalComposerContext } from '@lexical/react/LexicalComposerContext'
Expand All @@ -18,7 +19,7 @@ import {
type LexicalEditor,
} from 'lexical'
import { useEffect, useMemo, useRef } from 'react'
import { onComposerInsert } from '@/lib/composer-insert'
import { onComposerFocusRequest, onComposerInsert } from '@/lib/composer-insert'
import type { FunctionEntry } from '@/lib/functions'
import { FileMentionNode } from './lexical/FileMentionNode'
import { FileMentionsPlugin } from './lexical/FileMentionsPlugin'
Expand All @@ -33,6 +34,8 @@ interface LexicalShellProps {
onSubmit: () => void
placeholder?: string
disabled?: boolean
/** Put the caret in the editor on mount. Off by default. */
autoFocus?: boolean
}

const baseConfig = {
Expand Down Expand Up @@ -196,6 +199,19 @@ function ExternalInsertPlugin() {
return null
}

/**
* Take the caret when a surface asks for it, e.g. a "new chat" that reused
* the untouched one already open, where nothing remounts to focus itself.
*/
function FocusOnRequestPlugin({ enabled }: { enabled: boolean }) {
const [editor] = useLexicalComposerContext()
useEffect(() => {
if (!enabled) return
return onComposerFocusRequest(() => editor.focus())
}, [editor, enabled])
return null
}

/**
* Toggle the editor's editable state when `disabled` flips.
*/
Expand Down Expand Up @@ -227,6 +243,7 @@ export function LexicalShell({
onSubmit,
placeholder = 'send a message…',
disabled,
autoFocus,
clearToken,
initialContent,
functionEntries,
Expand Down Expand Up @@ -272,6 +289,12 @@ export function LexicalShell({
<SubmitOnEnterPlugin onSubmit={onSubmit} menuOpenRef={menuOpenRef} />
<HistoryNavPlugin onNav={onHistoryNav} menuOpenRef={menuOpenRef} />
<ExternalInsertPlugin />
{/* Opening a session is a request to write in it, so the first
keystroke should land in the message rather than be spent aiming.
Lexical's own plugin waits for the editable node, which a bare
focus() call on mount does not. */}
{autoFocus === true && disabled !== true ? <AutoFocusPlugin /> : null}
<FocusOnRequestPlugin enabled={disabled !== true} />
<EditablePlugin disabled={disabled} />
<MentionsPlugin
menuOpenRef={menuOpenRef}
Expand Down
3 changes: 3 additions & 0 deletions console/web/src/components/workspace/EmptyPane.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@ export function EmptyPane({
setActiveIndex(0)
}}
onKeyDown={onSearchKeyDown}
// This box opens focused, so without these the workspace keys
// spell themselves into the query instead of moving anywhere.
data-keybindings-allow="workspace.selectByIndex panel.split"
placeholder="search pages…"
aria-label="search pages"
role="combobox"
Expand Down
37 changes: 37 additions & 0 deletions console/web/src/hooks/use-conversations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type { Conversation } from '@/types/chat'
import {
appendMessageToConversation,
applyCatalogModelFallback,
isUntouchedDraft,
markBackgroundedStale,
mergeConversationMeta,
mergeHydratedTranscript,
Expand Down Expand Up @@ -473,3 +474,39 @@ describe('mergeConversationMeta / system_prompt', () => {
expect(next.systemPrompt?.strategy).toBe('enrich')
})
})

describe('isUntouchedDraft', () => {
it('recognises the chat nobody has written in yet', () => {
expect(isUntouchedDraft(conversation({ draft: true, messages: [] }))).toBe(
true,
)
})

it('refuses a draft that already carries work', () => {
expect(
isUntouchedDraft(
conversation({
draft: true,
messages: [],
draftText: 'half a thought',
}),
),
).toBe(false)
expect(
isUntouchedDraft(
conversation({
draft: true,
messages: [
{ id: 'm1', role: 'user', content: 'sent', createdAt: 1 },
] as Conversation['messages'],
}),
),
).toBe(false)
})

it('refuses a real session, which is never interchangeable', () => {
expect(isUntouchedDraft(conversation({ draft: false, messages: [] }))).toBe(
false,
)
})
})
24 changes: 23 additions & 1 deletion console/web/src/hooks/use-conversations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
type SystemPromptAddon,
type SystemPromptState,
} from '@/components/chat/system-prompt-selection'
import { requestComposerFocus } from '@/lib/composer-insert'
import { getIiiClient } from '@/lib/iii-client'
import { newSessionId } from '@/lib/session-id'
import {
Expand Down Expand Up @@ -105,6 +106,16 @@ function emptyConversation(defaultModel: ModelId | null): Conversation {
}
}

/** A chat nobody has written in yet: still local, no transcript, no draft
text. Two of these are the same chat as far as anyone can tell. */
export function isUntouchedDraft(conversation: Conversation): boolean {
return (
conversation.draft === true &&
conversation.messages.length === 0 &&
(conversation.draftText ?? '') === ''
)
}

function isMode(v: unknown): v is Mode {
return v === 'ask' || v === 'agent'
}
Expand Down Expand Up @@ -812,11 +823,22 @@ export function useConversations(
)

const createNew = useCallback(() => {
// Asking for a new chat while an untouched one is already open reads as
// "nothing happened": the second empty draft is indistinguishable from
// the first, and they pile up in the list. Hand back the one in front of
// you instead, and put the caret in it.
const current = conversations.find(
(conversation) => conversation.id === activeId,
)
if (current && isUntouchedDraft(current)) {
requestComposerFocus()
return current.id
}
const next = emptyConversation(loadLastModel())
setConversations((list) => [next, ...list])
setActiveId(next.id)
return next.id
}, [])
}, [conversations, activeId])

const select = useCallback((id: string) => setActiveId(id), [])

Expand Down
38 changes: 38 additions & 0 deletions console/web/src/hooks/use-keybindings.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { describe, expect, it } from 'vitest'
import { allowsWhileTyping } from './use-keybindings'

/** The guard reads one dataset entry, so a field stands in for the element. */
function field(allow?: string): EventTarget {
return {
dataset: allow === undefined ? {} : { keybindingsAllow: allow },
} as unknown as EventTarget
}

describe('allowsWhileTyping', () => {
it('hands back only the actions the field names', () => {
const input = field('workspace.selectByIndex panel.split')

expect(allowsWhileTyping(input, 'workspace.selectByIndex')).toBe(true)
expect(allowsWhileTyping(input, 'panel.split')).toBe(true)
// The workspace key is a letter, so this box still spells its own query.
expect(allowsWhileTyping(input, 'workspace.create')).toBe(false)
})

it('accepts a comma-separated list as readily as a spaced one', () => {
expect(
allowsWhileTyping(field('panel.split,app.settings'), 'app.settings'),
).toBe(true)
})

it('keeps every key for a field that opts into nothing', () => {
expect(allowsWhileTyping(field(), 'panel.split')).toBe(false)
expect(allowsWhileTyping(field(''), 'panel.split')).toBe(false)
expect(allowsWhileTyping(null, 'panel.split')).toBe(false)
})

it('does not match an action whose id merely starts the same way', () => {
expect(allowsWhileTyping(field('panel.splitter'), 'panel.split')).toBe(
false,
)
})
})
26 changes: 25 additions & 1 deletion console/web/src/hooks/use-keybindings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,25 @@ function isTyping(target: EventTarget | null): boolean {
return tag === 'INPUT' || tag === 'TEXTAREA' || tag === 'SELECT'
}

/**
* A field may hand specific shortcuts back with
* `data-keybindings-allow="workspace.selectByIndex panel.split"`. A search box
* that opens focused would otherwise swallow the navigation keys for as long
* as it holds the caret: after `t` opens a workspace, its page search has the
* focus, so `\` and the workspace digits typed characters instead of moving.
* Opt in per field and per action, never wholesale, so a field keeps every
* key it actually needs to spell its own query.
*/
export function allowsWhileTyping(
target: EventTarget | null,
actionId: KeybindingActionId,
): boolean {
const element = target as HTMLElement | null
const allow = element?.dataset?.keybindingsAllow
if (!allow) return false
return allow.split(/[\s,]+/).includes(actionId)
}

export function useKeybindings(handlers: KeybindingHandlers): void {
// Read through a ref so the listener binds once: handlers are rebuilt every
// render by every caller that passes inline arrows.
Expand All @@ -46,7 +65,12 @@ export function useKeybindings(handlers: KeybindingHandlers): void {
const typing = isTyping(event.target)
for (const definition of KEYBINDINGS) {
if (definition.scope !== 'global') continue
if (typing && !definition.firesWhileTyping) continue
if (
typing &&
!definition.firesWhileTyping &&
!allowsWhileTyping(event.target, definition.id)
)
continue
const run = handlersRef.current[definition.id]
if (!run) continue
if (definition.digitIndex) {
Expand Down
7 changes: 7 additions & 0 deletions console/web/src/hooks/use-media-query.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
import { useEffect, useState } from 'react'

/**
* A pointer you aim, on a screen with room for a keyboard that is already
* there. Width alone would call a tablet in landscape a desktop, and taking
* focus there raises the on-screen keyboard over whatever you were reading.
*/
export const DESKTOP_POINTER_QUERY = '(hover: hover) and (pointer: fine)'

export function useMediaQuery(query: string): boolean {
const [matches, setMatches] = useState(() =>
typeof window === 'undefined' || typeof window.matchMedia !== 'function'
Expand Down
19 changes: 19 additions & 0 deletions console/web/src/lib/composer-insert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,25 @@ export function insertIntoComposer(text: string): void {
for (const listener of listeners) listener(text)
}

type ComposerFocusListener = () => void

const focusListeners = new Set<ComposerFocusListener>()

/** Ask the mounted composer for the caret. Unlike an insert this is not
buffered: a focus nobody is around to take is a focus nobody wanted. */
export function requestComposerFocus(): void {
for (const listener of focusListeners) listener()
}

export function onComposerFocusRequest(
listener: ComposerFocusListener,
): () => void {
focusListeners.add(listener)
return () => {
focusListeners.delete(listener)
}
}

export function onComposerInsert(listener: ComposerInsertListener): () => void {
listeners.add(listener)
if (pending.length > 0) {
Expand Down
17 changes: 17 additions & 0 deletions console/web/src/lib/workspace-tabs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,23 @@ describe('withWorkspaceScreenOpened', () => {
})
})

it('stays put when the workspace you are on already shows that screen', () => {
const tabs: WorkspaceTab[] = [
{ id: 'first-chat', screens: [CHAT_SCREEN] },
{ id: 'chat-and-shell', screens: [CHAT_SCREEN, 'ext:shell'] },
]
// Opening chat from the second tab must not send you to the first one
// just because it was created earlier.
expect(
withWorkspaceScreenOpened(
tabs,
'chat-and-shell',
CHAT_SCREEN,
() => 'new',
),
).toEqual({ tabs, activeTabId: 'chat-and-shell' })
})

it('places beside chat, then creates a safe split when the active tab is full', () => {
const open = withWorkspaceScreenOpened(
[{ id: 'chat', columns: 2, screens: [CHAT_SCREEN, 'traces'] }],
Expand Down
9 changes: 8 additions & 1 deletion console/web/src/lib/workspace-tabs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -381,10 +381,17 @@ export function withWorkspaceScreenOpened(
screen: TabScreen,
makeTabId: () => string = newTabId,
): OpenWorkspaceScreenResult {
const active = tabs.find((tab) => tab.id === activeTabId) ?? tabs[0]
// Where you already are beats where it happens to be mounted: opening chat
// from a workspace that shows chat should stay put, not send you to
// whichever other tab was created first.
if (active?.screens.includes(screen)) {
return { tabs, activeTabId: active.id }
}

const existing = tabs.find((tab) => tab.screens.includes(screen))
if (existing) return { tabs, activeTabId: existing.id }

const active = tabs.find((tab) => tab.id === activeTabId) ?? tabs[0]
if (active) {
const placed = withScreenOpenedBeside(active, screen)
if (placed) {
Expand Down
Loading
Loading