fix(tui): update tool spacing before layout

This commit is contained in:
Kit Langton
2026-06-05 15:56:10 -04:00
committed by GitHub
parent a57fb32d95
commit c814f84c87
4 changed files with 97 additions and 37 deletions
@@ -30,6 +30,7 @@ import type {
} from "@opencode-ai/sdk/v2"
import { createEffect, createMemo, createSignal, For, Match, Show, Switch } from "solid-js"
import { collapseToolOutput } from "../../util/collapse-tool-output"
import { setPreLayoutSiblingMargin } from "../../util/layout"
const id = "internal:session-v2-debug"
const route = "session.v2.messages"
@@ -357,7 +358,7 @@ function AssistantText(props: { part: SessionMessageAssistantText; syntax: Synta
const { theme } = useTheme()
return (
<Show when={props.part.text.trim()}>
<box paddingLeft={3} marginTop={1} flexShrink={0} id="text">
<box paddingLeft={3} marginTop={1} flexShrink={0} id={`text-${props.part.id}`}>
<code
filetype="markdown"
drawUnstyledText={false}
@@ -569,7 +570,6 @@ function InlineTool(props: {
}) {
const { theme } = useTheme()
const renderer = useRenderer()
const [margin, setMargin] = createSignal(0)
const [hover, setHover] = createSignal(false)
const [showError, setShowError] = createSignal(false)
const error = createMemo(() => (props.part.state.status === "error" ? props.part.state.error.message : undefined))
@@ -592,7 +592,6 @@ function InlineTool(props: {
const attributes = createMemo(() => (denied() ? TextAttributes.STRIKETHROUGH : undefined))
return (
<box
marginTop={margin()}
paddingLeft={3}
flexShrink={0}
flexDirection="row"
@@ -605,16 +604,8 @@ function InlineTool(props: {
if (renderer.getSelection()?.getSelectedText()) return
setShowError((prev) => !prev)
}}
renderBefore={function () {
const el = this as BoxRenderable
const parent = el.parent
if (!parent) return
const previous = parent.getChildren()[parent.getChildren().indexOf(el) - 1]
if (!previous) {
setMargin(0)
return
}
if (previous.id.startsWith("text")) setMargin(1)
ref={(el: BoxRenderable) => {
setPreLayoutSiblingMargin(el, (previous) => (previous?.id.startsWith("text-") ? 1 : 0))
}}
>
<box flexShrink={0}>
@@ -80,6 +80,7 @@ import { QuestionPrompt } from "./question"
import { DialogExportOptions } from "../../ui/dialog-export-options"
import * as Model from "../../util/model"
import { formatTranscript } from "../../util/transcript"
import { setPreLayoutSiblingMargin } from "../../util/layout"
import { UI } from "@/cli/ui.ts"
import { useTuiConfig } from "../../context/tui-config"
import { nextThinkingMode, reasoningSummary, useThinkingMode, type ThinkingMode } from "../../context/thinking"
@@ -168,6 +169,7 @@ const context = createContext<{
showTimestamps: () => boolean
showDetails: () => boolean
showGenericToolOutput: () => boolean
userMessageIDs: () => ReadonlySet<string>
diffWrapMode: () => "word" | "none"
providers: () => ReadonlyMap<string, Provider>
sync: ReturnType<typeof useSync>
@@ -209,6 +211,14 @@ export function Session() {
),
),
)
const userMessageIDs = createMemo(
() =>
new Set(
messages()
.filter((message) => message.role === "user")
.map((message) => message.id),
),
)
const permissions = createMemo(() => {
if (session()?.parentID) return []
return children().flatMap((x) => sync.data.permission[x.id] ?? [])
@@ -1160,6 +1170,7 @@ export function Session() {
showTimestamps,
showDetails,
showGenericToolOutput,
userMessageIDs,
diffWrapMode,
providers,
sync,
@@ -1895,9 +1906,7 @@ function InlineTool(props: {
pending={props.pending}
spinner={props.spinner}
subagent={props.subagent}
separateAfter={(id) =>
sync.data.message[ctx.sessionID]?.some((message) => message.role === "user" && message.id === id) ?? false
}
separateAfter={(id) => id !== undefined && ctx.userMessageIDs().has(id)}
onMouseOver={() => clickable() && setHover(true)}
onMouseOut={() => setHover(false)}
onMouseUp={() => {
@@ -1934,35 +1943,24 @@ export function InlineToolRow(props: {
onMouseOut?: () => void
onMouseUp?: () => void
}) {
const [margin, setMargin] = createSignal(0)
return (
<box
id={props.id}
marginTop={margin()}
paddingLeft={3}
onMouseOver={props.onMouseOver}
onMouseOut={props.onMouseOut}
onMouseUp={props.onMouseUp}
renderBefore={function () {
const el = this as BoxRenderable
const parent = el.parent
if (!parent) {
return
}
const children = parent.getChildren()
const index = children.indexOf(el)
const previous = children[index - 1]
const previousInline = previous?.id.startsWith("tool-inline-") ?? false
const previousSubagent = previous?.id.startsWith("tool-inline-subagent-") ?? false
setMargin(
previous?.id.startsWith("text-") ||
ref={(el: BoxRenderable) => {
setPreLayoutSiblingMargin(el, (previous) => {
const previousInline = previous?.id.startsWith("tool-inline-") ?? false
const previousSubagent = previous?.id.startsWith("tool-inline-subagent-") ?? false
return previous?.id.startsWith("text-") ||
previous?.id.startsWith("tool-block-") ||
(previousInline && previousSubagent !== Boolean(props.subagent)) ||
props.separateAfter?.(previous?.id)
? 1
: 0,
)
: 0
})
}}
>
<Switch>
@@ -0,0 +1,25 @@
import type { BaseRenderable, BoxRenderable } from "@opentui/core"
const previousByParent = new WeakMap<
BaseRenderable,
{ frameID: number; previous: WeakMap<BaseRenderable, BaseRenderable | undefined> }
>()
export function setPreLayoutSiblingMargin(el: BoxRenderable, margin: (previous?: BaseRenderable) => number) {
// Run before Yoga layout so scroll geometry matches the rendered frame.
el.onLifecyclePass = () => {
const parent = el.parent
if (!parent) return
const cached = previousByParent.get(parent)
const previous = cached?.frameID === el.ctx.frameId ? cached.previous : previousSiblings(parent, el.ctx.frameId)
const value = margin(previous.get(el))
if (el.marginTop !== value) el.marginTop = value
}
}
function previousSiblings(parent: BaseRenderable, frameID: number) {
const previous = new WeakMap<BaseRenderable, BaseRenderable | undefined>()
parent.getChildren().forEach((child, index, children) => previous.set(child, children[index - 1]))
previousByParent.set(parent, { frameID, previous })
return previous
}
@@ -1,5 +1,6 @@
import { afterEach, describe, expect, test } from "bun:test"
import { For } from "solid-js"
import { createSignal, For, Show } from "solid-js"
import type { ScrollBoxRenderable } from "@opentui/core"
import { testRender, type JSX } from "@opentui/solid"
import {
formatCompletedSubagentDetail,
@@ -127,11 +128,30 @@ function LoadedReadBeforeSubagentFixture() {
)
}
function StickyScrollFixture(props: { separated: boolean; scroll: (scroll: ScrollBoxRenderable) => void }) {
return (
<scrollbox ref={props.scroll} stickyScroll={true} stickyStart="bottom" height={3} width={72}>
<box height={1}>
<text>First row</text>
</box>
<box height={1}>
<text>Second row</text>
</box>
<Show when={props.separated}>
<box id="text-before-tool">
<text>Assistant text</text>
</box>
</Show>
<InlineToolRow icon="→" complete={true} pending="">
Read src/cli/cmd/tui/routes/session/index.tsx
</InlineToolRow>
</scrollbox>
)
}
async function renderFrame(component: () => JSX.Element, options: { width: number; height: number }) {
testSetup = await testRender(component, options)
await testSetup.renderOnce()
await Bun.sleep(25)
await testSetup.renderOnce()
return testSetup
.captureCharFrame()
@@ -183,4 +203,30 @@ describe("TUI inline tool wrapping", () => {
test("separates a subagent group after an expanded read", async () => {
expect(await renderFrame(() => <LoadedReadBeforeSubagentFixture />, { width: 72, height: 8 })).toMatchSnapshot()
})
test("updates sticky-bottom geometry when a text separator mounts and unmounts", async () => {
const [separated, setSeparated] = createSignal(false)
let scroll: ScrollBoxRenderable | undefined
testSetup = await testRender(
() => <StickyScrollFixture separated={separated()} scroll={(value) => (scroll = value)} />,
{
width: 72,
height: 3,
},
)
await testSetup.renderOnce()
expect(scroll?.scrollHeight).toBe(3)
expect(scroll?.scrollTop).toBe(Math.max(0, scroll!.scrollHeight - scroll!.viewport.height))
setSeparated(true)
await testSetup.renderOnce()
expect(scroll?.scrollHeight).toBe(5)
expect(scroll?.scrollTop).toBe(Math.max(0, scroll!.scrollHeight - scroll!.viewport.height))
setSeparated(false)
await testSetup.renderOnce()
expect(scroll?.scrollHeight).toBe(3)
expect(scroll?.scrollTop).toBe(Math.max(0, scroll!.scrollHeight - scroll!.viewport.height))
})
})