From dc7510a902afae1b664e9cbfb52825b80a616b39 Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Wed, 5 Aug 2026 20:33:19 +0800 Subject: [PATCH] fix(ui-models): close the media block that swallowed the fetch dialog's styles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `@media (prefers-reduced-motion: reduce)` block never closed, so every rule after it — the whole fetch dialog, its candidate list, and the rows inside — applied only to viewers whose system asks for reduced motion. The sheet still parsed and the classes still attached, so the list painted with the browser's own bullets, indentation, and inline label, and the reported model id ran straight into the capacity beside it. Report the id alone: it is the string adoption writes, and the capacities the endpoint disclosed are adopted with it and editable in the row that appears. The candidate row's remaining font name is the one the theme declares. The styles gate now checks that the sheet's braces balance, and reads every theme sheet rather than the platform tokens alone, so a name declared in a sibling is not called undeclared and a `--dsh-` typo cannot pass as a token. --- .../ui-models/src/client/ModelListEditor.tsx | 6 ++--- .../src/client/ModelsSection.module.css | 9 ++----- .../client/ui-models/tests/styles.spec.ts | 27 ++++++++++++++----- 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/packages/client/ui-models/src/client/ModelListEditor.tsx b/packages/client/ui-models/src/client/ModelListEditor.tsx index 5cc0233fd1..27f706f642 100644 --- a/packages/client/ui-models/src/client/ModelListEditor.tsx +++ b/packages/client/ui-models/src/client/ModelListEditor.tsx @@ -427,10 +427,10 @@ export function ModelListEditor(props: ModelListEditorProps): ReactNode { checked={picked.has(candidate.id)} onChange={() => { toggle(candidate.id) }} /> + {/* The id alone: it is the string adoption writes, and the + capacities the endpoint reported are adopted with it and + editable in the row that appears. */} {candidate.id} - {candidate.contextWindow === undefined - ? null - : {candidate.contextWindow}} ))} diff --git a/packages/client/ui-models/src/client/ModelsSection.module.css b/packages/client/ui-models/src/client/ModelsSection.module.css index 797d67d4b4..e8232e2d81 100644 --- a/packages/client/ui-models/src/client/ModelsSection.module.css +++ b/packages/client/ui-models/src/client/ModelsSection.module.css @@ -583,6 +583,7 @@ select.input { .customizedSummary::before { transition: none; } +} .fetchDialog { max-width: 520px; @@ -620,13 +621,7 @@ select.input { .candidateId { flex: 1 1 auto; - font-family: var(--dsh-font-mono, monospace); + font-family: var(--ds-font-family-code); font-size: 13px; overflow-wrap: anywhere; } - -.candidateMeta { - color: var(--dsh-text-tertiary, #888); - font-size: 12px; - font-variant-numeric: tabular-nums; -} diff --git a/packages/client/ui-models/tests/styles.spec.ts b/packages/client/ui-models/tests/styles.spec.ts index 37ac894938..e13cd2baf5 100644 --- a/packages/client/ui-models/tests/styles.spec.ts +++ b/packages/client/ui-models/tests/styles.spec.ts @@ -6,17 +6,20 @@ * still renders and only the dark theme looks wrong. Checking the names against * the sheet that declares them is what turns that into a test failure. */ -import { readFileSync } from 'node:fs' +import { readdirSync, readFileSync } from 'node:fs' import { fileURLToPath } from 'node:url' import { describe, expect, it } from 'vitest' const css = readFileSync(fileURLToPath(new URL('../src/client/ModelsSection.module.css', import.meta.url)), 'utf8') // The theme package maps `./styles/*` to `./src/styles/*`, so the declarations // stay on the source plane rather than needing a build. -const tokens = readFileSync( - fileURLToPath(new URL('../../ui-theme/src/styles/design-platform.css', import.meta.url)), - 'utf8', -) +// Every theme sheet, not just the platform tokens: font and scrollbar +// variables are declared in siblings, and a gate reading one file would call +// their names undeclared. +const tokens = readdirSync(fileURLToPath(new URL('../../ui-theme/src/styles/', import.meta.url))) + .filter(name => name.endsWith('.css')) + .map(name => readFileSync(fileURLToPath(new URL(`../../ui-theme/src/styles/${name}`, import.meta.url)), 'utf8')) + .join('\n') /** The declarations of one top-level rule, by selector. */ function block(selector: string): string { @@ -31,12 +34,24 @@ describe('ModelsSection theme styles', () => { // resolves to whatever literal sits in its fallback slot, which is how this // section stayed light under the dark theme before. Undeclared names have // no fallback at all and inherit, so both spellings must fail here. - const named = [...css.matchAll(/var\((--dsw-[a-z0-9-]+)/g)].map(match => match[1]) + // Every theme-variable prefix the sheets actually use, not just `--dsw-`: + // a `--dsh-` name reads as a plausible sibling and would otherwise slip + // past this gate into a fallback literal. + const named = [...css.matchAll(/var\((--(?:dsw|dsh|ds)-[a-z0-9-]+)/g)].map(match => match[1]) const undeclared = [...new Set(named)].filter(name => !tokens.includes(` ${String(name)}:`)) expect(undeclared).toEqual([]) expect(css).not.toMatch(/var\(--(?:surface|text-|border|accent-strong)/) }) + it('closes every block, so no rule is swallowed by the one above it', () => { + // A missing `}` on an `@media` block is not a parse error: every rule after + // it silently becomes conditional, and the whole fetch dialog once painted + // unstyled for anyone whose system does not ask for reduced motion. Nothing + // downstream reports this — the sheet loads and the classes still attach. + const bare = css.replace(/\/\*[\s\S]*?\*\//g, '') + expect((bare.match(/\}/g) ?? []).length).toBe((bare.match(/\{/g) ?? []).length) + }) + it('separates the row card from the editor it expands into', () => { // `bg-layer-3` and `bg-module-platform` both resolve to neutral-bluish-800 // under the dark theme, so filling the row with either erases the nested