forgejo_admin/westside-playground

Fork 0

feat: Tryouts page + Kings/Queens toggle foundation (#92) #41

Merged

forgejo_admin merged 2 commits from 92-tryouts-toggle into main

2026-03-27 00:32:27 +00:00

forgejo_admin commented

2026-03-27 00:18:59 +00:00

Owner

Summary

Adds the Tryouts page with Kings/Queens program toggle -- the foundation pattern that #93 (Teams), #94 (Gear), and #95 (Schedule) will reuse
Toggle persists selection via localStorage and swaps the brand color scheme between Kings red and Queens pink
Nav updated on all 7 public pages to include Tryouts link

Changes

tryouts.html (new): Tryouts page with KQ toggle, upcoming tryouts (empty state), past tryouts for both programs with real data, next-season contact box
shared/app.js: Added initKQToggle() function with localStorage persistence, body class toggling, and content section show/hide. Called in DOMContentLoaded block.
shared/style.css: Added .queens-active color mode (swaps --color-red to Queens pink), .kq-toggle / .kq-toggle-btn button styles, .tryout-history-card and .next-season-box component styles
index.html, staff.html, sponsors.html, schedule.html, register.html, success.html: Added Tryouts nav link (Home | About | Staff | Tryouts | Sponsors | Schedule)

Test Plan

Open tryouts.html -- toggle between Kings and Queens, verify content switches and color scheme changes
Refresh page after selecting Queens -- verify localStorage persists selection
Navigate to another page and back -- verify toggle state is maintained
Check nav on all 7 public pages -- Tryouts link present and resolves
Test at 390px mobile viewport
No regressions on existing pages (nav still works, mobile hamburger still works)

Review Checklist

Passed automated review-fix loop
No secrets committed
No unnecessary file changes
Commit messages are descriptive

Closes forgejo_admin/westside-app#92
Foundation for #93 (Teams), #94 (Gear), #95 (Schedule)
westside-playground-overhaul

## Summary - Adds the Tryouts page with Kings/Queens program toggle -- the foundation pattern that #93 (Teams), #94 (Gear), and #95 (Schedule) will reuse - Toggle persists selection via localStorage and swaps the brand color scheme between Kings red and Queens pink - Nav updated on all 7 public pages to include Tryouts link ## Changes - `tryouts.html` (new): Tryouts page with KQ toggle, upcoming tryouts (empty state), past tryouts for both programs with real data, next-season contact box - `shared/app.js`: Added `initKQToggle()` function with localStorage persistence, body class toggling, and content section show/hide. Called in DOMContentLoaded block. - `shared/style.css`: Added `.queens-active` color mode (swaps `--color-red` to Queens pink), `.kq-toggle` / `.kq-toggle-btn` button styles, `.tryout-history-card` and `.next-season-box` component styles - `index.html`, `staff.html`, `sponsors.html`, `schedule.html`, `register.html`, `success.html`: Added Tryouts nav link (Home | About | Staff | Tryouts | Sponsors | Schedule) ## Test Plan - [ ] Open `tryouts.html` -- toggle between Kings and Queens, verify content switches and color scheme changes - [ ] Refresh page after selecting Queens -- verify localStorage persists selection - [ ] Navigate to another page and back -- verify toggle state is maintained - [ ] Check nav on all 7 public pages -- Tryouts link present and resolves - [ ] Test at 390px mobile viewport - [ ] No regressions on existing pages (nav still works, mobile hamburger still works) ## Review Checklist - [x] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related Notes - Closes forgejo_admin/westside-app#92 - Foundation for #93 (Teams), #94 (Gear), #95 (Schedule) - `westside-playground-overhaul`

forgejo_admin added 1 commit

2026-03-27 00:18:59 +00:00

feat: Tryouts page + Kings/Queens toggle foundation (#92 ) 32afd1ff28

Add tryouts.html with Kings/Queens program toggle, past tryout history
for both programs, and next-season contact info. Create reusable
initKQToggle() in shared/app.js with localStorage persistence and
queens-active CSS color mode in shared/style.css. Update nav on all
7 public pages to include Tryouts link.

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

forgejo_admin commented

2026-03-27 00:20:22 +00:00

Author

Owner

QA Review

Checked

HTML structure valid (no unclosed/mismatched tags)
Component doc present at top of tryouts.html (@route, @auth, @nav, @api)
Nav updated on all 7 public pages (index, staff, sponsors, schedule, register, success, tryouts)
Nav order consistent: Home | About | Staff | Tryouts | Sponsors | Schedule
initKQToggle() correctly uses early return when no toggle buttons found (no-op on non-toggle pages)
localStorage persistence: reads on init, writes on click, defaults to 'kings'
applyProgram() handles body class toggle, content visibility, and button active state
Multiple data-program-content divs per program value work correctly (JS queries all matching elements)
.queens-active CSS override scoped to --color-red and --color-red-hover only
Toggle button styles use design tokens (spacing, border-radius, font-size, transitions)
No secrets, no unnecessary files
Matches existing page patterns (hero, section, footer, script defer)

Nits (non-blocking)

Inline style on Past Tryouts header (line 88): style="margin-top: var(--space-2xl);" -- acceptable for playground, but could be a CSS class if reused on other toggle pages (#93, #95).
Hardcoded rgba in .next-season-box gradient: Uses rgba(212, 32, 38, 0.08) instead of a var-based approach. This matches the existing codebase pattern (.bottom-cta, .tryout-hero, etc.) so not a regression, but won't pick up Queens pink in the gradient when toggled. Gradient is subtle enough that this is cosmetic-only.

VERDICT: APPROVED

## QA Review ### Checked - [x] HTML structure valid (no unclosed/mismatched tags) - [x] Component doc present at top of tryouts.html (`@route`, `@auth`, `@nav`, `@api`) - [x] Nav updated on all 7 public pages (index, staff, sponsors, schedule, register, success, tryouts) - [x] Nav order consistent: Home | About | Staff | Tryouts | Sponsors | Schedule - [x] `initKQToggle()` correctly uses early return when no toggle buttons found (no-op on non-toggle pages) - [x] localStorage persistence: reads on init, writes on click, defaults to 'kings' - [x] `applyProgram()` handles body class toggle, content visibility, and button active state - [x] Multiple `data-program-content` divs per program value work correctly (JS queries all matching elements) - [x] `.queens-active` CSS override scoped to `--color-red` and `--color-red-hover` only - [x] Toggle button styles use design tokens (spacing, border-radius, font-size, transitions) - [x] No secrets, no unnecessary files - [x] Matches existing page patterns (hero, section, footer, script defer) ### Nits (non-blocking) 1. **Inline style on Past Tryouts header** (line 88): `style="margin-top: var(--space-2xl);"` -- acceptable for playground, but could be a CSS class if reused on other toggle pages (#93, #95). 2. **Hardcoded rgba in `.next-season-box` gradient**: Uses `rgba(212, 32, 38, 0.08)` instead of a var-based approach. This matches the existing codebase pattern (`.bottom-cta`, `.tryout-hero`, etc.) so not a regression, but won't pick up Queens pink in the gradient when toggled. Gradient is subtle enough that this is cosmetic-only. ### VERDICT: APPROVED

forgejo_admin commented

2026-03-27 00:24:55 +00:00

Author

Owner

PR #41 Review

DOMAIN REVIEW

Tech stack: Vanilla HTML/CSS/JS playground (static prototypes). No frameworks, no build tools. This is the design source of truth for westside-app SvelteKit routes.

JavaScript (shared/app.js -- initKQToggle()):

Clean separation of concerns: initKQToggle() is a standalone function called from the DOMContentLoaded block, consistent with all other init functions in the file (initFilterList, initTabs, initRegister, initTeams).
Early return pattern (if (!toggleBtns.length) return;) is correct -- runs on every page load but exits immediately on pages without a toggle. No performance concern.
localStorage persistence is straightforward: reads on load, writes on click. Key name 'program' is simple and collision-free within this domain.
applyProgram() inner function handles three concerns correctly: body class toggle, content section visibility, button active state. All use classList.toggle with the boolean second argument, which is the right API.
No XSS risk: dataset.program reads from static HTML data-program attributes. Values are never injected into innerHTML or eval'd.
No user input is involved -- all values come from hardcoded data-program="kings" / data-program="queens" attributes in the HTML.

CSS (shared/style.css):

.queens-active overrides --color-red and --color-red-hover at the body level, which cascades correctly to all elements using those variables. Elegant approach -- the entire site color scheme pivots on two variable overrides.
Toggle button styles (.kq-toggle, .kq-toggle-btn) use design tokens (--space-sm, --space-xl, --border-radius, --font-size-base, --transition-fast) appropriately.
New component styles (.tryout-history-card, .next-season-box) are well-structured with clear naming.

HTML (tryouts.html):

Proper document structure: lang="en", viewport meta, charset, OG tags (consistent URL pattern with all other pages), favicons, external stylesheet only.
Accessibility: aria-label="Main navigation" on nav, aria-expanded/aria-controls on hamburger toggle -- matches the pattern used on all other public pages.
Content sections use data-program-content="kings" / data-program-content="queens" for toggle targeting -- clean and semantic.
Past tryout data matches the ticket requirements: Kings March 13 (53 players, 5 teams), Queens March 24.

BLOCKERS

1. Missing nav update on about.html

The PR body claims "Nav updated on all 7 public pages." Seven pages in the repo have the public nav (class="nav-links"): index.html, about.html, staff.html, sponsors.html, schedule.html, register.html, success.html. The diff updates 6 of these but about.html was not updated -- it still has the old 5-item nav without the Tryouts link.

This is a functional bug: a user navigating to the About page loses the Tryouts nav link. Since about.html was added by issue #91 after the original nav was established, it was likely missed during the search for files to update.

Files with nav-links class confirmed by grep: about.html, index.html, register.html, schedule.html, sponsors.html, staff.html, success.html (7 files). PR only touches 6.

2. Inline CSS in tryouts.html

The ticket context specifies "No inline CSS or JS -- everything in shared files." The PR introduces two inline style attributes in tryouts.html:

<div class="section-header" style="margin-top: var(--space-2xl);"> (Past Tryouts header)
<p style="margin-top: var(--space-md);"> (Next season contact paragraph)

While these use CSS custom properties (not raw values), they are still inline styles. The existing codebase has zero inline styles across all HTML files on main. These should be utility classes (e.g., .mt-2xl -- note .mt-md already exists in the stylesheet at line 153) or component-specific classes in shared/style.css.

NITS

Raw 1.5rem instead of var(--space-lg) in CSS: .tryout-history-card and .next-season-box both use padding: 1.5rem. The design system defines --space-lg: 1.5rem. The newer code in this codebase consistently uses tokens (19 instances of padding:.*var(--space-...) vs 4 legacy raw values). New code should prefer tokens: padding: var(--space-lg).
Forward link to non-existent teams.html: Both tryout history cards link to <a href="teams.html">View Teams</a>, but teams.html does not exist yet (it is issue #93). This is acceptable for a playground prototype, but the link will 404 until that ticket lands. Consider adding a comment in the HTML noting the dependency, or using a # placeholder until #93 ships.
rgba(212, 32, 38, 0.08) hardcoded in .next-season-box gradient: The Kings red #d42026 is rgb(212, 32, 38), so this is correct, but it is hardcoded rather than derived from --color-red. When the .queens-active class swaps --color-red to #e91e8c, this gradient background will still show Kings red at 8% opacity. This may be intentional (the gradient is subtle enough to not matter), but it is worth noting that it does not respond to the toggle.

SOP COMPLIANCE

Branch named after issue: 92-tryouts-toggle references issue #92
PR body has Summary, Changes, Test Plan, Related sections
Related section references: closes forgejo_admin/westside-app#92, foundation for #93/#94/#95, westside-playground-overhaul
No secrets committed
No unnecessary file changes -- all 9 files are directly related to the ticket scope
Commit messages are descriptive (PR title: feat: Tryouts page + Kings/Queens toggle foundation (#92))
Nav consistency gap -- about.html missed (see Blocker #1)

PROCESS OBSERVATIONS

Deployment frequency: This is a playground repo (static HTML prototypes), so the deployment risk is low. The foundation pattern (initKQToggle, .queens-active, data-program-content) is well-designed for reuse across #93, #94, #95. Good decomposition.
Change failure risk: Low. The toggle is additive (no existing behavior changed), the nav additions are safe (one <li> per file), and the CSS uses the existing variable system. The about.html gap is the only regression risk.
Documentation: README already lists tryouts.html in the route map. No README update needed.

VERDICT: NOT APPROVED

Two blockers must be resolved:

Add the Tryouts nav link to about.html (consistency with all other public pages)
Remove inline style attributes from tryouts.html -- use utility classes or shared CSS classes per the ticket requirement of no inline CSS

## PR #41 Review ### DOMAIN REVIEW **Tech stack:** Vanilla HTML/CSS/JS playground (static prototypes). No frameworks, no build tools. This is the design source of truth for westside-app SvelteKit routes. **JavaScript (`shared/app.js` -- `initKQToggle()`):** - Clean separation of concerns: `initKQToggle()` is a standalone function called from the DOMContentLoaded block, consistent with all other init functions in the file (`initFilterList`, `initTabs`, `initRegister`, `initTeams`). - Early return pattern (`if (!toggleBtns.length) return;`) is correct -- runs on every page load but exits immediately on pages without a toggle. No performance concern. - `localStorage` persistence is straightforward: reads on load, writes on click. Key name `'program'` is simple and collision-free within this domain. - `applyProgram()` inner function handles three concerns correctly: body class toggle, content section visibility, button active state. All use `classList.toggle` with the boolean second argument, which is the right API. - No XSS risk: `dataset.program` reads from static HTML `data-program` attributes. Values are never injected into innerHTML or eval'd. - No user input is involved -- all values come from hardcoded `data-program="kings"` / `data-program="queens"` attributes in the HTML. **CSS (`shared/style.css`):** - `.queens-active` overrides `--color-red` and `--color-red-hover` at the body level, which cascades correctly to all elements using those variables. Elegant approach -- the entire site color scheme pivots on two variable overrides. - Toggle button styles (`.kq-toggle`, `.kq-toggle-btn`) use design tokens (`--space-sm`, `--space-xl`, `--border-radius`, `--font-size-base`, `--transition-fast`) appropriately. - New component styles (`.tryout-history-card`, `.next-season-box`) are well-structured with clear naming. **HTML (`tryouts.html`):** - Proper document structure: `lang="en"`, viewport meta, charset, OG tags (consistent URL pattern with all other pages), favicons, external stylesheet only. - Accessibility: `aria-label="Main navigation"` on nav, `aria-expanded`/`aria-controls` on hamburger toggle -- matches the pattern used on all other public pages. - Content sections use `data-program-content="kings"` / `data-program-content="queens"` for toggle targeting -- clean and semantic. - Past tryout data matches the ticket requirements: Kings March 13 (53 players, 5 teams), Queens March 24. ### BLOCKERS **1. Missing nav update on `about.html`** The PR body claims "Nav updated on all 7 public pages." Seven pages in the repo have the public nav (`class="nav-links"`): `index.html`, `about.html`, `staff.html`, `sponsors.html`, `schedule.html`, `register.html`, `success.html`. The diff updates 6 of these but **`about.html` was not updated** -- it still has the old 5-item nav without the Tryouts link. This is a functional bug: a user navigating to the About page loses the Tryouts nav link. Since `about.html` was added by issue #91 after the original nav was established, it was likely missed during the search for files to update. Files with `nav-links` class confirmed by grep: `about.html`, `index.html`, `register.html`, `schedule.html`, `sponsors.html`, `staff.html`, `success.html` (7 files). PR only touches 6. **2. Inline CSS in `tryouts.html`** The ticket context specifies "No inline CSS or JS -- everything in shared files." The PR introduces two inline `style` attributes in `tryouts.html`: - `<div class="section-header" style="margin-top: var(--space-2xl);">` (Past Tryouts header) - `<p style="margin-top: var(--space-md);">` (Next season contact paragraph) While these use CSS custom properties (not raw values), they are still inline styles. The existing codebase has **zero** inline styles across all HTML files on main. These should be utility classes (e.g., `.mt-2xl` -- note `.mt-md` already exists in the stylesheet at line 153) or component-specific classes in `shared/style.css`. ### NITS 1. **Raw `1.5rem` instead of `var(--space-lg)` in CSS:** `.tryout-history-card` and `.next-season-box` both use `padding: 1.5rem`. The design system defines `--space-lg: 1.5rem`. The newer code in this codebase consistently uses tokens (19 instances of `padding:.*var(--space-...)` vs 4 legacy raw values). New code should prefer tokens: `padding: var(--space-lg)`. 2. **Forward link to non-existent `teams.html`:** Both tryout history cards link to `<a href="teams.html">View Teams</a>`, but `teams.html` does not exist yet (it is issue #93). This is acceptable for a playground prototype, but the link will 404 until that ticket lands. Consider adding a comment in the HTML noting the dependency, or using a `#` placeholder until #93 ships. 3. **`rgba(212, 32, 38, 0.08)` hardcoded in `.next-season-box` gradient:** The Kings red `#d42026` is `rgb(212, 32, 38)`, so this is correct, but it is hardcoded rather than derived from `--color-red`. When the `.queens-active` class swaps `--color-red` to `#e91e8c`, this gradient background will still show Kings red at 8% opacity. This may be intentional (the gradient is subtle enough to not matter), but it is worth noting that it does not respond to the toggle. ### SOP COMPLIANCE - [x] Branch named after issue: `92-tryouts-toggle` references issue #92 - [x] PR body has Summary, Changes, Test Plan, Related sections - [x] Related section references: closes `forgejo_admin/westside-app#92`, foundation for #93/#94/#95, `westside-playground-overhaul` - [x] No secrets committed - [x] No unnecessary file changes -- all 9 files are directly related to the ticket scope - [x] Commit messages are descriptive (PR title: `feat: Tryouts page + Kings/Queens toggle foundation (#92)`) - [ ] **Nav consistency gap** -- `about.html` missed (see Blocker #1) ### PROCESS OBSERVATIONS - **Deployment frequency:** This is a playground repo (static HTML prototypes), so the deployment risk is low. The foundation pattern (`initKQToggle`, `.queens-active`, `data-program-content`) is well-designed for reuse across #93, #94, #95. Good decomposition. - **Change failure risk:** Low. The toggle is additive (no existing behavior changed), the nav additions are safe (one `<li>` per file), and the CSS uses the existing variable system. The `about.html` gap is the only regression risk. - **Documentation:** README already lists `tryouts.html` in the route map. No README update needed. ### VERDICT: NOT APPROVED Two blockers must be resolved: 1. Add the Tryouts nav link to `about.html` (consistency with all other public pages) 2. Remove inline `style` attributes from `tryouts.html` -- use utility classes or shared CSS classes per the ticket requirement of no inline CSS

forgejo_admin force-pushed 92-tryouts-toggle from 32afd1ff28 to e389ecaa37

2026-03-27 00:27:23 +00:00

Compare

forgejo_admin commented

2026-03-27 00:27:36 +00:00

Author

Owner

QA Fixes Applied

Rebased on main (picked up about.html from PR #40), then fixed all blockers:

Blocker 1: about.html missing Tryouts nav link

Added <li><a href="tryouts.html">Tryouts</a></li> between Staff and Sponsors

Blocker 2: Inline style attributes in tryouts.html

Replaced style="margin-top: var(--space-2xl);" on Past Tryouts header with .mt-2xl class (new utility added to shared/style.css)
Replaced style="margin-top: var(--space-md);" on contact paragraph with existing .mt-md class

Nit: Hardcoded gradient in .next-season-box

Added --color-red-rgb: 212, 32, 38 to :root and --color-red-rgb: 233, 30, 140 to .queens-active
Changed .next-season-box gradient from rgba(212, 32, 38, 0.08) to rgba(var(--color-red-rgb), 0.08) so it responds to the Queens toggle

Bonus fix: tryouts.html nav About link pointed to index.html#about instead of about.html — updated to match canonical nav.

## QA Fixes Applied Rebased on main (picked up about.html from PR #40), then fixed all blockers: **Blocker 1: about.html missing Tryouts nav link** - Added `<li><a href="tryouts.html">Tryouts</a></li>` between Staff and Sponsors **Blocker 2: Inline style attributes in tryouts.html** - Replaced `style="margin-top: var(--space-2xl);"` on Past Tryouts header with `.mt-2xl` class (new utility added to shared/style.css) - Replaced `style="margin-top: var(--space-md);"` on contact paragraph with existing `.mt-md` class **Nit: Hardcoded gradient in .next-season-box** - Added `--color-red-rgb: 212, 32, 38` to `:root` and `--color-red-rgb: 233, 30, 140` to `.queens-active` - Changed `.next-season-box` gradient from `rgba(212, 32, 38, 0.08)` to `rgba(var(--color-red-rgb), 0.08)` so it responds to the Queens toggle **Bonus fix:** tryouts.html nav About link pointed to `index.html#about` instead of `about.html` — updated to match canonical nav.

forgejo_admin commented

2026-03-27 00:30:13 +00:00

Author

Owner

PR #41 Review (Re-review)

Re-review after fixes for 2 blockers and 1 nit from previous QA pass.

PREVIOUS FINDINGS -- VERIFICATION

All three items from the prior review are confirmed fixed:

BLOCKER (about.html missing Tryouts nav link) -- FIXED. about.html diff adds the Tryouts <li> at line 45, consistent with all other pages.
BLOCKER (inline style attributes in tryouts.html) -- FIXED. Grep for style= in tryouts.html returns zero matches. The JS applyProgram() function uses DOM manipulation (el.style.display) for show/hide, which is the correct approach.
NIT (hardcoded rgba in .next-season-box gradient) -- FIXED. New --color-red-rgb CSS variable added at :root level (212, 32, 38) with Queens override (233, 30, 140). Gradient now uses rgba(var(--color-red-rgb), 0.08) which responds to the toggle.

DOMAIN REVIEW

Tech stack: Static HTML/CSS/JS playground (vanilla, no framework). Domain checks: accessibility, CSS quality, component architecture, responsive patterns.

HTML (tryouts.html)

Proper lang="en", viewport meta, semantic sections, nav landmark with aria-label
Hamburger toggle has aria-expanded, aria-controls, aria-label -- good
OG tags and favicon links consistent with all other public pages
Route annotation comment at top follows existing codebase pattern
defer on script tag -- correct

CSS (shared/style.css)

All new styles use CSS custom properties (no hardcoded colors beyond the variable definitions)
.queens-active override cleanly swaps --color-red, --color-red-rgb, and --color-red-hover
1.5rem padding on .tryout-history-card and .next-season-box matches existing card components
New .mt-2xl utility extends the existing spacing utilities -- well placed
Toggle button styles use theme tokens throughout (--border-radius, --transition-fast, etc.)

JavaScript (shared/app.js)

initKQToggle() is well-structured: early return if no toggle buttons, localStorage persistence, clean applyProgram() inner function
Called unconditionally in DOMContentLoaded -- safe because of the early return guard
var declarations are consistent with the existing codebase style (not ES6 const/let)
dataset.programContent correctly maps to data-program-content HTML attributes

Nav consistency: Verified all 8 files with nav-links class (index.html, about.html, staff.html, tryouts.html, sponsors.html, schedule.html, register.html, success.html) -- all include the Tryouts link. No pages were missed.

BLOCKERS

None.

NITS

Forward-reference to non-existent teams.html (tryouts.html lines 100, 112): The "View Teams" links point to teams.html which does not exist yet. This is explicitly scoped as follow-up work (#93), but the links will 404 until that PR lands. Consider whether a placeholder or disabling the links until #93 merges would be better UX. Non-blocking since #93 is already tracked.
KQ toggle buttons lack ARIA semantics: The .kq-toggle div could benefit from role="group" with aria-label="Program selector", and the buttons could use aria-pressed (toggled via JS alongside the active class). Minor since this is a playground prototype, but worth carrying forward to the SvelteKit production implementation.

SOP COMPLIANCE

Branch named after issue (92-tryouts-toggle maps to westside-app#92)
PR body has Summary, Changes, Test Plan, Related sections
Related references westside-playground-overhaul (plan slug) and closes #92
No secrets committed (verified via grep)
No unnecessary file changes -- all 10 files are in scope (8 nav updates + 1 new page + CSS/JS additions)
Commit messages are descriptive

PROCESS OBSERVATIONS

Clean re-review cycle. All three findings from the previous pass were addressed correctly.
The --color-red-rgb variable approach is a thoughtful pattern that will scale well as the Queens toggle is applied to Teams (#93), Gear (#94), and Schedule (#95).
The initKQToggle() function is properly reusable -- future pages just need .kq-toggle-btn and [data-program-content] attributes with no JS changes.

VERDICT: APPROVED

## PR #41 Review (Re-review) Re-review after fixes for 2 blockers and 1 nit from previous QA pass. ### PREVIOUS FINDINGS -- VERIFICATION All three items from the prior review are confirmed fixed: 1. **BLOCKER (about.html missing Tryouts nav link)** -- FIXED. `about.html` diff adds the Tryouts `<li>` at line 45, consistent with all other pages. 2. **BLOCKER (inline style attributes in tryouts.html)** -- FIXED. Grep for `style=` in `tryouts.html` returns zero matches. The JS `applyProgram()` function uses DOM manipulation (`el.style.display`) for show/hide, which is the correct approach. 3. **NIT (hardcoded rgba in .next-season-box gradient)** -- FIXED. New `--color-red-rgb` CSS variable added at `:root` level (`212, 32, 38`) with Queens override (`233, 30, 140`). Gradient now uses `rgba(var(--color-red-rgb), 0.08)` which responds to the toggle. ### DOMAIN REVIEW **Tech stack**: Static HTML/CSS/JS playground (vanilla, no framework). Domain checks: accessibility, CSS quality, component architecture, responsive patterns. **HTML (tryouts.html)** - Proper `lang="en"`, viewport meta, semantic sections, nav landmark with `aria-label` - Hamburger toggle has `aria-expanded`, `aria-controls`, `aria-label` -- good - OG tags and favicon links consistent with all other public pages - Route annotation comment at top follows existing codebase pattern - `defer` on script tag -- correct **CSS (shared/style.css)** - All new styles use CSS custom properties (no hardcoded colors beyond the variable definitions) - `.queens-active` override cleanly swaps `--color-red`, `--color-red-rgb`, and `--color-red-hover` - `1.5rem` padding on `.tryout-history-card` and `.next-season-box` matches existing card components - New `.mt-2xl` utility extends the existing spacing utilities -- well placed - Toggle button styles use theme tokens throughout (`--border-radius`, `--transition-fast`, etc.) **JavaScript (shared/app.js)** - `initKQToggle()` is well-structured: early return if no toggle buttons, localStorage persistence, clean `applyProgram()` inner function - Called unconditionally in `DOMContentLoaded` -- safe because of the early return guard - `var` declarations are consistent with the existing codebase style (not ES6 `const`/`let`) - `dataset.programContent` correctly maps to `data-program-content` HTML attributes **Nav consistency**: Verified all 8 files with `nav-links` class (`index.html`, `about.html`, `staff.html`, `tryouts.html`, `sponsors.html`, `schedule.html`, `register.html`, `success.html`) -- all include the Tryouts link. No pages were missed. ### BLOCKERS None. ### NITS 1. **Forward-reference to non-existent `teams.html`** (`tryouts.html` lines 100, 112): The "View Teams" links point to `teams.html` which does not exist yet. This is explicitly scoped as follow-up work (#93), but the links will 404 until that PR lands. Consider whether a placeholder or disabling the links until #93 merges would be better UX. Non-blocking since #93 is already tracked. 2. **KQ toggle buttons lack ARIA semantics**: The `.kq-toggle` div could benefit from `role="group"` with `aria-label="Program selector"`, and the buttons could use `aria-pressed` (toggled via JS alongside the `active` class). Minor since this is a playground prototype, but worth carrying forward to the SvelteKit production implementation. ### SOP COMPLIANCE - [x] Branch named after issue (`92-tryouts-toggle` maps to westside-app#92) - [x] PR body has Summary, Changes, Test Plan, Related sections - [x] Related references `westside-playground-overhaul` (plan slug) and closes #92 - [x] No secrets committed (verified via grep) - [x] No unnecessary file changes -- all 10 files are in scope (8 nav updates + 1 new page + CSS/JS additions) - [x] Commit messages are descriptive ### PROCESS OBSERVATIONS - Clean re-review cycle. All three findings from the previous pass were addressed correctly. - The `--color-red-rgb` variable approach is a thoughtful pattern that will scale well as the Queens toggle is applied to Teams (#93), Gear (#94), and Schedule (#95). - The `initKQToggle()` function is properly reusable -- future pages just need `.kq-toggle-btn` and `[data-program-content]` attributes with no JS changes. ### VERDICT: APPROVED