Establish design system + reduced-motion spinner (P0 a11y) - #4
Merged
Conversation
Establishes the project's design system and lands the first accessibility polish pass flagged by the impeccable critique/audit. Design system (impeccable init): - PRODUCT.md and DESIGN.md capture the product register (product, not brand) and six non-negotiable UI rules for RightClick. - .impeccable/design.json is the machine-readable sidecar: OKLCH canonicals, tonal ramps, component tiles, and the motion tokens that commit us to reduced-motion fallbacks. - AGENTS.md gains a Design Context pointer so future sessions honor the token discipline and the "Operator's Console" north star before touching UI code. - .impeccable/critique/ snapshot records the 33/40 design review and 15/20 technical audit that scoped the work below. Reduced-motion spinner (P0 accessibility finding): - Spinner gains a `reduced_motion: bool` field plus a `with_reduced_motion()` builder. When enabled, `tick()` and `next_frame()` are no-ops (frame frozen at its current index) and `is_running()` returns false, so render loops stop driving animation while still drawing a stable, style-consistent glyph. - UIConfig gains a `reduced_motion: bool` field with `#[serde(default)]` for backward-compatible deserialization of existing configs. Follows the project's mandatory Model -> Review -> Implement -> Verify flow: the transition is a pure boolean, no LLM in the loop, no text-driven state change. Verify: spinner 20/20 (6 new), config 34/34 (2 new), cargo build + fmt clean. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Establishes a committed design system for RightClick (human + machine-readable) and adds a reduced-motion mode to the Spinner component, plus a config flag to carry the preference through configuration.
Changes:
- Added design foundation docs:
PRODUCT.md,DESIGN.md,.impeccable/design.json, and a persisted critique snapshot. - Introduced
Spinner::with_reduced_motion(bool)and areduced_motionflag that freezes frame advancement and reportsis_running() == false. - Added
UIConfig.reduced_motionwith#[serde(default)]and a backward-compat deserialization witness test.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ui/spinner.rs | Adds reduced-motion behavior to the spinner and covers it with unit tests. |
| src/core/models/config.rs | Adds ui.reduced_motion config flag with serde defaults + tests for backward compatibility. |
| PRODUCT.md | New product register and accessibility commitments (incl. reduced motion). |
| DESIGN.md | New design system + UI rules and token references for consistent theming. |
| AGENTS.md | Adds pointers for future agent sessions to follow the design system and token discipline. |
| .impeccable/design.json | Machine-readable design system sidecar (colors, typography, motion tokens). |
| .impeccable/critique/2026-07-28T11-22-39Z__src-ui.md | Captures the critique/audit snapshot motivating the reduced-motion work. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f48db79dd
ℹ️ 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".
Address 5 review comments on PR #4. No behavioral regression; pure docs, token-consistency, and a convenience constructor. - config.rs: reword UIConfig.reduced_motion doc to drop the misleading "progress animations" claim (progress.rs renders a static bar). Keep the PRODUCT.md / DESIGN.md references. Plain ASCII punctuation. (comment 3668817519) - spinner.rs: update is_running() doc to state it returns true only when multi-frame AND reduced-motion is disabled (it reports active animation, not just frame count). No behavior change. (comment 3668817567) - spinner.rs: add Spinner::from_ui_config(&UIConfig) convenience constructor so future live surfaces honor the reduced_motion preference automatically, without manually threading the builder. Investigation confirmed zero live Spinner construction sites today (library-only), so no consumer is wired in this PR; the constructor encodes the intended pattern for adopters. +1 test covering the config path (default animates, reduced_motion=true freezes). (comment 3668824555, P1) - DESIGN.md: the Buttons section referenced an undefined {colors.button-hover-bg}. Replaced with the existing {colors.secondary} token (already defined as the rare hover accent), honoring the One Console Rule and the restrained palette. Verified all 17 {colors.*} references now resolve to a defined token. (comment 3668817607) - .impeccable/critique snapshot: corrected inaccurate WCAG contrast figures (muted was wrongly marked ~4.5:1 pass; actual 2.76:1 FAIL). Added an ERRATUM block dated 2026-07-29 and corrected inline numbers for muted (2.76:1), placeholder (2.35:1), and border (1.91:1) on Default/Tokyo Night, with reviewer-cited Dracula/Nord values flagged for formal verification. Health score unchanged (33/40). (comment 3668824566, P2) Verify: cargo build clean; spinner 21/21 (+1) + doctests 21/21; config 35/35 (+1); cargo fmt --check clean; cargo clippy clean. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
RightClick had no committed design system, so UI decisions were being made ad hoc and the accessibility baseline was unverified. This PR lands the design foundation (impeccable
init) and the first polish pass surfaced by the subsequentcritique+audit: a reduced-motion spinner — the P0 accessibility finding both evaluations converged on.What changed
Design system (impeccable init)
PRODUCT.md+DESIGN.mdcapture the product register (a tool, not a brand site) and six non-negotiable UI rules for RightClick..impeccable/design.jsonis the machine-readable sidecar: OKLCH canonicals, tonal ramps, component tiles, and motion tokens that commit us to reduced-motion fallbacks.AGENTS.mdgains a Design Context pointer so future sessions honor token discipline and the "Operator's Console" north star before touching UI code..impeccable/critique/2026-07-28T11-22-39Z__src-ui.mdpersists the review snapshot (design 33/40, technical audit 15/20) that scoped the work below.Reduced-motion spinner (P0 a11y)
Spinnergains areduced_motion: boolfield +with_reduced_motion()builder. When enabled,tick()andnext_frame()become no-ops (frame frozen at its current index) andis_running()returnsfalse, so render loops stop driving animation while still drawing a stable, style-consistent glyph.UIConfiggainsreduced_motion: boolwith#[serde(default)]so existing config files deserialize unchanged.Approach & non-obvious bits
⠋fordots) rather than swapping in an unrelated icon, so the visual identity is preserved.false.Spinneris currently a library component (exported, not yet instantiated in the live app), so this change is self-contained; wiring it into a live surface is deferred.Verify
cargo test --lib spinner→ 20/20 (6 new)cargo test --lib config→ 34/34 (2 new, incl. backward-compat witness)cargo build+cargo fmt --checkcleanOut of scope (flagged by the audit, each needs a design-identity decision)
#565f89is 2.76:1 (WCAG FAIL). Verified correct fix is#868fba(5.41:1, AA) — but it touches the canonical palette across all themes.#414868is 1.91:1 (FAIL, 3:1 non-text).Color::from_str().unwrap_or()appears at 50+ call sites — fragile, worth centralizing.