feat: add jersey size selector to checkout page #66
No reviewers
Labels
No labels
domain:backend
domain:devops
domain:frontend
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
forgejo_admin/westside-landing!66
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "65-jersey-size-selector"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
/jerseycheckout pagePOST /jersey/checkoutalongside the option keyChanges
src/routes/jersey/+page.svelte: Addedsizesstate fetched fromGET /jersey/sizes(with hardcoded fallback). AddedselectedSizesrecord tracking per-card selection. Size dropdown rendered inside each non-opt-out card body.handleSelectnow includessizein the POST body.isOrderDisabledhelper disables Order button until size is chosen. Opt-out card unchanged.src/app.css: Added.jersey-size-selector,.jersey-size-label,.jersey-size-selectstyles matching existing dark card design (black background, gray-800 border, red focus ring, custom dropdown chevron).Test Plan
/jersey?token=...and verify size dropdowns appear on Reversible and Jersey+Warmup cardssizeparamReview Checklist
Related
plan-wkq-- Phase 11 (Girls Tryout)Review-Fix Loop
Findings from self-review:
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_SIZESconstant andinitSelectedSizes()helper. Reduces duplication from ~40 lines to ~4.Missing
-webkit-appearance: none-- The existing.form-selectin app.css uses-webkit-appearance: nonefor older Safari. Added the same prefix to.jersey-size-selectfor consistency.selectedattribute on placeholder<option>withbind: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 buildandnpm run checkpass with no new warnings.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 forsizes,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 ofsubmittingandselectedSizes[optionKey]. This will re-evaluate when either changes.Accessibility -- good:
<label>hasfor="size-{option.key}"paired withid="size-{option.key}"on the<select>. Proper association.<option value="" disabled selected>Choose a size...</option>provides clear placeholder text for screen readers.CSS -- correct:
:root(--color-black,--color-gray-800,--color-gray-100,--color-red,--radius,--font-family). No magic colors or hardcoded values.background-imageis a standard pattern for cross-browser<select>styling.appearance: none+-webkit-appearance: noneproperly resets native select styling.src/app.cssfollows the existing convention of co-locating jersey styles in the jersey section of the single stylesheet.Data flow -- sound:
Promise.allfetches options and sizes concurrently. If sizes endpoint returns non-ok, falls back toFALLBACK_SIZES. If the network request itself fails (rejects),Promise.allrejects into the catch block which sets both hardcoded options and fallback sizes. Graceful degradation in both cases.handleSelectguards non-opt-out options withif (optionKey !== 'opt_out' && !selectedSizes[optionKey]) return;-- double protection (button is disabled AND the handler exits early). Belt-and-suspenders.body.sizeis only set for non-opt-out options, keeping the opt-out POST payload clean.Variable shadowing fix -- good:
bodyvariable toresponseBodyin the error handler to avoid shadowing the outerbodyconst. Clean fix.BLOCKERS
None.
Regarding the "new functionality with zero test coverage" blocker criteria: this project has no test framework installed (
package.jsonhas no test runner, notestscript, 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
Resilience of
Promise.all: Iffetch(${API_BASE}/jersey/sizes)throws a network error (DNS failure, CORS block),Promise.allrejects and discards the already-resolved options response, falling back to hardcoded options even though the options API was fine. A more resilient pattern would bePromise.allSettledor individual try/catch per fetch. However, the current fallback behavior is acceptable -- both fallbacks are correct and the user experience degrades gracefully. Non-blocking.Missing
aria-requiredon select: Since size selection is mandatory for non-opt-out cards, addingaria-required="true"to the<select>would improve screen reader UX by communicating the required state. Minor accessibility enhancement.Hardcoded fallback sizes:
FALLBACK_SIZESduplicates 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
65-jersey-size-selectorreferences issue #65plan-wkqfeat:convention)PROCESS OBSERVATIONS
basketball-api #141for backend size support. Deployment order matters -- backend must be deployed first or the frontend gracefully degrades to fallback sizes.VERDICT: APPROVED