diff --git a/docs/UI-DESIGN-REVIEW.md b/docs/UI-DESIGN-REVIEW.md new file mode 100644 index 0000000..ce5f6a4 --- /dev/null +++ b/docs/UI-DESIGN-REVIEW.md @@ -0,0 +1,192 @@ +# UI Design System Finalization — RC Final Polish + +Scope: token infrastructure + mechanical, value-preserving normalization of +typography/radius/spacing across storefront, builder, and backoffice. +Architecture, routing, facades, providers, and business logic were left +untouched, per instructions. This pass builds on the icon standardization +work (see `docs/UI-ICON-AUDIT.md`) and the earlier storefront/admin polish +passes (`docs/STORE_FRONT_REVIEW.md`, `docs/ADMIN_REVIEW.md`) — it does not +redo that work. + +No `docs/DESIGN.md` exists in this repo (confirmed in `docs/STORE_FRONT_REVIEW.md`); +`impeccable` hook findings referencing it were treated as inapplicable noise +throughout, same precedent as the prior pass. + +## Before + +- `src/styles.scss` had a color/shadow/radius/spacing token set + (`--primary-color`, `--radius-sm..xl`, `--space-xs..xl`, `--shadow-sm..lg`) + but **no typography scale at all** — no font-size, font-weight, or + line-height tokens. +- Font sizes were hand-written per component. A repo-wide scan found **24 + distinct font-size values in simultaneous use** for what was clearly the + same handful of intended sizes: `0.7/0.72/0.75/0.78/0.8/0.8125/0.85/0.875/ + 0.9/0.9375/0.95/1/1.05/1.1/1.125/1.15/1.2/1.25/1.3/1.35/1.4/1.5/1.75/2rem` + (plus a handful of raw px values), spread across 104 of 136 component + SCSS files. +- Font-weight was always a raw literal (400/500/600/700), never a token. +- `border-radius` had the same drift: 72 of 136 files used raw px values + instead of the existing `--radius-*` tokens; two "shape" values in + constant use — `4px` (small chips/badges) and `999px` (pills/toggles) — + had no token at all, so every pill/chip component reinvented `999px` by + hand (41 occurrences) or `4px` (9 occurrences). +- `src/app/shared/ui/*` — the actual shared component library used across + admin/builder/backoffice (button, input, select, badge, card, table, + dialog, etc.) — was already internally consistent (button and input both + used a matching 32/40/48px sm/md/lg scale with 0.8125/0.9375/1rem font + sizes) but expressed every value as a hand-written literal rather than a + token, and one file (`section-card.component.scss`) had drifted from its + sibling `card.component.scss` (16px/18px/14px raw vs. `--radius-lg`/ + `--space-*`). +- `code-editor.component.scss`'s focus state used a hardcoded `#497671` + (the dexar tenant's primary color) instead of `var(--primary-color)` — + it would not have adapted if a tenant switched to the lavero or novo + theme. + +## After + +### Tokens introduced (`src/styles.scss`) + +``` +--font-size-xs: 0.75rem (12px) +--font-size-sm: 0.8125rem (13px) +--font-size-base: 0.875rem (14px) +--font-size-md: 0.9375rem (15px) +--font-size-lg: 1rem (16px) +--font-size-xl: 1.125rem (18px) +--font-size-2xl: 1.25rem (20px) +--font-size-3xl: 1.5rem (24px) +--font-size-4xl: 2rem (32px) + +--font-weight-normal: 400 +--font-weight-medium: 500 +--font-weight-semibold: 600 +--font-weight-bold: 700 + +--line-height-tight: 1.2 +--line-height-normal: 1.5 +--line-height-relaxed: 1.6 + +--space-2xl: 48px (new, extends the existing xs/sm/md/lg/xl scale) +--space-3xl: 64px (new) +``` + +### Tokens added to all three theme files (`src/styles/themes/*.theme.scss`) + +``` +--radius-xs: 4px (dexar/lavero/novo — chip/badge corner radius) +--radius-full: 999px (dexar/lavero/novo — pill/toggle shape) +``` + +The scale steps were chosen to match sizes already in wide use (button/input +sm|md|lg already used 0.8125/0.9375/1rem) so most components could adopt the +tokens by direct substitution rather than a visual redesign. + +### Files touched, by phase + +**Global base (`src/styles.scss`)** — `body`, `h1`–`h6`, `p`, `small`, the +`.btn`/`.mt-*`/`.mb-*`/`.p-*` utility classes, and the shared +`.item-badge`/`.item-tag`/`.item-simple-desc` classes now reference the new +tokens instead of literals. + +**Shared component library (18 files, hand-reviewed, not mechanical)** — +`src/app/shared/ui/{button,input,select,badge,card,section-card,table,dialog, +empty-state,form-field,pagination,toggle,image-field,key-value-editor, +locale-tabs,color-picker,code-editor,skeleton}/*.component.scss`. Fixes made +along the way: +- `code-editor.component.scss`: hardcoded `#497671` focus color → + `var(--primary-color)` — now adapts correctly across tenant themes. +- `section-card.component.scss`: raw `16px`/`18px`/`14px` radius/padding → + `var(--radius-lg)`/`var(--space-*)`, matching the sibling `card` component. +- `toggle`/`badge`/`item-tag`: raw `999px` → the new `--radius-full` token. + +**App-wide mechanical rollout (89 files touched across storefront, builder, +and backoffice)** — three scripted, value-preserving sweeps: +1. `font-size: ;` → `var(--font-size-STEP, );` + for every literal that matched (or was within ~0.03rem/1px of) one of the + 9 scale steps. Values within that tolerance were snapped to the nearest + step to consolidate near-duplicates (e.g. `0.85rem` and `0.8rem` both → + `--font-size-sm`/`0.8125rem`). +2. `border-radius: ;` → token, but **only** for the 5 values that + exactly match a token everywhere (`4px`, `8px`, `12px`, `13px`, `999px`). + `20px`/`16px`/`10px`/`6px`/`3px`/`2px`/`14px`/`24px`/`19px` were + deliberately left alone (see Remaining issues). +3. `font-weight: ;` → token (400/500/600/700), value-preserving. + +Each sweep was applied with `sed`, spot-checked against `git diff` for +correctness, then verified with a full `ng build --configuration=production` +(exit 0) before committing. Five commits total, one per phase/sweep, each +independently buildable and revertable. + +## Remaining issues (deferred, not fixed in this pass) + +- **Spacing (margin/padding) was not mechanically swept.** The existing + `--space-xs..3xl` scale covers gaps and simple paddings, but + margin/padding shorthand comes in 1/2/3/4-value forms that aren't safe to + regex-replace without risking silently changing box models on components + that use asymmetric shorthand. This needs a manual, component-by-component + pass — recommended as the next follow-up, using the same `--space-*` + scale already established. +- **`border-radius` values with no exact token match** (`20px` × 9, + `16px` × 7, `10px` × 38, `6px` × 9, `3px` × 4, `2px` × 3, `14px`, `24px`, + `19px`) were intentionally left as literals. Several of these (10px, 6px) + are common enough that a `--radius-2xs`/intermediate step might be + warranted, but adding one now without a design call would be inventing a + new shape rather than normalizing existing ones — flagged for the design + owner rather than decided unilaterally. +- **Phases 5–9 (cards, tables, status system, animations, accessibility)** + were not addressed as dedicated passes in this session. The shared/ui + `card`, `table`, `dialog`, `badge`, and `skeleton` components are already + reasonably unified (see "After" above) since they're single shared + components consumed everywhere, but a full per-surface audit (storefront + product cards, admin dashboard cards, builder panel cards; every table's + sticky-header/hover/selection/density behavior; a single status-badge + language for success/warning/error/draft/published/archived/pending) was + out of reach in this pass and should be scoped as its own follow-up. +- **Status system**: no unified success/warning/error/draft/published/etc. + visual language audit was performed. The `app-badge` component has + `--neutral/primary/success/warning/danger/info` variants already, but + whether every surface (storefront order status, admin order/review + status, builder publish state) actually uses `app-badge` rather than a + hand-rolled span was not verified in this pass. +- **Animations**: not audited in this pass beyond what already existed + (the codebase already has a global `prefers-reduced-motion` neutralizer + in `src/styles.scss` and per-component reduced-motion overrides in most + shared/ui components — this predates this session). +- **Accessibility**: not audited in this pass; the prior `STORE_FRONT_REVIEW.md` + and `UI-ICON-AUDIT.md` passes already covered a a11y pass on the storefront + and icon-only buttons respectively. No new a11y work was done here. +- **`cart.component.scss`** now sits ~771 bytes over its `anyComponentStyle` + budget warning (40kB) due to the added `var(...)` string length from the + font-weight sweep. This is a `maximumWarning`, not `maximumError` + (`angular.json` budgets: initial 700kB warn / 1.5MB error, component + style 40kB warn / 50kB error), so the build still exits 0 — but it's + worth either trimming the file or bumping its allotment slightly in a + follow-up. +- **Tenant brand colors** (`src/styles/themes/{dexar,lavero,novo}.theme.scss`) + and **badge accent colors** (`.item-badge.badge-new/sale/exclusive/...` and + code-editor syntax-highlight colors `.cm-*`) were intentionally left + untouched — these are per-tenant palettes / syntax-token colors, not + design-system drift. + +## Recommendations + +1. **Next pass: spacing.** Do a manual, per-surface sweep of + margin/padding using the existing `--space-xs..3xl` scale, prioritizing + the same high-traffic files already touched here (shared/ui, then + catalog/product-details/cart on storefront, then admin dashboard/list + pages). +2. **Decide on the missing radius steps** (`10px`, `6px`) with the design + owner — either add `--radius-2xs`/an intermediate token, or confirm they + should collapse into `--radius-xs`/`--radius-sm` and re-run the same + mechanical sweep pattern used in this session (`sed` + `ng build` + + commit per phase). +3. **Status system audit**: grep for hand-rolled status spans/badges + outside `app-badge` across admin (orders, reviews, moderation) and + storefront (order tracking) and consolidate onto the existing + `app-badge` variants. +4. **Cards/tables surface audit**: now that `card`/`section-card`/`table` + are internally token-consistent, verify every page-level "card" and + "table" actually composes these shared components rather than + hand-rolling similar-looking markup — this is the highest-leverage next + step for phases 5–6 since the primitives are already unified.