Skip to content

feat(ui): visual redesign of storefront#23

Open
danielmeppiel wants to merge 1 commit into
mainfrom
feat/visual-redesign
Open

feat(ui): visual redesign of storefront#23
danielmeppiel wants to merge 1 commit into
mainfrom
feat/visual-redesign

Conversation

@danielmeppiel
Copy link
Copy Markdown
Contributor

Summary

Replaces the placeholder homepage with a Zava-branded design system, header, hero, feature strip, polished product cards, and footer.

Changes

  • app/globals.css (new) — design tokens (CSS variables), light/dark modes, typography stack, buttons, product cards, sticky header, footer.
  • app/layout.tsx (new) — branded sticky header (Z mark + nav + search/account/cart icons) and 4-column footer; proper <title>/<description> metadata.
  • app/page.tsx — hero with eyebrow + display-serif headline + dual CTA + gradient visual; feature strip; product grid using semantic classes; graceful empty/error states.

Why hand-written CSS (not Tailwind)

The previous code used Tailwind utility class names, but Tailwind is not in package.json — those classes were no-ops. Rather than introduce a new build dependency for a styling pass, this PR commits to a hand-written CSS design system with BEM-ish class names. If we want to revisit this and adopt Tailwind/CSS Modules later, that's a one-paragraph ADR worth its own discussion.

Security / dependency posture

  • No new runtime or dev dependencies.
  • No new HTTP handlers; no auth surface change.
  • No new DB queries; existing db.listProducts call is unchanged.
  • No logging, no PII; no secrets touched.

Pre-review panel feedback (acknowledged, deferred)

A panel review (architect + security) flagged a few items that are not addressed in this PR and are tracked as follow-ups:

  • Replace catch {} in page.tsx with structured logging.
  • Add export const revalidate = 60 (or unstable_cache) to avoid hitting the DB on every render.
  • Neutralize the placeholder href="/" links in nav/footer until real routes exist (SEO/analytics noise).
  • Extract <SiteHeader/>/<SiteFooter/> from RootLayout once a second page variant appears.

Per the panel's #1 recommendation, unrelated changes to tsconfig.json and apm.lock.yaml that were also in my working tree have been left out of this PR and will go in a separate housekeeping PR — the lockfile change in particular (drops deployed_file_hashes for the secure-baseline plugin) deserves its own review.

Verification

  • npm run dev → HTTP 200 on http://localhost:3001, page renders with new styling.
  • No new tests added (presentation-only change). No existing tests touched.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

Replace placeholder homepage with a Zava-branded design:

- Add app/globals.css design system (CSS variables, light/dark modes,
  typography, buttons, product cards, header/footer)
- Add app/layout.tsx with sticky header, nav, and 4-column footer
- Rebuild app/page.tsx with hero, feature strip, and product grid
- Add error/empty states for the products listing
- Update <title>/<description> metadata

The previous code referenced Tailwind utility classes, but Tailwind
is not installed in this repo (those classes were no-ops). This PR
commits to hand-written CSS rather than introducing a new build
dependency.

No new dependencies, no new HTTP handlers, no new DB queries.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown

🏛️ Panel Review — feat(ui): visual redesign of storefront

Automated review by the Zava architect + security panel. Triggered by panel-review label on PR #23.


🏛️ Architect review

What I see

This PR replaces a bare-bones Tailwind scaffold with a Zava-branded design system: a CSS custom-property token layer (globals.css, 581 lines), a new RootLayout shell with sticky header and 4-column footer, and a fully redesigned HomePage featuring a hero section, feature strip, and polished product grid. The DB call in page.tsx gains a try/catch with a user-visible error state. No new dependencies, no schema changes, no backend logic touched.

Concerns

  • [inconsistent] Single monolithic global stylesheet — The entire design system lives in one 581-line globals.css with BEM class names, while the rest of the codebase appears to use Tailwind utility classes (the deleted code uses className="container mx-auto p-8"). Two styling paradigms now coexist without an ADR or migration note. This is manageable today but compounds fast as the team grows. — Either commit to the CSS-variables + BEM approach and document the migration intent, or scope these styles to a CSS Module so the two systems don't bleed into each other.

  • [opportunity] All nav links point to / — Every <a href="/"> in the header and footer is a placeholder. This is expected for a scaffold, but shipping a nav where "New arrivals", "Collections", "Journal", "About", etc. all go to the same route creates a regression from whatever routing existed before. — Create a follow-up issue before merge, or add TODO: comments so the placeholders are discoverable by grep.

  • [opportunity] Layout shell is not extracted into a component — The entire header and footer are inlined directly into layout.tsx (109 lines). For a design system, this means any change to nav or footer touches the root layout file, which is harder to test and review in isolation. — Consider extracting <SiteHeader /> and <SiteFooter /> into app/components/ — not a blocker, but the right time is before more pages are added.

  • [opportunity] db.listProducts error swallowed silently — The new try/catch shows a user-friendly error state, which is an improvement. However, the catch block sets dbError = true with no logging. A DB failure will be invisible in observability. — Add a console.error or structured logger call in the catch block so the failure appears in traces.

Looks good

  • The try/catch + dbError pattern is a meaningful improvement over the previous bare await — blast radius is now contained to a graceful degradation message rather than a 500.
  • Dark mode via @media (prefers-color-scheme: dark) is handled at the token level, which is the correct place for it.
  • Responsive breakpoints are present for both the hero grid and footer grid.
  • aria-label, aria-hidden, and semantic landmark elements (<header>, <nav>, <footer>, <main>) are used correctly throughout.

🛡️ Security review

What I see

A pure frontend styling and layout change. No new HTTP handlers, no new API routes, no database query changes, no authentication boundaries touched, no new npm dependencies added. The only data rendered from the DB (p.name, p.description, p.priceCents) passes through React's JSX renderer, which escapes output by default.

Findings

  • [INFO] Add to cart button has no action handler — The <button className="product-card__add" type="button">Add to cart</button> renders with no onClick or form association. Clicking it is a no-op. This is not a security issue, but it means users can trigger a perceived interaction with no feedback — which could matter if cart state becomes session-sensitive later. — Wire up or disable before the feature is considered production-ready.

  • [INFO] Price rendered from DB without format validation${(p.priceCents / 100).toFixed(2)} renders a raw integer from the DB. React escapes this so XSS is not possible, but if priceCents is ever null or non-numeric (schema drift, bad seed data), it will render NaN to the customer. — Defensive: typeof p.priceCents === 'number' ? ... : '—'.

Checklist

  • No new secrets in diff
  • AuthN + AuthZ on new handlers — N/A (no new handlers)
  • Parameterized queries — N/A (no new queries; existing db.listProducts unchanged)
  • Dependencies justified — N/A (no new dependencies)
  • PII masked in logs — no logging added; no PII exposure
  • Error paths fail closed — DB error path shows generic user message, no stack trace leaked to client

✅ Advisory recommendation: APPROVED with follow-ups

No blockers. No security findings requiring action before merge. The two [opportunity] items (component extraction, DB error logging) and the placeholder nav links are worth tracking as immediate follow-ups — ideally as issues created before this merges so they don't evaporate. The styling paradigm inconsistency ([inconsistent]) is the only item worth a brief discussion with the team before more CSS accumulates.

Panel: 🏛️ Architect · 🛡️ Security | Skill: panel-review | PR: #23

Generated by PR Review Panel for issue #23 · ● 435.2K ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant