Skip to content

fix(session-ui): share timeline tool headers #46205

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
opencode-agent wants to merge 1 commit into
base: v2
Choose a base branch
from
Open

fix(session-ui): share timeline tool headers #46205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
126 changes: 126 additions & 0 deletions packages/session-ui/component-tests/tool-header.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
import { expect, story } from "../../storybook/playwright/story"

const id = "current-session-tool-headers--shared-headers"

story("shares compact title and detail metrics across tool families", async ({ mount }, info) => {
const root = await mount(id)
await root.getByRole("button", { name: /^Used 7 / }).click()
const headers = root.locator('[data-component="context-tool-group-list"] [data-component="tool-header"]')
await expect(headers).toHaveCount(7)
const titles = headers.locator('[data-slot="basic-tool-tool-title"]')
await expect(headers.locator('[data-component="text-shimmer"][aria-label="Write"]')).toBeVisible()
await expect(headers.locator('[data-component="text-shimmer"][aria-label="Edit"]')).toBeVisible()
for (const title of await titles.all()) {
await expect(title).toBeVisible()
await expect(title).toHaveCSS("font-family", /^Inter,/)
await expect(title).toHaveCSS("font-size", "13px")
await expect(title).toHaveCSS("line-height", "16px")
await expect(title).toHaveCSS("font-weight", "530")
await expect(title.locator('[data-component="text-shimmer"]')).toHaveAttribute("data-active", "false")
}
const details = headers.locator(
'[data-slot="basic-tool-tool-subtitle"], [data-slot="basic-tool-tool-arg"], [data-slot="tool-header-directory"]',
)
expect(await details.count()).toBeGreaterThan(7)
for (const detail of await details.all()) {
await expect(detail).toHaveCSS("font-size", "13px")
await expect(detail).toHaveCSS("line-height", "16px")
await expect(detail).toHaveCSS("font-weight", "440")
}
await expect(headers.locator('[data-slot="basic-tool-tool-arg"]')).toHaveText([
"offset=12",
"limit=40",
"pattern=header",
"include=*.tsx",
])
await root.locator('[data-component="session-timeline"]').screenshot({ path: info.outputPath("tool-headers.png") })
})

story("keeps pending file titles active without showing unfinished paths", async ({ mount }) => {
const root = await mount(id, { args: { phase: "streaming", pathKnown: false } })
await root
.locator(
'[data-component="collapsed-tool-group"] > [data-component="collapsible"] > [data-slot="collapsible-trigger"]',
)
.click()
for (const action of [undefined, "Provide paths", "Run tools"]) {
if (action) await root.getByRole("button", { name: action, exact: true }).click()
for (const name of ["edit", "write"]) {
const header = root.locator(`[data-timeline-part-id="tool_header_${name}"] [data-component="tool-header"]`)
await expect(header).toBeVisible()
await expect(header.locator('[data-component="text-shimmer"]')).toHaveAttribute("data-active", "true")
await expect(header.locator('[data-slot="basic-tool-tool-title"]')).toHaveCSS("line-height", "16px")
await expect(
header.locator('[data-slot="basic-tool-tool-subtitle"], [data-slot="tool-header-directory"]'),
).toHaveCount(0)
}
}
await root.getByRole("button", { name: "Complete tools", exact: true }).click()
const group = root.getByRole("button", { name: /^Used 7 / })
if ((await group.getAttribute("aria-expanded")) === "false") await group.click()
for (const name of ["edit", "write"]) {
const header = root.locator(`[data-timeline-part-id="tool_header_${name}"] [data-component="tool-header"]`)
await expect(header.locator('[data-slot="basic-tool-tool-subtitle"]')).toHaveText(`${name}.ts`)
await expect(header.locator('[data-slot="tool-header-directory"]')).toContainText("src/components")
await expect(header.locator('[data-component="text-shimmer"]')).toHaveAttribute("data-active", "false")
}
})

for (const theme of ["light", "dark"]) {
for (const width of [390, 1000]) {
story(`truncates long file headers at ${width}px in ${theme}`, async ({ mount, page }, info) => {
await page.setViewportSize({ width, height: 850 })
const root = await mount(id, { args: { longPath: true }, globals: { theme } })
await root.getByRole("button", { name: /^Used 7 / }).click()
for (const name of ["edit", "write"]) {
const header = root.locator(`[data-timeline-part-id="tool_header_${name}"] [data-component="tool-header"]`)
const filename = header.locator('[data-slot="basic-tool-tool-subtitle"]')
const directory = header.locator('[data-slot="tool-header-directory"] > span')
await expect(filename).toContainText(`${name}.ts`)
for (const text of [filename, directory]) {
await expect(text).toHaveCSS("text-overflow", "ellipsis")
await expect(text).toHaveCSS("white-space", "nowrap")
await expect(text).toHaveCSS("line-height", "16px")
}
expect(await filename.evaluate((node) => node.scrollWidth > node.clientWidth)).toBe(true)
const bounds = await header.boundingBox()
expect(bounds!.x + bounds!.width).toBeLessThanOrEqual(width)
}
expect(await page.evaluate(() => document.documentElement.scrollWidth <= window.innerWidth)).toBe(true)
await root
.locator('[data-component="session-timeline"]')
.screenshot({ path: info.outputPath("long-headers.png") })
})
}
}

story("preserves keyboard disclosures and the webfetch link", async ({ mount }) => {
const root = await mount(id)
await root.getByRole("button", { name: /^Used 7 / }).click()
for (const name of ["shell", "execute", "edit", "write"]) {
const row = root.locator(`[data-timeline-part-id="tool_header_${name}"]`)
const trigger = row.locator('[data-slot="collapsible-trigger"]').first()
const content = row.locator('[data-slot="collapsible-content"]').first()
await expect(trigger).toHaveAttribute("aria-expanded", "false")
await trigger.focus()
await trigger.press("Enter")
await expect(trigger).toHaveAttribute("aria-expanded", "true")
await expect(content).toBeVisible()
await expect(trigger).toBeFocused()
await trigger.press("Space")
await expect(trigger).toHaveAttribute("aria-expanded", "false")
await expect(content).toBeHidden()
await expect(trigger).toBeFocused()
}
const link = root.getByRole("link", { name: "https://example.com/docs" })
await expect(link).toBeVisible()
await expect(link).toHaveAttribute("href", "https://example.com/docs")
await expect(link).toHaveAttribute("target", "_blank")
await expect(link).toHaveAttribute("rel", /noopener/)
await expect(link).toHaveCSS("font-size", "13px")
await expect(link).toHaveCSS("font-weight", "440")
await expect(link).toHaveCSS("line-height", "16px")
await expect(link).toHaveCSS("letter-spacing", "-0.04px")
await link.focus()
await expect(link).toBeFocused()
})
42 changes: 0 additions & 42 deletions packages/session-ui/src/components/basic-tool.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,22 +83,11 @@

[data-slot="basic-tool-tool-title"] {
flex-shrink: 0;
font-family: var(--font-family-sans);
font-size: 14px;
font-style: normal;
font-weight: var(--font-weight-medium);
line-height: var(--line-height-large);
letter-spacing: var(--letter-spacing-normal);
color: var(--v2-text-text-base);

&.capitalize {
text-transform: capitalize;
}

&.agent-title {
color: var(--v2-text-text-base);
font-weight: var(--font-weight-medium);
}
}

[data-slot="basic-tool-tool-subtitle"] {
Expand All @@ -107,13 +96,6 @@
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
font-family: var(--font-family-sans);
font-variant-numeric: tabular-nums;
font-size: 14px;
font-style: normal;
font-weight: var(--font-weight-regular);
line-height: var(--line-height-large);
letter-spacing: var(--letter-spacing-normal);
color: var(--v2-text-text-muted);

&.clickable:not(.webfetch-link) {
Expand Down Expand Up @@ -153,13 +135,6 @@
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
font-family: var(--font-family-sans);
font-variant-numeric: tabular-nums;
font-size: 14px;
font-style: normal;
font-weight: var(--font-weight-regular);
line-height: var(--line-height-large);
letter-spacing: var(--letter-spacing-normal);
color: var(--v2-text-text-muted);
}

Expand All @@ -177,23 +152,6 @@
[data-slot="basic-tool-tool-info-main"] {
gap: 6px;
}

[data-slot="basic-tool-tool-title"] {
font-family: var(--v2-font-family-sans);
font-size: 13px;
font-weight: 530;
/* Keep compact text on the shared metric; solid 13px line boxes clip Inter descenders. */
line-height: var(--line-height-compact);
letter-spacing: -0.04px;
}

[data-slot="basic-tool-tool-subtitle"] {
font-family: var(--v2-font-family-sans);
font-size: 13px;
font-weight: 440;
line-height: var(--line-height-compact);
letter-spacing: -0.04px;
}
}

[data-component="task-tool-card"] {
Expand Down
67 changes: 8 additions & 59 deletions packages/session-ui/src/components/basic-tool.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {
createEffect,
createMemo,
For,
Match,
on,
onCleanup,
Expand All @@ -16,17 +15,9 @@ import { useI18n } from "@opencode-ai/ui/context/i18n"
import { createStore } from "solid-js/store"
import { Collapsible } from "@opencode-ai/ui/collapsible"
import type { IconProps } from "@opencode-ai/ui/icon"
import { TextShimmer } from "@opencode-ai/ui/text-shimmer"
import { ToolHeader, type ToolHeaderProps } from "./tool-header"

export type TriggerTitle = {
title: string
titleClass?: string
subtitle?: string
subtitleClass?: string
args?: string[]
argsClass?: string
action?: JSX.Element
}
export type TriggerTitle = Omit<ToolHeaderProps, "active" | "onSubtitleClick">

const isTriggerTitle = (val: unknown): val is TriggerTitle => {
if (typeof val !== "object" || val === null) return false
Expand Down Expand Up @@ -216,54 +207,12 @@ export function BasicTool(props: BasicToolProps) {
<Switch>
<Match when={triggerTitle()}>
{(title) => (
<div data-slot="basic-tool-tool-info-structured">
<div data-slot="basic-tool-tool-info-main">
<span
data-slot="basic-tool-tool-title"
classList={{
[title().titleClass ?? ""]: !!title().titleClass,
}}
>
<TextShimmer text={title().title} active={pending()} />
</span>
<Show when={!pending() || title().subtitle || title().args?.length}>
<Show when={title().subtitle}>
<span
data-slot="basic-tool-tool-subtitle"
classList={{
[title().subtitleClass ?? ""]: !!title().subtitleClass,
clickable: !!props.onSubtitleClick,
}}
onClick={(e) => {
if (props.onSubtitleClick) {
e.stopPropagation()
props.onSubtitleClick()
}
}}
>
{title().subtitle}
</span>
</Show>
<Show when={title().args?.length}>
<For each={title().args}>
{(arg) => (
<span
data-slot="basic-tool-tool-arg"
classList={{
[title().argsClass ?? ""]: !!title().argsClass,
}}
>
{arg}
</span>
)}
</For>
</Show>
</Show>
</div>
<Show when={!pending() && title().action}>
<span data-slot="basic-tool-tool-action">{title().action}</span>
</Show>
</div>
<ToolHeader
{...title()}
active={pending()}
onSubtitleClick={props.onSubtitleClick}
action={!pending() ? title().action : undefined}
/>
)}
</Match>
<Match when={true}>{triggerContent() as JSX.Element}</Match>
Expand Down
Loading
Loading