feat: parent dashboard shows player team schedule #218

Merged

forgejo_admin merged 1 commit from 214-parent-dashboard-schedule into main 2026-04-04 22:38:09 +00:00

Conversation 1 Commits 1 Files changed 1 +363 -11

forgejo_admin commented 2026-04-04 22:01:35 +00:00

Contributor

Copy link

Summary

Replaces the placeholder text "Schedule data will appear here once practice times are confirmed" on the parent dashboard (/my-players) with a functional schedule display. Fetches /account/players (with team_ids from basketball-api#334) and /public/schedule in parallel, filters practices and events by each player's team_ids, and groups the display by player name then by team.

Changes

• src/routes/(app)/my-players/+page.svelte — Added publicFetch import, schedule state variables, buildPlayerSchedules() grouping logic, time/date formatting helpers, and replaced the placeholder section with practice cards (green left border) and event cards (blue left border) grouped per player per team. Scoped CSS for the schedule components using the existing design system variables.

Test Plan

• svelte-check passes with 0 errors (7 pre-existing warnings, none in modified file)
• vite build succeeds
• Parent with 1 player on 1 team sees practice schedule with day, time, venue
• Parent with multiple players across multiple teams (Sandra Apaisa edge case: 3 players, 2 teams) sees all schedules grouped by player then team
• Events filtered by team_id match first, division fallback second
• Times display in 12-hour format with AM/PM
• Mock mode (?mock=offered) still works with team_ids added to mock data
• Schedule section shows loading state, error state, and empty state gracefully

Review Checklist

• svelte-check passes with 0 errors
• vite build succeeds
• Pure CSS vars, no Tailwind
• No backend changes — pure frontend, client-side filtering
• Placeholder text removed
• Schedule grouped by player, then by team
• 12-hour time format with AM/PM
• Division fallback for events without team_id

Related

• Closes #214
• basketball-api#333 / PR #334 (add team_ids to AccountPlayerResponse — merged)
• westside-landing#213 (coach dashboard schedule — sibling ticket)

Related Notes

• None

## Summary Replaces the placeholder text "Schedule data will appear here once practice times are confirmed" on the parent dashboard (`/my-players`) with a functional schedule display. Fetches `/account/players` (with `team_ids` from basketball-api#334) and `/public/schedule` in parallel, filters practices and events by each player's team_ids, and groups the display by player name then by team. ## Changes - `src/routes/(app)/my-players/+page.svelte` — Added `publicFetch` import, schedule state variables, `buildPlayerSchedules()` grouping logic, time/date formatting helpers, and replaced the placeholder section with practice cards (green left border) and event cards (blue left border) grouped per player per team. Scoped CSS for the schedule components using the existing design system variables. ## Test Plan - `svelte-check` passes with 0 errors (7 pre-existing warnings, none in modified file) - `vite build` succeeds - Parent with 1 player on 1 team sees practice schedule with day, time, venue - Parent with multiple players across multiple teams (Sandra Apaisa edge case: 3 players, 2 teams) sees all schedules grouped by player then team - Events filtered by team_id match first, division fallback second - Times display in 12-hour format with AM/PM - Mock mode (`?mock=offered`) still works with team_ids added to mock data - Schedule section shows loading state, error state, and empty state gracefully ## Review Checklist - [x] `svelte-check` passes with 0 errors - [x] `vite build` succeeds - [x] Pure CSS vars, no Tailwind - [x] No backend changes — pure frontend, client-side filtering - [x] Placeholder text removed - [x] Schedule grouped by player, then by team - [x] 12-hour time format with AM/PM - [x] Division fallback for events without team_id ## Related - Closes #214 - basketball-api#333 / PR #334 (add `team_ids` to `AccountPlayerResponse` — merged) - westside-landing#213 (coach dashboard schedule — sibling ticket) ## Related Notes - None

forgejo_admin added 1 commit 2026-04-04 22:01:35 +00:00

feat: replace parent dashboard schedule placeholder with functional display ... 6c43fe9850

Fetch /account/players (with team_ids from basketball-api#334) and
/public/schedule in parallel, then filter practices and events by each
player's team_ids. Groups schedule by player name, then by team.
Handles multi-player parents like Sandra Apaisa (3 players, 2 teams).

Closes #214

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

forgejo_admin commented 2026-04-04 22:03:55 +00:00

Author

Contributor

Copy link

PR #218 Review

DOMAIN REVIEW

Stack: SvelteKit / Svelte 5 ($state/$derived runes) / Pure CSS vars

Architecture: Single-file change to src/routes/(app)/my-players/+page.svelte (+363/-11). Adds schedule fetching via publicFetch, client-side filtering/grouping by player->team, and schedule card rendering with scoped CSS.

What works well:

• Promise.allSettled for parallel fetching with independent error handling -- good resilience pattern
• $derived for playerSchedules -- reactive recomputation when state changes, idiomatic Svelte 5
• Division fallback logic for events without team_id is well-thought-out with dedup guard (!existing.events.some(...))
• All three display states covered: loading, error, empty
• JSDoc type annotations throughout
• CSS uses design system vars consistently (--space-*, --color-*, --border-radius) with no Tailwind
• Green/blue left-border convention for practice vs event cards is a clear visual distinction
• formatTime handles edge cases: null/undefined input, already-formatted AM/PM strings, midnight (h=0 -> 12 AM)

Concerns:

1. DRY violation across sibling PR #217: The following functions are copy-pasted nearly verbatim between this PR and PR #217 (coach dashboard):

   • DAY_NAMES constant
   • formatTime() -- identical logic
   • dayName() -- identical logic
   • formatDate() -- nearly identical (only difference: parent includes weekday: 'short' in locale options)
   • formatDateRange() -- nearly identical (parent uses en-dash, coach uses hyphen)
   • sortEventsByDate() -- identical logic

   These should be extracted to a shared $lib/schedule-utils.js module. Six duplicated functions across two files is a maintenance divergence risk. However, this is not in an auth/security path, so it does not meet the BLOCKER threshold for DRY. It is a strong nit that should be tracked as a follow-up issue.

2. Double sort in template: sortPracticesByDay(team.practices) and sortEventsByDate(team.events) are called directly in {#each} blocks. These will re-execute on every render cycle. Consider pre-sorting inside buildPlayerSchedules or using $derived wrappers. Not a blocker for a dashboard page with small data sets, but worth noting.

3. margin-top: 1px and margin-top: 2px magic numbers: Lines in .parent-schedule-event-date, .parent-schedule-location, and .parent-schedule-notes use pixel values instead of spacing tokens. Minor inconsistency with the otherwise clean token usage.

4. Accessibility: Schedule cards lack ARIA semantics. Practice and event cards are purely visual <div> elements. Consider role="list" on .parent-schedule-cards and role="listitem" on each card for screen reader navigation. The color-coded left borders (green vs blue) carry meaning without a text-based alternative for colorblind users -- the "Practice" / "Events" sub-labels mitigate this, but adding aria-label on cards would strengthen it.

5. formatDate timezone nuance: new Date(dateStr + 'T00:00:00') creates a local-timezone date, which is correct for display purposes. Just noting this is intentional and works because all dates are local basketball schedule dates.

BLOCKERS

None. No unvalidated user input (data comes from authenticated API and public API, rendered as text content not innerHTML). No secrets. No auth/security path duplication. Test plan documented and builds pass per PR body.

Note on test coverage: This is a pure frontend SvelteKit page with no unit tests. The westside-landing repo appears to not have a test infrastructure (no test runner configured). Per the repo's conventions, svelte-check and vite build passing constitutes the test gate. The helper functions (formatTime, dayName, buildPlayerSchedules) would benefit from unit tests, but this is a repo-level gap, not a PR-level blocker.

NITS

1. Extract shared schedule utilities: DAY_NAMES, formatTime, dayName, formatDate, formatDateRange, sortEventsByDate are duplicated with PR #217. Extract to $lib/schedule-utils.js. Track as a follow-up issue.

2. Pre-sort instead of sort-in-template: Move sortPracticesByDay and sortEventsByDate into buildPlayerSchedules to avoid re-sorting on each render.

3. Replace magic pixel values: margin-top: 1px and margin-top: 2px in schedule card styles should use var(--space-xs) or a smaller token for consistency.

4. Minor en-dash inconsistency: PR #218 uses \u2013 (en-dash) in formatDateRange, while PR #217 uses - (hyphen). Extracting to shared utils would fix this naturally.

5. Accessibility: Add role="list" / role="listitem" to schedule card containers. Consider aria-label on cards summarizing the schedule item for screen readers.

SOP COMPLIANCE

• Branch named after issue: 214-parent-dashboard-schedule matches issue #214
• PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related all present
• Related references parent issue: "Closes #214" and cross-references basketball-api#333/#334 and sibling #213
• Related references plan slug: No plan slug referenced. PR body references story: labels only in sibling PR #217, not in this one. Minor gap.
• No secrets committed
• No scope creep: single file change, tightly scoped to parent dashboard schedule
• svelte-check and vite build pass per PR body

PROCESS OBSERVATIONS

• Deployment frequency: Clean single-file change with no backend dependency changes. Safe to deploy independently. basketball-api#334 (team_ids) is already merged, so no ordering concern.
• Change failure risk: Low. Pure frontend, client-side filtering only. Promise.allSettled means schedule failure does not break player loading. Graceful degradation built in.
• Documentation gap: The DRY duplication across #217 and #218 should be tracked as a follow-up issue for $lib/schedule-utils.js extraction before more dashboard pages adopt the same pattern.

VERDICT: APPROVED

Clean, well-structured implementation. No blockers. The DRY concern across sibling PRs is real but non-blocking -- track as a follow-up issue for shared utility extraction before the pattern spreads further.

## PR #218 Review ### DOMAIN REVIEW **Stack**: SvelteKit / Svelte 5 ($state/$derived runes) / Pure CSS vars **Architecture**: Single-file change to `src/routes/(app)/my-players/+page.svelte` (+363/-11). Adds schedule fetching via `publicFetch`, client-side filtering/grouping by player->team, and schedule card rendering with scoped CSS. **What works well**: - `Promise.allSettled` for parallel fetching with independent error handling -- good resilience pattern - `$derived` for `playerSchedules` -- reactive recomputation when state changes, idiomatic Svelte 5 - Division fallback logic for events without `team_id` is well-thought-out with dedup guard (`!existing.events.some(...)`) - All three display states covered: loading, error, empty - JSDoc type annotations throughout - CSS uses design system vars consistently (`--space-*`, `--color-*`, `--border-radius`) with no Tailwind - Green/blue left-border convention for practice vs event cards is a clear visual distinction - `formatTime` handles edge cases: null/undefined input, already-formatted AM/PM strings, midnight (h=0 -> 12 AM) **Concerns**: 1. **DRY violation across sibling PR #217**: The following functions are copy-pasted nearly verbatim between this PR and PR #217 (coach dashboard): - `DAY_NAMES` constant - `formatTime()` -- identical logic - `dayName()` -- identical logic - `formatDate()` -- nearly identical (only difference: parent includes `weekday: 'short'` in locale options) - `formatDateRange()` -- nearly identical (parent uses en-dash, coach uses hyphen) - `sortEventsByDate()` -- identical logic These should be extracted to a shared `$lib/schedule-utils.js` module. Six duplicated functions across two files is a maintenance divergence risk. **However**, this is not in an auth/security path, so it does not meet the BLOCKER threshold for DRY. It is a strong nit that should be tracked as a follow-up issue. 2. **Double sort in template**: `sortPracticesByDay(team.practices)` and `sortEventsByDate(team.events)` are called directly in `{#each}` blocks. These will re-execute on every render cycle. Consider pre-sorting inside `buildPlayerSchedules` or using `$derived` wrappers. Not a blocker for a dashboard page with small data sets, but worth noting. 3. **`margin-top: 1px` and `margin-top: 2px` magic numbers**: Lines in `.parent-schedule-event-date`, `.parent-schedule-location`, and `.parent-schedule-notes` use pixel values instead of spacing tokens. Minor inconsistency with the otherwise clean token usage. 4. **Accessibility**: Schedule cards lack ARIA semantics. Practice and event cards are purely visual `<div>` elements. Consider `role="list"` on `.parent-schedule-cards` and `role="listitem"` on each card for screen reader navigation. The color-coded left borders (green vs blue) carry meaning without a text-based alternative for colorblind users -- the "Practice" / "Events" sub-labels mitigate this, but adding `aria-label` on cards would strengthen it. 5. **`formatDate` timezone nuance**: `new Date(dateStr + 'T00:00:00')` creates a local-timezone date, which is correct for display purposes. Just noting this is intentional and works because all dates are local basketball schedule dates. ### BLOCKERS None. No unvalidated user input (data comes from authenticated API and public API, rendered as text content not innerHTML). No secrets. No auth/security path duplication. Test plan documented and builds pass per PR body. **Note on test coverage**: This is a pure frontend SvelteKit page with no unit tests. The westside-landing repo appears to not have a test infrastructure (no test runner configured). Per the repo's conventions, `svelte-check` and `vite build` passing constitutes the test gate. The helper functions (`formatTime`, `dayName`, `buildPlayerSchedules`) would benefit from unit tests, but this is a repo-level gap, not a PR-level blocker. ### NITS 1. **Extract shared schedule utilities**: `DAY_NAMES`, `formatTime`, `dayName`, `formatDate`, `formatDateRange`, `sortEventsByDate` are duplicated with PR #217. Extract to `$lib/schedule-utils.js`. Track as a follow-up issue. 2. **Pre-sort instead of sort-in-template**: Move `sortPracticesByDay` and `sortEventsByDate` into `buildPlayerSchedules` to avoid re-sorting on each render. 3. **Replace magic pixel values**: `margin-top: 1px` and `margin-top: 2px` in schedule card styles should use `var(--space-xs)` or a smaller token for consistency. 4. **Minor en-dash inconsistency**: PR #218 uses `\u2013` (en-dash) in `formatDateRange`, while PR #217 uses `-` (hyphen). Extracting to shared utils would fix this naturally. 5. **Accessibility**: Add `role="list"` / `role="listitem"` to schedule card containers. Consider `aria-label` on cards summarizing the schedule item for screen readers. ### SOP COMPLIANCE - [x] Branch named after issue: `214-parent-dashboard-schedule` matches issue #214 - [x] PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related all present - [x] Related references parent issue: "Closes #214" and cross-references basketball-api#333/#334 and sibling #213 - [ ] Related references plan slug: No plan slug referenced. PR body references `story:` labels only in sibling PR #217, not in this one. Minor gap. - [x] No secrets committed - [x] No scope creep: single file change, tightly scoped to parent dashboard schedule - [x] `svelte-check` and `vite build` pass per PR body ### PROCESS OBSERVATIONS - **Deployment frequency**: Clean single-file change with no backend dependency changes. Safe to deploy independently. basketball-api#334 (team_ids) is already merged, so no ordering concern. - **Change failure risk**: Low. Pure frontend, client-side filtering only. `Promise.allSettled` means schedule failure does not break player loading. Graceful degradation built in. - **Documentation gap**: The DRY duplication across #217 and #218 should be tracked as a follow-up issue for `$lib/schedule-utils.js` extraction before more dashboard pages adopt the same pattern. ### VERDICT: APPROVED Clean, well-structured implementation. No blockers. The DRY concern across sibling PRs is real but non-blocking -- track as a follow-up issue for shared utility extraction before the pattern spreads further.

forgejo_admin force-pushed 214-parent-dashboard-schedule from 6c43fe9850 to 571dbc5835 2026-04-04 22:35:16 +00:00 Compare

forgejo_admin merged commit 282741ec7a into main 2026-04-04 22:38:09 +00:00

forgejo_admin deleted branch 214-parent-dashboard-schedule 2026-04-04 22:38:09 +00:00

forgejo_admin referenced this pull request from a commit 2026-04-04 22:38:11 +00:00

feat: parent dashboard shows player team schedule (#218)

forgejo_admin referenced this pull request 2026-04-04 22:55:17 +00:00

Extract shared schedule utilities to $lib/schedule-utils.js #219

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

domain:backend

  

domain:devops

  

domain:frontend

No labels

domain:backend

domain:devops

domain:frontend

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

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

ldraney/westside-app!218

No description provided.