Files
marketplaces/docs/UI-DESIGN-REVIEW.md

193 lines
10 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 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: <literal>;``var(--font-size-STEP, <same-or-nearest-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: <literal>;` → 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: <literal>;` → 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 59 (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 56 since the primitives are already unified.