Repository navigation
feat(design): DS-X-16 gallery stories controls and inputs - #1712
Conversation
Test-First: src/design/registry.coverage.test.ts::every controls and inputs entry has a gallery story Test-First: src/design/gallery/stories.test.tsx::a forced hover state uses the hover style Co-Authored-By: GPT-6 <noreply@openai.com>
Test-First: src/design/registry.coverage.test.ts::every controls and inputs entry has a gallery story Test-First: src/design/gallery/stories.test.tsx::a forced hover state uses the hover style Test-First: src/design/gallery/stories.test.tsx::stories show only supported states with disabled and invalid semantics Co-Authored-By: GPT-6 <noreply@openai.com>
No-Test: plan text only Co-Authored-By: GPT-6 <noreply@openai.com>
Regression-For: c1e885b Co-Authored-By: GPT-6 <noreply@openai.com>
Test-First: src/design/gallery/stories.test.tsx::every interactive CSS selector has its forced-state twin in the same rule Test-First: src/design/gallery/stories.test.tsx::default cells include registered toggle states and visual variants Test-First: src/design/gallery/stories.test.tsx::each story sample is named by its component and state caption Co-Authored-By: GPT-6 <noreply@openai.com>
|
@Cuarroc Dieser PR hat einen Konflikt mit main und kann nicht in die Merge-Queue. Bitte main hineinmergen (kein Rebase, kein Force-Push) und den Konflikt loesen; das Label |
Test-First: src/design/gallery/stories.test.tsx::every interactive CSS selector has its forced-state twin in the same rule Test-First: src/design/gallery/stories.test.tsx::default cells include registered toggle states and visual variants Test-First: src/design/gallery/stories.test.tsx::each story sample is named by its component and state caption Co-Authored-By: GPT-6 <noreply@openai.com>
No-Test: merge commit, merge commit resolving main conflicts Co-Authored-By: gpt-6.1-sol <noreply@openai.com>
…n story Merging origin/main (c618784) broke two checks: main's controls test reads the press-token rule per control, and the new IconButton entry from DS-X-21 had no gallery story. Keep main's press-token rules separate, add the IconButton story, and let the gallery CSS-twin test accept an identical adjacent rule. Regression-For: c618784 Co-Authored-By: gpt-6.1-sol <noreply@openai.com> Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5bf3bf4adf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // Only the components' existing states are shown; this is fixture data, never app state. | ||
| const stories: Record<string, Story> = { | ||
| Button: { states: PRESS, render: (s, name) => (s === "default" ? buttonVariants : buttonVariants.slice(0, 1)).map((variant) => <Button key={variant} {...sampleProps(s, name)} variant={variant} disabled={s === "disabled"}>{sample}</Button>) }, |
There was a problem hiding this comment.
Include Button's loading state in its story
When the controls gallery is opened, this story omits loading, so its “Nicht anwendbar” list incorrectly says that loading does not apply to Button. The component already accepts loading, renders aria-busy and a spinner, and the registry lists loading as a supported state; add a loading cell that passes the prop so the gallery does not misdocument the component.
Useful? React with 👍 / 👎.
| | DS-X-11 | Zeile, Trenner und Marke: `surface/ListRow.tsx` (wählbare Zeile, Auswahl kommt vom Aufrufer per `aria-current` oder `aria-selected`, ein Tabstopp), `surface/Separator.tsx` (`<hr>`, dekorativ für Hilfstechnik verborgen) und `surface/Badge.tsx` (Zahl oder Stichwort als Text, ersetzt weder `.g-st` noch `.g-proof`); drei Registereinträge, Familie „Flächen“; die Halle (DS-H-05) nutzt die Zeile | Implementierer · Claude; Root nimmt ab | DS-Blueprint-Schnitt 16.10. (DS-X-11) | Server | B | Erledigt: #1673 (merged 2026-10-17); keine Bildschirmmigration, die Halle (DS-H-05) bindet die Zeile seit #1680 ein | Tests rot→grün; `vitest run src/design`, typecheck, `contrast-check` (746 Paarungen), `plan-check --offline` und prepush Exit 0; Bilder der Galerie hell und dunkel; zwei Stufe-B-Reviews fremder Nicht-Anthropic-Familien | – | | ||
| | DS-X-14 | Komponenten-Register und Abdeckungstest: `src/design/registry.ts` führt jede exportierte Designkomponente einmal (Familie, Varianten, Zustände, Bedienhilfe, Textquelle), `registry.coverage.test.ts` wird rot, wenn eine fehlt oder ein Eintrag keine Komponente mehr trifft | Implementierer · Claude; Root nimmt ab | DS-Blueprint-Schnitt 16.10. (DS-X-14) | Server | B | Erledigt: #1656 (merged 2026-10-16); 25 Komponenten gelistet, auch `hall/` wird gescannt (dort noch keine .tsx, DS-H-04 trägt seinen Eintrag selbst ein); Galerie DS-X-15 und Geschichten DS-X-16/17 lesen später dieses Register | Abdeckungstest rot→grün; `vitest run src/design`, typecheck, `plan-check --offline` und prepush Exit 0; zwei Stufe-B-Reviews fremder Nicht-Anthropic-Familien | – | | ||
| | DS-X-15 | Designsystem-Galerie, Hülle und Grundlagen (D6): Einstellungen › Version › „Designsystem ansehen“ öffnet `#/designsystem` (eigener `React.lazy`-Chunk, außerhalb von `ROUTES`, also nicht in Seitenleiste oder Palette); Navigation über die Register-Familien, Seiten Farben (Kontrast zur Grundfläche), Schrift, Abstand, Radius, Glas, Bewegung (ruht bei reduzierter Bewegung); keine echten Daten, Beispielinhalt trägt „Beispieldaten“ | Implementierer · Claude; Root nimmt ab | DS-Blueprint-Schnitt 16.10. (DS-X-15) | Server | B | Erledigt: #1662 (merged 2026-10-16); Geschichten je Baustein kommen mit DS-X-16/17, Werkzeugleiste und Übertrag aus #1656 (DS-X-14b) nicht enthalten | Tests rot→grün; `vitest run`, typecheck, `contrast-check`, `plan-check --offline` und prepush Exit 0; Bildschirmfotos hell/dunkel; zwei Stufe-B-Reviews fremder Nicht-Anthropic-Familien | – | | ||
| | DS-X-16 | Galerie-Geschichten für alle Registereinträge der Familien controls und inputs: Zustandsmatrix über data-force-state und die echten CSS-Regeln, nicht anwendbare Zustände benannt, sichtbare Beispieldaten, Texte aus de.ts | Implementierer · Codex; Root nimmt ab | DS-X-15 (#1662); DS-Blueprint-Schnitt 16.10. (DS-X-16) | Server | B | In Prüfung: Entwurfs-PR auf codex/ds-x-16; nach Merge sind 13 Geschichten mit Abdeckungstest vorhanden; weitere Familien bleiben DS-X-17, shot.mjs bleibt DS-H-07b | Tests rot→grün; Design-Tests, typecheck, contrast-check, plan-check --offline und eigene prepush Exit 0; Bilder beider Familien hell/dunkel geprüft; ein unabhängiger Stufe-B-Review, Root beauftragt | – | |
There was a problem hiding this comment.
Update the plan to count the IconButton story
The registry at this commit contains seven controls and seven inputs, and this change supplies a story for every one, including IconButton; the delivered total is therefore 14 rather than the 13 claimed here. Because this text explicitly describes the post-merge result, the canonical plan would become stale as soon as the change merges.
AGENTS.md reference: AGENTS.md:L121-L121
Useful? React with 👍 / 👎.
| Input: { states: INPUT, render: (s, name) => <Input {...inputProps(s, name)} defaultValue={sample} /> }, | ||
| Textarea: { states: INPUT, render: (s, name) => <Textarea {...inputProps(s, name)} defaultValue={sample} /> }, | ||
| Kbd: { states: STATIC, render: (s, name) => <Kbd {...sampleProps(s, name)}>{de["gallery.key"]}</Kbd> }, | ||
| Lamp: { states: ["default", "loading", "error"], render: (s, name) => (s === "default" ? lampShapes : [s === "loading" ? "busy" as const : "bad" as const]).map((state) => <Lamp key={state} {...sampleProps(s, name)} state={state} label={de[`gallery.state.${s}`]} />) }, |
There was a problem hiding this comment.
Label each Lamp shape in the default story
In the default Lamp cell, this renders ok, off, warn, and unknown as four adjacent glyphs under the single caption “Standard”, while every glyph also receives the same accessible name. A gallery visitor therefore cannot determine which shape corresponds to which LampState, visually or with a screen reader; render a visible and state-specific label for each shape.
Useful? React with 👍 / 👎.
| .g-btn--danger:hover { background: color-mix(in srgb, var(--g-bad-bg), var(--g-bad) 8%); border-color: var(--g-bad); } | ||
| .g-fchip:not([aria-pressed="true"]):hover { background: var(--g-tint-hi); } | ||
| .g-seg button:not([aria-checked="true"]):hover { background: var(--g-tint); } | ||
| .g-btn:hover, [data-force-state="hover"] .g-btn { background: var(--g-hover); } |
There was a problem hiding this comment.
Apply forced hover previews outside hover-capable media
On a touch-only or coarse-primary-pointer environment, this entire media query is false, so the new [data-force-state="hover"] selectors never apply and every gallery cell labelled “Zeiger darüber” displays the default appearance. Keep the real :hover rules guarded by the capability query, but place equivalent forced-preview rules outside it so the state matrix remains accurate on devices that cannot physically hover.
Useful? React with 👍 / 👎.
Merge Queue Status
This pull request spent 8 minutes 47 seconds in the queue, including 8 minutes 7 seconds running CI. Required conditions to merge
|
Die Galerie zeigt jetzt alle 13 Bedienelemente und Eingaben mit eigenen Beispielen.
Jedes Beispiel zeigt seine vorhandenen Zustände und benennt die nicht anwendbaren Zustände.
Die hellen und dunklen Bilder sind geprüft; die unabhängige Prüfung beauftragt Root.
Report
Plan-ID: DS-X-16
Base: main at f3fb146.
Candidate: f50ea50.
Size: 181 changed lines (164 additions, 17 deletions), including tests and exactly one PLAN row.
Added one rendered story for each controls/inputs registry entry. Every matrix cell has data-force-state; pseudo-class previews share the actual CSS declaration block, including the reduced-motion override. Disabled examples also use native disabled; invalid inputs expose aria-invalid and a linked error label. Lamp loading/error use its existing busy/bad props and unchanged production CSS. Natural-language UI strings come from de.ts; component identifiers come from the registry. Beispieldaten remains visible above the stories. The coverage test checks rendered articles on both family pages, with an explicit family list for DS-X-17 to extend. No dependency, seam, registry or shot.mjs change.
Applicable states (all other matrix states are listed as not applicable in each story):
Evidence (Linux, 2026-10-18):
npx vitest run src/design/registry.coverage.test.ts src/design/gallery/stories.test.tsx— exit 1 before implementation; both new checks fail because no story exists. Failing tests committed first as 729e4e5, implementation c1e885b, plan-only commit 3eb4176.npx vitest run src/design— exit 0; 47 files, 236 tests passed.npm run typecheck— exit 0.node scripts/contrast-check.mjs— exit 0; 760 pairs passed.node scripts/plan-check.mjs --offline— exit 0; no findings.bash scripts/ci/red-first.sh— exit 0; all three named new checks absent/red at the merge base and green at the candidate.VITE_TAURI_MOCKS=true node node_modules/.cache/ds-x-16-proof.mjs <artifact-directory>— exit 0. The retained local proof script starts Vite, awaits the existing shot.mjs captureRoute for both routes, then closes browser and server. No detached process or desktop app. Browser assertions confirm 6 controls and 7 inputs stories, equal real/forced Button hover background and focus outline in both themes, and no forced press transform under reduced motion.bash scripts/ci/gates.sh lane prepush— exit 1 at the hardcoded UI-string check (three internal component-name arguments and a separator). Fixed in f50ea50 by deriving identifiers from the registry and placing the separator in de.ts; no assertion or baseline was weakened. Focused hardcoded/coverage/story checks:npx vitest run src/i18n/hardcoded.test.ts src/design/registry.coverage.test.ts src/design/gallery/stories.test.tsx— exit 0, 27 tests.bash scripts/ci/gates.sh lane prepush— exit 0 on f50ea50 in this worktree; 410 seconds, all 28 gates green, including 2278 Rust tests passed (7 existing platform/capability skips).PA_PREPUSH=light git push -u origin codex/ds-x-16— exit 0; the supported branchpush hook also passed (200 seconds). Full prepush above was completed on the identical head first.git ls-remote origin codex/ds-x-16— exit 0; remote head f50ea50 matches local HEAD.Screenshots inspected:
ds-x-16-controls-light.png,ds-x-16-controls-dark.png,ds-x-16-inputs-light.png,ds-x-16-inputs-dark.png(existing shot.mjs, 1440×900, browser mocks, reduced motion). Additional inspectedds-x-16-{controls,inputs}-{light,dark}-full.pngimages show every story in a taller browser viewport. Artifact filenames are public-safe; the private handover contains their local locations.NICHT ABGEDECKT:
Review: pending, Root commissions (tier B; one independent non-author-family reviewer per this dispatch). No self-approval, labels, ready action or merge.
Integration note: controls.css and Gallery.tsx overlap the already assigned #1693/#1706 and #1701/#1702 changes. A later IconButton/loading registry addition needs a corresponding story when integrated; this candidate covers the 13 entries at its recorded base.
🤖 Generated with Claude Code on the ProjectA server worker
Review round 1 (18.10.)
This append records the review-fix delta; the earlier report above remains the historical evidence for f50ea50. No PLAN row was changed in this round.
Reviewed head: f50ea50 (Anthropic subscription review, APPROVE WITH CONDITIONS). New head: 1cd4fe4.
Changes and test boundaries:
Evidence (Linux, 2026-10-18):
npx vitest run src/design/gallery/stories.test.tsxbefore the fix — exit 1 (two expected regression failures).npx vitest run src/design/gallery/stories.test.tsx -t 'every interactive CSS selector has its forced-state twin in the same rule'with a deliberately removed CSS twin — exit 1, specifically the missing primary-hover twin. The mutation was restored and is not committed.npx vitest run src/design src/i18n/hardcoded.test.ts— exit 0 (48 files, 254 tests).npx vitest run src/designafter the final generic-selector change — exit 0 (47 files, 239 tests).npm run typecheck— exit 0.node scripts/contrast-check.mjs— exit 0 (760 pairs).node scripts/plan-check.mjs --offline— exit 0 (0 findings).VITE_TAURI_MOCKS=true node node_modules/.cache/ds-x-16-review-proof.mjs node_modules/.cache/ds-x-16-review-round1— exit 0. Uses existing shot.mjs, with browser mocks and an awaited Vite lifecycle. Checked both theme variants, six controls/seven inputs stories, checked Switch, pressed FilterChip, five Buttons, four default Lamp shapes, real/forced Button hover and focus, and reduced-motion press behavior. An initial proof attempt exited 1 because its fixed 180 ms delay sampled an unfinished transition; the final probe awaits the animation's finished promise and confirms real :hover before comparing colors.ds-x-16-controls-light-full.png,ds-x-16-controls-dark-full.png,ds-x-16-inputs-light-full.png,ds-x-16-inputs-dark-full.png. Standard route shots were also captured at 1440x900; full-page images show every story. Artifacts and the proof script are local ignored files.bash scripts/ci/red-first.sh— exit 0 on the new head; all six distinct named tests are absent/red at the merge base and green at the head (repeated trailers are also checked).bash scripts/ci/secret-scan.sh— exit 0 before each of the two new commits; no leaks. Both commits passed the regular precommit hook.bash scripts/ci/gates.sh lane prepush— exit 0 on the new head in this worktree; 551 seconds, all 28 gates green, including 1,639 frontend tests and 2,278 Rust tests passed (7 existing platform/capability skips).PA_PREPUSH=light git push -u origin codex/ds-x-16— exit 1: the supported branchpush hook stopped atsrc/screens/leitstand/perf/renderBudget.test.tsx:53after its unchanged 5,000 ms timeout (5,483 ms observed). All 1,638 other frontend tests passed. The hook correctly prevented the push. Render counts were within budget (mount 1/40, refresh 1/1, filter 1/0);uptimethen showed load averages 27.04/24.87/23.08 on 8 cores. Contention is a hypothesis, not a proven cause.npx vitest run src/screens/leitstand/perf/renderBudget.test.tsx— exit 0 without changes, 920 ms test duration. No timeout, retry setting, threshold or assertion was changed.PA_PREPUSH=light git push -u origin codex/ds-x-16— exit 0; supported branchpush hook green in 117 seconds, all 1,639 frontend tests passed with unchanged defaults. The full prepush lane above had already passed on the identical head. No hook was bypassed.git ls-remote origin codex/ds-x-16— exit 0; remote head 1cd4fe4 matches local HEAD. Working tree is clean.Size versus main (merge-base f3fb146): 9 files changed, 221 insertions(+), 17 deletions(-); 238 changed lines. Limit: 300 changed lines including tests.
NICHT ABGEDECKT:
Review: ausstehend (Stufe B). Review: pending, Root commissions the required non-OpenAI delta review. No ready action, label or merge.
🤖 Generated with Claude Code on the ProjectA server worker
Conflict fix (Codex, 2026-10-20)
BLOCKED: the authorized textual merge is resolved, but full local prepush exits 1 on two deterministic integration failures. No retry and no push were attempted; these are not the permitted SessionProvider timeout. Root must authorize a separate/broader correction before this candidate can pass and be pushed.
gpt-6.1-sol, efforthigh, provideropenai, read from this worker's harness transcript on 2026-10-20; provider quota was not observable. Native goal created/read for JOBcodex-dsx16-conflict, owner this worker, parent Root's PR feat(design): DS-X-16 gallery stories controls and inputs #1712 conflict task; acceptance remains unmet.git ls-remote origin refs/heads/codex/ds-x-16, exit 0, checked after the failed gate):1cd4fe43b829f5123533d9dfa4ce63523872c7bb.c618784818dd73f1c7f13acef3b5652a8638caeb; second parent/main:9f33f721d52a3a9188a3129118b52bab2cf1bfcd. Commit has the requested Cuarroc identity, merge message, No-Test trailer and observed-model co-author.git fetch origin: exit 0.git merge origin/main: exit 1 with exactly the four authorized conflicting files; all four were resolved, no additional conflict files. No rebase.node scripts/plan-check.mjs --offline: exit 0, 0 findings (also repeated after prepush).controls.css: retained main's press/disabled tokens, loading/icon-button rules and reduced-motion behavior; added the branch's forced-state selectors to the corresponding main rules, including the shared disabled rule.gallery.css: retained main's icon-gallery rule and appended the branch's story/matrix/cell layout rules.inputs.css: retained main's disabled token and transform-based meter rules; added the branch's error/disabled/focus preview selectors.npm ci: exit 0 (installed updater was 2.13.0 while the merged lockfile requires 2.14.0). No tracked dependency edits.bash scripts/ci/secret-scan.sh: exit 0, no leaks. Requestedgit ... commit: exit 0; all precommit gates passed, including cargo check and typecheck.git diff origin/main...HEAD --stat: exit 0; exactly the original nine PR paths, 221 insertions and 17 deletions (238 total), including only one added PLAN row.git diff --cached --check: exit 0 before commit.npm run dev:start-check: exit 0 before the full gate; 4.6 GB free, 0 cargo builds. Full gate ran inC:/Users/Cuarr/orca/ProjectA/.claude/worktrees/codex-dsx16at the local candidate withCARGO_TARGET_DIR=C:/Users/Cuarr/cargo-targets/projecta-c,CARGO_BUILD_JOBS=1.bash scripts/ci/gates.sh lane prepush, run 1: exit 1 after 176 s;fe-testfailed (230 test files: 228 passed/2 failed; 1692 tests: 1690 passed/2 failed). All earlier gates, including plan-check, red-first-plan, typecheck and lint, passed. No rerun.src/design/controls/controls.test.tsx:164matches only:activeimmediately before{; the preserved press-token rules now include the required gallery selectors, so its parser finds only the unchanged tabs rule (assertion at line 165).src/design/registry.coverage.test.ts:129requires a story for every controls/inputs registry entry; main addsIconButton(src/design/registry.ts:29), but the branch's story catalogue has none (src/design/gallery/stories.tsx:30). Test reports: unable to find article named IconButton.NICHT ABGEDECKT:
Integration fix (Codex, 2026-10-20)
BLOCKED at commit-msg: the requested second Test-First trailer contains commas in its exact test title. scripts/lib/test-first.sh:87-89 rejects every comma as a list. The regular commit exited 1 after all precommit gates passed; no new commit or push was made. Resume condition: Root authorizes the validated comma-free test filter shown below, or provides an authorized hook correction. No hook bypass, test rename, rebase, ready action, label, merge or queue comment.
Observed model: gpt-6.1-sol, effort high, from this job's harness turn_context on 2026-10-20. Provider quota was not observable. Native goal was created and read; it remains active because commit/push acceptance is unmet. The lage-jetzt mirror is unavailable in this worktree; live Git and PR evidence was used.
Old and rechecked remote head (git ls-remote origin refs/heads/codex/ds-x-16, exit 0): 1cd4fe4. New remote head: unchanged. Local HEAD remains c618784, with exactly three staged files: controls.css, gallery/stories.tsx and gallery/stories.test.tsx.
CSS resolutions:
Added the IconButton gallery story using the existing story matrix, existing Icon/check glyph and existing de.ts state labels. It shows default, hover, focus, active, disabled and loading; disabled is native and loading uses the component's existing prop. No de.ts change was needed. The branch-only story guard now also accepts an identical adjacent declaration block in the same parent/media context. Existing selector-removal mutation checks remain. No test inherited from main was changed; both named regression files are byte-unchanged from the starting merge.
Evidence (Windows, this worktree, 2026-10-20):
NICHT ABGEDECKT:
Review disposition: pending Root's required non-OpenAI delta review. This worker did not approve itself or change PR readiness, labels or merge state.
Review round 2 / delta and integration (Root, 2026-10-20)
f50ea504..5bf3bf4a(fix round1cd4fe43, mergec6187848, integration5bf3bf4a): APPROVE. Low follow-ups only:aria-labelis redundant next toaria-labelledby.5bf3bf4a(Regression-For: c6187848) does three things:Push-hook prepush exit 0;
git ls-remote=5bf3bf4a.5bf3bf4a: verdict "reject". Every high/medium finding was checked at the source and rejected:SegmentedPropshas nodisabled, so "nicht anwendbar" is correct[tabindex="0"]only; the test normalises itstories.test.tsx; the real.g-seg button:activerule is unchangedIconButtonPropsrequiresaria-labelby type; the label includes the state, so it stays unique.g-inplacks hover/active twinsinputs.csshas no:hover/:activerule, so there is nothing to twin; the twin test is greenSelectspreads...restonto the native<select class="g-inp">No confirmed high finding.
review-okbound to head5bf3bf4adf2bae45059f87eb2d0247ba93847619.