From 53bf77682c5dead16427d570bf5a669f1df0efce Mon Sep 17 00:00:00 2001 From: Kelvin Amoaba <97001695+AmoabaKelvin@users.noreply.github.com> Date: Mon, 12 Oct 2026 03:00:31 +0000 Subject: [PATCH] fix(native-chat): one set of rules for chat text size, colour, hover and focus (#26685) * fix(native-chat): put chat text on one type scale that follows Text size Sizes were picked per component, so the same kind of row came out 12, 13 or 14px and the Text size setting reached only a few of them. * fix(native-chat): scale chat spacing and icons with Text size Icons and gaps stayed fixed while text grew, so at larger sizes they looked undersized and sat off-centre. * fix(native-chat): implement PR #26685 review fixes - Scale row gap with text size so controls don't overlap - Clamp 'Match terminal interface' to 12-20 for labels/controls - Use existing --status-warning token for warning color - Rewrite stale test for scaled typography tokens - Use app colors for in-progress tasks in composer dock * fix(native-chat): leave the task list's dock colour as it was * fix(native-chat): message text follows the terminal size; controls and spacing stay in 12-20 * test(native-chat): labels and spacing follow the bounded chat size; main's new notice test uses the chat colour --------- Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> --- .oxlintrc.json | 17 ++++- AGENTS.md | 2 +- .../native-chat-design-tokens.mjs | 49 ++++++++++++++ .../native-chat-design-tokens-plugin.test.mjs | 66 +++++++++++++++++++ docs/STYLEGUIDE.md | 23 +++++++ src/renderer/src/assets/main.css | 26 ++++++++ .../NativeChatAgentMessageSenders.tsx | 2 +- .../native-chat/NativeChatApprovalCard.tsx | 10 +-- .../NativeChatAssistantMessageRow.tsx | 3 +- .../NativeChatAutocompleteMenus.tsx | 10 +-- .../NativeChatAwaitingInputRow.tsx | 17 +++-- .../NativeChatBackgroundTaskRun.tsx | 2 +- .../NativeChatBackgroundTasksStatus.tsx | 8 +-- .../native-chat/NativeChatCodeBlock.tsx | 2 +- .../native-chat/NativeChatComposerField.tsx | 2 +- .../native-chat/NativeChatComposerNotices.tsx | 2 +- .../NativeChatContextUsageRing.tsx | 2 +- .../native-chat/NativeChatDiffCard.tsx | 10 +-- .../native-chat/NativeChatDiffView.tsx | 8 +-- .../native-chat/NativeChatEmptyState.tsx | 4 +- .../native-chat/NativeChatInlineVisual.tsx | 2 +- .../native-chat/NativeChatLoadingCue.tsx | 2 +- ...iveChatMessageList.live-reasoning.test.tsx | 2 +- .../native-chat/NativeChatMessageList.tsx | 3 +- .../native-chat/NativeChatNoticeRow.test.tsx | 14 ++-- .../native-chat/NativeChatNoticeRow.tsx | 12 ++-- .../native-chat/NativeChatOlderHistoryRow.tsx | 2 +- .../NativeChatReasoningDisclosure.tsx | 6 +- .../native-chat/NativeChatReasoningRow.tsx | 6 +- .../NativeChatResolutionReceipt.tsx | 2 +- .../native-chat/NativeChatSubagentRun.tsx | 6 +- .../NativeChatSubagentSectionHead.tsx | 4 +- .../native-chat/NativeChatTaskList.test.tsx | 2 +- .../native-chat/NativeChatTaskList.tsx | 29 ++++---- .../native-chat/NativeChatToolLine.test.tsx | 2 +- .../native-chat/NativeChatToolLine.tsx | 6 +- .../NativeChatToolRun.appearance.test.tsx | 7 +- .../native-chat/NativeChatToolRun.test.tsx | 2 +- .../native-chat/NativeChatToolRun.tsx | 4 +- .../native-chat/NativeChatToolRunMembers.tsx | 2 +- .../NativeChatTranscriptChrome.tsx | 10 +-- .../native-chat/NativeChatTranscriptRow.tsx | 2 +- .../NativeChatTurnActivityLine.tsx | 6 +- .../native-chat/NativeChatUserMessageRow.tsx | 8 +-- .../native-chat/NativeChatWorkingStatus.tsx | 2 +- .../native-chat-appearance-style.test.ts | 30 +++++++-- .../native-chat-appearance-style.ts | 6 ++ .../native-chat/native-chat-appearance.css | 26 +++++--- .../native-chat-row-height-estimate.test.ts | 12 ++-- .../native-chat-row-height-estimate.ts | 40 +++++------ .../native-chat-type-scale.test.ts | 43 ++++++++++++ .../use-native-chat-row-typography.ts | 6 +- .../use-native-chat-transcript-window.ts | 14 +++- ...nt-child-row-fallback-pixels.test.tsx.snap | 4 +- src/renderer/src/lib/utils.ts | 7 +- 55 files changed, 438 insertions(+), 158 deletions(-) create mode 100644 config/oxlint-plugins/native-chat-design-tokens.mjs create mode 100644 config/scripts/native-chat-design-tokens-plugin.test.mjs create mode 100644 src/renderer/src/components/native-chat/native-chat-type-scale.test.ts diff --git a/.oxlintrc.json b/.oxlintrc.json index 78dee1acd90..200213e83e3 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -21,6 +21,10 @@ { "name": "renderer-scrollbar-style", "specifier": "./config/oxlint-plugins/renderer-scrollbar-style.mjs" + }, + { + "name": "native-chat-design-tokens", + "specifier": "./config/oxlint-plugins/native-chat-design-tokens.mjs" } ], "categories": { @@ -181,6 +185,14 @@ ] } }, + { + "files": ["src/renderer/src/components/native-chat/**/*.{ts,tsx}"], + "rules": { + "native-chat-design-tokens/no-arbitrary-font-size": "error", + "native-chat-design-tokens/no-softened-foreground": "error", + "native-chat-design-tokens/no-weakened-focus-ring": "error" + } + }, { "files": ["src/shared/**/*.ts"], "rules": { @@ -198,7 +210,10 @@ "rules": { "react/globals": "off", "quadratic-buffer-concat/no-loop-carried-concat": "off", - "renderer-scrollbar-style/require-styled-vertical-scrollbar": "off" + "renderer-scrollbar-style/require-styled-vertical-scrollbar": "off", + "native-chat-design-tokens/no-arbitrary-font-size": "off", + "native-chat-design-tokens/no-softened-foreground": "off", + "native-chat-design-tokens/no-weakened-focus-ring": "off" } }, { diff --git a/AGENTS.md b/AGENTS.md index d30faf95d34..2451994d21a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,6 +1,6 @@ # Design System -All UI work — layout, color, typography, spacing, component selection, UX behavior — must follow [`docs/STYLEGUIDE.md`](./docs/STYLEGUIDE.md). Most of it is linted: `pnpm run check:code-quality:changed` fails on new restyles of a `components/ui/` primitive, raw palette colors, and computed `className` strings; `pnpm lint` fails on any class Tailwind cannot generate. See the Enforcement section of the style guide before suppressing either. Use the tokens defined in `src/renderer/src/assets/main.css` (the canonical source) and the shadcn primitives in `src/renderer/src/components/ui/`. Don't invent new color values, font sizes, or shadow tiers when a documented one already covers the role. When STYLEGUIDE.md is silent, follow the resolution order in its final section. +All UI work — layout, color, typography, spacing, component selection, UX behavior — must follow [`docs/STYLEGUIDE.md`](./docs/STYLEGUIDE.md). Most of it is linted: `pnpm run check:code-quality:changed` fails on new restyles of a `components/ui/` primitive, raw palette colors, and computed `className` strings; `pnpm lint` fails on any class Tailwind cannot generate. See the Enforcement section of the style guide before suppressing either. Use the tokens defined in `src/renderer/src/assets/main.css` (the canonical source) and the shadcn primitives in `src/renderer/src/components/ui/`. Don't invent new color values, font sizes, or shadow tiers when a documented one already covers the role. When STYLEGUIDE.md is silent, follow the resolution order in its final section. In `components/native-chat/`, the "Chat type scale" and "Chat text colour, hover and focus" sections are binding, and `pnpm lint` fails on arbitrary font sizes, softened `text-foreground`, and weakened focus rings there. ## Electron UI Validation diff --git a/config/oxlint-plugins/native-chat-design-tokens.mjs b/config/oxlint-plugins/native-chat-design-tokens.mjs new file mode 100644 index 00000000000..26c0e56a622 --- /dev/null +++ b/config/oxlint-plugins/native-chat-design-tokens.mjs @@ -0,0 +1,49 @@ +import { plainClassName } from './renderer-scrollbar-style.mjs' + +const CHECKS = { + 'no-arbitrary-font-size': { + pattern: /^text-\[(?:length:)?[\d.]+(?:px|rem|em)\]$/, + message: + 'Use a chat type step (text-sm, text-xs, text-2xs, text-3xs, text-chat-code), not an arbitrary size. See "Chat type scale" in docs/STYLEGUIDE.md.' + }, + 'no-softened-foreground': { + pattern: /^text-foreground\/\d+$/, + message: + "Don't soften text-foreground by hand: use text-chat-foreground on the chat canvas or text-muted-foreground on a lifted surface. See docs/STYLEGUIDE.md." + }, + 'no-weakened-focus-ring': { + pattern: /^ring-ring\/\d+$/, + message: + 'A focus ring at reduced opacity fails 3:1 contrast on the chat canvas: use ring-ring. See docs/STYLEGUIDE.md.' + } +} + +function literalTexts(node) { + if (node.type === 'Literal') { + return typeof node.value === 'string' ? [node.value] : [] + } + return node.quasis.map((quasi) => quasi.value.cooked ?? quasi.value.raw) +} + +function createRule({ pattern, message }) { + return { + create(context) { + const check = (node) => { + const offending = literalTexts(node) + .flatMap((text) => text.split(/\s+/)) + .find((token) => pattern.test(plainClassName(token))) + if (offending) { + context.report({ node, message: `${offending}: ${message}` }) + } + } + return { Literal: check, TemplateLiteral: check } + } + } +} + +export default { + meta: { name: 'native-chat-design-tokens' }, + rules: Object.fromEntries( + Object.entries(CHECKS).map(([name, check]) => [name, createRule(check)]) + ) +} diff --git a/config/scripts/native-chat-design-tokens-plugin.test.mjs b/config/scripts/native-chat-design-tokens-plugin.test.mjs new file mode 100644 index 00000000000..718390c5b93 --- /dev/null +++ b/config/scripts/native-chat-design-tokens-plugin.test.mjs @@ -0,0 +1,66 @@ +import path from 'node:path' +import { describe, expect, it } from 'vitest' +import { runOxlintPluginOnSource } from './oxlint-plugin-test-runner.mjs' + +const pluginPath = path.resolve('config/oxlint-plugins/native-chat-design-tokens.mjs') + +function lintSource(source) { + return runOxlintPluginOnSource({ + pluginName: 'native-chat-design-tokens', + pluginPath, + source, + rules: { + 'native-chat-design-tokens/no-arbitrary-font-size': 'warn', + 'native-chat-design-tokens/no-softened-foreground': 'warn', + 'native-chat-design-tokens/no-weakened-focus-ring': 'warn' + } + }) +} + +const violations = [ + ['arbitrary px size', 'export const X = () =>

'], + ['arbitrary rem size', 'export const X = () =>

'], + ['arbitrary size behind a variant', 'export const X = () =>

'], + ['arbitrary size in a class constant', "const ROW = 'flex text-[10px]'"], + [ + 'arbitrary size in a composer', + "export const X = () =>

" + ], + ['softened foreground', 'export const X = () =>

'], + [ + 'weakened focus ring', + 'export const X = () => diff --git a/src/renderer/src/components/native-chat/NativeChatTaskList.test.tsx b/src/renderer/src/components/native-chat/NativeChatTaskList.test.tsx index 17ffa840333..5bca8de61f7 100644 --- a/src/renderer/src/components/native-chat/NativeChatTaskList.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatTaskList.test.tsx @@ -25,7 +25,7 @@ describe('NativeChatTaskList', () => { it('shows tri-state glyphs, progress, and activeForm in the first checklist', () => { const { container } = render() expect(screen.getByText('Read')).toHaveClass('line-through') - expect(screen.getByText('Writing').closest('li')).toHaveClass('text-foreground') + expect(screen.getByText('Writing').closest('li')).toHaveClass('text-chat-foreground-strong') expect(screen.getByText('Test')).toBeInTheDocument() expect(screen.getByLabelText('1 of 3 tasks completed')).toHaveTextContent('1/3') for (const glyph of ['circle', 'circle-dot', 'circle-check']) { diff --git a/src/renderer/src/components/native-chat/NativeChatTaskList.tsx b/src/renderer/src/components/native-chat/NativeChatTaskList.tsx index c7db6cceaf6..332ba886059 100644 --- a/src/renderer/src/components/native-chat/NativeChatTaskList.tsx +++ b/src/renderer/src/components/native-chat/NativeChatTaskList.tsx @@ -46,8 +46,8 @@ function TaskRow({ task, label }: { task: NativeChatTask; label?: string }): Rea return (

  • @@ -66,9 +66,7 @@ function TaskRow({ task, label }: { task: NativeChatTask; label?: string }): Rea function Checklist({ list }: { list: TaskList }): React.JSX.Element { return list.tasks.length === 0 ? ( -

    - {translate('components.native-chat.taskList.empty', 'No tasks')} -

    +

    {translate('components.native-chat.taskList.empty', 'No tasks')}

    ) : (
      -
      +
      {list.explanation ? (

      @@ -126,8 +124,8 @@ export function NativeChatTaskList({ } const changes = previous ? diffNativeChatTaskLists(previous, list) : null return ( -

      -
      +
      +
      {translate('components.native-chat.taskList.title', 'Tasks')} @@ -156,14 +154,19 @@ export function NativeChatTaskList({ ))}
    ) : ( -

    +

    {translate('components.native-chat.taskList.unchanged', 'Tasks unchanged')}

    )} - - - {translate('components.native-chat.taskList.showAll', 'Full task list')} + + @@ -174,7 +177,7 @@ export function NativeChatTaskList({ )} {list.explanation ? ( -

    +

    {list.explanation}

    ) : null} diff --git a/src/renderer/src/components/native-chat/NativeChatToolLine.test.tsx b/src/renderer/src/components/native-chat/NativeChatToolLine.test.tsx index 59ad6729386..2d30bd738b0 100644 --- a/src/renderer/src/components/native-chat/NativeChatToolLine.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatToolLine.test.tsx @@ -214,7 +214,7 @@ describe('tool sentence rows', () => { 'bg-chat-code-surface', 'border-chat-code-border', 'rounded-lg', - 'text-xs' + 'text-chat-code' ) fireEvent.click(button) expect(screen.queryByText('command failed')).toBeNull() diff --git a/src/renderer/src/components/native-chat/NativeChatToolLine.tsx b/src/renderer/src/components/native-chat/NativeChatToolLine.tsx index d4d610101f4..56dce7855e3 100644 --- a/src/renderer/src/components/native-chat/NativeChatToolLine.tsx +++ b/src/renderer/src/components/native-chat/NativeChatToolLine.tsx @@ -59,7 +59,7 @@ function ToolLineDetail({ {!diff && fullCommand ? ( 
               {commandLanguage ? (
             
@@ -72,7 +72,7 @@ function ToolLineDetail({
         
    @@ -134,7 +134,7 @@ export function NativeChatToolLine({
         type="button"
         onClick={() => hasDetail && setExpanded(!expanded)}
         className={cn(
-          'group/tool-line flex min-h-[26px] w-full items-center gap-2 text-left font-sans text-[13px]',
+          'group/tool-line flex min-h-6.5 w-full items-center gap-2 rounded-md text-left font-sans text-sm focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-ring',
           hasDetail ? 'cursor-pointer' : 'cursor-default'
         )}
         aria-expanded={hasDetail ? expanded : undefined}
diff --git a/src/renderer/src/components/native-chat/NativeChatToolRun.appearance.test.tsx b/src/renderer/src/components/native-chat/NativeChatToolRun.appearance.test.tsx
index f7d2cff4d24..22a46a38ae2 100644
--- a/src/renderer/src/components/native-chat/NativeChatToolRun.appearance.test.tsx
+++ b/src/renderer/src/components/native-chat/NativeChatToolRun.appearance.test.tsx
@@ -49,7 +49,7 @@ import { i18n } from '@/i18n/i18n'
       )
       expect(style['--chat-content-max-width']).toBe('46rem')
       expect(style['--chat-font-family']).toContain('Menlo')
-      const summary = container.querySelector('span.native-chat-message-text')
+      const summary = container.querySelector('span.line-clamp-2')
       expect(summary).toHaveTextContent(live ? 'running 1 agent' : 'ran 1 agent')
       expect(summary).toHaveClass('min-w-0', 'line-clamp-2', 'whitespace-normal', 'break-words')
       expect(summary).not.toHaveClass('truncate', 'whitespace-nowrap', 'font-mono')
@@ -74,7 +74,7 @@ import { i18n } from '@/i18n/i18n'
         
       )
 
-      const summary = container.querySelector('span.native-chat-message-text')
+      const summary = container.querySelector('span.line-clamp-2')
       expect(summary).toHaveClass('line-clamp-2', 'break-words')
       expect(summary?.textContent).toContain('printf')
       expect(summary?.textContent?.length).toBeLessThan(command.length)
@@ -104,9 +104,8 @@ import { i18n } from '@/i18n/i18n'
       const header = screen.getByRole('button')
       // Top-aligned in the summary's own type, so `1lh` is one summary line.
       expect(header).toHaveClass('items-start', 'text-sm', 'leading-relaxed')
-      expect(header).toHaveClass('native-chat-message-text')
       expect(header).not.toHaveClass('items-center')
-      const summary = container.querySelector('span.native-chat-message-text')
+      const summary = container.querySelector('span.line-clamp-2')
       expect(summary?.parentElement).toBe(header)
       expect(summary).toHaveClass('min-w-0', 'line-clamp-2')
 
diff --git a/src/renderer/src/components/native-chat/NativeChatToolRun.test.tsx b/src/renderer/src/components/native-chat/NativeChatToolRun.test.tsx
index 1d6d73da0b1..dd317c28522 100644
--- a/src/renderer/src/components/native-chat/NativeChatToolRun.test.tsx
+++ b/src/renderer/src/components/native-chat/NativeChatToolRun.test.tsx
@@ -325,7 +325,7 @@ describe('NativeChatToolRun', () => {
     it('keeps the run in the transcript type, not a monospace dump', () => {
       const { container } = render()
 
-      const label = runHeader(container).querySelector('span.native-chat-message-text')
+      const label = runHeader(container).querySelector('span.line-clamp-2')
       expect(label).toHaveClass('text-sm')
       expect(label).not.toHaveClass('font-mono')
     })
diff --git a/src/renderer/src/components/native-chat/NativeChatToolRun.tsx b/src/renderer/src/components/native-chat/NativeChatToolRun.tsx
index dcb46a0e092..4aa8bddf6dd 100644
--- a/src/renderer/src/components/native-chat/NativeChatToolRun.tsx
+++ b/src/renderer/src/components/native-chat/NativeChatToolRun.tsx
@@ -229,7 +229,7 @@ export function NativeChatToolRun({
         
    Agents
    • Failed child · failed
    • Background agent · status unavailable
    • Cancelled child
    • Approve edits · needs approval
    • Summarize logs900
    • Review the parser18.2k · 10m 0s
    • Explore · failed8m 20s
    • Old host child6m 40s
    Shell
    • npm run dev11m 40s
    Monitors
    • tail -f server.log
    "`;
+exports[`chat strip rows from a legacy background-task roster > render exactly as before 1`] = `"
    Agents
    • Failed child · failed
    • Background agent · status unavailable
    • Cancelled child
    • Approve edits · needs approval
    • Summarize logs900
    • Review the parser18.2k · 10m 0s
    • Explore · failed8m 20s
    • Old host child6m 40s
    Shell
    • npm run dev11m 40s
    Monitors
    • tail -f server.log
    "`;
 
-exports[`chat strip rows from a legacy background-task roster > render exactly as before 2`] = `"
    Agents
    • Failed child · failed
    • Background agent · status unavailable
    • Cancelled child
    • Approve edits · needs approval
    • Summarize logs900
    • Review the parser18.2k · 10m 0s
    • Explore · failed8m 20s
    • Old host child6m 40s
    Shell
    • npm run dev11m 40s
    Monitors
    • tail -f server.log
    "`;
+exports[`chat strip rows from a legacy background-task roster > render exactly as before 2`] = `"
    Agents
    • Failed child · failed
    • Background agent · status unavailable
    • Cancelled child
    • Approve edits · needs approval
    • Summarize logs900
    • Review the parser18.2k · 10m 0s
    • Explore · failed8m 20s
    • Old host child6m 40s
    Shell
    • npm run dev11m 40s
    Monitors
    • tail -f server.log
    "`;
 
 exports[`sidebar child rows from a legacy subagents snapshot > render a stale parent and a lost transport exactly as before 1`] = `
 [
diff --git a/src/renderer/src/lib/utils.ts b/src/renderer/src/lib/utils.ts
index fed2fe91e4d..dcab3e4df04 100644
--- a/src/renderer/src/lib/utils.ts
+++ b/src/renderer/src/lib/utils.ts
@@ -1,5 +1,10 @@
 import { clsx, type ClassValue } from 'clsx'
-import { twMerge } from 'tailwind-merge'
+import { extendTailwindMerge } from 'tailwind-merge'
+
+// Unregistered, tailwind-merge reads the theme's extra text steps as colors and drops them.
+const twMerge = extendTailwindMerge({
+  extend: { theme: { text: ['2xs', '3xs', 'chat-code'] } }
+})
 
 export function cn(...inputs: ClassValue[]) {
   return twMerge(clsx(inputs))