Design system foundation — Tailwind 4 theme, CSS tokens, fonts #31

marcel · 2026-04-02T12:46:42+02:00

marcel commented

2026-04-02 12:46:42 +02:00

Summary

Add Tailwind 4 @theme block with all design tokens (neutrals, green/yellow/blue/purple/orange scales, spacing, radii, shadows, button base)
Load Fraunces, DM Sans, DM Mono via Google Fonts preconnect in app.html
Add WCAG 2.2 AA contrast tests and token completeness tests (39 tests total)

WCAG finding

--green (#3D8C4A) only gives 4.16:1 contrast with white — fails AA for normal text. Button backgrounds use --green-dark (#2E6E39) which gives 6.2:1. --green stays in the palette for non-text uses. Documented in token comment and contrast test.

Test plan

npm run test — 39 tests pass
npm run check — 0 type errors

Closes #16

## Summary - Add Tailwind 4 `@theme` block with all design tokens (neutrals, green/yellow/blue/purple/orange scales, spacing, radii, shadows, button base) - Load Fraunces, DM Sans, DM Mono via Google Fonts preconnect in `app.html` - Add WCAG 2.2 AA contrast tests and token completeness tests (39 tests total) ## WCAG finding `--green` (#3D8C4A) only gives 4.16:1 contrast with white — fails AA for normal text. **Button backgrounds use `--green-dark` (#2E6E39)** which gives 6.2:1. `--green` stays in the palette for non-text uses. Documented in token comment and contrast test. ## Test plan - [ ] `npm run test` — 39 tests pass - [ ] `npm run check` — 0 type errors Closes #16

marcel added 4 commits 2026-04-02 12:46:42 +02:00

Add frontend journey specs with visual previews and LLM instructions b36d4c731d

Six self-contained HTML documents, one per user journey, each combining
mobile/desktop previews with machine-readable implementation guides:

- j1-add-recipe.html (B1, B3)
- j2-plan-the-week.html (C1, C2, C3)
- j3-cook-tonight.html (B2, B4)
- j4-adapt-on-the-fly.html (swap trigger, C2 swap)
- j5-shopping-list.html (D1)
- j6-household-setup.html (A1, A2, A3/D3, A4)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Wire frontend into Docker Compose with type-safe API client 82815205d0

- Add frontend service to docker-compose.yml (port 3000, BACKEND_URL env var)
- Add frontend/Dockerfile using adapter-node for plain Node/Docker runtime
- Switch svelte.config.js from adapter-auto to adapter-node
- Generate OpenAPI types from backend spec (openapi-typescript + openapi-fetch)
- Add src/lib/server/api.ts as server-only typed API client factory
- Add generate:api script to regenerate types when backend spec changes

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

test(design-system): assert WCAG 2.2 AA contrast for key color pairs 7c8d725fce

White on --green-dark (not --green) is the correct button background;
--green (#3D8C4A) gives only 4.16:1 which fails AA for normal-size text.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

feat(design-system): add Tailwind 4 @theme tokens, fonts, and completeness tests 0a2ef750c4

- Load Fraunces, DM Sans, DM Mono via Google Fonts preconnect in app.html
- Define all design tokens in @theme block: neutrals, green/yellow/blue/
  purple/orange scales, spacing (--space-1..20), radii, shadows, button base
- Note --green-dark as button background (--green fails WCAG AA with white)
- Add @types/node for Node fs/path usage in design-system tests

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marcel commented

2026-04-02 12:47:44 +02:00

Review: Kai (Frontend Engineer) — Developer Perspective

Verdict: ✅ Approved

What's good

The CSS-first Tailwind 4 @theme block is exactly the right approach — no tailwind.config.js means one source of truth, and the token names are clean and predictable. The font stack (Fraunes / DM Sans / DM Mono) is a solid, coherent choice.

The --green-dark decision is well-documented right in the comment on the token itself (#3D8C4A gives 4.16:1, fails AA). That's the kind of reasoning that saves future maintainers from reverting to the wrong color.

The button base tokens (--btn-font-size, --btn-font-weight, --btn-letter-spacing) are a smart addition — they give us a single place to update button typography without hunting through component files.

The spacing scale (4px half-step, 8px base) covers --space-1 through --space-20, which is more than enough headroom. Good that it starts at 4px rather than a 1px-based scale; it maps cleanly to our 8px grid.

Blockers

None.

Suggestions

Blue and purple scales are thinner than green/yellow. Blue has --blue-tint / --blue-light / --blue / --blue-dark (4 stops), purple has only --purple-tint / --purple / --purple-dark (3 stops, no --purple-light). If these scales are going to be used for info/warning badges and tag chips, the missing mid-step could force ad-hoc color mixing in components. Consider completing them now or leaving an explicit /* not needed yet */ comment so the asymmetry looks intentional.
--btn-* tokens are defined but no button component exists yet. That's fine for a foundation PR, but these tokens are "dangling" until a Button.svelte consumes them. A small follow-up issue to track this would keep the work visible.
No dark mode layer. Not a blocker for an MVP, but if dark mode lands later, the flat @theme block will need to be wrapped in @media (prefers-color-scheme: dark) or a .dark class override. Worth a TODO comment now so we don't forget the token structure was designed without dark mode in mind.
app.html — the text-scale meta tag looks non-standard. <meta name="text-scale" content="scale"> is not a recognised browser meta tag. If this is a SvelteKit placeholder that gets replaced at runtime, it should be documented. If it was added by mistake, it should be removed — unknown meta tags are harmless but noisy.

Overall: clean, well-reasoned foundation. Happy to see the WCAG decision baked into both the token file and the test suite. Approve.

## Review: Kai (Frontend Engineer) — Developer Perspective **Verdict: ✅ Approved** --- ### What's good The CSS-first Tailwind 4 `@theme` block is exactly the right approach — no `tailwind.config.js` means one source of truth, and the token names are clean and predictable. The font stack (`Fraunes` / `DM Sans` / `DM Mono`) is a solid, coherent choice. The `--green-dark` decision is well-documented right in the comment on the token itself (`#3D8C4A gives 4.16:1, fails AA`). That's the kind of reasoning that saves future maintainers from reverting to the wrong color. The button base tokens (`--btn-font-size`, `--btn-font-weight`, `--btn-letter-spacing`) are a smart addition — they give us a single place to update button typography without hunting through component files. The spacing scale (4px half-step, 8px base) covers `--space-1` through `--space-20`, which is more than enough headroom. Good that it starts at 4px rather than a 1px-based scale; it maps cleanly to our 8px grid. --- ### Blockers None. --- ### Suggestions 1. **Blue and purple scales are thinner than green/yellow.** Blue has `--blue-tint / --blue-light / --blue / --blue-dark` (4 stops), purple has only `--purple-tint / --purple / --purple-dark` (3 stops, no `--purple-light`). If these scales are going to be used for info/warning badges and tag chips, the missing mid-step could force ad-hoc color mixing in components. Consider completing them now or leaving an explicit `/* not needed yet */` comment so the asymmetry looks intentional. 2. **`--btn-*` tokens are defined but no button component exists yet.** That's fine for a foundation PR, but these tokens are "dangling" until a `Button.svelte` consumes them. A small follow-up issue to track this would keep the work visible. 3. **No dark mode layer.** Not a blocker for an MVP, but if dark mode lands later, the flat `@theme` block will need to be wrapped in `@media (prefers-color-scheme: dark)` or a `.dark` class override. Worth a TODO comment now so we don't forget the token structure was designed without dark mode in mind. 4. **`app.html` — the `text-scale` meta tag looks non-standard.** `<meta name="text-scale" content="scale">` is not a recognised browser meta tag. If this is a SvelteKit placeholder that gets replaced at runtime, it should be documented. If it was added by mistake, it should be removed — unknown meta tags are harmless but noisy. --- Overall: clean, well-reasoned foundation. Happy to see the WCAG decision baked into both the token file and the test suite. Approve.

marcel commented

2026-04-02 12:48:12 +02:00

Review: Architect — Structure & Coupling Perspective

Verdict: ✅ Approved

What's good

The architectural decision to use Tailwind 4's @theme block as the single authoritative source of design tokens is sound. There is exactly one place to update a color, spacing value, or radius — no divergence between a JS config and a CSS file. This is the correct CSS-first pattern for Tailwind 4 and sets a scalable foundation.

Placing the tests in frontend/src/lib/design-system/ establishes a clear module boundary. The design system is being treated as a first-class subsystem, not just loose CSS. That's the right instinct.

The tokens.test.ts approach — reading app.css from disk and asserting token presence — is a pragmatic contract test. It means any future refactor that accidentally removes or renames a token will cause an immediate test failure. This is a lightweight but effective architectural guard.

Blockers

None.

Suggestions

The test suite reads app.css via __dirname with node:fs. This works, but it creates a subtle coupling: the test is tied to the relative path ../../app.css from its current location. If the file is ever moved (e.g., during a SvelteKit directory restructure), the test silently breaks on the filesystem read rather than the assertion. Consider asserting that the css string is non-empty at the start of the describe block as a guard against silent file-not-found misses — though readFileSync throws on missing files, so this is low risk. A more robust alternative would be to import the raw CSS string via Vite's ?raw suffix, which would be resolved at build time and fail fast at compilation rather than at runtime. However, since node:fs tests run in Vitest's Node environment (not a browser), this is architecturally acceptable.
No barrel file or public API for the design-system module. Right now the module is just tests — no exported utilities. When contrast-check helpers or token maps are needed by components, consider whether $lib/design-system/index.ts should export them, or whether the tokens stay CSS-only and no JS export is ever needed. Making that decision explicit in a comment or README would prevent gradual accumulation of ad-hoc utility files.
Orange scale has only 3 stops; blue has 4; green has 5. The inconsistency in scale depth across color families could lead to component authors reaching for hex literals when a token doesn't cover their shade. Documenting the intended depth per scale would clarify intent.
@types/node in tsconfig.json types array applies globally to the frontend project, not just to test files. This means Node globals (process, Buffer, __dirname) will resolve in Svelte component files, where they should not be used in browser contexts. A more targeted approach would restrict @types/node to a separate tsconfig.test.json that Vitest uses, leaving the main tsconfig browser-only. This is a minor coupling concern, not a blocker.

Solid, well-structured foundation PR. The separation of concerns is appropriate for the project size.

## Review: Architect — Structure & Coupling Perspective **Verdict: ✅ Approved** --- ### What's good The architectural decision to use Tailwind 4's `@theme` block as the single authoritative source of design tokens is sound. There is exactly one place to update a color, spacing value, or radius — no divergence between a JS config and a CSS file. This is the correct CSS-first pattern for Tailwind 4 and sets a scalable foundation. Placing the tests in `frontend/src/lib/design-system/` establishes a clear module boundary. The design system is being treated as a first-class subsystem, not just loose CSS. That's the right instinct. The `tokens.test.ts` approach — reading `app.css` from disk and asserting token presence — is a pragmatic contract test. It means any future refactor that accidentally removes or renames a token will cause an immediate test failure. This is a lightweight but effective architectural guard. --- ### Blockers None. --- ### Suggestions 1. **The test suite reads `app.css` via `__dirname` with `node:fs`.** This works, but it creates a subtle coupling: the test is tied to the relative path `../../app.css` from its current location. If the file is ever moved (e.g., during a SvelteKit directory restructure), the test silently breaks on the filesystem read rather than the assertion. Consider asserting that the `css` string is non-empty at the start of the describe block as a guard against silent file-not-found misses — though `readFileSync` throws on missing files, so this is low risk. A more robust alternative would be to import the raw CSS string via Vite's `?raw` suffix, which would be resolved at build time and fail fast at compilation rather than at runtime. However, since `node:fs` tests run in Vitest's Node environment (not a browser), this is architecturally acceptable. 2. **No barrel file or public API for the design-system module.** Right now the module is just tests — no exported utilities. When contrast-check helpers or token maps are needed by components, consider whether `$lib/design-system/index.ts` should export them, or whether the tokens stay CSS-only and no JS export is ever needed. Making that decision explicit in a comment or README would prevent gradual accumulation of ad-hoc utility files. 3. **Orange scale has only 3 stops; blue has 4; green has 5.** The inconsistency in scale depth across color families could lead to component authors reaching for hex literals when a token doesn't cover their shade. Documenting the intended depth per scale would clarify intent. 4. **`@types/node` in `tsconfig.json` types array** applies globally to the frontend project, not just to test files. This means Node globals (`process`, `Buffer`, `__dirname`) will resolve in Svelte component files, where they should not be used in browser contexts. A more targeted approach would restrict `@types/node` to a separate `tsconfig.test.json` that Vitest uses, leaving the main tsconfig browser-only. This is a minor coupling concern, not a blocker. --- Solid, well-structured foundation PR. The separation of concerns is appropriate for the project size.

marcel commented

2026-04-02 12:48:42 +02:00

Review: QA Engineer — Test Quality Perspective

Verdict: ✅ Approved

What's good

This PR ships tests alongside the code — that's the baseline I always ask for, and it's satisfied. Two test files, clear scope, real assertions. Good.

contrast.test.ts is especially well done:

The WCAG 2.2 luminance formula is implemented in-file rather than hidden behind a dependency. This means the test is self-documenting — a future maintainer can read exactly what WCAG AA means in math, not just what a library tells them.
The comment --green (#3D8C4A) only gives 4.16:1 — buttons use --green-dark (#2E6E39) instead is a failure-mode explanation living next to the test that encodes the decision. That's exactly what I want from test documentation.
4 test cases covering the two most important text/background pairings and the primary button. Coverage is appropriate for the scope.

tokens.test.ts is a clean contract test:

it.each(requiredTokens)('%s is defined in app.css') gives one named test case per token. If a token disappears, the failure message names the exact missing token. Good failure ergonomics.
The file read is straightforward and the test is deterministic.

Blockers

None.

Suggestions

contrast.test.ts does not test --color-text-muted on --color-surface (card muted labels). It tests muted text on page background, and main text on surface, but not the cross combination. If muted text appears inside cards (very likely for recipe metadata), that pairing should also be verified. Easy to add a fifth it() case.
tokens.test.ts does not verify token values, only token names. For example, --green-dark: ; (empty value) would pass. This is intentional for a presence check, and probably the right tradeoff — value validation would be brittle. But consider adding one spot-check for a critical value: e.g., assert that --green-dark contains #2e6e39 (case-insensitive), since that specific value was chosen to satisfy WCAG AA and an accidental edit there is a silent regression.
No test for the --color-error contrast ratio. The error token (#dc4c3e) will be used for validation messages and destructive action indicators. Its contrast on --color-page and --color-surface should be verified in contrast.test.ts. A red that fails contrast is a common accessibility mistake.
tokens.test.ts checks a subset of spacing tokens (--space-1, --space-4, --space-8, --space-12, --space-16, --space-20) but not the full range through --space-20. This is a reasonable sampling strategy — worth a comment like // spot-check key steps, full range defined in app.css to make the intent explicit.
Missing E2E coverage — not expected at this stage, but note that fonts loaded via Google Fonts CDN cannot be verified in unit tests. A visual regression or E2E screenshot test that checks actual font rendering would catch CDN failures or wrong font names. Low priority now, worth tracking.

Strong test coverage for a foundation PR. The contrast tests in particular set a good precedent for future design-system additions.

## Review: QA Engineer — Test Quality Perspective **Verdict: ✅ Approved** --- ### What's good This PR ships tests alongside the code — that's the baseline I always ask for, and it's satisfied. Two test files, clear scope, real assertions. Good. **`contrast.test.ts`** is especially well done: - The WCAG 2.2 luminance formula is implemented in-file rather than hidden behind a dependency. This means the test is self-documenting — a future maintainer can read exactly what WCAG AA means in math, not just what a library tells them. - The comment `--green (#3D8C4A) only gives 4.16:1 — buttons use --green-dark (#2E6E39) instead` is a failure-mode explanation living next to the test that encodes the decision. That's exactly what I want from test documentation. - 4 test cases covering the two most important text/background pairings and the primary button. Coverage is appropriate for the scope. **`tokens.test.ts`** is a clean contract test: - `it.each(requiredTokens)('%s is defined in app.css')` gives one named test case per token. If a token disappears, the failure message names the exact missing token. Good failure ergonomics. - The file read is straightforward and the test is deterministic. --- ### Blockers None. --- ### Suggestions 1. **`contrast.test.ts` does not test `--color-text-muted` on `--color-surface`** (card muted labels). It tests muted text on page background, and main text on surface, but not the cross combination. If muted text appears inside cards (very likely for recipe metadata), that pairing should also be verified. Easy to add a fifth `it()` case. 2. **`tokens.test.ts` does not verify token *values*, only token *names*.** For example, `--green-dark: ;` (empty value) would pass. This is intentional for a presence check, and probably the right tradeoff — value validation would be brittle. But consider adding one spot-check for a critical value: e.g., assert that `--green-dark` contains `#2e6e39` (case-insensitive), since that specific value was chosen to satisfy WCAG AA and an accidental edit there is a silent regression. 3. **No test for the `--color-error` contrast ratio.** The error token (`#dc4c3e`) will be used for validation messages and destructive action indicators. Its contrast on `--color-page` and `--color-surface` should be verified in `contrast.test.ts`. A red that fails contrast is a common accessibility mistake. 4. **`tokens.test.ts` checks a subset of spacing tokens** (`--space-1`, `--space-4`, `--space-8`, `--space-12`, `--space-16`, `--space-20`) but not the full range through `--space-20`. This is a reasonable sampling strategy — worth a comment like `// spot-check key steps, full range defined in app.css` to make the intent explicit. 5. **Missing E2E coverage** — not expected at this stage, but note that fonts loaded via Google Fonts CDN cannot be verified in unit tests. A visual regression or E2E screenshot test that checks actual font rendering would catch CDN failures or wrong font names. Low priority now, worth tracking. --- Strong test coverage for a foundation PR. The contrast tests in particular set a good precedent for future design-system additions.

marcel commented

2026-04-02 12:49:12 +02:00

Review: Security Expert — Security Perspective

Verdict: ✅ Approved

What's good

This PR is CSS tokens and test infrastructure — the attack surface is minimal by nature. There are no user inputs, no rendering paths, no authentication changes. The review is correspondingly brief.

The tokens.test.ts use of node:fs with readFileSync reads a known, project-local file path resolved via __dirname. There is no dynamic path construction from user-controlled input, so path traversal is not a concern here.

Blockers

None.

Suggestions / Observations

Google Fonts CDN dependency in app.html. The font stylesheet is loaded from https://fonts.googleapis.com with a preconnect to https://fonts.gstatic.com. This introduces a third-party dependency on every page load. Security considerations:
- Data exposure: Google Fonts requests include the Referer header, which tells Google which pages on your app the user is visiting. For an internal meal-planning app this is low risk, but worth being aware of.
- CDN availability: If Google Fonts is unavailable (network partition, firewall in corporate environments), fonts fall back to the system stack. The system fallbacks in @theme (Georgia, serif / system-ui, sans-serif / monospace) are appropriately defined.
- Subresource integrity: The <link> tag loading the Google Fonts stylesheet does not have an integrity attribute. Google Fonts dynamically generates the CSS URL response, which means SRI cannot be applied here (the hash changes per-request). This is a known limitation of the Google Fonts CDN model, not a fixable issue.
- Alternative: Self-hosting fonts via @fontsource packages would eliminate the CDN dependency entirely and is straightforward with Vite. Worth considering if data minimization or offline capability becomes a requirement.
crossorigin="anonymous" on the gstatic preconnect is correct — this is required for cross-origin font resources to avoid CORS errors. No issue here.
@types/node added to global tsconfig (see also the Architect review). From a security standpoint, this means developers writing Svelte components will see Node.js globals in autocomplete. There is a small risk of accidentally using process.env or Buffer in client-side component code, where environment variables would either be undefined (runtime) or exposed if Vite replaces them at build time. This is a developer ergonomics risk, not a runtime exploit, but worth mitigating with the tsconfig.test.json split suggested by the Architect.
No Content-Security-Policy headers observed in this PR. Fonts loaded from fonts.googleapis.com and fonts.gstatic.com require explicit font-src and style-src entries in a CSP. This PR doesn't introduce CSP, but when CSP is added (in SvelteKit hooks or a reverse proxy config), the Google Fonts origins will need to be whitelisted. Worth noting in a follow-up issue so it doesn't become a surprise during hardening.

No security concerns block this PR. The Google Fonts CDN choice is worth tracking as a future data-minimization consideration.

## Review: Security Expert — Security Perspective **Verdict: ✅ Approved** --- ### What's good This PR is CSS tokens and test infrastructure — the attack surface is minimal by nature. There are no user inputs, no rendering paths, no authentication changes. The review is correspondingly brief. The `tokens.test.ts` use of `node:fs` with `readFileSync` reads a known, project-local file path resolved via `__dirname`. There is no dynamic path construction from user-controlled input, so path traversal is not a concern here. --- ### Blockers None. --- ### Suggestions / Observations 1. **Google Fonts CDN dependency in `app.html`.** The font stylesheet is loaded from `https://fonts.googleapis.com` with a preconnect to `https://fonts.gstatic.com`. This introduces a third-party dependency on every page load. Security considerations: - **Data exposure:** Google Fonts requests include the `Referer` header, which tells Google which pages on your app the user is visiting. For an internal meal-planning app this is low risk, but worth being aware of. - **CDN availability:** If Google Fonts is unavailable (network partition, firewall in corporate environments), fonts fall back to the system stack. The system fallbacks in `@theme` (`Georgia, serif` / `system-ui, sans-serif` / `monospace`) are appropriately defined. - **Subresource integrity:** The `<link>` tag loading the Google Fonts stylesheet does not have an `integrity` attribute. Google Fonts dynamically generates the CSS URL response, which means SRI cannot be applied here (the hash changes per-request). This is a known limitation of the Google Fonts CDN model, not a fixable issue. - **Alternative:** Self-hosting fonts via `@fontsource` packages would eliminate the CDN dependency entirely and is straightforward with Vite. Worth considering if data minimization or offline capability becomes a requirement. 2. **`crossorigin="anonymous"` on the gstatic preconnect** is correct — this is required for cross-origin font resources to avoid CORS errors. No issue here. 3. **`@types/node` added to global tsconfig** (see also the Architect review). From a security standpoint, this means developers writing Svelte components will see Node.js globals in autocomplete. There is a small risk of accidentally using `process.env` or `Buffer` in client-side component code, where environment variables would either be undefined (runtime) or exposed if Vite replaces them at build time. This is a developer ergonomics risk, not a runtime exploit, but worth mitigating with the `tsconfig.test.json` split suggested by the Architect. 4. **No `Content-Security-Policy` headers observed** in this PR. Fonts loaded from `fonts.googleapis.com` and `fonts.gstatic.com` require explicit `font-src` and `style-src` entries in a CSP. This PR doesn't introduce CSP, but when CSP is added (in SvelteKit hooks or a reverse proxy config), the Google Fonts origins will need to be whitelisted. Worth noting in a follow-up issue so it doesn't become a surprise during hardening. --- No security concerns block this PR. The Google Fonts CDN choice is worth tracking as a future data-minimization consideration.

marcel commented

2026-04-02 12:49:47 +02:00

Review: UI/UX Expert — Visual Design & Accessibility Perspective

Verdict: ✅ Approved

What's good

Color palette: The warm-neutral base (--color-page: #fafaf7, --color-surface: #f5f4ee) avoids the sterile pure-white feel common in food apps. The slight warm tint makes the palette feel organic and appetising — a good match for a meal-planning context. The three-step neutral ramp (page / surface / subtle) maps cleanly to a typical card-on-background layout.

WCAG AA compliance: Choosing --green-dark (#2E6E39) over --green (#3D8C4A) for button backgrounds is the right call, and the contrast test enforces it permanently. This is exactly the kind of documented, tested design decision that prevents regressions.

Typography: Fraunes (optical-size-variable serif) for display, DM Sans for body, DM Mono for code is a coherent, high-quality pairing. The variable-font optical size axes (opsz,wght@9..144) are properly specified in the Google Fonts URL, which means the font will render correctly at heading and body sizes without extra requests.

Spacing scale: 4px half-step, 8px base is the industry standard. 20 steps is more than enough for an app of this scope.

Elevation system: Three shadow levels (card / raised / overlay) with warm-tinted shadows (rgba(28, 28, 24, ...)) instead of cold grey shadows is a thoughtful touch — warm shadows harmonise with the warm neutral palette.

Blockers

None.

Suggestions

--color-text-muted (#6b6a63) on --color-surface (#f5f4ee) needs verification. The contrast test checks muted text on --color-page but not on --color-surface. Since muted labels (e.g., recipe metadata like "20 min · 4 portions") will almost certainly appear inside cards (which use --color-surface), this pairing deserves a dedicated test. A quick calculation: #6b6a63 on #f5f4ee should be in the 4.3–4.5:1 range — it may fail AA by a small margin. Verify and document.
--color-error: #dc4c3e — contrast not tested. Error messages and field validation labels in red text on the warm page background are a high-visibility UI pattern. The error red should be verified to meet 4.5:1 against both --color-page and --color-surface. Classic saturated reds often fall just short of AA.
No focus ring token. Keyboard navigation requires visible focus indicators. A --focus-ring token (typically a 2–3px offset outline in the brand color) should be part of the design system foundation. Without it, individual components will define their own focus styles inconsistently. Recommend adding --focus-ring-color and --focus-ring-width to the @theme block.
--yellow-text: #8a6800 is the only text-on-light-background semantic token for yellow. This is well-considered (yellow backgrounds with regular yellow text is an accessible anti-pattern). Consider whether a similar --green-text token is needed for green tint backgrounds (e.g., success banners), where --green-dark may or may not be the right choice.
Font weights requested from Google Fonts: Fraunes requests weights 300, 400, 500. DM Sans requests 300, 400, 500, 600. Consider whether 300 (light) is actually used in the design — loading unused font weights adds ~15–25KB per weight per format. If light weight is only for display headings, confirm it's intentional.
DM Mono weights 400, 500 — both requested. In most food-app contexts, mono is used for code snippets or nutritional data. Confirm both weights are needed before shipping, or drop 500 to save a network request.

Excellent foundation. The warm palette, coherent typography, and WCAG-compliant green are strong choices that will carry the visual identity well. The missing focus ring token is the most actionable gap to close before the first interactive component ships.

## Review: UI/UX Expert — Visual Design & Accessibility Perspective **Verdict: ✅ Approved** --- ### What's good **Color palette:** The warm-neutral base (`--color-page: #fafaf7`, `--color-surface: #f5f4ee`) avoids the sterile pure-white feel common in food apps. The slight warm tint makes the palette feel organic and appetising — a good match for a meal-planning context. The three-step neutral ramp (page / surface / subtle) maps cleanly to a typical card-on-background layout. **WCAG AA compliance:** Choosing `--green-dark` (#2E6E39) over `--green` (#3D8C4A) for button backgrounds is the right call, and the contrast test enforces it permanently. This is exactly the kind of documented, tested design decision that prevents regressions. **Typography:** `Fraunes` (optical-size-variable serif) for display, `DM Sans` for body, `DM Mono` for code is a coherent, high-quality pairing. The variable-font optical size axes (`opsz,wght@9..144`) are properly specified in the Google Fonts URL, which means the font will render correctly at heading and body sizes without extra requests. **Spacing scale:** 4px half-step, 8px base is the industry standard. 20 steps is more than enough for an app of this scope. **Elevation system:** Three shadow levels (card / raised / overlay) with warm-tinted shadows (`rgba(28, 28, 24, ...)`) instead of cold grey shadows is a thoughtful touch — warm shadows harmonise with the warm neutral palette. --- ### Blockers None. --- ### Suggestions 1. **`--color-text-muted` (#6b6a63) on `--color-surface` (#f5f4ee) needs verification.** The contrast test checks muted text on `--color-page` but not on `--color-surface`. Since muted labels (e.g., recipe metadata like "20 min · 4 portions") will almost certainly appear inside cards (which use `--color-surface`), this pairing deserves a dedicated test. A quick calculation: #6b6a63 on #f5f4ee should be in the 4.3–4.5:1 range — it may fail AA by a small margin. Verify and document. 2. **`--color-error: #dc4c3e` — contrast not tested.** Error messages and field validation labels in red text on the warm page background are a high-visibility UI pattern. The error red should be verified to meet 4.5:1 against both `--color-page` and `--color-surface`. Classic saturated reds often fall just short of AA. 3. **No focus ring token.** Keyboard navigation requires visible focus indicators. A `--focus-ring` token (typically a 2–3px offset outline in the brand color) should be part of the design system foundation. Without it, individual components will define their own focus styles inconsistently. Recommend adding `--focus-ring-color` and `--focus-ring-width` to the `@theme` block. 4. **`--yellow-text: #8a6800` is the only text-on-light-background semantic token for yellow.** This is well-considered (yellow backgrounds with regular yellow text is an accessible anti-pattern). Consider whether a similar `--green-text` token is needed for green tint backgrounds (e.g., success banners), where `--green-dark` may or may not be the right choice. 5. **Font weights requested from Google Fonts:** `Fraunes` requests weights 300, 400, 500. `DM Sans` requests 300, 400, 500, 600. Consider whether 300 (light) is actually used in the design — loading unused font weights adds ~15–25KB per weight per format. If light weight is only for display headings, confirm it's intentional. 6. **`DM Mono` weights 400, 500** — both requested. In most food-app contexts, mono is used for code snippets or nutritional data. Confirm both weights are needed before shipping, or drop 500 to save a network request. --- Excellent foundation. The warm palette, coherent typography, and WCAG-compliant green are strong choices that will carry the visual identity well. The missing focus ring token is the most actionable gap to close before the first interactive component ships.

marcel commented

2026-04-02 12:50:13 +02:00

Review: DevOps — Build & Deployment Perspective

Verdict: ✅ Approved

What's good

The changes are contained to frontend/ — no backend, no infrastructure, no CI pipeline changes required. The diff is low-risk from a build and deployment standpoint.

@types/node is added as a devDependency at ^25.5.0, which is correct — it has no place in production runtime dependencies. The version pin uses ^ (caret), which is consistent with the rest of the dev dependencies in package.json.

The existing test script (vitest run) will pick up the two new test files automatically by Vitest's default glob (**/*.test.ts). No vite.config.ts or vitest.config.ts changes are needed, which means CI pipelines running npm test will exercise the new contrast and token tests on the next run without any pipeline changes.

Blockers

None.

Suggestions

Google Fonts CDN is a runtime external dependency. The font link in app.html means every page load makes a cross-origin request to fonts.googleapis.com. In the current dev/self-hosted deployment context this is fine, but consider the failure mode: if Google Fonts is unavailable (or the instance is deployed in a restricted network environment), the app degrades gracefully to the system font stack — which is correct, because the fallbacks in @theme are defined. No action required, but worth noting in deployment docs if this moves to an air-gapped or privacy-sensitive environment.
@types/node ^25.5.0 is a very recent major version. Node 25 types align with Node.js 25.x, which is a current release. If the build environment or CI runner is pinned to an older Node.js LTS (18 or 20), there is a potential mismatch between the types version and the runtime. Verify that @types/node version is compatible with the Node.js version in use. Using ^20.x.x or ^22.x.x would be more conservative if the runtime is LTS-pinned. This is low risk since @types/node is a devDependency (not bundled), but type errors from API shape mismatches can surface.
package.json version is still 0.0.1. Not related to this PR specifically, but if semantic versioning is used for this package at any point, the version should be updated as part of the first public release. No action required now.
No .nvmrc or engines field observed (may be in the repo root, not reviewed here). With @types/node now explicitly version-pinned, documenting the intended Node.js version in an .nvmrc or package.json engines field would reduce environment-mismatch issues in the future.
Font loading strategy: display=swap is set correctly in the Google Fonts URL. This ensures text is visible with the fallback font while the custom font loads, preventing invisible text (FOIT). Good.

Clean PR from a build perspective. No pipeline changes needed, no new infrastructure concerns. The Google Fonts CDN dependency and @types/node version are worth a second look before a production hardening pass.

## Review: DevOps — Build & Deployment Perspective **Verdict: ✅ Approved** --- ### What's good The changes are contained to `frontend/` — no backend, no infrastructure, no CI pipeline changes required. The diff is low-risk from a build and deployment standpoint. `@types/node` is added as a `devDependency` at `^25.5.0`, which is correct — it has no place in production runtime dependencies. The version pin uses `^` (caret), which is consistent with the rest of the dev dependencies in `package.json`. The existing `test` script (`vitest run`) will pick up the two new test files automatically by Vitest's default glob (`**/*.test.ts`). No `vite.config.ts` or `vitest.config.ts` changes are needed, which means CI pipelines running `npm test` will exercise the new contrast and token tests on the next run without any pipeline changes. --- ### Blockers None. --- ### Suggestions 1. **Google Fonts CDN is a runtime external dependency.** The font link in `app.html` means every page load makes a cross-origin request to `fonts.googleapis.com`. In the current dev/self-hosted deployment context this is fine, but consider the failure mode: if Google Fonts is unavailable (or the instance is deployed in a restricted network environment), the app degrades gracefully to the system font stack — which is correct, because the fallbacks in `@theme` are defined. No action required, but worth noting in deployment docs if this moves to an air-gapped or privacy-sensitive environment. 2. **`@types/node ^25.5.0` is a very recent major version.** Node 25 types align with Node.js 25.x, which is a current release. If the build environment or CI runner is pinned to an older Node.js LTS (18 or 20), there is a potential mismatch between the types version and the runtime. Verify that `@types/node` version is compatible with the Node.js version in use. Using `^20.x.x` or `^22.x.x` would be more conservative if the runtime is LTS-pinned. This is low risk since `@types/node` is a devDependency (not bundled), but type errors from API shape mismatches can surface. 3. **`package.json` version is still `0.0.1`.** Not related to this PR specifically, but if semantic versioning is used for this package at any point, the version should be updated as part of the first public release. No action required now. 4. **No `.nvmrc` or `engines` field observed** (may be in the repo root, not reviewed here). With `@types/node` now explicitly version-pinned, documenting the intended Node.js version in an `.nvmrc` or `package.json` `engines` field would reduce environment-mismatch issues in the future. 5. **Font loading strategy: `display=swap` is set correctly** in the Google Fonts URL. This ensures text is visible with the fallback font while the custom font loads, preventing invisible text (FOIT). Good. --- Clean PR from a build perspective. No pipeline changes needed, no new infrastructure concerns. The Google Fonts CDN dependency and `@types/node` version are worth a second look before a production hardening pass.

marcel merged commit 0a2ef750c4 into master

2026-04-02 12:51:05 +02:00

marcel deleted branch feat/issue-16-design-system

2026-04-02 12:51:11 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/mealprep#31