|
1 | 1 | --- |
2 | 2 | name: emcn-design-review |
3 | | -description: Review UI code for alignment with the emcn design system — components, tokens, patterns, and conventions |
| 3 | +description: Review product UI changes for design drift using the local conformance check, EMCN components, and global styles. |
4 | 4 | argument-hint: "[scope] [fix=true|false]" |
5 | 5 | --- |
6 | 6 |
|
7 | | -# EMCN Design Review |
| 7 | +# EMCN design review |
8 | 8 |
|
9 | | -Arguments: |
10 | | -- scope: what to review (default: your current changes). Examples: "diff to main", "PR #123", "src/components/", "whole codebase" |
11 | | -- fix: whether to apply fixes (default: true). Set to false to only propose changes. |
| 9 | +Review the requested product UI scope (default: current changes). When `fix=false`, explain proposed changes without applying them. |
12 | 10 |
|
13 | | -User arguments: $ARGUMENTS |
| 11 | +1. During UI work, run `bun run check:design --base origin/staging --working-tree` from the repo root, substituting the actual PR target for `origin/staging`. After committing, use `--head HEAD` for the immutable PR comparison. Exit 1 means findings to review; exit 2 means the check failed and must be repaired or reported. CI is warning-only for findings and fails on incomplete analysis. |
| 12 | +2. For each new finding or relevant review signal, inspect the cited source, the applicable public EMCN export in `packages/emcn/src/index.ts`, and tokens and recipes in `apps/sim/app/_styles/globals.css`. Reuse a suitable component, prop, variant, or global token when it expresses the design intent. Avoid near-duplicate local colours or overriding EMCN chrome merely for convenience. |
| 13 | +3. A genuinely new product treatment may remain an Extra. Explain its visual intent and why existing EMCN or global styling does not fit in the PR. The check does not decide design approval and must not be silenced by adding an arbitrary token, broad exclusion, or fake component wrapper. Ask the designer or engineer when changing a shared recipe would have broad or ambiguous effects. |
| 14 | +4. Keep unresolved `unchecked` inputs and inspection failures distinct from proven violations. A quiet diff means no *new detected* debt, not proof of complete visual conformance. Existing debt stays quiet; a new copy can warn. Landing and docs are out of product scope; Monaco presentation, provider branding, and block identity palettes have deliberate exclusions. See `scripts/design-conformance/README.md` for exact rule boundaries. |
14 | 15 |
|
15 | | -## Context |
16 | | - |
17 | | -This codebase uses **emcn**, a custom component library built on Radix UI primitives with CVA variants and CSS variable design tokens. All UI must use emcn components and tokens. |
18 | | - |
19 | | -## Deterministic conformance and fixes |
20 | | - |
21 | | -For committed product UI changes, run `bun run check:design --base <actual-target-branch> --head HEAD`. This checks central derivation, component ownership and system changes; it does not judge whether an approved red variant or monospace font is a good product choice. The style review advice below may inform suggestions, but must not be presented as additional deterministic linter rules. |
22 | | - |
23 | | -With fixes enabled, correct straightforward local violations using the cited central token, recipe or component prop. For a small gray adjustment, choose the approved token that fits its role (body, muted, icon or surface); do not hardcode the nearest hex. Ask the engineer when a correction has broad impact or ambiguous intent, especially before changing shared definitions or adding variants. An intentional change or justified artwork exception can proceed with an explanation in the PR; retain the warning. New shared standards should involve the designer. Do not enlarge this task into a whole-codebase cleanup or invent a central token just to pass. |
24 | | - |
25 | | -Untouched debt stays quiet. Bringing noncompliant styling into a new file or component is an opportunity to fix it, even when its value is unchanged. Product artwork should use the registered central libraries; central asset changes notify. Landing pages, user media and sanctioned provider branding are separate scope. Unknown HTML illustrations may still receive normal styling warnings. Unresolved inputs are not proof of conformance. Only committed blobs are checked, and exit 2 means an operational failure; CI findings currently warn without blocking. See `scripts/design-conformance/README.md` for exact coverage. |
26 | | - |
27 | | -## Steps |
28 | | - |
29 | | -1. Read the emcn public barrel at `packages/emcn/src/index.ts` (re-exports components, Calendar, Table*, and icons) to know what's available; for the full icon set read `packages/emcn/src/icons/index.ts` |
30 | | -2. Read `apps/sim/app/_styles/globals.css` for CSS variable tokens |
31 | | -3. Analyze the specified scope against every rule below |
32 | | -4. If fix=true, apply the fixes. If fix=false, propose the fixes without applying. |
33 | | - |
34 | | ---- |
35 | | - |
36 | | -## Imports |
37 | | - |
38 | | -- Components, `cn`, and tokens from the `@sim/emcn` barrel, never component subpaths |
39 | | -- Icons from `@sim/emcn/icons` |
40 | | - |
41 | | -## Design Tokens |
42 | | - |
43 | | -Use CSS variable pattern (`text-[var(--text-primary)]`), never Tailwind semantics (`text-muted-foreground`) or hardcoded colors (`text-gray-500`, `#333`). |
44 | | - |
45 | | -**Text**: `--text-primary`, `--text-secondary`, `--text-tertiary`, `--text-muted`, `--text-body` (canonical value text), `--text-icon`, `--text-placeholder`, `--text-subtle`, `--text-inverse`, `--text-error` |
46 | | -**Surfaces**: `--bg`, `--surface-1` through `--surface-7`, `--surface-hover`, `--surface-active` |
47 | | -**Borders**: `--border` (`--border-1`/`--border-muted` are legacy aliases resolving to it — flag new uses) |
48 | | -**Brand/accent**: `--brand-secondary`, `--brand-accent` |
49 | | -**Z-Index**: `--z-dropdown` (100), `--z-toast` (150), `--z-modal` (200), `--z-popover` (300), `--z-tooltip` (400), `--z-takeover` (500), `--z-shell-gate` (600) |
50 | | -**Shadows**: `shadow-subtle`, `shadow-medium`, `shadow-overlay`, `shadow-card` |
51 | | -**Badges**: `--badge-*` semantic families (success/error/gray/blue/purple/orange/amber/teal/cyan/pink, each with `-bg`/`-text`) |
52 | | - |
53 | | -## Buttons |
54 | | - |
55 | | -Intent-to-variant mapping (read the actual `buttonVariants` in `packages/emcn/src/components/button/button.tsx` for the full variant set — it exposes more than listed here): |
56 | | - |
57 | | -| Action | Variant | |
58 | | -|--------|---------| |
59 | | -| Toolbar, icon-only | `ghost` | |
60 | | -| Create, save, submit | `primary` | |
61 | | -| Cancel, close | `default` | |
62 | | -| Delete, remove | `destructive` | |
63 | | -| Selected state | `active` | |
64 | | -| Toggle | `outline` | |
65 | | - |
66 | | -## Delete/Remove Confirmations |
67 | | - |
68 | | -`ChipModal` `size='sm'`, title "Delete/Remove {ItemType}", destructive confirm button, plain Cancel (follow the chip footer layout in `.claude/rules/emcn-components.md`). Use `text-[var(--text-error)]` for irreversible warnings. |
69 | | - |
70 | | -## Toast |
71 | | - |
72 | | -`toast.success()`, `toast.error()`, `toast()` from `@sim/emcn`. Never custom notification UI. |
73 | | - |
74 | | -## Badges |
75 | | - |
76 | | -`red`=error/failed, `gray-secondary`=metadata/roles, `type`=type annotations, `green`=success/active, `gray`=neutral, `amber`=processing, `orange`=paused, `blue`=info. Use `dot` prop for status indicators. |
77 | | - |
78 | | -## Icons |
79 | | - |
80 | | -Default: `size-[14px]`. Color: `text-[var(--text-icon)]`. Scale: 14px > 16px > 12px > 20px. Use the `size-*` shorthand — flag `h-[Npx] w-[Npx]` and `h-N w-N` pairs as refactor targets. |
81 | | - |
82 | | -## Anti-patterns to flag |
83 | | - |
84 | | -- Raw `<button>`/`<input>`, or legacy `Input`/`Textarea`/`Modal`, instead of the canonical chip components (`ChipInput`/`ChipTextarea`/`ChipModal`) |
85 | | -- Hand-rolled field rows inside a `ChipModalBody` instead of `ChipModalField` |
86 | | -- Hardcoded colors (`text-gray-*`, `#hex`, `rgb()`) |
87 | | -- Tailwind semantics (`text-muted-foreground`) instead of CSS variables |
88 | | -- Template literal className instead of `cn()` |
89 | | -- Inline styles for colors/static values (dynamic values OK) |
90 | | -- Importing from emcn subpaths instead of barrel |
91 | | -- Arbitrary z-index instead of tokens |
92 | | -- Wrong button variant for action type |
| 16 | +Do not turn this review into an unrelated whole-codebase cleanup. Preserve intended appearance when migrating product UI and use before/after screenshots when a treatment changes. |
0 commit comments