From 64c0411edb8e83a5cb278b3415ff0ec8b582c873 Mon Sep 17 00:00:00 2001 From: Luke Parker <10430890+Hona@users.noreply.github.com> Date: Mon, 24 Aug 2026 09:31:45 +1000 Subject: [PATCH] fix(session-ui): align tool error card with figma (#44543) --- .../src/components/tool-error-card.css | 89 +++++++++++++- .../components/tool-error-card.stories.tsx | 34 +++++- .../src/components/tool-error-card.tsx | 113 ++++++++++-------- .../src/message/current-message.test.ts | 17 +-- .../src/message/current-tool-state.ts | 4 +- .../session-ui/src/tools/tool-renderer.tsx | 29 ++++- 6 files changed, 217 insertions(+), 69 deletions(-) diff --git a/packages/session-ui/src/components/tool-error-card.css b/packages/session-ui/src/components/tool-error-card.css index 9a6d5395d03..48a5d42b6bb 100644 --- a/packages/session-ui/src/components/tool-error-card.css +++ b/packages/session-ui/src/components/tool-error-card.css @@ -1,22 +1,94 @@ [data-component="card"][data-kind="tool-error-card"] { --card-pad-y: 0px; - --card-line-pad: 4px; + --card-line-pad: 0px; + --card-pad-r: 0px; + &::before { + border-radius: 0; + } + + /* Figma's leading-none would clip Inter descenders at 13px; keep the compact metric. */ [data-slot="basic-tool-tool-title"] { + font-family: var(--v2-font-family-sans); + font-size: 13px; + font-weight: 530; + line-height: var(--line-height-compact); + letter-spacing: -0.04px; color: var(--v2-text-text-base); } [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; color: var(--v2-text-text-muted); } + [data-slot="tool-error-card-dot"] { + flex-shrink: 0; + font-family: var(--v2-font-family-sans); + font-size: 11px; + font-weight: 530; + line-height: var(--line-height-tight); + letter-spacing: 0.05px; + color: var(--v2-text-text-muted); + } + + [data-slot="tool-error-card-summary"], + [data-slot="tool-error-card-message"] { + font-family: var(--v2-font-family-sans); + font-size: 13px; + font-weight: 440; + line-height: var(--line-height-compact); + letter-spacing: -0.04px; + color: var(--v2-state-fg-danger); + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + } + + [data-slot="tool-error-card-summary"] { + flex-shrink: 1; + min-width: 0; + } + + [data-slot="tool-error-card-message"] { + /* Text column indent: 16px icon + 8px gap. */ + padding-inline-start: 24px; + } + + [data-slot="basic-tool-tool-info-main"] { + align-items: center; + gap: 6px; + } + + [data-slot="basic-tool-tool-trigger-content"] { + /* Reserve the 14px arrow + 6px gap. */ + max-width: calc(100% - 20px); + } + + [data-slot="collapsible-arrow"] { + width: 14px; + height: 14px; + margin-inline-start: 6px; + /* Always visible; the base collapsible reveals arrows only on hover. */ + opacity: 1; + } + [data-slot="collapsible-arrow"], [data-slot="collapsible-arrow-icon"] { color: var(--v2-text-text-faint); } > [data-component="collapsible"].tool-collapsible { - gap: 0px; + /* Figma's 4px gap minus the 1.5px the compact line box adds above the summary em. */ + gap: 2.5px; + + > [data-slot="collapsible-trigger"] { + height: 24px; + } > [data-slot="collapsible-content"] { border-inline-start: none; @@ -31,7 +103,8 @@ } > [data-component="collapsible"].tool-collapsible[data-open="true"] { - gap: 4px; + /* The compact line box alone yields Figma's spacing under the header. */ + gap: 0px; } [data-component="tool-error-card-icon"] [data-slot="icon-svg"] { @@ -41,11 +114,19 @@ [data-slot="tool-error-card-content"] { position: relative; padding-left: 24px; - margin-bottom: 8px; -webkit-user-select: text; user-select: text; } + [data-slot="tool-error-card-content"] [data-slot="card-description"] { + font-family: var(--v2-font-family-sans); + font-size: 13px; + font-weight: 440; + line-height: var(--line-height-compact); + letter-spacing: -0.04px; + color: var(--v2-text-text-faint); + } + > [data-component="collapsible"].tool-collapsible[data-open="true"] [data-slot="tool-error-card-content"] { padding-right: 40px; } diff --git a/packages/session-ui/src/components/tool-error-card.stories.tsx b/packages/session-ui/src/components/tool-error-card.stories.tsx index f3979b86b0b..f2aebe8789f 100644 --- a/packages/session-ui/src/components/tool-error-card.stories.tsx +++ b/packages/session-ui/src/components/tool-error-card.stories.tsx @@ -12,7 +12,18 @@ Tool call failure summary styled like a tool trigger. - Collapsible; click header to expand/collapse. ` -const samples = [ +const samples: { tool: string; error: string; subtitle?: string; defaultOpen?: boolean }[] = [ + { + tool: "shell", + subtitle: "sleep 30", + error: "Tool execution interrupted", + }, + { + tool: "shell", + subtitle: "sleep 30", + error: "Tool execution interrupted", + defaultOpen: true, + }, { tool: "patch", error: @@ -62,8 +73,9 @@ export default { }, }, args: { - tool: "patch", + tool: samples[0].tool, error: samples[0].error, + subtitle: samples[0].subtitle, }, argTypes: { tool: { @@ -73,9 +85,12 @@ export default { error: { control: "text", }, + subtitle: { + control: "text", + }, }, - render: (props: { tool: string; error: string }) => { - return + render: (props: { tool: string; error: string; subtitle?: string }) => { + return }, } @@ -83,7 +98,16 @@ export const All = { render: () => { return (
- {(item) => } + + {(item) => ( + + )} +
) }, diff --git a/packages/session-ui/src/components/tool-error-card.tsx b/packages/session-ui/src/components/tool-error-card.tsx index 65bc0e1b620..4c854e484d7 100644 --- a/packages/session-ui/src/components/tool-error-card.tsx +++ b/packages/session-ui/src/components/tool-error-card.tsx @@ -70,19 +70,16 @@ export function ToolErrorCard(props: ToolErrorCardProps) { return value }) - const subtitle = createMemo(() => { - if (split.subtitle) return split.subtitle - const parts = tail().split(": ") - if (parts.length <= 1) return i18n.t("ui.toolErrorCard.failed") - const head = (parts[0] ?? "").trim() + const summary = createMemo(() => { + const head = (tail().split(": ")[0] ?? "").trim() if (!head) return i18n.t("ui.toolErrorCard.failed") - return head[0] ? head[0].toUpperCase() + head.slice(1) : i18n.t("ui.toolErrorCard.failed") + return head[0].toUpperCase() + head.slice(1) }) - const body = createMemo(() => { + const detail = createMemo(() => { const parts = tail().split(": ") - if (parts.length <= 1) return cleaned() - return parts.slice(1).join(": ").trim() || cleaned() + if (parts.length <= 1) return "" + return parts.slice(1).join(": ").trim() }) const copy = async () => { @@ -100,27 +97,36 @@ export function ToolErrorCard(props: ToolErrorCardProps) {
- + {/* 20px-viewBox path at 16px: 1.25 renders the 1px stroke Figma specifies. */} +
@@ -129,33 +135,38 @@ export function ToolErrorCard(props: ToolErrorCardProps) {
- -
- -
- - } - size="normal" - variant="ghost" - onMouseDown={(e) => e.preventDefault()} - onClick={(e) => { - e.stopPropagation() - void copy() - }} - aria-label={copied() ? i18n.t("ui.message.copied") : i18n.t("ui.toolErrorCard.copyError")} - /> - -
-
- {(value) => {value()}} -
-
+ +
{summary()}
+
+ + +
+ +
+ + } + size="normal" + variant="ghost" + onMouseDown={(e) => e.preventDefault()} + onClick={(e) => { + e.stopPropagation() + void copy() + }} + aria-label={copied() ? i18n.t("ui.message.copied") : i18n.t("ui.toolErrorCard.copyError")} + /> + +
+
+ {detail()} +
+
+
) diff --git a/packages/session-ui/src/message/current-message.test.ts b/packages/session-ui/src/message/current-message.test.ts index 3f8dd30ca55..a1337f87d95 100644 --- a/packages/session-ui/src/message/current-message.test.ts +++ b/packages/session-ui/src/message/current-message.test.ts @@ -29,20 +29,23 @@ describe("current content default open", () => { expect(currentContentDefaultOpen(tool("patch"), false, false)).toBe(true) }) - test("collapses failed patches", () => { - const patch: SessionMessageAssistantTool = { + test("collapses errored tools regardless of disclosure preferences", () => { + const errored = (name: string): SessionMessageAssistantTool => ({ type: "tool", - id: "tool_patch", - name: "patch", + id: `tool_${name}`, + name, state: { status: "error", input: {}, - error: { type: "ToolError", message: "Verification failed" }, + error: { type: "ToolError", message: "Tool execution interrupted" }, metadata: {}, }, time: { created: 1, completed: 2 }, - } - expect(currentContentDefaultOpen(patch, false, false)).toBe(false) + }) + expect(currentContentDefaultOpen(errored("shell"), true, true)).toBe(false) + expect(currentContentDefaultOpen(errored("execute"), true, true)).toBe(false) + expect(currentContentDefaultOpen(errored("edit"), true, true)).toBe(false) + expect(currentContentDefaultOpen(errored("patch"), false, false)).toBe(false) }) test("opens deletion-only patches", () => { diff --git a/packages/session-ui/src/message/current-tool-state.ts b/packages/session-ui/src/message/current-tool-state.ts index bd566d0e134..b8ef9a7cd6f 100644 --- a/packages/session-ui/src/message/current-tool-state.ts +++ b/packages/session-ui/src/message/current-tool-state.ts @@ -35,8 +35,10 @@ export function currentContentDefaultOpen( editExpanded: boolean, ) { if (content.type !== "tool") return undefined + // Errored tools render the error card, which starts collapsed. + if (content.state.status === "error") return false if (content.name === "shell" || content.name === "execute") return shellExpanded - if (content.name === "patch") return content.state.status !== "error" + if (content.name === "patch") return true if (content.name !== "edit" && content.name !== "write") return undefined if (!editExpanded) return false const files = currentToolMetadata(content).files diff --git a/packages/session-ui/src/tools/tool-renderer.tsx b/packages/session-ui/src/tools/tool-renderer.tsx index fac419e20e5..896defa476e 100644 --- a/packages/session-ui/src/tools/tool-renderer.tsx +++ b/packages/session-ui/src/tools/tool-renderer.tsx @@ -772,6 +772,7 @@ export function ToolDisplay( if (typeof value === "string" && value) return value return taskId() }) + const errorSubtitle = createMemo(() => toolErrorSubtitle(props, i18n)) const error = createMemo(() => toolDisplayError(props, i18n.t("ui.toolErrorCard.failed"))) const render = createMemo(() => ToolRegistry.render(props.tool) ?? GenericTool) @@ -799,7 +800,7 @@ export function ToolDisplay( defaultOpen={props.defaultOpen} open={props.open} onOpenChange={props.onOpenChange} - subtitle={taskSubtitle()} + subtitle={taskSubtitle() ?? errorSubtitle()} href={taskHref()} onSubtitleClick={(event) => { if (!data.navigateToSession) return @@ -822,6 +823,32 @@ export function ToolDisplay( ) } +// Each branch must stay in sync with its tool trigger's subtitle expression so +// failed rows read like their non-error counterparts ("Shell sleep 30"). +function toolErrorSubtitle(props: ToolProps, i18n: UiI18n) { + const text = (value: unknown) => (typeof value === "string" && value ? value : undefined) + if (props.tool === "shell") return text(props.input.command) ?? text(props.metadata.command) + if (props.tool === "execute") return text(props.input.code) + if (props.tool === "read") return getFilename(readToolPath(props.input) ?? "") + if (props.tool === "edit" || props.tool === "write") return getFilename(text(props.input.path) ?? "") + if (props.tool === "list" || props.tool === "glob" || props.tool === "grep") + return displayDirectory(text(props.input.path) ?? "/") + if (props.tool === "webfetch") return text(props.input.url) + if (props.tool === "websearch") return text(props.input.query) + if (props.tool === "skill") return skillToolName(props.input, props.metadata) + if (props.tool === "patch") { + const count = patchFileGroups(props.metadata.files).length + if (count === 0) return undefined + return `${count} ${i18n.plural("ui.common.file", count)}` + } + if (props.tool === "question") { + const count = Array.isArray(props.input.questions) ? props.input.questions.filter(questionInfo).length : 0 + if (count === 0) return undefined + return `${count} ${i18n.plural("ui.common.question", count)}` + } + return undefined +} + function toolDisplayError(props: ToolProps & { error?: string }, fallback: string) { if (props.status === "error") return props.error if (props.tool !== "execute") return undefined