From 79a2c7fcbf6fa2ce7f187b761d6feeb664a065ac Mon Sep 17 00:00:00 2001 From: murashit Date: Wed, 24 Jun 2026 15:54:51 +0900 Subject: [PATCH] Eliminate dynamic CSS class exemptions --- scripts/lint/check-css-usage.mjs | 142 ++---------------- .../message-stream/detail-view.ts | 8 +- .../presentation/message-stream/text-view.ts | 5 +- src/features/chat/ui/goal.tsx | 16 +- .../chat/ui/message-stream/detail.tsx | 9 +- .../chat/ui/message-stream/status.tsx | 19 ++- src/features/chat/ui/toolbar.tsx | 27 +++- src/features/threads-view/renderer.tsx | 8 +- src/shared/diff/render.ts | 18 ++- src/styles/24-chat-stream-items.css | 4 - .../ui/message-stream/stream-items.test.tsx | 2 +- tests/scripts/development-scripts.test.ts | 21 +-- 12 files changed, 110 insertions(+), 169 deletions(-) diff --git a/scripts/lint/check-css-usage.mjs b/scripts/lint/check-css-usage.mjs index 0ab99288..dbfd711e 100644 --- a/scripts/lint/check-css-usage.mjs +++ b/scripts/lint/check-css-usage.mjs @@ -3,64 +3,11 @@ import path from "node:path"; const root = process.cwd(); const stylesDir = path.join(root, "src", "styles"); -const reportDynamicPrefixes = process.argv[2] === "--report-dynamic-prefixes"; -if (process.argv.length > (reportDynamicPrefixes ? 3 : 2)) { - console.error("Usage: node scripts/lint/check-css-usage.mjs [--report-dynamic-prefixes]"); +if (process.argv.length > 2) { + console.error("Usage: node scripts/lint/check-css-usage.mjs"); process.exit(1); } -const dynamicCssClassPatterns = [ - { - prefix: "codex-panel-diff__line--", - classes: [ - "codex-panel-diff__line--added", - "codex-panel-diff__line--context", - "codex-panel-diff__line--file", - "codex-panel-diff__line--hunk", - "codex-panel-diff__line--removed", - ], - }, - { - prefix: "codex-panel-diff__word--", - classes: ["codex-panel-diff__word--added", "codex-panel-diff__word--removed"], - }, - { - prefix: "codex-panel-threads__row--", - classes: ["codex-panel-threads__row--open", "codex-panel-threads__row--pending", "codex-panel-threads__row--running"], - }, - { - prefix: "codex-panel__connection-diagnostics-row--", - classes: ["codex-panel__connection-diagnostics-row--error", "codex-panel__connection-diagnostics-row--warning"], - }, - { - prefix: "codex-panel__execution--", - classes: ["codex-panel__execution--completed", "codex-panel__execution--failed", "codex-panel__execution--running"], - }, - { - prefix: "codex-panel__goal--", - classes: [ - "codex-panel__goal--active", - "codex-panel__goal--blocked", - "codex-panel__goal--budgetLimited", - "codex-panel__goal--complete", - "codex-panel__goal--paused", - "codex-panel__goal--usageLimited", - ], - }, - { - prefix: "codex-panel__limit-panel-meter--", - classes: ["codex-panel__limit-panel-meter--5", "codex-panel__limit-panel-meter--7"], - }, - { - prefix: "codex-panel__limit-panel-row--", - classes: ["codex-panel__limit-panel-row--danger", "codex-panel__limit-panel-row--warn"], - }, - { - prefix: "codex-panel__task-step--", - classes: ["codex-panel__task-step--completed", "codex-panel__task-step--inProgress"], - }, -]; - const cssFiles = await orderedCssFiles(); const sourceFiles = await filesInTree(path.join(root, "src"), new Set([".ts", ".tsx"]), { excludedPrefixes: [path.join(root, "src", "generated")], @@ -72,24 +19,14 @@ const sourceTexts = await readTexts(sourceFiles); const testTexts = await readTexts(testFiles); const sourceText = sourceTexts.map((item) => item.text).join("\n"); const dynamicPrefixes = collectDynamicClassPrefixes(sourceText); -const dynamicCssClasses = dynamicCssClassMap(dynamicCssClassPatterns); const testOnlyCandidates = []; const candidates = []; -const dynamicPrefixMatches = new Map(); for (const [className, locations] of cssClasses) { const sourceMatches = locationsInTexts(sourceTexts, className); if (sourceMatches.length > 0) continue; - const dynamicClass = dynamicCssClasses.get(className); - if (dynamicClass && dynamicPrefixes.includes(dynamicClass.prefix)) { - const matches = dynamicPrefixMatches.get(dynamicClass.prefix) ?? []; - matches.push({ className, locations }); - dynamicPrefixMatches.set(dynamicClass.prefix, matches); - continue; - } - const testMatches = locationsInTexts(testTexts, className); if (testMatches.length > 0) { testOnlyCandidates.push({ className, locations, testMatches }); @@ -99,18 +36,8 @@ for (const [className, locations] of cssClasses) { candidates.push({ className, locations }); } -const dynamicConfigurationErrors = dynamicCssConfigurationErrors({ - cssClasses, - dynamicCssClassPatterns, - dynamicPrefixes, -}); - -if (reportDynamicPrefixes) { - printDynamicPrefixReport({ dynamicPrefixes, dynamicPrefixMatches }); -} - -if (candidates.length + testOnlyCandidates.length + dynamicConfigurationErrors.length > 0) { - printCandidates({ testOnlyCandidates, candidates, dynamicConfigurationErrors }); +if (candidates.length + testOnlyCandidates.length + dynamicPrefixes.length > 0) { + printCandidates({ testOnlyCandidates, candidates, dynamicPrefixes }); process.exit(1); } @@ -152,33 +79,6 @@ function collectDynamicClassPrefixes(text) { return [...prefixes].sort((left, right) => left.localeCompare(right)); } -function dynamicCssClassMap(patterns) { - const result = new Map(); - for (const pattern of patterns) { - for (const className of pattern.classes) { - if (!className.startsWith(pattern.prefix)) { - throw new Error(`Dynamic CSS class ${className} does not match prefix ${pattern.prefix}.`); - } - result.set(className, { prefix: pattern.prefix }); - } - } - return result; -} - -function dynamicCssConfigurationErrors({ cssClasses, dynamicCssClassPatterns, dynamicPrefixes }) { - const errors = []; - for (const pattern of dynamicCssClassPatterns) { - const existingClasses = pattern.classes.filter((className) => cssClasses.has(className)); - if (existingClasses.length > 0 && !dynamicPrefixes.includes(pattern.prefix)) { - errors.push({ - title: "Dynamic CSS class configuration references prefixes that are not present in source templates:", - details: [` ${pattern.prefix}`, ...existingClasses.map((className) => ` ${className}`)], - }); - } - } - return errors; -} - async function readTexts(files) { const result = []; for (const file of files) { @@ -219,37 +119,13 @@ async function collectFiles(directory, extensions, excludedPrefixes, result) { } } -function printDynamicPrefixReport({ dynamicPrefixes, dynamicPrefixMatches }) { - console.log("Dynamic CSS class prefix exemptions:"); - if (dynamicPrefixMatches.size === 0) { - console.log(" none"); - } else { - for (const [prefix, matches] of [...dynamicPrefixMatches.entries()].sort(([left], [right]) => left.localeCompare(right))) { - console.log(` ${prefix}`); - for (const match of matches) { - console.log(` ${match.className}`); - console.log(` css: ${match.locations.join(", ")}`); - } - } - } - - const prefixesWithoutExemptions = dynamicPrefixes.filter((prefix) => !dynamicPrefixMatches.has(prefix)); - if (prefixesWithoutExemptions.length === 0) return; - - console.log(""); - console.log("Detected dynamic prefixes with no CSS exemptions:"); - for (const prefix of prefixesWithoutExemptions) { - console.log(` ${prefix}`); - } -} - -function printCandidates({ testOnlyCandidates, candidates, dynamicConfigurationErrors }) { +function printCandidates({ testOnlyCandidates, candidates, dynamicPrefixes }) { console.error("CSS usage check failed."); - for (const error of dynamicConfigurationErrors) { + if (dynamicPrefixes.length > 0) { console.error(""); - console.error(error.title); - for (const detail of error.details) { - console.error(detail); + console.error("Dynamic CSS class prefixes are not allowed:"); + for (const prefix of dynamicPrefixes) { + console.error(` ${prefix}`); } } diff --git a/src/features/chat/presentation/message-stream/detail-view.ts b/src/features/chat/presentation/message-stream/detail-view.ts index e0d155e8..c6d5edf7 100644 --- a/src/features/chat/presentation/message-stream/detail-view.ts +++ b/src/features/chat/presentation/message-stream/detail-view.ts @@ -158,7 +158,7 @@ function agentDetailView(item: AgentMessageStreamItem): DetailView { function genericToolDetailView(item: ToolCallMessageStreamItem | HookMessageStreamItem, workspaceRoot?: string | null): DetailView { return detailViewBase( item, - detailItemClassName(item.kind), + "codex-panel__detail-item", item.toolName ?? item.kind, messageDetailKey(item.id, "details"), [...genericToolDetails(item), ...outputSection(item.kind === "hook" ? "Hook output" : "Output", item.output)], @@ -187,7 +187,7 @@ function approvalDetailView(item: ApprovalResultMessageStreamItem): DetailView { function genericDetailView(item: MessageStreamItem, workspaceRoot?: string | null): DetailView { return detailViewBase( item, - detailItemClassName(item.kind), + "codex-panel__detail-item", detailLabel(item), messageDetailKey(item.id, "details"), genericDetailSections(item, workspaceRoot), @@ -199,10 +199,6 @@ function messageDetailKey(itemId: string, suffix: string): string { return `${itemId}:${suffix}`; } -function detailItemClassName(kind: MessageStreamItem["kind"]): string { - return kind === "hook" ? "codex-panel__detail-item codex-panel__detail-item--hook" : "codex-panel__detail-item"; -} - function resultDetailView( item: ApprovalResultMessageStreamItem | ReviewResultMessageStreamItem, label: string, diff --git a/src/features/chat/presentation/message-stream/text-view.ts b/src/features/chat/presentation/message-stream/text-view.ts index d60722c0..fda5551f 100644 --- a/src/features/chat/presentation/message-stream/text-view.ts +++ b/src/features/chat/presentation/message-stream/text-view.ts @@ -162,7 +162,10 @@ function userInputQuestionDetailViews(questions: readonly MessageStreamUserInput } function executionClassName(state: ExecutionState): string { - return state ? ` codex-panel__execution codex-panel__execution--${state}` : ""; + if (state === "completed") return " codex-panel__execution codex-panel__execution--completed"; + if (state === "failed") return " codex-panel__execution codex-panel__execution--failed"; + if (state === "running") return " codex-panel__execution codex-panel__execution--running"; + return ""; } function textItemClass(item: MessageStreamItem): string { diff --git a/src/features/chat/ui/goal.tsx b/src/features/chat/ui/goal.tsx index 6785bdf9..b9cf6a6a 100644 --- a/src/features/chat/ui/goal.tsx +++ b/src/features/chat/ui/goal.tsx @@ -126,7 +126,7 @@ export function GoalPanel({ }; return ( -
+
Goal @@ -241,6 +241,20 @@ function terminalGoalStatus(status: ThreadGoalStatus): boolean { return status === "complete" || status === "blocked" || status === "usageLimited" || status === "budgetLimited"; } +function goalClassName(status: ThreadGoalStatus | null, terminal: boolean): string { + return ["codex-panel__goal", goalStatusClassName(status), terminal ? "is-terminal" : ""].filter(Boolean).join(" "); +} + +function goalStatusClassName(status: ThreadGoalStatus | null): string { + if (status === "active") return "codex-panel__goal--active"; + if (status === "blocked") return "codex-panel__goal--blocked"; + if (status === "budgetLimited") return "codex-panel__goal--budgetLimited"; + if (status === "complete") return "codex-panel__goal--complete"; + if (status === "paused") return "codex-panel__goal--paused"; + if (status === "usageLimited") return "codex-panel__goal--usageLimited"; + return ""; +} + function syncGoalObjectiveHeight(textarea: HTMLTextAreaElement | null): void { syncTextareaHeight(textarea, { minHeightFallback: 56, diff --git a/src/features/chat/ui/message-stream/detail.tsx b/src/features/chat/ui/message-stream/detail.tsx index b6b70e8a..33cfc097 100644 --- a/src/features/chat/ui/message-stream/detail.tsx +++ b/src/features/chat/ui/message-stream/detail.tsx @@ -22,7 +22,7 @@ function Detail({ view, context }: { view: DetailView; context: DetailRenderCont view.className, "codex-panel__detail", view.sections.length === 0 ? "codex-panel__detail--plain" : "", - view.state ? `codex-panel__execution codex-panel__execution--${view.state}` : "", + executionClassName(view.state), open ? "is-open" : "", ] .filter(Boolean) @@ -56,6 +56,13 @@ function Detail({ view, context }: { view: DetailView; context: DetailRenderCont ); } +function executionClassName(state: DetailView["state"]): string { + if (state === "completed") return "codex-panel__execution codex-panel__execution--completed"; + if (state === "failed") return "codex-panel__execution codex-panel__execution--failed"; + if (state === "running") return "codex-panel__execution codex-panel__execution--running"; + return ""; +} + function DetailHeader({ view }: { view: DetailView }): UiNode { const content = {view.label}; return view.sections.length > 0 ? ( diff --git a/src/features/chat/ui/message-stream/status.tsx b/src/features/chat/ui/message-stream/status.tsx index b35f54fe..ff2278ae 100644 --- a/src/features/chat/ui/message-stream/status.tsx +++ b/src/features/chat/ui/message-stream/status.tsx @@ -44,7 +44,7 @@ function TaskProgress({ view }: { view: Extract {view.checklist.map((step) => ( -
  • +
  • {taskStatusMarker(step.status)} {step.step}
    • @@ -61,6 +61,12 @@ function taskStatusMarker(status: StatusChecklistItem["status"]): string { return "[ ]"; } +function taskStepClassName(status: StatusChecklistItem["status"]): string { + if (status === "completed") return "codex-panel__task-step codex-panel__task-step--completed"; + if (status === "inProgress") return "codex-panel__task-step codex-panel__task-step--inProgress"; + return "codex-panel__task-step"; +} + function ContextCompaction({ view }: { view: Extract }): UiNode { return ( @@ -106,9 +112,7 @@ function StatusMessage({ state: ExecutionState; children: UiNode; }): UiNode { - const classes = [createStatusMessageClassName(className), state ? `codex-panel__execution codex-panel__execution--${state}` : ""] - .filter(Boolean) - .join(" "); + const classes = [createStatusMessageClassName(className), executionClassName(state)].filter(Boolean).join(" "); return (
    {label}
    @@ -117,6 +121,13 @@ function StatusMessage({ ); } +function executionClassName(state: ExecutionState): string { + if (state === "completed") return "codex-panel__execution codex-panel__execution--completed"; + if (state === "failed") return "codex-panel__execution codex-panel__execution--failed"; + if (state === "running") return "codex-panel__execution codex-panel__execution--running"; + return ""; +} + function AgentSummaryRows({ view }: { view: AgentRunSummaryView }): UiNode { if (view.rows.length === 0 && view.additionalAgents === 0) return null; return ( diff --git a/src/features/chat/ui/toolbar.tsx b/src/features/chat/ui/toolbar.tsx index d1b99894..e9a0b67e 100644 --- a/src/features/chat/ui/toolbar.tsx +++ b/src/features/chat/ui/toolbar.tsx @@ -191,10 +191,7 @@ function RateLimitPanel({ rateLimit }: { rateLimit: RateLimitSummary | null }):
    Usage limit
    {rateLimit.rows.map((row) => ( -
    +
    {row.label}
    {row.value}
    @@ -202,7 +199,7 @@ function RateLimitPanel({ rateLimit }: { rateLimit: RateLimitSummary | null }): className={[ "codex-panel__limit-panel-meter", row.meterDivisions ? "codex-panel__limit-panel-meter--divided" : "", - row.meterDivisions ? `codex-panel__limit-panel-meter--${String(row.meterDivisions)}` : "", + rateLimitMeterDivisionClassName(row.meterDivisions), ] .filter(Boolean) .join(" ")} @@ -254,13 +251,31 @@ function DiagnosticSection({ section }: { section: ToolbarDiagnosticSection }): function DiagnosticRow({ row }: { row: ToolbarDiagnosticRow }): UiNode { return ( -
    +
    {row.label}
    {row.value}
    ); } +function rateLimitRowClassName(level: RateLimitSummary["rows"][number]["level"]): string { + if (level === "danger") return "codex-panel__limit-panel-row codex-panel__limit-panel-row--danger"; + if (level === "warn") return "codex-panel__limit-panel-row codex-panel__limit-panel-row--warn"; + return "codex-panel__limit-panel-row"; +} + +function rateLimitMeterDivisionClassName(divisions: RateLimitSummary["rows"][number]["meterDivisions"]): string { + if (divisions === 5) return "codex-panel__limit-panel-meter--5"; + if (divisions === 7) return "codex-panel__limit-panel-meter--7"; + return ""; +} + +function diagnosticRowClassName(level: NonNullable): string { + if (level === "error") return "codex-panel__connection-diagnostics-row codex-panel__connection-diagnostics-row--error"; + if (level === "warning") return "codex-panel__connection-diagnostics-row codex-panel__connection-diagnostics-row--warning"; + return "codex-panel__connection-diagnostics-row"; +} + function ThreadList({ threads, actions }: { threads: ToolbarThreadRow[]; actions: ToolbarActions }): UiNode { if (threads.length === 0) { return ( diff --git a/src/features/threads-view/renderer.tsx b/src/features/threads-view/renderer.tsx index 7ce8a02e..665f7da2 100644 --- a/src/features/threads-view/renderer.tsx +++ b/src/features/threads-view/renderer.tsx @@ -77,7 +77,7 @@ function ThreadRow({ row, actions }: { row: ThreadsRowModel; actions: ThreadsVie const mainClassName = [ "codex-panel-ui__nav-item", "codex-panel-threads__row-main", - row.live ? `codex-panel-threads__row--${row.live.status}` : "", + row.live ? liveStatusClassName(row.live.status) : "", row.selected ? "codex-panel-threads__row--selected" : "", ] .filter(Boolean) @@ -256,6 +256,12 @@ function RenameRow({ row, actions, className }: { row: ThreadsRowModel; actions: ); } +function liveStatusClassName(status: NonNullable["status"]): string { + if (status === "open") return "codex-panel-threads__row--open"; + if (status === "pending") return "codex-panel-threads__row--pending"; + return "codex-panel-threads__row--running"; +} + function ThreadsToolbarButton({ icon, label, diff --git a/src/shared/diff/render.ts b/src/shared/diff/render.ts index cfad7140..02477009 100644 --- a/src/shared/diff/render.ts +++ b/src/shared/diff/render.ts @@ -136,20 +136,34 @@ function oppositeChangeLineClass(className: ChangeLineClass): ChangeLineClass { } function renderLine(parent: HTMLElement, line: RenderDiffLine, inlineParts: InlineDiffPart[] | null = null): void { - const lineEl = parent.createEl("span", { cls: `codex-panel-diff__line codex-panel-diff__line--${line.className}` }); + const lineEl = parent.createEl("span", { cls: diffLineClassName(line.className) }); if (!inlineParts) { lineEl.textContent = displayDiffLineText(line.text, line.className); return; } for (const part of inlineParts) { if (part.changed) { - lineEl.createSpan({ cls: `codex-panel-diff__word codex-panel-diff__word--${line.className}`, text: part.text }); + lineEl.createSpan({ cls: diffWordClassName(line.className), text: part.text }); } else { lineEl.createSpan({ text: part.text }); } } } +function diffLineClassName(className: DiffLineClass): string { + if (className === "added") return "codex-panel-diff__line codex-panel-diff__line--added"; + if (className === "context") return "codex-panel-diff__line codex-panel-diff__line--context"; + if (className === "file") return "codex-panel-diff__line codex-panel-diff__line--file"; + if (className === "hunk") return "codex-panel-diff__line codex-panel-diff__line--hunk"; + return "codex-panel-diff__line codex-panel-diff__line--removed"; +} + +function diffWordClassName(className: DiffLineClass): string { + if (className === "added") return "codex-panel-diff__word codex-panel-diff__word--added"; + if (className === "removed") return "codex-panel-diff__word codex-panel-diff__word--removed"; + return "codex-panel-diff__word"; +} + function inlineDiff(removedText: string, addedText: string): InlineDiff | null { if (removedText.length + addedText.length > MAX_INLINE_DIFF_CHARS) return null; diff --git a/src/styles/24-chat-stream-items.css b/src/styles/24-chat-stream-items.css index 20c1284d..eb7e982a 100644 --- a/src/styles/24-chat-stream-items.css +++ b/src/styles/24-chat-stream-items.css @@ -50,10 +50,6 @@ border-left-color: var(--codex-panel-color-danger); } -.codex-panel__detail-item--hook:not(.codex-panel__execution) { - border-left-color: var(--text-accent); -} - .codex-panel__detail-header { display: flex; align-items: center; diff --git a/tests/features/chat/ui/message-stream/stream-items.test.tsx b/tests/features/chat/ui/message-stream/stream-items.test.tsx index 58c52cad..5037c7f7 100644 --- a/tests/features/chat/ui/message-stream/stream-items.test.tsx +++ b/tests/features/chat/ui/message-stream/stream-items.test.tsx @@ -263,7 +263,7 @@ describe("message stream item renderer decisions", () => { expect(element).toBeDefined(); expect(element.querySelector(":scope > summary")?.textContent).toBe("Work details"); - expect(element.querySelector(".codex-panel__detail-item--hook")?.classList.contains("codex-panel__execution--completed")).toBe(true); + expect(element.querySelector(".codex-panel__detail-item")?.classList.contains("codex-panel__execution--completed")).toBe(true); expect(element.querySelector(".codex-panel__stream-summary")?.textContent).toBe("postToolUse: Formatted 1 file."); expect(element.querySelector(".codex-panel__meta-grid")?.textContent).toContain("statuscompleted"); expect(element.querySelector(".codex-panel__meta-grid")?.textContent).toContain("eventpostToolUse"); diff --git a/tests/scripts/development-scripts.test.ts b/tests/scripts/development-scripts.test.ts index b6786312..4bce2944 100644 --- a/tests/scripts/development-scripts.test.ts +++ b/tests/scripts/development-scripts.test.ts @@ -149,19 +149,22 @@ describe("development scripts", () => { expect(result.stderr).toContain("codex-panel__unused"); }); - it("reports dynamic CSS prefix exemptions on demand", async () => { + it("fails CSS usage checks when source contains dynamic class prefixes", async () => { const cwd = await cssUsageFixture({ - "src/styles/10-component.css": ".codex-panel__task-step--completed { display: block; }\n", - "src/component.ts": "export const className = `codex-panel__task-step--${status}`;\n", + "src/styles/10-component.css": ".codex-panel__used { display: block; }\n", + "src/component.ts": [ + 'export const used = "codex-panel__used";', + "export const dynamicClassName = `codex-panel__task-step--${status}`;", + ].join("\n"), }); - const result = runNodeScript("scripts/lint/check-css-usage.mjs", ["--report-dynamic-prefixes"], cwd); + const result = runNodeScript("scripts/lint/check-css-usage.mjs", [], cwd); - expect(result.status).toBe(0); - expect(result.stderr).toBe(""); - expect(result.stdout).toContain("Dynamic CSS class prefix exemptions:"); - expect(result.stdout).toContain("codex-panel__task-step--"); - expect(result.stdout).toContain("codex-panel__task-step--completed"); + expect(result.status).toBe(1); + expect(result.stdout).toBe(""); + expect(result.stderr).toContain("CSS usage check failed."); + expect(result.stderr).toContain("Dynamic CSS class prefixes are not allowed:"); + expect(result.stderr).toContain("codex-panel__task-step--"); }); it("does not let dynamic CSS prefixes hide unconfigured class candidates", async () => {