From 8a70d70006ad5858c02dd0148bb9029e4e53f056 Mon Sep 17 00:00:00 2001 From: starptech Date: Thu, 16 Jul 2026 21:45:16 +0200 Subject: [PATCH] revert(tui): remove message navigation shortcuts --- packages/tui/src/config/v1/keybind.ts | 10 +- packages/tui/src/routes/session/index.tsx | 147 +++++----------- .../src/routes/session/message-navigation.ts | 48 ----- packages/tui/src/routes/session/rows.ts | 30 ---- .../test/cli/tui/message-navigation.test.ts | 164 ------------------ .../tui/test/cli/tui/session-rows.test.ts | 16 +- packages/tui/test/config.test.tsx | 10 -- packages/tui/test/keymap.test.tsx | 58 +------ 8 files changed, 53 insertions(+), 430 deletions(-) delete mode 100644 packages/tui/src/routes/session/message-navigation.ts delete mode 100644 packages/tui/test/cli/tui/message-navigation.test.ts diff --git a/packages/tui/src/config/v1/keybind.ts b/packages/tui/src/config/v1/keybind.ts index d0bc864731..67f64bc68e 100644 --- a/packages/tui/src/config/v1/keybind.ts +++ b/packages/tui/src/config/v1/keybind.ts @@ -136,11 +136,9 @@ export const Definitions = { messages_half_page_down: keybind("ctrl+alt+d", "Scroll messages down by half page"), messages_first: keybind("ctrl+g,home", "Navigate to first message"), messages_last: keybind("ctrl+alt+g,end", "Navigate to last message"), - messages_next: keybind("alt+shift+down", "Navigate to next message"), - messages_previous: keybind("alt+shift+up", "Navigate to previous message"), - messages_next_user: keybind("alt+down", "Navigate to next user message"), - messages_previous_user: keybind("alt+up", "Navigate to previous user message"), - messages_last_user: keybind("alt+end", "Navigate to last user message"), + messages_next: keybind("none", "Navigate to next message"), + messages_previous: keybind("none", "Navigate to previous message"), + messages_last_user: keybind("none", "Navigate to last user message"), messages_copy: keybind("y", "Copy message"), messages_undo: keybind("u", "Undo message"), messages_redo: keybind("r", "Redo message"), @@ -337,8 +335,6 @@ export const CommandMap = { messages_last: "session.last", messages_next: "session.message.next", messages_previous: "session.message.previous", - messages_next_user: "session.message.user.next", - messages_previous_user: "session.message.user.previous", messages_last_user: "session.messages_last_user", messages_copy: "messages.copy", messages_undo: "session.undo", diff --git a/packages/tui/src/routes/session/index.tsx b/packages/tui/src/routes/session/index.tsx index 11370dbd53..4835ce247e 100644 --- a/packages/tui/src/routes/session/index.tsx +++ b/packages/tui/src/routes/session/index.tsx @@ -71,15 +71,11 @@ import { usePluginRuntime } from "../../plugin/runtime" import { OPENCODE_BASE_MODE, useBindings, useCommandShortcut } from "../../keymap" import { usePathFormatter } from "../../context/path-format" import { useLocation } from "../../context/location" -import { createSessionRows, messageBoundaryIDs, resolvePart, type PartRef, type SessionRow } from "./rows" +import { createSessionRows, resolvePart, type PartRef, type SessionRow } from "./rows" import { switchLabel } from "../../util/model" -import { findMessageBoundary, messageNavigationSlack } from "./message-navigation" addDefaultParsers(parsers.parsers) -// Exclude temporary bottom space when measuring the real transcript height. -const NAVIGATION_SLACK_ID = "session-navigation-slack" - const context = createContext<{ width: number sessionID: string @@ -184,23 +180,6 @@ export function Session() { const client = useClient() const editor = useEditorContext() const rows = createSessionRows(() => route.sessionID) - const boundaries = createMemo(() => messageBoundaryIDs(rows, messages())) - const [navigationMessage, setNavigationMessage] = createSignal() - const [navigationSlack, setNavigationSlack] = createSignal(0) - - const clearMessageNavigation = () => { - setNavigationSlack(0) - setNavigationMessage(undefined) - } - - createEffect( - on( - () => [dimensions().width, dimensions().height] as const, - (_, previous) => { - if (previous) clearMessageNavigation() - }, - ), - ) createEffect( on([descendantSessionIDs, () => client.connection.status()], ([sessionIDs, status]) => { @@ -260,55 +239,53 @@ export function Session() { dialog.clear() } - const alignMessage = (messageID: string, top: number) => { - scroll.stickyScroll = false - setNavigationMessage(messageID) - setNavigationSlack( - messageNavigationSlack({ - top, - viewportHeight: scroll.viewport.height, - scrollHeight: scroll.scrollHeight, - currentSlack: scroll.getRenderable(NAVIGATION_SLACK_ID)?.height ?? 0, - }), - ) - requestAnimationFrame(() => { - requestAnimationFrame(() => { - if (scroll.isDestroyed || navigationMessage() !== messageID) return - scroll.scrollTo(top) + // Helper: Find next visible message boundary in direction + const findNextVisibleMessage = (direction: "next" | "prev"): string | null => { + const children = scroll.getChildren() + const messagesList = messages() + const scrollTop = scroll.y + + // Get visible messages sorted by position, filtering for valid non-synthetic, non-ignored content + const visibleMessages = children + .filter((c) => { + if (!c.id) return false + const message = messagesList.find((m) => m.id === c.id) + if (!message) return false + + if (message.type === "user") return Boolean(message.text.trim()) + return ( + message.type === "assistant" && + message.content.some((content) => content.type === "text" && content.text.trim()) + ) }) - }) + .sort((a, b) => a.y - b.y) + + if (visibleMessages.length === 0) return null + + if (direction === "next") { + // Find first message below current position + return visibleMessages.find((c) => c.y > scrollTop + 10)?.id ?? null + } + // Find last message above current position + return [...visibleMessages].reverse().find((c) => c.y < scrollTop - 10)?.id ?? null } - const scrollToMessage = (direction: "next" | "prev", dialog: ReturnType, userOnly = false) => { - const target = findMessageBoundary({ - direction, - children: scroll.getChildren(), - messages: messages(), - scrollTop: scroll.scrollTop, - viewportY: scroll.viewport.y, - currentID: navigationMessage(), - userOnly, - }) + // Helper: Scroll to message in direction or fallback to page scroll + const scrollToMessage = (direction: "next" | "prev", dialog: ReturnType) => { + const targetID = findNextVisibleMessage(direction) - if (!target) { + if (!targetID) { + scroll.scrollBy(direction === "next" ? scroll.height : -scroll.height) dialog.clear() return } - alignMessage(target.id, target.top) + const child = scroll.getChildren().find((c) => c.id === targetID) + if (child) scroll.scrollBy(child.y - scroll.y - 1) dialog.clear() } - const jumpToMessage = (messageID: string) => { - const child = scroll.getRenderable(messageID) - if (!child) return - const y = scroll.scrollTop + child.y - scroll.viewport.y - const message = data.session.message.get(route.sessionID, messageID) - alignMessage(messageID, Math.max(0, y - (message?.type === "assistant" ? 1 : 0))) - } - function toBottom() { - clearMessageNavigation() setTimeout(() => { if (!scroll || scroll.isDestroyed) return scroll.scrollTo(scroll.scrollHeight) @@ -322,7 +299,6 @@ export function Session() { category: "Session", hidden: true, run: () => { - clearMessageNavigation() scroll.scrollBy(-scroll.height / 2) dialog.clear() }, @@ -333,7 +309,6 @@ export function Session() { category: "Session", hidden: true, run: () => { - clearMessageNavigation() scroll.scrollBy(scroll.height / 2) dialog.clear() }, @@ -344,7 +319,6 @@ export function Session() { category: "Session", hidden: true, run: () => { - clearMessageNavigation() scroll.scrollBy(-1) dialog.clear() }, @@ -355,7 +329,6 @@ export function Session() { category: "Session", hidden: true, run: () => { - clearMessageNavigation() scroll.scrollBy(1) dialog.clear() }, @@ -366,7 +339,6 @@ export function Session() { category: "Session", hidden: true, run: () => { - clearMessageNavigation() scroll.scrollBy(-scroll.height / 4) dialog.clear() }, @@ -377,7 +349,6 @@ export function Session() { category: "Session", hidden: true, run: () => { - clearMessageNavigation() scroll.scrollBy(scroll.height / 4) dialog.clear() }, @@ -391,7 +362,6 @@ export function Session() { category: "Session", hidden: true, run: () => { - clearMessageNavigation() scroll.scrollTo(0) dialog.clear() }, @@ -402,7 +372,6 @@ export function Session() { category: "Session", hidden: true, run: () => { - clearMessageNavigation() scroll.scrollTo(scroll.scrollHeight) dialog.clear() }, @@ -443,7 +412,8 @@ export function Session() { sessionID={route.sessionID} onMove={(messageID) => { if (!messageID) return - jumpToMessage(messageID) + const child = scroll.getChildren().find((child) => child.id === messageID) + if (child) scroll.scrollBy(child.y - scroll.y - 1) }} /> )) @@ -604,7 +574,10 @@ export function Session() { const message = messages[i] if (!message || message.type !== "user" || !message.text.trim()) continue { - jumpToMessage(message.id) + const child = scroll.getChildren().find((child) => { + return child.id === message.id + }) + if (child) scroll.scrollBy(child.y - scroll.y - 1) break } } @@ -624,20 +597,6 @@ export function Session() { hidden: true, run: () => scrollToMessage("prev", dialog), }, - { - title: "Next user message", - name: "session.message.user.next", - category: "Session", - hidden: true, - run: () => scrollToMessage("next", dialog, true), - }, - { - title: "Previous user message", - name: "session.message.user.previous", - category: "Session", - hidden: true, - run: () => scrollToMessage("prev", dialog, true), - }, { title: "Copy last assistant message", name: "messages.copy", @@ -851,10 +810,7 @@ export function Session() { createEffect( on( () => route.sessionID, - () => { - setComposer("open", false) - clearMessageNavigation() - }, + () => setComposer("open", false), ), ) @@ -890,17 +846,16 @@ export function Session() { foregroundColor: themeV2.border(), }, }} - stickyScroll={!navigationMessage()} + stickyScroll={true} stickyStart="bottom" flexGrow={1} scrollAcceleration={scrollAcceleration()} > - {(row, index) => ( + {(row) => ( data.session.message.get(route.sessionID, messageID)} - boundaryID={boundaries()[index()]} /> )} @@ -915,9 +870,6 @@ export function Session() { files={session()!.revert!.files ?? []} /> - - {(height) => } - SessionMessageInfo | undefined - boundaryID?: string -}) { +function SessionRowView(props: { row: SessionRow; message: (messageID: string) => SessionMessageInfo | undefined }) { return ( - + {(row) => ( @@ -1615,6 +1563,7 @@ function UserMessage(props: { message: SessionMessageUser }) { return ( [message.id, message])) - const visible = input.children - .flatMap((child) => { - if (!child.id) return [] - const message = messages.get(child.id) - if (!message) return [] - if (message.type === "user" && message.text.trim()) { - const y = input.scrollTop + child.y - input.viewportY - return [{ id: child.id, y, top: y }] - } - if (input.userOnly || message.type !== "assistant") return [] - if (!message.content.some((content) => content.type === "text" && content.text.trim())) return [] - const y = input.scrollTop + child.y - input.viewportY - return [{ id: child.id, y, top: Math.max(0, y - 1) }] - }) - .sort((a, b) => a.y - b.y) - - const current = visible.findIndex((child) => child.id === input.currentID) - if (current !== -1) return visible[current + (input.direction === "next" ? 1 : -1)] ?? null - if (input.direction === "next") return visible.find((child) => child.y > input.scrollTop) ?? null - return visible.findLast((child) => child.y < input.scrollTop) ?? null -} diff --git a/packages/tui/src/routes/session/rows.ts b/packages/tui/src/routes/session/rows.ts index dc4a681bb9..471b282097 100644 --- a/packages/tui/src/routes/session/rows.ts +++ b/packages/tui/src/routes/session/rows.ts @@ -285,36 +285,6 @@ export function reduceSessionRows(messages: SessionMessageInfo[], inputs = new S }, []) } -export function messageBoundaryIDs(rows: SessionRow[], messages: SessionMessageInfo[]) { - const byID = new Map(messages.map((message) => [message.id, message])) - const seen = new Set() - return rows.map((row) => { - const id = rowBoundaryMessageID(row, byID) - if (!id || seen.has(id)) return undefined - seen.add(id) - return id - }) -} - -function rowBoundaryMessageID(row: SessionRow, messages: Map) { - if (row.type === "message") { - const message = messages.get(row.messageID) - if (message?.type === "user" && message.text.trim()) return message.id - return undefined - } - const messageID = - row.type === "part" - ? row.ref.messageID - : row.type === "group" - ? row.refs[0]?.messageID - : row.type === "assistant-footer" - ? row.messageID - : undefined - if (!messageID) return undefined - const message = messages.get(messageID) - if (message?.type === "assistant") return message.id -} - export function resolvePart(message: SessionMessageAssistant, partID: string) { const tool = message.content.find((part) => part.type === "tool" && part.id === partID) if (tool) return tool diff --git a/packages/tui/test/cli/tui/message-navigation.test.ts b/packages/tui/test/cli/tui/message-navigation.test.ts deleted file mode 100644 index 97a29458f3..0000000000 --- a/packages/tui/test/cli/tui/message-navigation.test.ts +++ /dev/null @@ -1,164 +0,0 @@ -import { expect, test } from "bun:test" -import type { SessionMessageAssistant, SessionMessageInfo } from "@opencode-ai/client" -import { findMessageBoundary, messageNavigationSlack } from "../../../src/routes/session/message-navigation" - -const messages: SessionMessageInfo[] = [ - { type: "user", id: "user-1", text: "First", time: { created: 0 } }, - assistant("assistant-1", "Response"), - { type: "user", id: "user-2", text: "Second", time: { created: 2 } }, -] -const children = [ - { id: "user-1", y: 0 }, - { id: "assistant-1", y: 20 }, - { id: "user-2", y: 40 }, -] - -test("adds only enough slack to align the selected message", () => { - expect(messageNavigationSlack({ top: 80, viewportHeight: 50, scrollHeight: 100, currentSlack: 0 })).toBe(30) - expect(messageNavigationSlack({ top: 20, viewportHeight: 50, scrollHeight: 130, currentSlack: 30 })).toBe(0) -}) - -test("finds the next user message without stopping at an assistant message", () => { - expect( - findMessageBoundary({ - direction: "next", - children, - messages, - scrollTop: 0, - viewportY: 0, - userOnly: true, - }), - ).toEqual({ id: "user-2", y: 40, top: 40 }) -}) - -test("finds the previous user message without stopping at an assistant message", () => { - expect( - findMessageBoundary({ - direction: "prev", - children: children.map((child) => ({ ...child, y: child.y - 35 })), - messages, - scrollTop: 35, - viewportY: 0, - userOnly: true, - }), - ).toEqual({ id: "user-1", y: 0, top: 0 }) -}) - -test("preserves navigation across both user and assistant messages", () => { - expect( - findMessageBoundary({ - direction: "next", - children, - messages, - scrollTop: 0, - viewportY: 0, - }), - ).toEqual({ id: "assistant-1", y: 20, top: 19 }) - expect( - findMessageBoundary({ - direction: "prev", - children: children.map((child) => ({ ...child, y: child.y - 35 })), - messages, - scrollTop: 35, - viewportY: 0, - }), - ).toEqual({ id: "assistant-1", y: 20, top: 19 }) -}) - -test("uses the selected message when the viewport is too tall to scroll", () => { - expect( - findMessageBoundary({ - direction: "next", - children, - messages, - scrollTop: 0, - viewportY: 0, - currentID: "user-1", - userOnly: true, - }), - ).toEqual({ id: "user-2", y: 40, top: 40 }) - expect( - findMessageBoundary({ - direction: "prev", - children, - messages, - scrollTop: 0, - viewportY: 0, - currentID: "user-2", - userOnly: true, - }), - ).toEqual({ id: "user-1", y: 0, top: 0 }) -}) - -test("stops at the first and last selected user message", () => { - expect( - findMessageBoundary({ - direction: "next", - children, - messages, - scrollTop: 0, - viewportY: 0, - currentID: "user-2", - userOnly: true, - }), - ).toBeNull() - expect( - findMessageBoundary({ - direction: "prev", - children, - messages, - scrollTop: 0, - viewportY: 0, - currentID: "user-1", - userOnly: true, - }), - ).toBeNull() -}) - -test("keeps the logical boundary when layout temporarily moves it outside the viewport", () => { - expect( - findMessageBoundary({ - direction: "next", - children, - messages, - scrollTop: 0, - viewportY: 0, - currentID: "user-2", - userOnly: true, - }), - ).toBeNull() -}) - -test("stops at the first and last message", () => { - expect( - findMessageBoundary({ - direction: "next", - children, - messages, - scrollTop: 0, - viewportY: 0, - currentID: "user-2", - }), - ).toBeNull() - expect( - findMessageBoundary({ - direction: "prev", - children, - messages, - scrollTop: 0, - viewportY: 0, - currentID: "user-1", - }), - ).toBeNull() -}) - -function assistant(id: string, text: string): SessionMessageAssistant { - return { - type: "assistant", - id, - agent: "build", - model: { providerID: "test", id: "test" }, - content: [{ type: "text", text }], - time: { created: 1, completed: 1 }, - } -} diff --git a/packages/tui/test/cli/tui/session-rows.test.ts b/packages/tui/test/cli/tui/session-rows.test.ts index c84237a3b7..e158a377b0 100644 --- a/packages/tui/test/cli/tui/session-rows.test.ts +++ b/packages/tui/test/cli/tui/session-rows.test.ts @@ -1,20 +1,6 @@ import { expect, test } from "bun:test" import type { SessionMessageAssistant, SessionMessageInfo } from "@opencode-ai/client" -import { messageBoundaryIDs, reduceSessionRows } from "../../../src/routes/session/rows" - -test("assigns assistant boundaries to the first rendered row instead of the first text row", () => { - const messages: SessionMessageInfo[] = [ - { type: "user", id: "user-1", text: "Question", time: { created: 0 } }, - assistant("assistant-1", [ - { type: "reasoning", text: "Thinking" }, - { type: "text", text: "First" }, - { type: "text", text: "Second" }, - ]), - ] - const rows = reduceSessionRows(messages) - - expect(messageBoundaryIDs(rows, messages)).toEqual(["user-1", "assistant-1", undefined, undefined]) -}) +import { reduceSessionRows } from "../../../src/routes/session/rows" test("groups exploration parts across assistant messages until a delimiter", () => { const messages: SessionMessageInfo[] = [ diff --git a/packages/tui/test/config.test.tsx b/packages/tui/test/config.test.tsx index 1a53d94e26..96f7430d53 100644 --- a/packages/tui/test/config.test.tsx +++ b/packages/tui/test/config.test.tsx @@ -86,16 +86,6 @@ test("resolves a session move keybind", () => { expect(config.keybinds.get("session.move")).toMatchObject([{ key: "ctrl+o" }]) }) -test("resolves message navigation defaults", () => { - const config = resolve({}, { terminalSuspend: true }) - - expect(config.keybinds.get("session.message.previous")).toMatchObject([{ key: "alt+shift+up" }]) - expect(config.keybinds.get("session.message.next")).toMatchObject([{ key: "alt+shift+down" }]) - expect(config.keybinds.get("session.message.user.previous")).toMatchObject([{ key: "alt+up" }]) - expect(config.keybinds.get("session.message.user.next")).toMatchObject([{ key: "alt+down" }]) - expect(config.keybinds.get("session.messages_last_user")).toMatchObject([{ key: "alt+end" }]) -}) - test("opens the subagent picker with down", () => { const config = resolve({}, { terminalSuspend: true }) diff --git a/packages/tui/test/keymap.test.tsx b/packages/tui/test/keymap.test.tsx index f22c4824bf..3f2ebcdf76 100644 --- a/packages/tui/test/keymap.test.tsx +++ b/packages/tui/test/keymap.test.tsx @@ -1,10 +1,9 @@ /** @jsxImportSource @opentui/solid */ import { createDefaultOpenTuiKeymap } from "@opentui/keymap/opentui" import { createBindingLookup } from "@opentui/keymap/extras" -import { type TextareaRenderable } from "@opentui/core" import { testRender, useRenderer } from "@opentui/solid" import { expect, test } from "bun:test" -import { onCleanup, onMount } from "solid-js" +import { onCleanup } from "solid-js" import { TuiKeybind } from "../src/config/keybind" import { formatKeySequence, @@ -111,61 +110,6 @@ test("formats navigation keys as arrows", async () => { } }) -test("dispatches user message navigation while the composer is focused", async () => { - for (const kittyKeyboard of [false, true]) { - const counts = { - "session.message.user.previous": 0, - "session.message.user.next": 0, - "session.messages_last_user": 0, - } - - function Harness() { - const renderer = useRenderer() - const keymap = createDefaultOpenTuiKeymap(renderer) - const config = createResolvedKeymapConfig() - const offKeymap = registerOpencodeKeymap(keymap, renderer, config) - const commands = Object.keys(counts) as (keyof typeof counts)[] - const offLayer = keymap.registerLayer({ - commands: commands.map((name) => ({ - name, - run() { - counts[name]++ - }, - })), - bindings: commands.flatMap((command) => config.keybinds.get(command)), - }) - let textarea: TextareaRenderable - onMount(() => textarea.focus()) - onCleanup(() => { - offLayer() - offKeymap() - }) - - return ( - -