Add copy button for markdown write details#1647
Conversation
|
| Filename | Overview |
|---|---|
| packages/app/src/components/tool-call-details.tsx | Adds WriteDetailSection and CopyTextButton for markdown write panels. The copy button logic (timer ref, useCallback, cleanup useEffect) is correct. The main structural concern is that ds.sectionFillStyle is applied both on the outer callsite wrapper and again inside WriteDetailSection, producing a redundant nested View layer. |
| packages/app/src/components/tool-call-details.test.tsx | New test file for the copy button feature. Uses a wide set of test patterns the project explicitly bans: vi.hoisted, vi.mock (×8), JSDOM, component mounting, and vi.stubGlobal. The extractable pure logic (isMarkdownPath, clipboard call) would be better tested directly; component-level integration belongs in an E2E harness. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[buildDetailSections] -->|detail.type === write| B["<View key=write style=ds.sectionFillStyle>"]
B --> C{detail.content?}
C -->|yes| D[WriteDetailSection]
C -->|no| E[null]
D --> F["<View style=[ds.sectionFillStyle, copyableSection]>"]
F --> G{isMarkdownPath?}
G -->|yes| H[CopyTextButton]
G -->|no| I[no button]
F --> J[ScrollableTextSection]
H --> K{copied?}
K -->|yes| L["Check icon, label=Copied"]
K -->|no| M["Copy icon, label=Copy"]
H --> N[handlePress]
N --> O[Clipboard.setStringAsync]
O --> P[setCopied true]
P --> Q[setTimeout 1500ms]
Q --> R[setCopied false]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[buildDetailSections] -->|detail.type === write| B["<View key=write style=ds.sectionFillStyle>"]
B --> C{detail.content?}
C -->|yes| D[WriteDetailSection]
C -->|no| E[null]
D --> F["<View style=[ds.sectionFillStyle, copyableSection]>"]
F --> G{isMarkdownPath?}
G -->|yes| H[CopyTextButton]
G -->|no| I[no button]
F --> J[ScrollableTextSection]
H --> K{copied?}
K -->|yes| L["Check icon, label=Copied"]
K -->|no| M["Copy icon, label=Copy"]
H --> N[handlePress]
N --> O[Clipboard.setStringAsync]
O --> P[setCopied true]
P --> Q[setTimeout 1500ms]
Q --> R[setCopied false]
Comments Outside Diff (1)
-
packages/app/src/components/tool-call-details.tsx, line 776-783 (link)Double application of
sectionFillStyleThe outer
<View key="write" style={ds.sectionFillStyle}>already applies[styles.section, shouldFill && styles.fillHeight]. Inside,WriteDetailSectioncreates another<View style={[ds.sectionFillStyle, styles.copyableSection]}>with the exact same base style. Every markdown write panel now has two stacked Views carryinggap: spacing[2](andflex: 1/minHeight: 0whenshouldFillis true).WriteDetailSectionshould own its own style entirely; the outer wrapper at the callsite should be removed (or only carry the React key).
Reviews (1): Last reviewed commit: "Add copy button for markdown write detai..." | Re-trigger Greptile
| // @vitest-environment jsdom | ||
| import React, { act } from "react"; | ||
| import { createRoot, type Root } from "react-dom/client"; | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; | ||
|
|
||
| const { theme, setStringAsync } = vi.hoisted(() => ({ | ||
| theme: { | ||
| spacing: { 1: 4, 2: 8, 3: 12, 4: 16 }, | ||
| borderWidth: { 1: 1 }, | ||
| borderRadius: { sm: 4, md: 6, lg: 8, xl: 12, full: 999 }, | ||
| fontFamily: { mono: "monospace", ui: "system-ui" }, | ||
| fontSize: { xs: 11, sm: 13, base: 15, code: 13 }, | ||
| fontWeight: { normal: "400", medium: "500", semibold: "600" }, |
There was a problem hiding this comment.
Banned test patterns throughout this file
The project's test discipline rules explicitly ban vi.hoisted, vi.mock, JSDOM as a substitute for app behavior, component mounting tests, and monkey-patched globals (vi.stubGlobal). This file uses all of them. The logic under test — isMarkdownPath and the clipboard call — is pure enough to extract from the component and test directly without mounting a component tree. The component behavior itself belongs in an E2E test with a real browser harness, not a JSDOM simulation. The mock surface here (6+ vi.mock calls, a full synthetic theme, a patched Pressable, and stubbed globals) is wider than the behavior being verified.
Rule Used: # Code Review Pattern Reference: Slop, Tests, Feat... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| const handlePress = useCallback(async () => { | ||
| if (!content) return; | ||
| await Clipboard.setStringAsync(content); |
There was a problem hiding this comment.
The
content prop is typed string (non-optional), so the !content guard is unnecessary defensive code. Validations belong at the boundary; inside a typed interface the type system should be trusted.
| const handlePress = useCallback(async () => { | |
| if (!content) return; | |
| await Clipboard.setStringAsync(content); | |
| const handlePress = useCallback(async () => { | |
| await Clipboard.setStringAsync(content); |
Rule Used: # Code Review Pattern Reference: Slop, Tests, Feat... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Summary
Writetool detail panels.md,.mdx, and.markdownfilesWhy
Markdown write results can be long, and copying them manually requires selecting the rendered content inside a scrollable tool detail. This adds the same kind of one-click copy affordance already used elsewhere in the app.
Verification
npm --prefix packages/app test -- src/components/tool-call-details.test.tsxnpm run lint -- packages/app/src/components/tool-call-details.tsx packages/app/src/components/tool-call-details.test.tsxnpm run typecheck --workspace=@getpaseo/appnpm run typecheck --workspaces --if-present