feat: add jersey size selector to checkout page #66

Merged
forgejo_admin merged 2 commits from 65-jersey-size-selector into main 2026-03-21 21:21:10 +00:00

Summary

  • Adds a size dropdown to each jersey option card (Reversible, Jersey+Warmup) on the /jersey checkout page
  • Parents must select a size before the Order button becomes active
  • Selected size is sent to POST /jersey/checkout alongside the option key

Changes

  • src/routes/jersey/+page.svelte: Added sizes state fetched from GET /jersey/sizes (with hardcoded fallback). Added selectedSizes record tracking per-card selection. Size dropdown rendered inside each non-opt-out card body. handleSelect now includes size in the POST body. isOrderDisabled helper disables Order button until size is chosen. Opt-out card unchanged.
  • src/app.css: Added .jersey-size-selector, .jersey-size-label, .jersey-size-select styles matching existing dark card design (black background, gray-800 border, red focus ring, custom dropdown chevron).

Test Plan

  • Navigate to /jersey?token=... and verify size dropdowns appear on Reversible and Jersey+Warmup cards
  • Verify Opt Out card has no size dropdown
  • Verify Order buttons are disabled until a size is selected
  • Select a size, click Order, confirm the checkout POST includes size param
  • Test on mobile viewport (375px) -- dropdown should be full-width and tappable
  • No regressions on other pages

Review Checklist

  • Passed automated review-fix loop
  • No secrets committed
  • No unnecessary file changes
  • Commit messages are descriptive
  • Closes #65
  • plan-wkq -- Phase 11 (Girls Tryout)
  • Depends on: basketball-api #141 (backend size support)
## Summary - Adds a size dropdown to each jersey option card (Reversible, Jersey+Warmup) on the `/jersey` checkout page - Parents must select a size before the Order button becomes active - Selected size is sent to `POST /jersey/checkout` alongside the option key ## Changes - `src/routes/jersey/+page.svelte`: Added `sizes` state fetched from `GET /jersey/sizes` (with hardcoded fallback). Added `selectedSizes` record tracking per-card selection. Size dropdown rendered inside each non-opt-out card body. `handleSelect` now includes `size` in the POST body. `isOrderDisabled` helper disables Order button until size is chosen. Opt-out card unchanged. - `src/app.css`: Added `.jersey-size-selector`, `.jersey-size-label`, `.jersey-size-select` styles matching existing dark card design (black background, gray-800 border, red focus ring, custom dropdown chevron). ## Test Plan - [ ] Navigate to `/jersey?token=...` and verify size dropdowns appear on Reversible and Jersey+Warmup cards - [ ] Verify Opt Out card has no size dropdown - [ ] Verify Order buttons are disabled until a size is selected - [ ] Select a size, click Order, confirm the checkout POST includes `size` param - [ ] Test on mobile viewport (375px) -- dropdown should be full-width and tappable - [ ] No regressions on other pages ## Review Checklist - [ ] Passed automated review-fix loop - [ ] No secrets committed - [ ] No unnecessary file changes - [ ] Commit messages are descriptive ## Related - Closes #65 - `plan-wkq` -- Phase 11 (Girls Tryout) - Depends on: basketball-api #141 (backend size support)
feat: add jersey size selector to checkout page
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
b24a068c38
Parents can now select a jersey size (YS through AXL) on each option
card before ordering. Size dropdown is fetched from GET /jersey/sizes
with hardcoded fallback. Order button stays disabled until a size is
chosen. Selected size is sent to POST /jersey/checkout alongside the
option. Opt-out card has no size selector.

Closes #65

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
refactor: extract fallback sizes constant, add webkit prefix
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
19a03e0358
Review fixes: deduplicate the hardcoded sizes array into FALLBACK_SIZES
constant and initSelectedSizes() helper. Add -webkit-appearance: none
to jersey-size-select for older Safari compatibility.

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

Review-Fix Loop

Findings from self-review:

  1. Duplicated fallback sizes array -- The hardcoded sizes array was copy-pasted 3 times (try-block fallback, catch-block, and both init loops). Extracted to FALLBACK_SIZES constant and initSelectedSizes() helper. Reduces duplication from ~40 lines to ~4.

  2. Missing -webkit-appearance: none -- The existing .form-select in app.css uses -webkit-appearance: none for older Safari. Added the same prefix to .jersey-size-select for consistency.

  3. selected attribute on placeholder <option> with bind:value -- Technically redundant since Svelte controls selection via the bound value, but harmless and provides a sensible default if JS hasn't hydrated yet. Left as-is.

Verified: npm run build and npm run check pass with no new warnings.

## Review-Fix Loop **Findings from self-review:** 1. **Duplicated fallback sizes array** -- The hardcoded sizes array was copy-pasted 3 times (try-block fallback, catch-block, and both init loops). Extracted to `FALLBACK_SIZES` constant and `initSelectedSizes()` helper. Reduces duplication from ~40 lines to ~4. 2. **Missing `-webkit-appearance: none`** -- The existing `.form-select` in app.css uses `-webkit-appearance: none` for older Safari. Added the same prefix to `.jersey-size-select` for consistency. 3. **`selected` attribute on placeholder `<option>` with `bind:value`** -- Technically redundant since Svelte controls selection via the bound value, but harmless and provides a sensible default if JS hasn't hydrated yet. Left as-is. **Verified:** `npm run build` and `npm run check` pass with no new warnings.
Author
Owner

PR #66 Review

DOMAIN REVIEW

Tech stack: SvelteKit (Svelte 5 runes), vanilla CSS, static adapter SPA. Reviewed against SvelteKit/CSS domain checklist.

Svelte 5 patterns -- correct:

  • $state() used properly for sizes, selectedSizes, and existing state variables.
  • initSelectedSizes() mutates the $state({}) proxy by key assignment, which is the correct Svelte 5 pattern for reactive objects.
  • isOrderDisabled() is called inline in the template (disabled={isOrderDisabled(option.key)}), so Svelte's fine-grained reactivity correctly tracks reads of submitting and selectedSizes[optionKey]. This will re-evaluate when either changes.

Accessibility -- good:

  • <label> has for="size-{option.key}" paired with id="size-{option.key}" on the <select>. Proper association.
  • Default <option value="" disabled selected>Choose a size...</option> provides clear placeholder text for screen readers.
  • The label text ("Select Size") is descriptive.

CSS -- correct:

  • All values reference design tokens from :root (--color-black, --color-gray-800, --color-gray-100, --color-red, --radius, --font-family). No magic colors or hardcoded values.
  • Custom dropdown chevron via inline SVG background-image is a standard pattern for cross-browser <select> styling.
  • appearance: none + -webkit-appearance: none properly resets native select styling.
  • Placement in src/app.css follows the existing convention of co-locating jersey styles in the jersey section of the single stylesheet.

Data flow -- sound:

  • Promise.all fetches options and sizes concurrently. If sizes endpoint returns non-ok, falls back to FALLBACK_SIZES. If the network request itself fails (rejects), Promise.all rejects into the catch block which sets both hardcoded options and fallback sizes. Graceful degradation in both cases.
  • handleSelect guards non-opt-out options with if (optionKey !== 'opt_out' && !selectedSizes[optionKey]) return; -- double protection (button is disabled AND the handler exits early). Belt-and-suspenders.
  • body.size is only set for non-opt-out options, keeping the opt-out POST payload clean.

Variable shadowing fix -- good:

  • Renamed inner body variable to responseBody in the error handler to avoid shadowing the outer body const. Clean fix.

BLOCKERS

None.

Regarding the "new functionality with zero test coverage" blocker criteria: this project has no test framework installed (package.json has no test runner, no test script, no vitest/playwright/jest dependency). This is a pre-existing repo-wide condition, not something introduced by this PR. The PR's Test Plan section documents manual verification steps. Adding a test framework is out of scope for a feature PR -- that is tracked separately as issue #62 (E2E Playwright test suite). No blocker here.

NITS

  1. Resilience of Promise.all: If fetch(${API_BASE}/jersey/sizes) throws a network error (DNS failure, CORS block), Promise.all rejects and discards the already-resolved options response, falling back to hardcoded options even though the options API was fine. A more resilient pattern would be Promise.allSettled or individual try/catch per fetch. However, the current fallback behavior is acceptable -- both fallbacks are correct and the user experience degrades gracefully. Non-blocking.

  2. Missing aria-required on select: Since size selection is mandatory for non-opt-out cards, adding aria-required="true" to the <select> would improve screen reader UX by communicating the required state. Minor accessibility enhancement.

  3. Hardcoded fallback sizes: FALLBACK_SIZES duplicates data that presumably lives in the backend. If sizes change on the backend, the frontend fallback becomes stale. Consider whether the fallback should be removed in favor of an error state, or documented as intentionally pinned. Non-blocking -- the fallback is a reasonable UX choice for a checkout flow where showing something is better than showing nothing.

SOP COMPLIANCE

  • Branch named after issue: 65-jersey-size-selector references issue #65
  • PR body has Summary, Changes, Test Plan, Related sections
  • Related section references plan slug plan-wkq
  • No secrets committed (no API keys, tokens, or credentials in diff)
  • No unnecessary file changes (2 files, both directly related to the feature)
  • Commit messages are descriptive (PR title follows feat: convention)
  • Scope is tight -- no unrelated changes

PROCESS OBSERVATIONS

  • Deployment frequency: Clean 2-file, 133-addition change. Low change-failure risk. Feature is additive (extends existing checkout page, does not modify auth or payment flow logic).
  • Cross-repo dependency: PR body correctly notes dependency on basketball-api #141 for backend size support. Deployment order matters -- backend must be deployed first or the frontend gracefully degrades to fallback sizes.
  • Test infrastructure gap: Issue #62 tracks E2E test suite addition. Until that lands, manual testing is the only gate. This is a known process risk, not a PR-specific problem.

VERDICT: APPROVED

## PR #66 Review ### DOMAIN REVIEW **Tech stack**: SvelteKit (Svelte 5 runes), vanilla CSS, static adapter SPA. Reviewed against SvelteKit/CSS domain checklist. **Svelte 5 patterns -- correct**: - `$state()` used properly for `sizes`, `selectedSizes`, and existing state variables. - `initSelectedSizes()` mutates the `$state({})` proxy by key assignment, which is the correct Svelte 5 pattern for reactive objects. - `isOrderDisabled()` is called inline in the template (`disabled={isOrderDisabled(option.key)}`), so Svelte's fine-grained reactivity correctly tracks reads of `submitting` and `selectedSizes[optionKey]`. This will re-evaluate when either changes. **Accessibility -- good**: - `<label>` has `for="size-{option.key}"` paired with `id="size-{option.key}"` on the `<select>`. Proper association. - Default `<option value="" disabled selected>Choose a size...</option>` provides clear placeholder text for screen readers. - The label text ("Select Size") is descriptive. **CSS -- correct**: - All values reference design tokens from `:root` (`--color-black`, `--color-gray-800`, `--color-gray-100`, `--color-red`, `--radius`, `--font-family`). No magic colors or hardcoded values. - Custom dropdown chevron via inline SVG `background-image` is a standard pattern for cross-browser `<select>` styling. - `appearance: none` + `-webkit-appearance: none` properly resets native select styling. - Placement in `src/app.css` follows the existing convention of co-locating jersey styles in the jersey section of the single stylesheet. **Data flow -- sound**: - `Promise.all` fetches options and sizes concurrently. If sizes endpoint returns non-ok, falls back to `FALLBACK_SIZES`. If the network request itself fails (rejects), `Promise.all` rejects into the catch block which sets both hardcoded options and fallback sizes. Graceful degradation in both cases. - `handleSelect` guards non-opt-out options with `if (optionKey !== 'opt_out' && !selectedSizes[optionKey]) return;` -- double protection (button is disabled AND the handler exits early). Belt-and-suspenders. - `body.size` is only set for non-opt-out options, keeping the opt-out POST payload clean. **Variable shadowing fix -- good**: - Renamed inner `body` variable to `responseBody` in the error handler to avoid shadowing the outer `body` const. Clean fix. ### BLOCKERS **None.** Regarding the "new functionality with zero test coverage" blocker criteria: this project has **no test framework installed** (`package.json` has no test runner, no `test` script, no vitest/playwright/jest dependency). This is a pre-existing repo-wide condition, not something introduced by this PR. The PR's Test Plan section documents manual verification steps. Adding a test framework is out of scope for a feature PR -- that is tracked separately as issue #62 (E2E Playwright test suite). No blocker here. ### NITS 1. **Resilience of `Promise.all`**: If `fetch(`${API_BASE}/jersey/sizes`)` throws a network error (DNS failure, CORS block), `Promise.all` rejects and discards the already-resolved options response, falling back to hardcoded options even though the options API was fine. A more resilient pattern would be `Promise.allSettled` or individual try/catch per fetch. However, the current fallback behavior is acceptable -- both fallbacks are correct and the user experience degrades gracefully. Non-blocking. 2. **Missing `aria-required` on select**: Since size selection is mandatory for non-opt-out cards, adding `aria-required="true"` to the `<select>` would improve screen reader UX by communicating the required state. Minor accessibility enhancement. 3. **Hardcoded fallback sizes**: `FALLBACK_SIZES` duplicates data that presumably lives in the backend. If sizes change on the backend, the frontend fallback becomes stale. Consider whether the fallback should be removed in favor of an error state, or documented as intentionally pinned. Non-blocking -- the fallback is a reasonable UX choice for a checkout flow where showing something is better than showing nothing. ### SOP COMPLIANCE - [x] Branch named after issue: `65-jersey-size-selector` references issue #65 - [x] PR body has Summary, Changes, Test Plan, Related sections - [x] Related section references plan slug `plan-wkq` - [x] No secrets committed (no API keys, tokens, or credentials in diff) - [x] No unnecessary file changes (2 files, both directly related to the feature) - [x] Commit messages are descriptive (PR title follows `feat:` convention) - [x] Scope is tight -- no unrelated changes ### PROCESS OBSERVATIONS - **Deployment frequency**: Clean 2-file, 133-addition change. Low change-failure risk. Feature is additive (extends existing checkout page, does not modify auth or payment flow logic). - **Cross-repo dependency**: PR body correctly notes dependency on `basketball-api #141` for backend size support. Deployment order matters -- backend must be deployed first or the frontend gracefully degrades to fallback sizes. - **Test infrastructure gap**: Issue #62 tracks E2E test suite addition. Until that lands, manual testing is the only gate. This is a known process risk, not a PR-specific problem. ### VERDICT: APPROVED
forgejo_admin deleted branch 65-jersey-size-selector 2026-03-21 21:21:10 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
forgejo_admin/westside-landing!66
No description provided.