Review: Westside playground complete overhaul -- unified HTML/CSS mock

review-380-2026-03-25 Review

review needs-refinement

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Lineage -- present ("Standalone")
  • [x] Repo -- present but WRONG REPO (body says forgejo_admin/westside-playground, issue filed on forgejo_admin/westside-app)
  • [x] User Story -- present
  • [x] Context -- present and thorough
  • [x] File Targets -- present and detailed
  • [x] Acceptance Criteria -- present (9 items)
  • [x] Test Expectations -- present
  • [x] Constraints -- present
  • [x] Checklist -- present
  • [x] Related -- present

File Targets

CSS inventory (verified)

  • [x] shared/app.css (2621 lines) -- exists, authenticated page styles. Ticket correctly identifies.
  • [x] css/style.css (804 lines) -- exists, public page styles. Ticket correctly identifies.
  • [x] shared/westside-style.css (804 lines) -- exists, exact duplicate of css/style.css. Ticket says remove.
  • [ ] shared/style.css -- does NOT exist yet. Ticket correctly identifies as the merge target.

JS inventory (verified)

  • [ ] shared/app.js (409 lines) -- ISSUE: ticket says merge into single "light interactivity" JS file, but app.js is already 409 lines with significant logic. Ticket does not specify what happens to this file (keep? rename? gut?).
  • [x] shared/westside-main.js (20 lines) -- exists, nav toggle only. Ticket does not mention removing this file despite saying "single JS file."
  • [x] js/main.js (20 lines) -- exists, exact duplicate of shared/westside-main.js. Ticket does not mention this file or the js/ directory removal.

HTML pages -- existing (verified)

  • [x] index.html -- exists (210 lines), has @route doc
  • [x] staff.html -- exists (164 lines), NO @route doc (uses css/style.css)
  • [x] sponsors.html -- exists (121 lines), NO @route doc (uses css/style.css)
  • [x] register.html -- exists (313 lines), has @route doc
  • [x] admin.html -- exists (197 lines), has @route doc
  • [x] admin-players.html -- exists (222 lines), has @route doc
  • [x] admin-teams.html -- exists (133 lines), has @route doc
  • [x] coach.html -- exists (247 lines), has @route doc
  • [x] coach-profile.html -- exists (134 lines), has @route doc
  • [x] parent.html -- exists (174 lines), has @route doc
  • [x] player-profile.html -- exists (333 lines), has @route doc
  • [x] billing.html -- exists (259 lines), has @route doc

HTML pages -- missing (need creation, verified)

  • [ ] schedule.html -- does not exist. NEW page.
  • [ ] login.html -- does not exist. NOTE: signin.html exists (79 lines, Keycloak redirect mock). Ticket says create login.html but does not mention what happens to signin.html.
  • [ ] forgot-password.html -- does not exist. NEW page.
  • [ ] reset-password.html -- does not exist. NEW page.
  • [ ] checkout.html -- does not exist. NEW page.
  • [ ] checkout-success.html -- does not exist. NOTE: success.html exists (121 lines, registration confirmation). Different scope from checkout success.
  • [ ] checkout-cancel.html -- does not exist. NEW page.
  • [ ] jersey.html -- does not exist. NEW page.
  • [ ] jersey-success.html -- does not exist. NEW page.

Unmapped existing files -- ISSUE

  • [ ] signin.html -- exists (79 lines, @route /signin). Ticket creates login.html but does not say to remove or rename signin.html.
  • [ ] success.html -- exists (121 lines, registration confirmation). Not in the 21-page list. Remove? Keep?
  • [ ] team.html -- exists (192 lines, @route /teams/[id], team detail page). Not in the 21-page list at all. This is an active, documented page that should either be listed or explicitly removed.

Component docs -- partially already done

  • [ ] ISSUE: 13 of 18 existing pages already have @route/@auth/@api doc comments (the established pattern). The ticket describes a different format ("HTML comment block documenting Route, Auth requirement, Data contracts, Nav state"). These are the same fields but described differently, which could cause an agent to rewrite existing docs unnecessarily.

Repo Placement

CRITICAL MISMATCH: Issue #84 is filed on forgejo_admin/westside-app but all work targets forgejo_admin/westside-playground. The westside-playground repo exists on Forgejo (repo id: 40). The issue must be re-filed on the correct repo or the agent will clone the wrong repo.

Dependencies

  • Board #356 (next_up): "Public site: travel schedule page + nav updates" -- creates schedule content that overlaps with schedule.html in this ticket. These will conflict if not sequenced.
  • Board #355 (todo): "Add Coach Manny to staff page" -- directly modifies staff.html content this overhaul will rebuild. Should complete first or be absorbed into this ticket.
  • Phase 15 (in_progress): "Production Port -- playground to SvelteKit" -- this phase consumes the playground as source of truth. The playground overhaul should logically complete before Phase 15 continues porting pages. Not documented in the ticket.
  • Neither #355 nor #356 are mentioned in the ticket as dependencies or related items.

Acceptance Criteria

  • [x] "Single shared/style.css" -- testable, clear
  • [x] "Single shared/app.js" -- testable but ambiguous (current app.js is 409 lines -- keep as-is or gut?)
  • [x] "One HTML file per production route (21 pages)" -- testable but count is wrong: 12 exist + 9 new = 21, but 3 existing pages (signin, success, team) are not accounted for. Real count is 24 pages to manage.
  • [ ] "Every page has component doc comment" -- 13 pages already have this. Criterion should say "standardize existing + add to new pages."
  • [x] "Auth flow mocked" -- PARTIALLY testable. Ticket doesn't specify the static HTML mechanism for "redirect to dashboard" (link? JS? just describe it?). Keycloak mock pages are a novel pattern with no prior art in this playground.
  • [x] "No stale tryout references" -- testable via grep. Currently 60+ tryout references across 12 files. The removal scope is large.
  • [x] "All pages viewable on phone" -- requires manual review (per feedback_playground_gate)

Blast Radius

  • Phase 15 port: Any page renamed or restructured here breaks the Phase 15 promotion pipeline. The playground IS the source for SvelteKit pages.
  • westside-app production site: Public pages (staff, sponsors, index) on the live westside-app were recently copied FROM the playground (commit 6cac776). Playground changes will need to be re-promoted.
  • Board item conflicts: #355 and #356 directly modify pages this ticket rebuilds. Merging order matters.
  • CSS merge risk: Merging 2621-line app.css + 804-line style.css into one file is non-trivial. These use different CSS variable namespaces (app.css uses the Phase 8 design system; style.css uses simpler brand tokens). An agent may produce a broken merge without visual review.

Decomposition Assessment

At 8 points, this ticket covers 4 distinct workstreams that should be separate tickets:
  • CSS/JS unification + stale file cleanup (3 pts) -- merge CSS, consolidate JS, remove duplicates and stale files
  • Missing public pages (2 pts) -- schedule.html, login.html, forgot-password.html, reset-password.html
  • Missing checkout flow pages (2 pts) -- checkout.html, checkout-success.html, checkout-cancel.html, jersey.html, jersey-success.html
  • Content cleanup + doc standardization (1 pt) -- strip tryout refs, standardize component docs, resolve signin/success/team page gaps
Decomposing lets Lucas review the CSS merge on phone (the riskiest part) before 9 new pages are built on top of it. Per feedback_playground_gate, playground must be approved on phone before promotion -- reviewing 21 pages in one shot is much harder to iterate on than 4 focused PRs.

Recommendation

Before this ticket is READY, the following must be addressed:
  • Re-file on correct repo: Move issue to forgejo_admin/westside-playground or close #84 and re-create there.
  • Resolve unmapped files: Explicitly state what happens to signin.html, success.html, and team.html.
  • Clarify JS strategy: Current shared/app.js is 409 lines. Does it stay? Get renamed? Get gutted to "light interactivity"?
  • Add missing cleanup targets: shared/westside-main.js and js/ directory (including js/main.js) need explicit removal instructions.
  • Document dependencies: Board items #355 and #356 directly conflict. Phase 15 depends on playground state.
  • Consider decomposition: 4 workstreams in one ticket risks a broken CSS merge under 9 new pages. CSS unification needs phone review before new pages are built on it.
  • Clarify auth mock mechanism: How does static HTML represent a Keycloak redirect? Link-based navigation between pages? Describe the pattern for the agent.