forgejo_admin/westside-landing

Fork 0

feat: home page redesign + role redirect + fix Sign In (Phase 10f-1, 10f-2, 10f-3) #29

Merged

forgejo_admin merged 2 commits from 28-home-page-redesign-role-redirect-fix-sig into main

2026-03-15 05:07:38 +00:00

forgejo_admin commented

2026-03-15 05:02:28 +00:00

Owner

Summary

Replace the raw API stats dashboard with a proper landing page for unauthenticated visitors, ported from the west-side-basketball static site. Authenticated users now get redirected to their role-appropriate dashboard. The broken Sign In button (double-nested <button> from <SignIn> component) is replaced with a direct form POST.

Changes

src/routes/+page.server.js — Rewrote from API stats fetch to session-based role redirect. Authenticated users go to /admin, /coach, or /player. Unauthenticated users see the landing page with no API call.
src/routes/+page.svelte — Complete rewrite. Replaced stats dashboard with full landing page: hero with logo, about section (3 cards + Marcus quote), tryout details, how-it-works steps, FAQ, bottom CTA. Uses direct <form method="POST" action="/signin"> instead of the broken <SignIn> component. All styles use app.css design tokens, no hardcoded hex values.
src/routes/+layout.svelte — Conditionally hide AuthStatus nav component on the / route for unauthenticated visitors to avoid duplicate/broken sign-in buttons.
static/logo.jpeg — Copied from west-side-basketball repo for the hero section.

Test Plan

Visit / unauthenticated -- should see full landing page with hero, about, tryouts, FAQ sections
Click "Member Sign In" button in hero -- should redirect to Keycloak login
Click "Member Sign In" button in bottom CTA -- same behavior
Sign in as admin -- should redirect to /admin
Sign in as coach -- should redirect to /coach
Sign in as player -- should redirect to /player
Verify no double-nested <button> elements in the sign-in form
Verify AuthStatus nav bar appears on /admin, /coach, /player pages
Verify AuthStatus nav bar is hidden on / for unauthenticated visitors
Test at 390px mobile viewport -- content should be readable and well-spaced
Test at 768px tablet viewport -- grids should show 2 columns
Test at 1280px desktop viewport -- grids should show 3 columns, larger hero text
npm run build passes

Review Checklist

No hardcoded hex values -- all colors use app.css design tokens
Content faithfully ported from west-side-basketball/index.html
Mobile-first responsive design (390px primary, 640px tablet, 1024px desktop)
No modifications to admin/coach/player/teams pages
<SignIn> component not used -- direct form POST avoids broken double-nested buttons
npm run build passes

Closes #28

## Summary Replace the raw API stats dashboard with a proper landing page for unauthenticated visitors, ported from the west-side-basketball static site. Authenticated users now get redirected to their role-appropriate dashboard. The broken Sign In button (double-nested `<button>` from `<SignIn>` component) is replaced with a direct form POST. ## Changes - **`src/routes/+page.server.js`** — Rewrote from API stats fetch to session-based role redirect. Authenticated users go to `/admin`, `/coach`, or `/player`. Unauthenticated users see the landing page with no API call. - **`src/routes/+page.svelte`** — Complete rewrite. Replaced stats dashboard with full landing page: hero with logo, about section (3 cards + Marcus quote), tryout details, how-it-works steps, FAQ, bottom CTA. Uses direct `<form method="POST" action="/signin">` instead of the broken `<SignIn>` component. All styles use `app.css` design tokens, no hardcoded hex values. - **`src/routes/+layout.svelte`** — Conditionally hide `AuthStatus` nav component on the `/` route for unauthenticated visitors to avoid duplicate/broken sign-in buttons. - **`static/logo.jpeg`** — Copied from `west-side-basketball` repo for the hero section. ## Test Plan - [ ] Visit `/` unauthenticated -- should see full landing page with hero, about, tryouts, FAQ sections - [ ] Click "Member Sign In" button in hero -- should redirect to Keycloak login - [ ] Click "Member Sign In" button in bottom CTA -- same behavior - [ ] Sign in as admin -- should redirect to `/admin` - [ ] Sign in as coach -- should redirect to `/coach` - [ ] Sign in as player -- should redirect to `/player` - [ ] Verify no double-nested `<button>` elements in the sign-in form - [ ] Verify AuthStatus nav bar appears on `/admin`, `/coach`, `/player` pages - [ ] Verify AuthStatus nav bar is hidden on `/` for unauthenticated visitors - [ ] Test at 390px mobile viewport -- content should be readable and well-spaced - [ ] Test at 768px tablet viewport -- grids should show 2 columns - [ ] Test at 1280px desktop viewport -- grids should show 3 columns, larger hero text - [ ] `npm run build` passes ## Review Checklist - [x] No hardcoded hex values -- all colors use `app.css` design tokens - [x] Content faithfully ported from `west-side-basketball/index.html` - [x] Mobile-first responsive design (390px primary, 640px tablet, 1024px desktop) - [x] No modifications to admin/coach/player/teams pages - [x] `<SignIn>` component not used -- direct form POST avoids broken double-nested buttons - [x] `npm run build` passes ## Related - Closes #28

forgejo_admin added 1 commit

2026-03-15 05:02:28 +00:00

feat: home page redesign + role redirect + fix Sign In button 4d93f4dde3

Replace the raw API stats dashboard with a proper landing page for
unauthenticated visitors, ported from the west-side-basketball static
site. Authenticated users are now redirected to their role-appropriate
dashboard (/admin, /coach, /player) instead of seeing stats.

- Rewrite +page.server.js: session-based role redirect, no API fetch
- Rewrite +page.svelte: hero, about, tryouts, how-it-works, FAQ, CTA
- Use direct form POST to /signin instead of broken <SignIn> component
- Conditionally hide AuthStatus nav on landing page for unauthenticated
- Add logo.jpeg to static/
- Mobile-first responsive design using app.css design tokens

Closes #28

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin added 1 commit

2026-03-15 05:03:36 +00:00

fix: replace hardcoded #fff with var(--color-text) design token 5791631823

Removes the last hardcoded hex values from the landing page CSS,
using the design token as required by the issue constraints.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-03-15 05:03:51 +00:00

Author

Owner

Self-Review (Dev Agent)

Reviewed the diff for code quality, SOP compliance, and issue constraints.

Finding: Hardcoded `#fff` hex values (FIXED)

Two instances of color: #fff in .btn-primary and .step-number violated the issue constraint "NO new hardcoded hex values." Replaced with var(--color-text) in commit 5791631.

Other observations (no action needed)

rgba(212, 32, 38, ...) in radial gradients -- These reference the brand red for the hero/CTA glow effects. CSS custom properties cannot be interpolated inside rgba() without color-mix() (limited browser support). The reference CSS (style.css) uses the same pattern. Acceptable.
Form POST to /signin with providerId -- The /signin route's action is signIn from Auth.js, which reads providerId from form data. Only one provider (Keycloak) is configured, so this is correct and explicit.
+layout.svelte conditional AuthStatus -- Uses $derived with $page.data.session?.user || $page.url.pathname !== '/'. Since authenticated users always redirect away from /, this correctly hides the nav sign-in button only on the unauthenticated landing page.

VERDICT: APPROVED (after self-fix)

## Self-Review (Dev Agent) Reviewed the diff for code quality, SOP compliance, and issue constraints. ### Finding: Hardcoded `#fff` hex values (FIXED) Two instances of `color: #fff` in `.btn-primary` and `.step-number` violated the issue constraint "NO new hardcoded hex values." Replaced with `var(--color-text)` in commit 5791631. ### Other observations (no action needed) - **`rgba(212, 32, 38, ...)` in radial gradients** -- These reference the brand red for the hero/CTA glow effects. CSS custom properties cannot be interpolated inside `rgba()` without `color-mix()` (limited browser support). The reference CSS (`style.css`) uses the same pattern. Acceptable. - **Form POST to `/signin` with `providerId`** -- The `/signin` route's action is `signIn` from Auth.js, which reads `providerId` from form data. Only one provider (Keycloak) is configured, so this is correct and explicit. - **`+layout.svelte` conditional AuthStatus** -- Uses `$derived` with `$page.data.session?.user || $page.url.pathname !== '/'`. Since authenticated users always redirect away from `/`, this correctly hides the nav sign-in button only on the unauthenticated landing page. ### VERDICT: APPROVED (after self-fix)

forgejo_admin commented

2026-03-15 05:06:21 +00:00

Author

Owner

PR #29 Review

DOMAIN REVIEW

Tech stack: SvelteKit 5 (Svelte 5 runes), Auth.js/SvelteKit, Keycloak OIDC. This is a frontend landing page rewrite with server-side role redirect logic.

Server-side redirect (+page.server.js)

The redirect logic is correct: authenticated users get role-based redirect (admin -> /admin, coach -> /coach, fallback -> /player), unauthenticated users see the landing page with no API call. Clean simplification from the previous stats-fetching approach.

DRY observation (non-blocking): The role-priority redirect pattern (admin > coach > player) is now duplicated across src/routes/+page.server.js and src/routes/signin/+page.server.js. Both files have identical logic:

const roles = session.roles || [];
if (roles.includes('admin')) throw redirect(303, '/admin');
if (roles.includes('coach')) throw redirect(303, '/coach');
throw redirect(303, '/player');

This is NOT in an auth/security enforcement path (it is convenience routing, not access control -- the actual guards live in each route's own +page.server.js), so it does not trigger the DRY-in-auth-path BLOCKER. However, if the role priority ever changes, both files must be updated in lockstep. Worth extracting to a shared utility eventually.

Sign-in form approach

Using <form method="POST" action="/signin"> with <input type="hidden" name="providerId" value="keycloak" /> is a clean fix for the double-nested <button> problem from the <SignIn> component. The /signin/+page.server.js has export const actions = { default: signIn } which correctly processes this form submission via Auth.js's signIn action. The providerId field name is what Auth.js expects.

CSRF consideration: Auth.js/SvelteKit handles CSRF protection automatically through SvelteKit's built-in form actions mechanism (the __superform_id / SvelteKit action token). The direct form POST to /signin will be processed by SvelteKit's action handler which includes CSRF protection. No issue here.

Layout conditional (+layout.svelte)

The showAuthStatus derived value uses a sound approach: show AuthStatus when the user has a session OR when the pathname is not /. Since authenticated users hitting / get server-redirected before the page renders, the landing page is only ever seen by unauthenticated visitors. The logic is correct.

The import of page from $app/stores is the correct SvelteKit 5 pattern for accessing page data in components.

Landing page markup (+page.svelte)

Semantic HTML: Good use of <section>, <blockquote>, <footer>, and heading hierarchy (h1 -> h2 -> h3). The FAQ section uses plain divs rather than <details>/<summary> -- acceptable for a static FAQ but noted below as a nit.
Image handling: Logo has explicit width="80" height="80" attributes to prevent layout shift (CLS). Good.
No hardcoded hex values in CSS: Confirmed -- all color references use var(--color-*) design tokens. The rgba(212, 32, 38, ...) values in the gradient backgrounds are the only raw color values. These are the brand red (#d42026 = rgb(212, 32, 38)) used at very low opacity for decorative radial gradients. There is no CSS custom property for semi-transparent brand overlays in app.css, so inline rgba is the pragmatic choice here. Noted as a minor nit.
Responsive design: Mobile-first with breakpoints at 640px (tablet) and 1024px (desktop). Grids go 1-col -> 2-col -> 3-col. The PR description mentions 390px/640px/1024px which aligns with the CSS. Good coverage.
No <script> block: The page has no JavaScript at all -- pure SSR HTML + CSS. This is ideal for a public landing page (zero JS bundle for this route).
Content accuracy: Tryout date is "Friday, March 13, 2026" -- March 13, 2026 is indeed a Friday. Contact info includes phone and email. Coach attribution is correct.

Accessibility audit:

The logo <img> has descriptive alt text ("Westside Kings and Queens logo"). Good.
Form buttons have visible text labels ("Member Sign In"). Good.
Heading hierarchy is sequential (h1 -> h2 -> h3) with no skipped levels. Good.
The section elements have id attributes for potential anchor navigation.
Color contrast: white/light text on dark backgrounds with red brand accents. The design tokens show --color-text: #f0f0f0 on --color-bg: #0a0a0a which passes WCAG AAA. --color-text-muted: #a3a3a3 on #0a0a0a gives ~7:1 contrast ratio, passes AA.
Missing: no <main> landmark wrapping the page content. The layout has AuthStatus nav + page content but no <main> tag. This is a pre-existing issue in the layout, not introduced by this PR.
Missing: the sign-in forms lack aria-label on the form elements. Minor -- the button text is self-describing.

BLOCKERS

None.

No new interactive functionality requiring test coverage. This is a static landing page (zero JS) with a server-side redirect that reuses the existing Auth.js session mechanism. The redirect logic mirrors the already-shipping /signin route pattern.
No user input beyond the form POST (which delegates to Auth.js's existing validated handler).
No secrets or credentials in code.
The DRY duplication of redirect logic is in a convenience routing path, not an auth enforcement path. The actual access control guards remain in each protected route's own +page.server.js.

NITS

Hardcoded rgba brand color in gradients (rgba(212, 32, 38, ...) appears 3 times in the <style> block). Consider adding --color-brand-glow or --color-brand-overlay tokens to app.css for these semi-transparent brand usages. Low priority since the values are purely decorative.
FAQ section could use <details>/<summary> for progressive disclosure. The current flat list works fine for 7 items, but collapsible FAQ is a common UX pattern that would reduce visual density on mobile. Non-blocking.
Copyright year hardcoded ("2026 Westside Kings & Queens"). If the app lives beyond this year, the footer will go stale. Consider new Date().getFullYear() -- though this would require adding a <script> block to an otherwise zero-JS page, so the tradeoff may not be worth it.
Extract redirect helper (future): The admin > coach > player redirect pattern in both +page.server.js and signin/+page.server.js could be a shared $lib/server/redirectByRole.js utility. Not blocking since it is convenience routing, not security enforcement.
@ts-ignore comment in +page.server.js line 9 (// @ts-ignore -- roles added via Auth.js callback). This is a pre-existing pattern used consistently across the codebase, so not a regression, but worth noting that a proper type augmentation for the session would eliminate all these suppressions project-wide.

SOP COMPLIANCE

Branch named after issue (28-home-page-redesign-role-redirect-fix-sig -- derived from issue #28)
PR body has Summary, Changes, Test Plan, Related sections
Related section references Closes #28
No secrets committed
No unnecessary file changes (4 files: layout, page server, page component, logo asset -- all scoped to the issue)
Commit messages are descriptive (PR title follows conventional commit format)
Related section does not reference the plan slug (plan-2026-03-08-tryout-prep). The PR body says Closes #28 but does not mention the plan. Minor -- the issue title itself references "Phase 10f-1, 10f-2, 10f-3" which provides traceability.

PROCESS OBSERVATIONS

Deployment frequency: This PR replaces the entire home page. It is a single atomic change that can deploy cleanly. No migration needed, no env var changes, no infrastructure dependencies. Low change failure risk.
Test coverage context: This repo has zero automated tests (no test runner configured in package.json, no test directories). The lack of tests for this specific PR is consistent with the repo's current state. The Test Plan in the PR body is comprehensive (13 manual verification items including responsive breakpoints). A future issue should address adding a test framework to this repo.
Content correctness risk: The tryout details (date, location, pricing) are hardcoded in the template. Any changes to these details require a code change + deploy. If these details change frequently, consider making them configurable (env vars, API endpoint, or CMS). For a tryout prep context, hardcoding is acceptable.

VERDICT: APPROVED

## PR #29 Review ### DOMAIN REVIEW **Tech stack:** SvelteKit 5 (Svelte 5 runes), Auth.js/SvelteKit, Keycloak OIDC. This is a frontend landing page rewrite with server-side role redirect logic. **Server-side redirect (`+page.server.js`)** The redirect logic is correct: authenticated users get role-based redirect (admin -> /admin, coach -> /coach, fallback -> /player), unauthenticated users see the landing page with no API call. Clean simplification from the previous stats-fetching approach. **DRY observation (non-blocking):** The role-priority redirect pattern (`admin > coach > player`) is now duplicated across `src/routes/+page.server.js` and `src/routes/signin/+page.server.js`. Both files have identical logic: ```js const roles = session.roles || []; if (roles.includes('admin')) throw redirect(303, '/admin'); if (roles.includes('coach')) throw redirect(303, '/coach'); throw redirect(303, '/player'); ``` This is NOT in an auth/security enforcement path (it is convenience routing, not access control -- the actual guards live in each route's own `+page.server.js`), so it does not trigger the DRY-in-auth-path BLOCKER. However, if the role priority ever changes, both files must be updated in lockstep. Worth extracting to a shared utility eventually. **Sign-in form approach** Using `<form method="POST" action="/signin">` with `<input type="hidden" name="providerId" value="keycloak" />` is a clean fix for the double-nested `<button>` problem from the `<SignIn>` component. The `/signin/+page.server.js` has `export const actions = { default: signIn }` which correctly processes this form submission via Auth.js's `signIn` action. The `providerId` field name is what Auth.js expects. **CSRF consideration:** Auth.js/SvelteKit handles CSRF protection automatically through SvelteKit's built-in form actions mechanism (the `__superform_id` / SvelteKit action token). The direct form POST to `/signin` will be processed by SvelteKit's action handler which includes CSRF protection. No issue here. **Layout conditional (`+layout.svelte`)** The `showAuthStatus` derived value uses a sound approach: show AuthStatus when the user has a session OR when the pathname is not `/`. Since authenticated users hitting `/` get server-redirected before the page renders, the landing page is only ever seen by unauthenticated visitors. The logic is correct. The import of `page` from `$app/stores` is the correct SvelteKit 5 pattern for accessing page data in components. **Landing page markup (`+page.svelte`)** - **Semantic HTML:** Good use of `<section>`, `<blockquote>`, `<footer>`, and heading hierarchy (h1 -> h2 -> h3). The FAQ section uses plain divs rather than `<details>/<summary>` -- acceptable for a static FAQ but noted below as a nit. - **Image handling:** Logo has explicit `width="80" height="80"` attributes to prevent layout shift (CLS). Good. - **No hardcoded hex values in CSS:** Confirmed -- all color references use `var(--color-*)` design tokens. The `rgba(212, 32, 38, ...)` values in the gradient backgrounds are the only raw color values. These are the brand red (`#d42026` = `rgb(212, 32, 38)`) used at very low opacity for decorative radial gradients. There is no CSS custom property for semi-transparent brand overlays in `app.css`, so inline rgba is the pragmatic choice here. Noted as a minor nit. - **Responsive design:** Mobile-first with breakpoints at 640px (tablet) and 1024px (desktop). Grids go 1-col -> 2-col -> 3-col. The PR description mentions 390px/640px/1024px which aligns with the CSS. Good coverage. - **No `<script>` block:** The page has no JavaScript at all -- pure SSR HTML + CSS. This is ideal for a public landing page (zero JS bundle for this route). - **Content accuracy:** Tryout date is "Friday, March 13, 2026" -- March 13, 2026 is indeed a Friday. Contact info includes phone and email. Coach attribution is correct. **Accessibility audit:** - The logo `<img>` has descriptive alt text ("Westside Kings and Queens logo"). Good. - Form buttons have visible text labels ("Member Sign In"). Good. - Heading hierarchy is sequential (h1 -> h2 -> h3) with no skipped levels. Good. - The `section` elements have `id` attributes for potential anchor navigation. - Color contrast: white/light text on dark backgrounds with red brand accents. The design tokens show `--color-text: #f0f0f0` on `--color-bg: #0a0a0a` which passes WCAG AAA. `--color-text-muted: #a3a3a3` on `#0a0a0a` gives ~7:1 contrast ratio, passes AA. - Missing: no `<main>` landmark wrapping the page content. The layout has AuthStatus nav + page content but no `<main>` tag. This is a pre-existing issue in the layout, not introduced by this PR. - Missing: the sign-in forms lack `aria-label` on the form elements. Minor -- the button text is self-describing. ### BLOCKERS **None.** - No new interactive functionality requiring test coverage. This is a static landing page (zero JS) with a server-side redirect that reuses the existing Auth.js session mechanism. The redirect logic mirrors the already-shipping `/signin` route pattern. - No user input beyond the form POST (which delegates to Auth.js's existing validated handler). - No secrets or credentials in code. - The DRY duplication of redirect logic is in a convenience routing path, not an auth enforcement path. The actual access control guards remain in each protected route's own `+page.server.js`. ### NITS 1. **Hardcoded rgba brand color in gradients** (`rgba(212, 32, 38, ...)` appears 3 times in the `<style>` block). Consider adding `--color-brand-glow` or `--color-brand-overlay` tokens to `app.css` for these semi-transparent brand usages. Low priority since the values are purely decorative. 2. **FAQ section could use `<details>/<summary>`** for progressive disclosure. The current flat list works fine for 7 items, but collapsible FAQ is a common UX pattern that would reduce visual density on mobile. Non-blocking. 3. **Copyright year hardcoded** ("2026 Westside Kings & Queens"). If the app lives beyond this year, the footer will go stale. Consider `new Date().getFullYear()` -- though this would require adding a `<script>` block to an otherwise zero-JS page, so the tradeoff may not be worth it. 4. **Extract redirect helper (future)**: The `admin > coach > player` redirect pattern in both `+page.server.js` and `signin/+page.server.js` could be a shared `$lib/server/redirectByRole.js` utility. Not blocking since it is convenience routing, not security enforcement. 5. **`@ts-ignore` comment** in `+page.server.js` line 9 (`// @ts-ignore -- roles added via Auth.js callback`). This is a pre-existing pattern used consistently across the codebase, so not a regression, but worth noting that a proper type augmentation for the session would eliminate all these suppressions project-wide. ### SOP COMPLIANCE - [x] Branch named after issue (`28-home-page-redesign-role-redirect-fix-sig` -- derived from issue #28) - [x] PR body has Summary, Changes, Test Plan, Related sections - [x] Related section references `Closes #28` - [x] No secrets committed - [x] No unnecessary file changes (4 files: layout, page server, page component, logo asset -- all scoped to the issue) - [x] Commit messages are descriptive (PR title follows conventional commit format) - [ ] Related section does not reference the plan slug (`plan-2026-03-08-tryout-prep`). The PR body says `Closes #28` but does not mention the plan. Minor -- the issue title itself references "Phase 10f-1, 10f-2, 10f-3" which provides traceability. ### PROCESS OBSERVATIONS - **Deployment frequency:** This PR replaces the entire home page. It is a single atomic change that can deploy cleanly. No migration needed, no env var changes, no infrastructure dependencies. Low change failure risk. - **Test coverage context:** This repo has zero automated tests (no test runner configured in `package.json`, no test directories). The lack of tests for this specific PR is consistent with the repo's current state. The Test Plan in the PR body is comprehensive (13 manual verification items including responsive breakpoints). A future issue should address adding a test framework to this repo. - **Content correctness risk:** The tryout details (date, location, pricing) are hardcoded in the template. Any changes to these details require a code change + deploy. If these details change frequently, consider making them configurable (env vars, API endpoint, or CMS). For a tryout prep context, hardcoding is acceptable. ### VERDICT: APPROVED