Skip to content

feat(design): complete motion tokens and layout-animation contract (DS-01) - #1561

Merged
mergify[bot] merged 4 commits into
mainfrom
codex/ds-01-motion-tokens
Oct 15, 2026
Merged

mergify[bot] merged 4 commits into
mainfrom
codex/ds-01-motion-tokens

Conversation

@Cuarroc

@Cuarroc Cuarroc commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner

Das Designsystem erhält eine gemeinsame Bewegungsskala und eine begrenzte Staffelung für Listen.
Die Nachbesserung schaltet auch eigenständig verwendete Einstiegseffekte bei reduzierter Bewegung aus und erweitert den Schutz vor Layout-Animationen.
Die Review-Befunde sind unten einzeln beantwortet; die unabhängige Prüfung der Nachbesserung steht noch aus.

Report

Change and scope

DS-01 adds duration, easing, distance and calm-default tokens while retaining existing token names and values. Typed semantic exports reference CSS properties. Stagger indices are capped at eight and delay defaults to zero. Duplicate g-rise and g-fade definitions were removed from layout/layout.css.

Fix round 1 adds .g-stagger > *, .g-sheet, .g-view and .g-is-new to the existing OS reduced-motion kill switch, including standalone use outside shell/portal roots. The existing layout override still covers tab panels, sheets and toasts. The layout-motion guard now includes logical dimensions, flex basis, gaps, font size, line height and border widths. Comments document stagger composition and the unconsumed preference tokens.

The CSS contract parses every design stylesheet with PostCSS provided transitively by Vite, not a directly declared dependency. This corrects the previous report's ambiguous “existing dependency” wording using the explicit report-correction option in R1561-A1. No dependency installation, manifest/lockfile change or license addition in this round. The parser still relies on npm's current hoisted dependency layout; strict dependency layouts or future Vite changes require follow-up.

Scope: four package files; 154 additions + 3 deletions = 157 diff lines. Round-one delta: 31 additions + 1 deletion in three files. No seams, Rust, base.css, architecture exceptions or unrelated test changes. The two existing meter-width transition exceptions remain occurrence-sensitive and may only shrink in DS-05.

Evidence — 2026-10-15

  • Merge-base: 3015d841d18fd95d1d0192e193b23955307e4bec.
  • Reviewed starting head: db2135599048187f4d37d205a14aa4123a8da624.
  • New red-test commit: 5263de62306b1d32a505322fcfc912284e70231a.
  • Fix candidate: e3e10d05013a9f8e44a31a488adefc5c67a84e70.
  • Parent: DS-01/design-system polish; ticket FIX-PR1561-R1. Worker goal ends at verified push/report handover; Root owns external review and integration. Native goal create/get output was recorded in the worker session.
Command / observation Exit Result
npx --no-install vitest run src/design/motion.test.ts with new tests on db213559 1 Two compiling assertion failures: missing standalone stagger override and missing inline-size detection; ten existing tests passed
npm run typecheck on the red-test tree 0 Compiling regression tests
npx --no-install vitest run src/design after the fix 0 20 files, 103 tests passed
node --input-type=module (isolated Playwright Chromium fixture) 0 45 entries per CSS order: 45 active animations normally, zero with reduced motion; reduced entries had opacity 1 and transform none
bash scripts/ci/red-first.sh 0 All named original and new tests absent at merge-base and green at candidate; temporary base reused installed node_modules only after byte-identical manifest and lockfile checks, with no install
bash scripts/ci/secret-scan.sh before both commits 0 No leaks; both commit hooks also passed
git diff --check and git diff origin/main...HEAD --check 0 Passed
Initial bash scripts/ci/gates.sh lane prepush 1 Render-budget test exceeded its unchanged 5000ms timeout (5501ms); 1,427 other tests passed; later gates did not run
npx --no-install vitest run src/screens/leitstand/perf/renderBudget.test.tsx --coverage 1 Diagnostic: test itself passed in 4.67s; global coverage thresholds correctly failed for a one-test selection
VITEST_MAX_WORKERS=1 bash scripts/ci/gates.sh lane prepush 0 All 27 gates green at e3e10d05, 744 seconds; 1,428 frontend tests and 2,273 Rust tests passed
VITEST_MAX_WORKERS=1 git push -u origin codex/ds-01-motion-tokens 0 Full prepush hook passed again (407 seconds); both new commits pushed without rewriting history
git ls-remote origin codex/ds-01-motion-tokens 0 Remote equals e3e10d05013a9f8e44a31a488adefc5c67a84e70

Runtime fixture method: load the real motion.css and layout/layout.css via Playwright page.setContent, in both stylesheet orders; render ten stagger children and five standalone entry/layout classes, repeated outside roots, within .g-shell, and within [data-g-portal]. Use reducedMotion: 'no-preference' and 'reduce', with --g-dur-2: 100s and --g-stagger: 1s to keep normal animations observable. Read getComputedStyle and getAnimations() for all 45 entries and assert the resting state under reduced motion. No desktop app, live queue or server was started.

The timeout investigation observed load averages above 26 on the shared host. Subsequent full validation uses VITEST_MAX_WORKERS=1 to limit this worker's concurrency; test selection, timeout, coverage thresholds and assertions remain unchanged. Startcheck capacity waits observed two and then three Cargo builds; validation resumed after the count returned to zero. At startup, PR #1547 was still open (gh pr view 1547 --json state,mergeCommit), so no main merge was required. Provider usage and actual model were not independently observable; the memory mirror was unavailable, so live Git/PR evidence was used.

Review disposition

Reviews at db213559: Anthropic APPROVE WITH CONDITIONS (local review record), Google REQUEST_CHANGES (accepted finding supplied by Root in the task). R1561-G1 is the local disposition ID for that supplied Google finding.

Finding Source / severity Disposition
R1561-G1 — stagger entry survives reduced motion outside roots (motion.css, .g-stagger > *) Google / accepted by Root Accepted in e3e10d05: explicit reduced-motion selectors, red-first regression, and measured zero active animations in both stylesheet orders
R1561-A1 — PostCSS is only transitive (motion.test.ts:3) Anthropic / medium Addressed by the review's explicitly permitted report-correction alternative: dependency provenance and hoisting limitation stated above; no new dependency or install
R1561-A2 — missing logical/layout properties (motion.test.ts:15) Anthropic / low Accepted in e3e10d05: expanded guard plus a new red-first test covering transitions, transition-property and keyframes
R1561-A3 — child animation conflicts (motion.css, stagger rule) Anthropic / low Accepted in e3e10d05: comment prohibits independent child animations. The index-eight cap remains; DS-02 must validate late insertion with nonzero stagger
R1561-A4 — unused preference/typed tokens and easing ambiguity (motion.css, motion/tokens.ts) Anthropic / low, informational Documented in e3e10d05 and this report: --g-enter-dist is 4px but currently unconsumed; actual rise stays 8px. Ambient/trail tokens and typed exports await DS-02/DS-05. Semantic standard means --g-ease-move; legacy --g-ease remains ease-out

Review: ausstehend (Stufe B)

Root schedules the non-OpenAI delta review. This worker did not review, set review-ok, enqueue or merge the PR. The existing draft state is preserved for handover.

NICHT ABGEDECKT

  • Nextest skipped seven tests; the Denkraum Node suite skipped one Windows-only test. HQ also skips platform-specific tests on Linux. This is the prepush lane, not the full Linux workflow lane; production build and app browser smoke were not rerun in this fix round.

  • Linux does not compile or run #[cfg(windows)] tests, including ConPTY, DPAPI, run_in_pty and npm_shim, or Windows clippy paths. The Windows lane runs in the merge queue.

  • Browser evidence covers an isolated Chromium fixture, not installed desktop/WebView behavior, animation smoothness or all themes/viewports. No new visual screenshot claim is made in this round; original screenshot/build evidence belongs to the earlier db213559 delivery and was not re-observed here.

  • The contrast script and color tokens were untouched; its previous 704-pair result was not rerun in this round.

  • DS-02 owns the Ruhig/Lebendig/Aus switch and preference consumers. DS-05 owns component migration and removal of the two meter-width layout-animation exceptions.

  • PostCSS remains a transitive dependency; strict installation layouts were not tested. External services and provider limits were not verified.

…ract

Test-First: src/design/motion.test.ts::provides the complete additive duration easing distance and stagger scale

Test-First: src/design/motion.test.ts::owns one rise and fade definition and bounds stagger to eight steps
Test-First: src/design/motion.test.ts::provides the complete additive duration easing distance and stagger scale

Test-First: src/design/motion.test.ts::owns one rise and fade definition and bounds stagger to eight steps
Test-First: src/design/motion.test.ts::disables standalone entry stagger and layout animations under reduced motion
Test-First: src/design/motion.test.ts::rejects logical sizing spacing typography and border width motion
Disable stagger, sheet, view and new-row animations under the OS preference, extend the layout-property guard, and document stagger and future token consumers.

Test-First: src/design/motion.test.ts::disables standalone entry stagger and layout animations under reduced motion
Test-First: src/design/motion.test.ts::rejects logical sizing spacing typography and border width motion
@Cuarroc

Cuarroc commented Oct 15, 2026

Copy link
Copy Markdown
Owner Author

Root 2026-10-15 18:36:21, head e3e10d0: Claude delta APPROVE (A1 resolved via report correction). Gemini @e3e10d05 REQUEST_CHANGES adjudicated: G1 (medium, motion.css:90 '9 delay buckets') REJECTED - :nth-child(n+9) { --i: 8 } gives 8 stagger increments after the first item, which is exactly what the test 'bounds stagger to eight steps' (motion.test.ts:61) pins; cosmetic semantics, no defect. G2 (low, dynamic import with absolute path, motion.test.ts:46) -> follow-up: tests run under vitest/Vite, which resolves absolute paths; CI windows lane will show it. Marking ready; review-ok only after ALL checks incl. CodeQL are green on this head.

@Cuarroc
Cuarroc marked this pull request as ready for review October 15, 2026 16:36
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-15T16:41:50.303616Z e3e10d0 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e3e10d0501

ℹ️ 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".

Comment thread src/design/motion.test.ts
return findings;
}

describe("DS-01 motion scale", () => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add a valid red-first trailer to the source commit

This commit changes production CSS and TypeScript without a Test-First:, Regression-For:, or No-Test: trailer. Running BASE_SHA=3015d841… HEAD_SHA=3b9baeba… bash scripts/ci/red-first.sh --plan exits 1 on this exact head, so the required red-first check will block integration regardless of the test results documented in the commit body. Preserve the named red-test evidence in valid commit trailers.

AGENTS.md reference: AGENTS.md:L18-L20

Useful? React with 👍 / 👎.

Comment thread src/design/motion.test.ts
"inputs/inputs.css|.g-meter b|transition:width var(--g-dur-3, 320ms) var(--g-ease, cubic-bezier(.2, .8, .2, 1))",
"motion.css|.g-meter b|transition:width var(--g-dur-3) var(--g-ease-out)",
];
const layoutProperty = /^(?:(?:min-|max-)?(?:width|height|(?:inline|block)-size)|top|left|right|bottom|inset(?:-.+)?|margin(?:-.+)?|padding(?:-.+)?|grid-template(?:-.+)?|flex-basis|(?:row-|column-)?gap|font-size|line-height|border(?:-.+)?-width)$/;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject the remaining layout-affecting properties

When a design stylesheet adds animation for properties such as flex-grow, grid-auto-columns, order, or the border shorthand, this regex does not classify it as layout motion, so the repository-wide test still passes despite the claim that only the two meter exceptions are allowed. For example, both transition: flex-grow 200ms and transition-property: grid-auto-columns currently produce no finding; expand the classification to cover the other layout-affecting properties.

Useful? React with 👍 / 👎.

Comment thread src/design/motion.test.ts
Comment on lines +24 to +26
const transition = /^(transition|transition-property)$/.test(prop) && list.comma(decl.value).some((part) => {
const property = list.space(part)[0].toLowerCase();
return layoutProperty.test(property) || property === "all" || /^(?:[.\d-]|var\(|ease|linear|steps\(|cubic-bezier\()/i.test(property);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Parse the full transition shorthand before accepting it

When the property is not the first component of the order-independent transition shorthand, only examining list.space(part)[0] lets layout motion bypass the guard. For example, the valid transition: allow-discrete width 200ms returns no finding because the first token is the transition behavior; inspect the parsed shorthand for its property rather than assuming token position.

Useful? React with 👍 / 👎.

@Cuarroc

Cuarroc commented Oct 15, 2026

Copy link
Copy Markdown
Owner Author

Root review gate 2026-10-15 18:48:31, bound to head e3e10d0: Claude delta APPROVE at this head; Gemini G1 rejected, G2 follow-up (see earlier comment); all checks green incl. CodeQL (gh pr checks: no fail, no pending). Setting review-ok.

@Cuarroc Cuarroc added the review-ok Pipeline: review disposition complete, no open high finding label Oct 15, 2026
@Cuarroc

Cuarroc commented Oct 15, 2026

Copy link
Copy Markdown
Owner Author

Root auto-queue: head e3e10d0 unchanged since review-ok, all checks green. @Mergifyio queue

@mergify mergify Bot added the queued label Oct 15, 2026
@mergify
mergify Bot merged commit 2f3625e into main Oct 15, 2026
20 checks passed
@mergify

mergify Bot commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

This pull request spent 8 minutes 59 seconds in the queue, including 8 minutes 16 seconds running CI.

Required conditions to merge
  • check-success = gates (linux)
  • check-success = gates (windows)
  • check-success = red-first
  • github-review-approved [🛡 GitHub repository ruleset rule main-pr-required-checks]
  • label != do-not-merge
  • any of [🛡 GitHub repository ruleset rule main-pr-required-checks]:
    • check-success = @github-actions/gates (linux)
    • check-neutral = @github-actions/gates (linux)
    • check-skipped = @github-actions/gates (linux)
  • any of [🛡 GitHub repository ruleset rule main-pr-required-checks]:
    • check-success = @github-actions/gates (windows)
    • check-neutral = @github-actions/gates (windows)
    • check-skipped = @github-actions/gates (windows)
  • any of [🛡 GitHub repository ruleset rule main-pr-required-checks]:
    • check-success = @github-actions/red-first
    • check-neutral = @github-actions/red-first
    • check-skipped = @github-actions/red-first
  • any of [🛡 GitHub repository ruleset rule main-pr-required-checks]:
    • check-neutral = @mergify/Mergify Merge Protections
    • check-skipped = @mergify/Mergify Merge Protections
    • check-success = @mergify/Mergify Merge Protections

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-ok Pipeline: review disposition complete, no open high finding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant