pal-e-docs

Review: SvelteKit public site — layout shell + 8 pages

review-431-2026-03-26-r3 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Third review. Issue body fully rewritten with route groups, publicFetch, nav cleanup, and restructuring scope. Massive improvement over prior versions. Two remaining issues found.

Template Completeness

  • [x] Type — Feature
  • [x] Lineage
  • [x] Repo — forgejo_admin/westside-app
  • [x] User Story
  • [x] Context
  • [x] Architecture: Route Groups — bonus section, well-detailed
  • [x] Scope — 7 subsections covering restructuring, layout, CSS, publicFetch, static pages, dynamic pages, nav cleanup
  • [x] Translation Reference — @svelte-notes patterns documented
  • [x] File Targets — 16 entries, comprehensive
  • [x] Acceptance Criteria — 12 items
  • [x] Test Expectations — 4 items
  • [x] Constraints — 6 items
  • [x] Checklist — 10 items
  • [x] Depends On — bb-176, bb-177
  • [x] Related — convention-sveltekit-spa, playground PR, ws-96

Traceability

  • [x] story:WS-S26 label — public site story
  • [x] arch:westside-app label — correct component
  • [x] Forgejo issue — westside-app#98, open

File Targets

  • [x] src/routes/+layout.svelte — verified: exists (142 lines), currently has Keycloak auth, nav, bottom-nav. Will be replaced with minimal CSS-only version.
  • [x] src/routes/+layout.js — verified: exists, exports ssr=false, prerender=false
  • [x] src/routes/(public)/+layout.svelte — NEW, correct
  • [x] src/routes/(public)/+page.svelte through (public)/sponsors/ — 8 NEW pages, correct
  • [x] src/routes/(app)/+layout.svelte — NEW (existing Keycloak layout moves here)
  • [x] src/routes/(app)/ — 13 existing route dirs to move (admin, checkout, coach, coaches, forgot-password, jersey, my-players, players, register, reset-password, signin, teams, tryouts) — all 13 verified present
  • [x] src/lib/public-api.js — NEW, uses import.meta.env.VITE_API_URL per convention
  • [x] src/app.css — verified: exists with design tokens already
  • [x] Cleanup: tryouts/+page.svelte lines 9-15 — verified: inline nav at lines 10-15
  • [x] Cleanup: register/+page.svelte line 182 — verified: inline nav at lines 182-187
  • [ ] src/routes/+page.svelte — ISSUE: existing root page has auth redirect logic (lines 6-11: redirects authenticated users to role dashboard). Issue body does not mention deleting this file or migrating the redirect. See Finding 1.

Repo Placement

OK. Issue filed on forgejo_admin/westside-app. All changes target westside-app. Dependencies correctly reference basketball-api as separate issues.

Dependencies

  • basketball-api#176 (public teams endpoint) — OPEN. Board item #429, in_progress. Required for dynamic teams page.
  • basketball-api#177 (public coaches endpoint) — OPEN. Board item #430, next_up. Required for dynamic staff page.
  • westside-app#96 (Svelte promotion prep, @svelte-notes) — OPEN. Board item #416, next_up. Soft dependency: annotations help translation but not strictly blocking since playground HTML is the source of truth.
  • Dependencies correctly documented in issue body under Depends On section and on board item labels.

Acceptance Criteria

All 12 criteria are agent-verifiable. Build success, auth flow check, toggle persistence, fetch validation, mobile hamburger — all automatable with Playwright or manual verification. Test expectations include npm run build which is the definitive check for adapter-static compatibility.

Blast Radius

  • Route collision: /tryouts. The existing tryouts/+page.svelte maps to /tryouts. Moving it to (app)/tryouts/+page.svelte still maps to /tryouts. The new (public)/tryouts/+page.svelte also maps to /tryouts. SvelteKit will error on duplicate routes across groups. See Finding 2.
  • No other route collisions. teams/[id] vs (public)/teams/ = different paths (/teams/:id vs /teams). coaches/[id] vs (public)/staff/ = different URLs entirely.
  • VITE_API_URL env var. The publicFetch helper correctly uses import.meta.env.VITE_API_URL per convention. The existing api.js hardcodes the URL — pre-existing deviation from convention, not in scope for this ticket.
  • Inline nav. Only 2 pages have inline nav (tryouts, register) — both identified in scope. No others found.

Findings

Finding 1: Root +page.svelte disposition unclear

The existing src/routes/+page.svelte has auth redirect logic (if authenticated, redirect to role dashboard). When the new (public)/+page.svelte takes over the / route, the existing root page must be deleted — but its redirect logic needs to live somewhere (likely in (app)/+layout.svelte). The issue should explicitly state: (a) delete existing root +page.svelte, and (b) where the authenticated-user redirect moves.

Finding 2: Route collision at /tryouts

Both (app)/tryouts/+page.svelte (moved from existing) and (public)/tryouts/+page.svelte (new) resolve to /tryouts. SvelteKit does not allow duplicate routes across route groups. The issue must clarify: is the existing auth tryouts page replaced by the public version (delete from (app)), or relocated to a different URL? The existing tryouts page is a hardcoded girls tryout registration landing — likely it should be fully replaced by the new public tryouts page.

Recommendation

Two additions needed to the issue body before this is READY:
  • Add to Scope section 1 (Route group restructuring): explicitly state that the existing root +page.svelte is deleted, and the auth-redirect logic moves to (app)/+layout.svelte.
  • Add to Scope section 1: clarify that the existing tryouts/+page.svelte is NOT moved to (app)/tryouts/ — it is replaced by the new (public)/tryouts/+page.svelte. (Or choose an alternative.)