From b504f8b2fbd87ac1c73d7c3edbd55272bf6de1a8 Mon Sep 17 00:00:00 2001 From: Karn Date: Wed, 12 Aug 2026 15:52:46 +0530 Subject: [PATCH 1/3] fix(web): Shift+Enter adds a line instead of sending the message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit xterm.js encodes Enter as a bare carriage return whatever Shift is doing — faithful to the hardware, and useless in front of an agent CLI. Claude Code and Codex both read a bare CR as "send this", so in a browser terminal there was no way to put a second line in a prompt. Send ESC CR for Shift+Enter. That is not a sequence flue invented: a line editor reads it as Alt+Enter, `claude /terminal-setup` writes exactly those two bytes into VS Code's keybindings.json for `shift+enter`, and it is what iTerm2, Ghostty, WezTerm, Kitty, Warp and Windows Terminal already send. It goes in the emulator rather than the view because the view cannot put bytes on the wire without reaching past the seam. `term.input` routes them through onData, so the key bar's latched Ctrl and the replay mute gate still apply. Ctrl+Shift+Enter is left alone — the view takes that one on the way down for focus mode. Co-Authored-By: Claude Opus 5 (1M context) --- web/src/emulator/emulator.test.ts | 101 +++++++++++++++++++++++++++++- web/src/emulator/xterm.ts | 43 +++++++++++++ 2 files changed, 143 insertions(+), 1 deletion(-) diff --git a/web/src/emulator/emulator.test.ts b/web/src/emulator/emulator.test.ts index 2909a19..02a1c98 100644 --- a/web/src/emulator/emulator.test.ts +++ b/web/src/emulator/emulator.test.ts @@ -3,7 +3,14 @@ import { dirname, resolve } from 'node:path' import { fileURLToPath } from 'node:url' import { Terminal } from '@xterm/xterm' import { describe, expect, it, vi } from 'vitest' -import { createXtermEmulator, extractGrid, loadWebglRenderer, openTerminalLink, TERMINAL_FONT_FAMILY } from './xterm' +import { + createXtermEmulator, + extractGrid, + loadWebglRenderer, + NEWLINE_CHORD_BYTES, + openTerminalLink, + TERMINAL_FONT_FAMILY, +} from './xterm' import type { Emulator } from './types' /** @@ -259,6 +266,98 @@ describe('openTerminalLink', () => { }) }) +describe('Shift+Enter', () => { + /** + * Mount an emulator and hand back what it sends and the element xterm + * listens for keys on. + * + * The helper textarea is where every real keystroke lands: xterm keeps the + * caret in it and reads keydown from it, so dispatching anywhere else would + * measure a listener nobody uses. + */ + function mounted() { + const el = document.createElement('div') + document.body.appendChild(el) + const em = createXtermEmulator({ cols: 20, rows: 4 }) + em.attachTo(el) + const out: string[] = [] + em.onData((b) => out.push(new TextDecoder().decode(b))) + const textarea = el.querySelector('textarea')! + const press = (init: KeyboardEventInit) => + textarea.dispatchEvent( + new KeyboardEvent('keydown', { + key: 'Enter', + keyCode: 13, + bubbles: true, + cancelable: true, + ...init, + }), + ) + return { + press, + sent: () => out.join(''), + done: () => { + em.dispose() + el.remove() + }, + } + } + + it('sends ESC CR, so an agent CLI takes a newline instead of a submit', () => { + // The bug this exists for: xterm encodes Enter as a bare CR whatever + // Shift is doing, and Claude Code and Codex both read a bare CR as + // "send this message". A newline mid-prompt was unreachable in a browser. + const t = mounted() + t.press({ shiftKey: true }) + expect(t.sent()).toBe(NEWLINE_CHORD_BYTES) + t.done() + }) + + it('leaves the browser out of it, so no line break lands in the helper', () => { + // xterm's caret lives in a real textarea. An Enter left to the browser + // puts a line break in that element's value, and xterm's own input + // listener then reads a value it never wrote. + const t = mounted() + const event = new KeyboardEvent('keydown', { + key: 'Enter', + keyCode: 13, + shiftKey: true, + bubbles: true, + cancelable: true, + }) + document.querySelector('textarea')!.dispatchEvent(event) + expect(event.defaultPrevented).toBe(true) + t.done() + }) + + it('leaves a plain Enter as the carriage return every shell expects', () => { + const t = mounted() + t.press({}) + expect(t.sent()).toBe('\r') + t.done() + }) + + it('keeps its hands off the focus-mode chord', () => { + // Ctrl+Shift+Enter belongs to the terminal view, which takes it on the + // way down (components/terminal.tsx). Claiming it here as a newline would + // mean the two agree only by accident of listener order. + const t = mounted() + t.press({ shiftKey: true, ctrlKey: true }) + expect(t.sent()).not.toBe(NEWLINE_CHORD_BYTES) + t.done() + }) + + it('sends the same bytes for Alt+Enter, which is xterm doing it', () => { + // Not a claim about this handler — a check that the sequence chosen is + // the one xterm already emits for the other chord that means "newline", + // so both spellings arrive at the CLI as one thing. + const t = mounted() + t.press({ altKey: true }) + expect(t.sent()).toBe(NEWLINE_CHORD_BYTES) + t.done() + }) +}) + describe('device-query suppression', () => { const bytes = (s: string) => new TextEncoder().encode(s) diff --git a/web/src/emulator/xterm.ts b/web/src/emulator/xterm.ts index 8ccb51c..fe45c64 100644 --- a/web/src/emulator/xterm.ts +++ b/web/src/emulator/xterm.ts @@ -27,6 +27,23 @@ const SCREEN_SELECTOR = '.xterm-screen' export const TERMINAL_FONT_FAMILY = "ui-monospace, 'SFMono-Regular', 'SF Mono', Menlo, Consolas, monospace" +/** + * What a "newline, do not send yet" chord puts on the wire: ESC then CR. + * + * There is no such thing at the VT level. Enter is a carriage return and + * Shift does not change that — xterm.js is faithful to the hardware and + * encodes both the same way, which is why Shift+Enter in a browser terminal + * submits an agent's prompt rather than adding a line to it. + * + * ESC CR is the sequence the CLIs themselves settled on. A line editor reads + * it as Alt+Enter, `claude /terminal-setup` writes exactly these two bytes + * into VS Code's keybindings.json for `shift+enter`, and iTerm2, Ghostty, + * WezTerm, Kitty, Warp and Windows Terminal send it out of the box — which is + * what "Shift+Enter is natively supported" means in those tools. So this is + * flue matching a convention, not inventing one. + */ +export const NEWLINE_CHORD_BYTES = '\x1b\r' + /** * xterm.js behind the Emulator seam. * @@ -64,6 +81,32 @@ export function createXtermEmulator(opts: XtermOptions = {}): Emulator { // through the same guard so there is exactly one place that opens links. term.loadAddon(new WebLinksAddon((_event, uri) => openTerminalLink(uri))) + /* + * Shift+Enter, which xterm has no notion of. + * + * Here rather than in the terminal view, and that is the whole argument for + * this seam: the view's own keydown handler has no way to put bytes on the + * wire without reaching past it. `term.input` sends them through onData, so + * the key bar's latched Ctrl and the replay mute gate both still apply, and + * the daemon counts the keystroke as activity exactly as it would any other. + * + * keydown alone — keyup and keypress arrive for the same press, and either + * would send the sequence a second time. Ctrl, Alt and Cmd are all excluded: + * Ctrl+Shift+Enter is the view's focus-mode chord, and Alt+Enter already + * produces these bytes through xterm's own encoder. + */ + term.attachCustomKeyEventHandler((e) => { + if (e.type !== 'keydown') return true + if (e.key !== 'Enter' || !e.shiftKey || e.ctrlKey || e.altKey || e.metaKey) return true + // xterm keeps the caret in a real textarea, and refusing the event is not + // the same as cancelling it: left to the browser, this Enter writes a line + // into that element and xterm's input listener then reads a value nothing + // typed. xterm's own cancel never runs, because `false` returns above it. + e.preventDefault() + term.input(NEWLINE_CHORD_BYTES, true) + return false + }) + const encoder = new TextEncoder() let disposed = false From 11a5ac069b4af6e800e9ba05a26c9155da0f5700 Mon Sep 17 00:00:00 2001 From: Karn Date: Wed, 12 Aug 2026 16:33:43 +0530 Subject: [PATCH 2/3] feat(web): ask what a session is before starting one, and open it in its own tab MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three things that were one thing. A session row now opens in a tab of its own. This list is what people come back to between sessions, and opening a terminal over it made the way back the browser's back button — which drops the terminal and its scrollback with it. The `+` inside a terminal pointed at `/?cwd=`, which is the sessions dashboard, so a session started from a terminal came up behind the whole list — page, shell, rows and all — for as long as the daemon took to answer. There is a page for it now: /new spawns, applies the metadata, and replaces itself with the terminal, on the terminal's own ground. And every way of starting a session goes through one dialog first: name, directory, machine, tags, prefilled from whatever the press implied. `spawn` carries no metadata, so a name and a tag could only ever be applied after the session existed — which meant going back to the list to do it, which nobody does. Asked here, they ride the address to the page that starts the session and land on the first frame that has an id. The page is a page and not a click handler because starting a session takes a round trip: a screen that spawned and then called window.open from the reply would be opening a popup from a continuation, which Safari refuses outright and Chrome refuses once the gesture has aged out. A link opens on the click itself. A blocked popup falls back to this tab. `flue open`'s `?cwd=` handover is untouched — it still spawns from the sessions screen, which is the one caller `adopt` has left. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/new-session-dialog.test.tsx | 187 ++++++++++++ web/src/components/new-session-dialog.tsx | 265 ++++++++++++++++++ web/src/components/session-table.test.tsx | 35 ++- web/src/components/session-table.tsx | 12 +- web/src/components/tag-editor.tsx | 125 +-------- web/src/components/tag-field.tsx | 189 +++++++++++++ web/src/components/terminal.test.tsx | 34 ++- web/src/components/terminal.tsx | 43 ++- web/src/router.test.tsx | 78 +++++- web/src/router.tsx | 19 ++ web/src/routes/new-session.test.tsx | 221 +++++++++++++++ web/src/routes/new-session.tsx | 259 +++++++++++++++++ web/src/routes/sessions.test.tsx | 240 +++++++++------- web/src/routes/sessions.tsx | 94 ++++--- web/src/routes/terminal.tsx | 86 +++++- web/src/sessions/new-session.test.ts | 72 +++++ web/src/sessions/new-session.ts | 91 ++++++ web/src/sessions/open-new-session.ts | 48 ++++ 18 files changed, 1802 insertions(+), 296 deletions(-) create mode 100644 web/src/components/new-session-dialog.test.tsx create mode 100644 web/src/components/new-session-dialog.tsx create mode 100644 web/src/components/tag-field.tsx create mode 100644 web/src/routes/new-session.test.tsx create mode 100644 web/src/routes/new-session.tsx create mode 100644 web/src/sessions/new-session.test.ts create mode 100644 web/src/sessions/new-session.ts create mode 100644 web/src/sessions/open-new-session.ts diff --git a/web/src/components/new-session-dialog.test.tsx b/web/src/components/new-session-dialog.test.tsx new file mode 100644 index 0000000..d747d92 --- /dev/null +++ b/web/src/components/new-session-dialog.test.tsx @@ -0,0 +1,187 @@ +import { render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { describe, expect, it, vi } from 'vitest' + +import type { NewSessionRequest } from '@/sessions/new-session' +import { NewSessionDialog, type NewSessionDialogProps } from './new-session-dialog' + +const MACHINES = [ + { id: 'local', name: 'mesa.local' }, + { id: 'attic-pi', name: 'Attic Pi' }, +] + +function show(over: Partial = {}) { + const props: NewSessionDialogProps = { + open: true, + initial: {}, + machines: MACHINES, + known: [], + onSubmit: vi.fn(), + onClose: vi.fn(), + ...over, + } + const view = render() + const submitted = () => (props.onSubmit as ReturnType).mock.calls[0]?.[0] as + | NewSessionRequest + | undefined + return { ...view, props, submitted, rerender: view.rerender } +} + +const start = () => userEvent.click(screen.getByRole('button', { name: 'Start session' })) + +describe('NewSessionDialog', () => { + it('submits the ridden machine and nothing else when nothing is typed', async () => { + // The bar this had to clear to be allowed in front of a one-click button: + // opening it and pressing Start must be the old behaviour exactly. + const { submitted } = show({ initial: { machineId: 'local' } }) + + await start() + + expect(submitted()).toEqual({ machineId: 'local', cwd: '', name: '', tags: [] }) + }) + + it('carries a name, a directory and tags', async () => { + const user = userEvent.setup() + const { submitted } = show({ initial: { machineId: 'local' } }) + + await user.type(screen.getByLabelText('Name'), 'deploy') + await user.type(screen.getByLabelText('Directory'), '/srv/app') + await user.type(screen.getByLabelText('Tags'), 'ops{Enter}api{Enter}') + await start() + + expect(submitted()).toEqual({ + machineId: 'local', + cwd: '/srv/app', + name: 'deploy', + tags: ['ops', 'api'], + }) + }) + + it('counts a tag typed but never entered', async () => { + // Somebody who typed a tag and reached straight for Start is done, and a + // dialog that threw the keystroke away would be disagreeing with them in + // silence — the chips are gone before anyone can read what was sent. + const user = userEvent.setup() + const { submitted } = show() + + await user.type(screen.getByLabelText('Tags'), 'staging') + await start() + + expect(submitted()?.tags).toEqual(['staging']) + }) + + it('does not start a session on the Enter that finishes a tag', async () => { + // The tag field sits inside a real form, so its own Enter has to be + // stopped: a set the reader was still assembling is not an answer. + const user = userEvent.setup() + const { props } = show() + + await user.type(screen.getByLabelText('Tags'), 'ops{Enter}') + + expect(props.onSubmit).not.toHaveBeenCalled() + expect(screen.getByRole('button', { name: 'Remove ops' })).toBeTruthy() + }) + + it('submits on Enter from the name field, like every other form', async () => { + const user = userEvent.setup() + const { submitted } = show({ initial: { machineId: 'attic-pi' } }) + + await user.type(screen.getByLabelText('Name'), 'quick{Enter}') + + expect(submitted()).toEqual({ machineId: 'attic-pi', cwd: '', name: 'quick', tags: [] }) + }) + + it('opens on what the press implied, and lets it be edited', async () => { + const user = userEvent.setup() + const { submitted } = show({ + initial: { machineId: 'attic-pi', cwd: '/srv', tags: ['api'] }, + }) + + expect(screen.getByLabelText('Directory')).toHaveProperty('value', '/srv') + + // A prefill and not a decision: the chip is there to be taken off again. + await user.click(screen.getByRole('button', { name: 'Remove api' })) + await start() + + expect(submitted()).toEqual({ machineId: 'attic-pi', cwd: '/srv', name: '', tags: [] }) + }) + + it('offers the fleet’s own tags, minus the ones already chosen', async () => { + const user = userEvent.setup() + const { submitted } = show({ known: ['api', 'ops'], initial: { tags: ['api'] } }) + + expect(screen.queryByRole('button', { name: 'Add api' })).toBeNull() + await user.click(screen.getByRole('button', { name: 'Add ops' })) + await start() + + expect(submitted()?.tags).toEqual(['api', 'ops']) + }) + + it('puts the chosen tags under their field, not above its heading', async () => { + // Leading chips are right for a dialog whose whole subject is tags. Here + // they read as a stray line belonging to the field before them: "No tags + // yet." landed between the Directory input and a heading called Tags. + const user = userEvent.setup() + show() + + await user.type(screen.getByLabelText('Tags'), 'ops{Enter}') + + const field = screen.getByLabelText('Tags') + const chip = screen.getByRole('button', { name: 'Remove ops' }) + expect(field.compareDocumentPosition(chip) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy() + }) + + it('asks which machine only when there is a choice to make', () => { + const { unmount } = show({ machines: [MACHINES[0]!] }) + expect(screen.queryByRole('combobox', { name: 'Machine' })).toBeNull() + unmount() + + show() + expect(screen.getByRole('combobox', { name: 'Machine' })).toBeTruthy() + }) + + it('falls back to the first machine when the press named one that has gone', async () => { + // A heading for a machine that dropped between render and click. Without + // the fallback the trigger renders blank over a form pointing at an id no + // option carries. + const { submitted } = show({ initial: { machineId: 'vanished' } }) + + await start() + + expect(submitted()?.machineId).toBe('local') + }) + + it('takes the machine the fleet has not named yet, once it names it', async () => { + // The terminal screen subscribes to the fleet and the first delivery can + // land after this has rendered. A machine chosen once, at mount, would + // leave the picker empty for good. + const { rerender, submitted, props } = show({ machines: [], initial: { machineId: 'local' } }) + + expect(screen.getByText(/no machine is reachable/i)).toBeTruthy() + + rerender() + await start() + + expect(submitted()?.machineId).toBe('local') + }) + + it('refuses in words when no machine is reachable', async () => { + const { props } = show({ machines: [] }) + + expect(screen.getByRole('button', { name: 'Start session' })).toHaveProperty('disabled', true) + + await start() + + expect(props.onSubmit).not.toHaveBeenCalled() + }) + + it('closes itself after a submit, and on Cancel', async () => { + const { props } = show({ initial: { machineId: 'local' } }) + + await start() + expect(props.onClose).toHaveBeenCalledTimes(1) + + await userEvent.click(screen.getByRole('button', { name: 'Cancel' })) + expect(props.onClose).toHaveBeenCalledTimes(2) + }) +}) diff --git a/web/src/components/new-session-dialog.tsx b/web/src/components/new-session-dialog.tsx new file mode 100644 index 0000000..4db11be --- /dev/null +++ b/web/src/components/new-session-dialog.tsx @@ -0,0 +1,265 @@ +import { useId, useRef, useState, type RefObject } from 'react' + +import { TagField, withTag } from '@/components/tag-field' +import { Button } from '@/components/ui/button' +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from '@/components/ui/dialog' +import { Input } from '@/components/ui/input' +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from '@/components/ui/select' +import type { NewSessionRequest } from '@/sessions/new-session' + +/** A machine this dialog may start a session on. */ +export interface NewSessionMachine { + id: string + /** What to call it. Empty falls back to the id, as every other screen does. */ + name: string +} + +export interface NewSessionDialogProps { + open: boolean + /** + * What the press that opened this already implies: the machine of the + * heading, the directory of the terminal underneath, the tag of the group. + * Everything absent opens empty. + */ + initial: Partial + /** The machines that can carry one. An empty list disables the dialog. */ + machines: NewSessionMachine[] + /** Every tag in use across the fleet, offered as one-click additions. */ + known: string[] + /** Called with the whole request. The caller decides where it opens. */ + onSubmit(want: NewSessionRequest): void + /** Dismissal, however it happened, including after a submit. */ + onClose(): void +} + +/** + * The one place a session is asked for. + * + * It exists because of the order the daemon imposes: `spawn` carries no + * metadata, so a name and a tag can only be applied after the session already + * exists. Left to that order, naming a session means starting it, watching a + * terminal come up, going back to the list and renaming the row — and nobody + * does that, so sessions stay called after whatever shell they run. Asking + * first turns the same two round trips into one form: what is typed here is + * carried to the page that starts the session, which applies it the moment + * there is an id to apply it to. + * + * Nothing is required. Every field opens either empty or on what the press + * implied, and submitting all four untouched is exactly the old one-click + * behaviour — which is the bar this had to clear to be allowed in front of it. + * + * The form state lives one component down, inside the content Radix unmounts + * on close, for the reason the rename dialog gives: the next open builds a new + * form over new props, so a name abandoned on one press cannot follow the + * reader to the next. + */ +export function NewSessionDialog({ + open, + initial, + machines, + known, + onSubmit, + onClose, +}: NewSessionDialogProps) { + const field = useRef(null) + + return ( + { + // Escape, the overlay, and the corner X all arrive here as `false`. + // The dialog is controlled by the caller, so this is a request to + // close rather than a closing. + if (!next) onClose() + }} + > + { + // The name, which is the field this dialog exists for. Left to + // itself Radix takes the first thing it can reach, and where that + // lands depends on which prefills happened to render. + event.preventDefault() + field.current?.focus() + }} + > + + New session + + Everything here is optional. The session opens in a tab of its own. + + + { + onSubmit(want) + onClose() + }} + /> + + + ) +} + +/** + * The four fields and the two buttons. + * + * A real form element, so Enter starts the session the way Enter submits + * everywhere else — the browser's own implicit submission, reached identically + * by the button and by the keyboard. The tag field stops its own Enter (see + * TagField), which is what keeps "finish this tag" from meaning "start now". + * + * The machine picker renders only when there is a choice to make. A fleet of + * one is the ordinary case, and a select holding a single option is a control + * that asks a question with one answer. + */ +function NewSessionForm({ + field, + initial, + machines, + known, + onSubmit, + onCancel, +}: { + field: RefObject + initial: Partial + machines: NewSessionMachine[] + known: string[] + onSubmit(want: NewSessionRequest): void + onCancel(): void +}) { + const nameId = useId() + const cwdId = useId() + const machineId = useId() + + const [name, setName] = useState(initial.name ?? '') + const [cwd, setCwd] = useState(initial.cwd ?? '') + const [tags, setTags] = useState(() => [...(initial.tags ?? [])]) + const [draft, setDraft] = useState('') + /** + * The machine the reader picked, or null for "nobody has picked one". + * + * Null rather than a seeded id, so `on` below stays a *derivation* of the + * list rather than a snapshot of it taken once. The list moves: a machine + * can drop while the form is open, and on the terminal screen the fleet's + * first delivery can land after the dialog has already rendered. A seeded + * value would leave the trigger blank and the form pointing at an id no + * option carries. + */ + const [picked, setPicked] = useState(null) + const wanted = picked ?? initial.machineId + const on = machines.find((m) => m.id === wanted)?.id ?? machines[0]?.id ?? '' + + const nothingReachable = machines.length === 0 + + return ( +
{ + event.preventDefault() + if (nothingReachable) return + // The tag field is part of the answer: somebody who typed a tag and + // reached straight for Start is done, and throwing that keystroke away + // on the way out would be disagreeing with them in silence. + onSubmit({ machineId: on, cwd: cwd.trim(), name: name.trim(), tags: withTag(tags, draft) }) + }} + > +
+ + setName(event.target.value)} + /> +
+ +
+ + setCwd(event.target.value)} + className="font-mono" + /> +
+ + {machines.length > 1 && ( +
+ + +
+ )} + + + + {nothingReachable && ( + // Said rather than hidden. The dialog opens from a button that was + // pressed, and a form that silently refused would leave the reader + // pressing Start at a fleet that is not there. +

+ No machine is reachable, so nothing can be started right now. +

+ )} + + + + + + + ) +} diff --git a/web/src/components/session-table.test.tsx b/web/src/components/session-table.test.tsx index 13cbb86..7790c5b 100644 --- a/web/src/components/session-table.test.tsx +++ b/web/src/components/session-table.test.tsx @@ -129,7 +129,11 @@ describe('SessionTable', () => { }) const names = screen.getAllByRole('link').map((a) => a.getAttribute('aria-label')) - expect(names).toEqual(['Open zeta', 'Open alpha', 'Open mid']) + expect(names).toEqual([ + 'Open zeta in a new tab', + 'Open alpha in a new tab', + 'Open mid in a new tab', + ]) }) it('asks for a toggle rather than deciding one', async () => { @@ -170,7 +174,7 @@ describe('SessionTable', () => { // 'name' as always on, whatever the columns preference says. await renderTable({ columns: ['state'] }) - expect(screen.getByRole('link', { name: 'Open zsh' })).toBeTruthy() + expect(screen.getByRole('link', { name: 'Open zsh in a new tab' })).toBeTruthy() expect(screen.getByText('zsh')).toBeTruthy() }) @@ -364,25 +368,28 @@ describe('SessionTable', () => { }) describe('opening', () => { - it('makes the whole row one link to its session', async () => { - const { router } = await renderTable() + it('makes the whole row one link to its session, in a tab of its own', async () => { + await renderTable() - const link = screen.getByRole('link', { name: 'Open zsh' }) + const link = screen.getByRole('link', { name: 'Open zsh in a new tab' }) // A real href on a real anchor: this is what a middle click, a copied // address and a Ctrl/Cmd click all read. expect(link.getAttribute('href')).toBe('/d/m1/s/a1') - - await userEvent.click(link) - expect(router.state.location.pathname).toBe('/d/m1/s/a1') + expect(link.getAttribute('target')).toBe('_blank') + expect(link.getAttribute('rel')).toBe('noopener') }) - it('leaves a modified click to the browser, which owns the new tab', async () => { - // Ctrl/Cmd and middle clicks mean "a new tab" and only the browser can - // honour that. The router must not swallow them — TanStack's Link - // stands aside for a modified click, so the location holds still here - // while a real browser would be opening the terminal beside this tab. + it('leaves the click to the browser, which owns the new tab', async () => { + // A target the router honours by standing aside — TanStack's Link hands + // any click with a target other than _self straight to the browser — so + // the location holds still here while a real browser opens the terminal + // beside this list. Ctrl, Cmd and middle clicks were already the + // browser's, and stay that way. const { router } = await renderTable() - const link = screen.getByRole('link', { name: 'Open zsh' }) + const link = screen.getByRole('link', { name: 'Open zsh in a new tab' }) + + await userEvent.click(link) + expect(router.state.location.pathname).toBe('/sessions') fireEvent.click(link, { ctrlKey: true }) expect(router.state.location.pathname).toBe('/sessions') diff --git a/web/src/components/session-table.tsx b/web/src/components/session-table.tsx index 60069b5..2ee872a 100644 --- a/web/src/components/session-table.tsx +++ b/web/src/components/session-table.tsx @@ -184,7 +184,17 @@ function SessionRow({ diff --git a/web/src/components/tag-editor.tsx b/web/src/components/tag-editor.tsx index 5dfd351..bea03ba 100644 --- a/web/src/components/tag-editor.tsx +++ b/web/src/components/tag-editor.tsx @@ -1,7 +1,6 @@ -import { useId, useRef, useState, type RefObject } from 'react' -import { PlusIcon, XMarkIcon } from '@heroicons/react/16/solid' +import { useRef, useState, type RefObject } from 'react' -import { Badge } from '@/components/ui/badge' +import { TagField, withTag } from '@/components/tag-field' import { Button } from '@/components/ui/button' import { Dialog, @@ -11,7 +10,6 @@ import { DialogHeader, DialogTitle, } from '@/components/ui/dialog' -import { Input } from '@/components/ui/input' export interface TagEditorProps { open: boolean @@ -86,25 +84,6 @@ export function TagEditor({ open, current, known, onSubmit, onClose }: TagEditor ) } -/** - * The set with one more tag in it: trimmed, and unchanged if the tag is blank - * or already there. - * - * Three routes reach this rule and it cannot hold for two of them — a typed - * Enter, a clicked suggestion, and Save over a field the reader never pressed - * Enter on. That third one is the reason this is a function rather than four - * lines inside the first handler that needed them. - * - * The comparison is exact rather than case-folded: `API` and `api` are two - * strings until the daemon says otherwise, and folding them here would drop a - * tag the reader had just watched themselves type. - */ -function withTag(held: string[], tag: string): string[] { - const clean = tag.trim() - if (clean === '' || held.includes(clean)) return held - return [...held, clean] -} - /** * The chips, the field, the suggestions, and the two buttons. * @@ -117,7 +96,8 @@ function withTag(held: string[], tag: string): string[] { * State seeded once from `current` and never resynced: the content this sits * in is unmounted on close, so the seeding happens exactly when the dialog * opens and a set abandoned on one session cannot follow the reader to the - * next. + * next. The draft is held here rather than inside the field for the reason + * Save's own comment gives — it is part of the answer. */ function TagForm({ field, @@ -132,100 +112,19 @@ function TagForm({ onSubmit(tags: string[]): void onCancel(): void }) { - const fieldId = useId() - const suggestionsId = useId() const [tags, setTags] = useState(() => [...current]) const [draft, setDraft] = useState('') - /** A typed Enter and a clicked suggestion, both of which empty the field. */ - const add = (tag: string) => { - setDraft('') - setTags((held) => withTag(held, tag)) - } - - // What the fleet knows, minus what this session already carries, narrowed by - // what has been typed so far. A prefix rather than a substring: a reader - // typing `de` is reaching for a tag they can already half-remember, and - // matching the middle of words would answer with tags they were not naming. - const needle = draft.trim().toLowerCase() - const offered = known.filter( - (tag) => !tags.includes(tag) && tag.toLowerCase().startsWith(needle), - ) - return (
-
- {tags.length === 0 ? ( -

No tags yet.

- ) : ( - tags.map((tag) => ( - /* - The chip is the remove control, which is why its accessible name - says so: the word on screen is the tag, and a reader hearing - only "api, button" would have no idea that pressing it takes the - tag away. The X, and the destructive tint under the pointer, say - the same thing to the eye. - */ - - - - )) - )} -
- -
- - setDraft(event.target.value)} - onKeyDown={(event) => { - if (event.key !== 'Enter') return - // Nothing above would submit on Enter today, but this field will - // one day sit inside something that does. - event.preventDefault() - add(draft) - }} - /> -
- - {offered.length > 0 && ( -
-

- Suggestions -

- {/* Capped in height and scrolled: a fleet with forty tags in it - must not push Save off the bottom of the screen. */} -
- {offered.map((tag) => ( - - ))} -
-
- )} + + + )) + )} +
+ ) + + return ( +
+ {chipsFirst && chips} + +
+ + onDraft(event.target.value)} + onKeyDown={(event) => { + if (event.key !== 'Enter') return + // Stopped whether or not it adds anything. This field sits inside a + // real form in the new-session dialog, and an Enter left to bubble + // would submit that form — starting a session on the keystroke that + // was meant to finish a tag. + event.preventDefault() + add(draft) + }} + /> + {!chipsFirst && chips} +
+ + {offered.length > 0 && ( +
+

+ Suggestions +

+ {/* Capped in height and scrolled: a fleet with forty tags in it must + not push the buttons off the bottom of the screen. */} +
+ {offered.map((tag) => ( + + ))} +
+
+ )} +
+ ) +} diff --git a/web/src/components/terminal.test.tsx b/web/src/components/terminal.test.tsx index a13ea7a..3f8e01a 100644 --- a/web/src/components/terminal.test.tsx +++ b/web/src/components/terminal.test.tsx @@ -1,5 +1,6 @@ import { StrictMode, type ReactNode } from 'react' import { act, fireEvent, render, screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { FlueClientProvider } from '@/client/provider' @@ -1658,9 +1659,16 @@ describe('the terminal theme', () => { }) }) -describe('the new-session link', () => { - it('carries the session’s directory and opens a new tab', () => { - const { sock } = mountTerminal((em) => ) +describe('the new-session control', () => { + it('hands this session’s directory up, for whoever owns the form', async () => { + // It used to be a link to `/?cwd=`, which is the dashboard — so a session + // started from a terminal came up behind the whole list. The chip asks + // above it now, and the directory is the one thing only this component + // knows: the list is where a session's cwd arrives. + const onNewSession = vi.fn() + const { sock } = mountTerminal((em) => ( + + )) // Asked for on mount: the list is where the cwd comes from. expect(sock.ofType('list')).toHaveLength(1) @@ -1669,8 +1677,22 @@ describe('the new-session link', () => { sock.emitControl({ type: 'sessions', sessions: [session({ cwd: '/tmp/with space' })] }), ) - const link = screen.getByRole('link', { name: 'New session in this directory' }) - expect(link.getAttribute('href')).toBe(`/?cwd=${encodeURIComponent('/tmp/with space')}`) - expect(link.getAttribute('target')).toBe('_blank') + await userEvent.click(screen.getByRole('button', { name: 'New session in this directory' })) + + expect(onNewSession).toHaveBeenCalledWith('/tmp/with space') + }) + + it('says so rather than guessing when the list has not answered yet', async () => { + // Null, not ''. The form prefills its directory field from this, and an + // empty string there reads as "the reader cleared it" — which is a + // different instruction to the daemon than "nothing is known". + const onNewSession = vi.fn() + mountTerminal((em) => ( + + )) + + await userEvent.click(screen.getByRole('button', { name: 'New session in this directory' })) + + expect(onNewSession).toHaveBeenCalledWith(null) }) }) diff --git a/web/src/components/terminal.tsx b/web/src/components/terminal.tsx index 73d52fb..de6af04 100644 --- a/web/src/components/terminal.tsx +++ b/web/src/components/terminal.tsx @@ -51,6 +51,16 @@ export interface TerminalProps { onRestarted?: (sessionId: string) => void /** Called after Close has closed the dead session; navigate away here. */ onClosed?: () => void + /** + * Called by the `+` in the control strip, with this session's directory when + * the list has said what it is. + * + * The dialog it opens lives above this component, not in it, and that is the + * same bargain the rest of the file keeps: the form needs the fleet's + * machines and the fleet's tags, and a terminal that knew the fleet existed + * would be a terminal that could not be mounted without one. + */ + onNewSession?: (cwd: string | null) => void } /** Named so the test and the markup cannot drift apart. */ @@ -124,6 +134,7 @@ export function Terminal({ createEmulator = createXtermEmulator, onRestarted, onClosed, + onNewSession, }: TerminalProps) { const client = useFlueClient() const switcher = useSwitcher() @@ -883,15 +894,11 @@ export function Terminal({