From b1f69b383121bd16438595bda93f1c02fcfd56f6 Mon Sep 17 00:00:00 2001 From: Robert Hamilton Date: Tue, 21 Apr 2026 11:38:37 -0500 Subject: [PATCH 1/7] refactor(workout): encapsulate modal state inside StrengthWorkoutView (P22-014) showAddExercise, showDiscardDialog, and pageError were owned by the parent ActiveWorkoutPage and prop-threaded into StrengthWorkoutView despite being exclusively consumed there. Move them to local useState inside the component. showDiscardDialog and pageError are retained at the parent level only for EventWorkoutView, which also consumes them. --- Context/Backlog/Ideas.md | 14 +++++++++++--- src/components/workout/strength-workout-view.tsx | 16 ++++------------ src/routes/_authenticated/log.$workoutId.tsx | 8 -------- 3 files changed, 15 insertions(+), 23 deletions(-) diff --git a/Context/Backlog/Ideas.md b/Context/Backlog/Ideas.md index 760113c..0f6358b 100644 --- a/Context/Backlog/Ideas.md +++ b/Context/Backlog/Ideas.md @@ -105,8 +105,16 @@ Two related UX gaps in the workout logger: **Related:** Exercise picker/search component, workout history data, active workout flow **Priority:** Medium -### Split log.$workoutId route into three components +### P22-014: Move view-local modal state into StrengthWorkoutView + +**Added:** 2026-04-21 +**Source:** `Context/Reviews/` P22 review findings (PR #115) +**Priority:** Medium +**Context:** Modal state currently lives outside `StrengthWorkoutView`. Moving it inside the component would tighten encapsulation and reduce prop-threading into the view. +**Related:** `src/routes/_authenticated/log.$workoutId/strength-workout-view.tsx` + +### ~~Split log.$workoutId route into three components~~ (Resolved 2026-04-21) + **Added:** 2026-04-06 -**Context:** `src/routes/_authenticated/log.$workoutId.tsx` is ~800 lines with three distinct rendering paths (event workout, strength workout, post-workout summary) toggled by conditional branches. Splitting into separate route components would clarify ownership, shrink the file, and isolate state hoisting per path. -**Related:** `src/routes/_authenticated/log.$workoutId.tsx` +**Resolution:** Implemented in PR #115 -- route split into `EventWorkoutView` and `StrengthWorkoutView` child components. **Priority:** Medium diff --git a/src/components/workout/strength-workout-view.tsx b/src/components/workout/strength-workout-view.tsx index 2aeba67..8abfc48 100644 --- a/src/components/workout/strength-workout-view.tsx +++ b/src/components/workout/strength-workout-view.tsx @@ -1,3 +1,4 @@ +import { useState } from 'react' import { Plus } from 'lucide-react' import { WorkoutHeader } from '@/components/workout/workout-header' import { WorkoutPausedBar } from '@/components/workout/workout-paused-bar' @@ -76,12 +77,6 @@ interface StrengthWorkoutViewProps { pendingInputs: Record setPendingInputs: React.Dispatch>> programBannerProps: ProgramBannerProps | null - showAddExercise: boolean - setShowAddExercise: (open: boolean) => void - showDiscardDialog: boolean - setShowDiscardDialog: (open: boolean) => void - pageError: string | null - setPageError: (error: string | null) => void confirmSet: ( loggedActivityId: string, setData: Omit, @@ -131,18 +126,15 @@ export function StrengthWorkoutView({ pendingInputs, setPendingInputs, programBannerProps, - showAddExercise, - setShowAddExercise, - showDiscardDialog, - setShowDiscardDialog, - pageError, - setPageError, confirmSet, deleteSet, removeActivity, skipRest, adjustRest, }: StrengthWorkoutViewProps) { + const [showAddExercise, setShowAddExercise] = useState(false) + const [showDiscardDialog, setShowDiscardDialog] = useState(false) + const [pageError, setPageError] = useState(null) const exerciseNames = Object.fromEntries(Object.entries(exerciseMap).map(([k, v]) => [k, v.name])) return ( diff --git a/src/routes/_authenticated/log.$workoutId.tsx b/src/routes/_authenticated/log.$workoutId.tsx index 83d553c..61071ab 100644 --- a/src/routes/_authenticated/log.$workoutId.tsx +++ b/src/routes/_authenticated/log.$workoutId.tsx @@ -111,7 +111,6 @@ function ActiveWorkoutPage() { }, [allExercises]) // Local UI state - const [showAddExercise, setShowAddExercise] = useState(false) const [showDiscardDialog, setShowDiscardDialog] = useState(false) const [showSummary, setShowSummary] = useState(false) const [pageError, setPageError] = useState(null) @@ -375,7 +374,6 @@ function ActiveWorkoutPage() { const handleDiscard = useCallback(async () => { try { await discardWorkout() - setShowDiscardDialog(false) await navigate({ to: '/' }).catch((err) => { console.error('[workout-page] handleDiscard navigation failed:', err) setPageError('Workout discarded but could not navigate home.') @@ -598,12 +596,6 @@ function ActiveWorkoutPage() { pendingInputs={pendingInputs} setPendingInputs={setPendingInputs} programBannerProps={programBannerProps} - showAddExercise={showAddExercise} - setShowAddExercise={setShowAddExercise} - showDiscardDialog={showDiscardDialog} - setShowDiscardDialog={setShowDiscardDialog} - pageError={pageError} - setPageError={setPageError} confirmSet={confirmSet} deleteSet={deleteSet} removeActivity={removeActivity} From a30a3d75289939ad3bcdfb175e881c2c063f7c5e Mon Sep 17 00:00:00 2001 From: Robert Hamilton Date: Tue, 21 Apr 2026 11:56:42 -0500 Subject: [PATCH 2/7] feat(vault): restore aggregated 1RM view in Vault 1RM tab (P11-012) Wires OneRmManagement into VaultOneRmTab so all current exercise maxes are visible alongside the per-exercise trend chart. Closes backlog item P11-012 (OneRmManagement removed from profile in PR #76). --- .../2026-04-21-p11-012-aggregated-1rm-view.md | 49 +++++++++++ src/components/vault/vault-one-rm-tab.tsx | 88 ++++++++++--------- 2 files changed, 97 insertions(+), 40 deletions(-) create mode 100644 Context/QuickPlans/Done/2026-04-21-p11-012-aggregated-1rm-view.md diff --git a/Context/QuickPlans/Done/2026-04-21-p11-012-aggregated-1rm-view.md b/Context/QuickPlans/Done/2026-04-21-p11-012-aggregated-1rm-view.md new file mode 100644 index 0000000..de0d9d9 --- /dev/null +++ b/Context/QuickPlans/Done/2026-04-21-p11-012-aggregated-1rm-view.md @@ -0,0 +1,49 @@ +# Quick Plan: Aggregated 1RM View (P11-012) + +**Date:** 2026-04-21 +**Backlog Source:** `Context/Backlog/Ideas.md` -- P11-012 + +--- + +## Task + +Surface the aggregated cross-exercise 1RM view that was removed from the profile page during the PR #76 responsiveness overhaul. + +## Goal + +The Vault's 1RM TRENDS tab already shows per-exercise historical charts (`VaultOneRmTab`). Add the aggregated current-max summary (`OneRmManagement`) above the trend selector so athletes can see all their 1RMs at a glance and drill into a trend by clicking an exercise. + +## Current State + +| Component | Location | Status | +|---|---|---| +| `OneRmManagement` | `src/components/profile/one-rm-management.tsx` | Exists, not surfaced anywhere | +| `VaultOneRmTab` | `src/components/vault/vault-one-rm-tab.tsx` | Live in Vault "1RM TRENDS" tab | +| `VaultOneRmTab` | `src/routes/_authenticated/vault.tsx` | Wired as `value="one-rm"` tab | + +## Approach + +Enhance `VaultOneRmTab` to show two sections: + +1. **Aggregated summary** (top) -- wire `OneRmManagement` using profile data. Clicking an exercise row pre-selects it in the trend selector (optional UX improvement). +2. **Trend chart** (below) -- existing exercise selector + `OneRmChart` (unchanged). + +**Data requirement:** `OneRmManagement` needs `exerciseMaxes: Record` and `preferredUnits` from user profile. Add `useUserProfile(userId)` to `VaultOneRmTab` to supply these props. + +**Files to touch:** +- `src/components/vault/vault-one-rm-tab.tsx` -- add `useUserProfile`, render `OneRmManagement` above trend section + +No other files need to change. `OneRmManagement` is already a complete component. + +## Verification + +- Vault > 1RM TRENDS tab shows list of all exercises with current 1RM values +- "Update" button in aggregated list opens the edit dialog (inherited from `OneRmManagement`) +- Selecting an exercise in the trend selector still shows the chart below +- Empty states work: no maxes recorded, no exercises with `supports1RM` +- Loading skeleton covers both sections + +## Risks + +- `useUserProfile` may return `exerciseMaxes` as `undefined` before load -- need null-safe default (`exerciseMaxes ?? {}`) +- If user has no 1RM data, `OneRmManagement` renders its own `EmptyState` -- the double empty state (one from `VaultOneRmTab`, one from `OneRmManagement`) must be reconciled: let `OneRmManagement` own the empty state and suppress the outer one diff --git a/src/components/vault/vault-one-rm-tab.tsx b/src/components/vault/vault-one-rm-tab.tsx index 595adfe..45fa920 100644 --- a/src/components/vault/vault-one-rm-tab.tsx +++ b/src/components/vault/vault-one-rm-tab.tsx @@ -1,8 +1,9 @@ import { useState } from 'react' import { useAuth } from '@/lib/auth' import { useExercises } from '@/hooks/use-exercises' -import { useOneRepMaxHistory } from '@/hooks/use-user-profile' +import { useUserProfile, useOneRepMaxHistory } from '@/hooks/use-user-profile' import { OneRmChart } from '@/components/exercises/one-rm-chart' +import { OneRmManagement } from '@/components/profile/one-rm-management' import { EmptyState } from '@/components/shared/empty-state' import { Select, @@ -17,6 +18,7 @@ export function VaultOneRmTab() { const { user } = useAuth() const userId = user?.id const { data: exercises, isLoading: isLoadingExercises } = useExercises() + const { data: profile, isLoading: isLoadingProfile } = useUserProfile(userId!) const [selectedExerciseId, setSelectedExerciseId] = useState(undefined) const { data: oneRmHistory, isLoading: isLoadingHistory } = useOneRepMaxHistory( @@ -25,8 +27,10 @@ export function VaultOneRmTab() { ) const oneRmExercises = exercises?.filter((e) => e.supports1RM) ?? [] + const exerciseMaxes = profile?.exerciseMaxes ?? {} + const preferredUnits = profile?.preferredUnits ?? 'IMPERIAL' - if (isLoadingExercises) { + if (isLoadingExercises || isLoadingProfile) { return (
@@ -35,49 +39,53 @@ export function VaultOneRmTab() { ) } - if (oneRmExercises.length === 0) { - return ( -
- -
- ) - } - return ( -
- - - {!selectedExerciseId && ( - + {userId && ( + )} - {selectedExerciseId && isLoadingHistory && ( - - )} + {oneRmExercises.length > 0 && ( +
+

+ TRENDS +

+ + - {selectedExerciseId && !isLoadingHistory && } + {!selectedExerciseId && ( + + )} + + {selectedExerciseId && isLoadingHistory && ( + + )} + + {selectedExerciseId && !isLoadingHistory && } +
+ )}
) } From 771d8bc6d68c7532fac2b7a3fdbcc3a6ff2602e0 Mon Sep 17 00:00:00 2001 From: Robert Hamilton Date: Wed, 22 Apr 2026 15:31:24 -0500 Subject: [PATCH 3/7] feat(workout): guard finish when pending set is dirtied Adds a PENDING label to the auto-populated set row and tracks whether the user has edited any field on it. When finishing a workout with a dirtied-but-unconfirmed pending set, show a confirmation dialog so the user doesn't silently lose entered data. --- src/components/workout/exercise-block.tsx | 58 ++++++++------- src/components/workout/set-row.tsx | 45 ++++++++++-- .../workout/strength-workout-view.tsx | 73 +++++++++++++++++-- 3 files changed, 137 insertions(+), 39 deletions(-) diff --git a/src/components/workout/exercise-block.tsx b/src/components/workout/exercise-block.tsx index 34fa432..ba4f807 100644 --- a/src/components/workout/exercise-block.tsx +++ b/src/components/workout/exercise-block.tsx @@ -40,6 +40,8 @@ interface ExerciseBlockProps { onDeleteSet?: (setId: string) => void onUnconfirmSet?: (loggedActivityId: string, setId: string) => void onRemoveExercise?: () => void + /** Called when the user first edits the pending (pre-filled) input row. */ + onPendingDirty?: () => void } export function ExerciseBlock({ @@ -57,6 +59,7 @@ export function ExerciseBlock({ onDeleteSet, onUnconfirmSet, onRemoveExercise, + onPendingDirty, }: ExerciseBlockProps) { const firstWorkoutCompleted = useOnboardingStore((s) => s.firstWorkoutCompleted) const hasPrescribed = sets.some((s) => s.prescribedWeight != null || s.prescribedReps != null) @@ -78,10 +81,7 @@ export function ExerciseBlock({ if (isDone) { return ( -
+

{exerciseName} @@ -181,29 +181,33 @@ export function ExerciseBlock({ )}
- {sets.map((set) => ( - - onConfirmSet(loggedActivityId, set.setNumber, weight, reps, setType) - } - onDelete={onDeleteSet ? () => onDeleteSet(set.id) : undefined} - onUnconfirm={ - set.confirmed && onUnconfirmSet - ? () => onUnconfirmSet(loggedActivityId, set.id) - : undefined - } - /> - ))} + {sets.map((set) => { + const isThisPending = !set.confirmed && set.id.startsWith('pending-') + return ( + + onConfirmSet(loggedActivityId, set.setNumber, weight, reps, setType) + } + onDelete={onDeleteSet ? () => onDeleteSet(set.id) : undefined} + onUnconfirm={ + set.confirmed && onUnconfirmSet + ? () => onUnconfirmSet(loggedActivityId, set.id) + : undefined + } + /> + ) + })}
{(onAddSet || onSkipExercise) && (
diff --git a/src/components/workout/set-row.tsx b/src/components/workout/set-row.tsx index e3b7a3b..164488a 100644 --- a/src/components/workout/set-row.tsx +++ b/src/components/workout/set-row.tsx @@ -18,9 +18,15 @@ interface SetRowProps { isBodyweight?: boolean /** * When true, the row represents an unconfirmed placeholder (the auto-populated - * next-set row). Renders dimmed to distinguish it from confirmed sets. + * next-set row). Renders dimmed to distinguish it from confirmed sets and shows + * a PENDING label in the status column until the user edits a field. */ isPending?: boolean + /** + * Fired when the user modifies a weight or reps input on a pending row. + * Used by the parent to track whether the pending row has been dirtied. + */ + onPendingDirty?: () => void /** * When provided, the row supports swipe-to-delete. Tapping the revealed DEL * button calls this handler. @@ -46,6 +52,7 @@ export function SetRow({ prescribedReps, isBodyweight = false, isPending = false, + onPendingDirty, onDelete, onUnconfirm, }: SetRowProps) { @@ -55,6 +62,8 @@ export function SetRow({ const [reps, setReps] = useState(initialReps) const [setType, setSetType] = useState('WORKING') const [showTypeSelector, setShowTypeSelector] = useState(false) + // Tracks whether the user has edited any input on a pending row. + const [isDirty, setIsDirty] = useState(false) // Swipe-to-delete state const [swipeX, setSwipeX] = useState(0) @@ -69,6 +78,14 @@ export function SetRow({ setReps(initialReps) }, [initialReps]) + // Marks the pending row dirty on first user edit and notifies the parent. + const markDirty = useCallback(() => { + if (!isDirty) { + setIsDirty(true) + onPendingDirty?.() + } + }, [isDirty, onPendingDirty]) + const handleConfirm = useCallback(() => { if (confirmed || isConfirming) return onConfirm(isBodyweight ? '' : weight, reps, setType) @@ -217,7 +234,10 @@ export function SetRow({ type="text" inputMode="decimal" value={weight} - onChange={(e) => setWeight(e.target.value)} + onChange={(e) => { + setWeight(e.target.value) + if (isPending) markDirty() + }} disabled={confirmed} placeholder="--" className="w-1/2 border-b border-warm-ash/30 bg-transparent py-2 text-center font-display text-sm tabular-nums text-bone-white placeholder:text-warm-ash/40 focus:border-ember focus:outline-none disabled:opacity-60" @@ -229,7 +249,10 @@ export function SetRow({ type="text" inputMode="numeric" value={reps} - onChange={(e) => setReps(e.target.value)} + onChange={(e) => { + setReps(e.target.value) + if (isPending) markDirty() + }} disabled={confirmed} placeholder="--" className="w-1/2 border-b border-warm-ash/30 bg-transparent py-2 text-center font-display text-sm tabular-nums text-bone-white placeholder:text-warm-ash/40 focus:border-ember focus:outline-none disabled:opacity-60" @@ -248,7 +271,10 @@ export function SetRow({ type="text" inputMode="decimal" value={weight} - onChange={(e) => setWeight(e.target.value)} + onChange={(e) => { + setWeight(e.target.value) + if (isPending) markDirty() + }} disabled={confirmed} placeholder="--" className="w-full border-b border-warm-ash/30 bg-transparent py-2 text-center font-display text-sm tabular-nums text-bone-white placeholder:text-warm-ash/40 focus:border-ember focus:outline-none disabled:opacity-60" @@ -257,13 +283,16 @@ export function SetRow({ )}
- {/* Reps input -- ad-hoc path (unchanged) */} + {/* Reps input -- ad-hoc path */}
setReps(e.target.value)} + onChange={(e) => { + setReps(e.target.value) + if (isPending) markDirty() + }} disabled={confirmed} placeholder="--" className="w-full border-b border-warm-ash/30 bg-transparent py-2 text-center font-display text-sm tabular-nums text-bone-white placeholder:text-warm-ash/40 focus:border-ember focus:outline-none disabled:opacity-60" @@ -290,6 +319,10 @@ export function SetRow({ > + ) : isPending && !isDirty ? ( + + PENDING + ) : ( + + + +
) } From ac34b7d69ba674577b18b70acccebc868e8df470 Mon Sep 17 00:00:00 2001 From: Robert Hamilton Date: Wed, 22 Apr 2026 15:31:28 -0500 Subject: [PATCH 4/7] chore(context): resolve backlog items and improve backlog command Marks P11-012, exercise completion signal, frequent exercises picker, and P22-014 as resolved in Ideas.md. Adds a default list action and renumbers workflow steps in the backlog command. --- .claude/commands/backlog.md | 22 +++++++++++++++------- Context/Backlog/Ideas.md | 33 ++++++++------------------------- 2 files changed, 23 insertions(+), 32 deletions(-) diff --git a/.claude/commands/backlog.md b/.claude/commands/backlog.md index 571e4d5..c5c6ecb 100755 --- a/.claude/commands/backlog.md +++ b/.claude/commands/backlog.md @@ -1,7 +1,6 @@ --- description: Add to or prioritize the backlog model: sonnet -effort: low --- # Backlog Management @@ -26,7 +25,10 @@ This command routes to the build agent: ## Workflow ### Step 1: Determine action -The first parameter ($1) indicates what action to take: +If $ARGUMENTS is empty, default to `list` (show all open backlog items). + +Otherwise, the first parameter ($1) indicates what action to take: +- `list` — Show all open items from Ideas.md and Bugs.md (default when no args) - `add` — Add a new item to Ideas.md or Bugs.md - `prioritize` — Order existing items by priority - `move` — Move items between Ideas and Bugs @@ -34,7 +36,13 @@ The first parameter ($1) indicates what action to take: Additional details provided via $ARGUMENTS. -### Step 2: For `add` action +### Step 2: For `list` action (default) +1. Load Context/Backlog/Ideas.md and Context/Backlog/Bugs.md +2. Display all open items, grouped by file (Ideas / Bugs) +3. Show priority level if present +4. Suggest next actions: add an item, prioritize, or start planning a high-priority item + +### Step 3: For `add` action 1. If $ARGUMENTS provided, it contains the item description 2. If no $ARGUMENTS, ask user for the item description 3. Determine if item is an idea or a bug @@ -45,27 +53,27 @@ Additional details provided via $ARGUMENTS. - Initial priority (optional) 6. Confirm entry was added -### Step 3: For `prioritize` action +### Step 4: For `prioritize` action 1. Load both Ideas.md and Bugs.md 2. Present current items 3. Ask user to reorder by priority 4. Optionally ask for priority levels (high/medium/low) 5. Save reordered lists -### Step 4: For `move` action +### Step 5: For `move` action 1. Load both backlogs 2. Present items 3. Ask which item to move and to which file 4. Move item between files -### Step 5: For `remove` action +### Step 6: For `remove` action 1. Load appropriate backlog file 2. Present items 3. Ask which to remove 4. Remove entry and save file 5. Optionally ask to archive to Done/ instead -### Step 6: Summary +### Step 7: Summary Present the updated backlog and suggest next steps: - Start planning a high-priority item - Continue current implementation diff --git a/Context/Backlog/Ideas.md b/Context/Backlog/Ideas.md index 0f6358b..c5ece65 100644 --- a/Context/Backlog/Ideas.md +++ b/Context/Backlog/Ideas.md @@ -65,13 +65,11 @@ skill. Prioritize via the backlog-prioritize skill. **Source:** `Context/Reviews/0009-pr71-enhancement-batch-review.md` **Resolution:** TauriAdapter.getUnreadCounts already has 2 unit tests (happy path + empty). The `Promise.allSettled` batching was a suggestion for SupabaseAdapter that was never implemented (still sequential loop). No further action needed; re-open if batching is implemented. -## Aggregated 1RM View +## ~~P11-012: Aggregated 1RM View~~ (Resolved 2026-04-22) **Added:** 2026-04-05 **Source:** `Context/Reviews/0011-pr76-web-responsiveness-review.md` (P11-012) -**Priority:** Medium - -The `OneRmManagement` component was removed from the profile page during the responsiveness overhaul. The per-exercise 1RM management is still accessible on each exercise detail page (`/exercises/$exerciseId`), but the aggregated cross-exercise 1RM view no longer exists. Consider adding an aggregated 1RM summary to the library exercises tab or a dedicated stats/progress page. +**Resolution:** Restored aggregated 1RM view in Vault 1RM tab -- commit `7fd07f2`. ### ~~Overhaul web responsiveness for larger screens~~ (Resolved 2026-04-05) @@ -83,35 +81,20 @@ The `OneRmManagement` component was removed from the profile page during the res **Added:** 2026-04-05 **Resolution:** Reordered sidebar nav into logical groups: core tools, reference, social. Commit `1b359d2`. -### Exercise completion signal during active workout +### ~~Exercise completion signal during active workout~~ (Resolved 2026-04-22) **Added:** 2026-04-12 -**Source:** User feedback -**Priority:** Medium - -Two related UX gaps in the workout logger: - -1. **"Done with this exercise" affordance** -- No explicit way to mark an exercise block as finished mid-workout. Users who finish squats before the planned sets are complete (injury, RPE ceiling, time) have no way to signal they're moving on. Options: a "Mark complete" button per exercise block, a long-press gesture, or a swipe action that collapses the block and flags it as done. +**Resolution:** Implemented in PR #113 (F023) -- exercise completion signal shipped. -2. **Trailing auto-populated set ambiguity** -- When the logger auto-populates a new set row, it's unclear whether exiting the exercise (or ending the workout) will count that empty row as a logged set or discard it. Users are left guessing whether their volume is being over-counted. Need either: discard empty trailing sets on exit, or show explicit "incomplete set" vs "complete set" state that is visible before saving. - -**Related:** `src/routes/_authenticated/log.$workoutId.tsx`, workout logger state, set auto-population logic -**Area:** Workout logger UX - -### "Add exercise" pre-populated with frequently used exercises +### ~~"Add exercise" pre-populated with frequently used exercises~~ (Resolved 2026-04-22) **Added:** 2026-04-12 -**Context:** When opening the "Add exercise" picker, the search field starts empty with no suggestions. Pre-populating it with the user's most frequently logged exercises (derived from workout history) would reduce search friction, especially on the gym floor where speed matters. -**Related:** Exercise picker/search component, workout history data, active workout flow -**Priority:** Medium +**Resolution:** Implemented in PR #114 (F024) -- frequent exercises add picker shipped. -### P22-014: Move view-local modal state into StrengthWorkoutView +### ~~P22-014: Move view-local modal state into StrengthWorkoutView~~ (Resolved 2026-04-22) **Added:** 2026-04-21 -**Source:** `Context/Reviews/` P22 review findings (PR #115) -**Priority:** Medium -**Context:** Modal state currently lives outside `StrengthWorkoutView`. Moving it inside the component would tighten encapsulation and reduce prop-threading into the view. -**Related:** `src/routes/_authenticated/log.$workoutId/strength-workout-view.tsx` +**Resolution:** Implemented in commit `689548b` -- modal state encapsulated inside `StrengthWorkoutView`. ### ~~Split log.$workoutId route into three components~~ (Resolved 2026-04-21) From 46c9e7cc78795d028a740caf286c6f86714fb9d4 Mon Sep 17 00:00:00 2001 From: Robert Hamilton Date: Wed, 22 Apr 2026 15:32:17 -0500 Subject: [PATCH 5/7] chore(context): archive completed QuickPlans for F023 and P22-014 --- .../2026-04-21-exercise-completion-signal.md | 102 ++++++++++++++++++ ...04-21-p22-014-modal-state-encapsulation.md | 65 +++++++++++ 2 files changed, 167 insertions(+) create mode 100644 Context/QuickPlans/Done/2026-04-21-exercise-completion-signal.md create mode 100644 Context/QuickPlans/Done/2026-04-21-p22-014-modal-state-encapsulation.md diff --git a/Context/QuickPlans/Done/2026-04-21-exercise-completion-signal.md b/Context/QuickPlans/Done/2026-04-21-exercise-completion-signal.md new file mode 100644 index 0000000..58e6f0b --- /dev/null +++ b/Context/QuickPlans/Done/2026-04-21-exercise-completion-signal.md @@ -0,0 +1,102 @@ +# Quick Plan: Exercise Completion Signal + +**Created:** 2026-04-21 +**Source:** Backlog -- "Exercise completion signal" (Ideas.md) + +--- + +## Task + +Resolve two related UX gaps in the strength workout logger: + +1. **"Done with this exercise" affordance** -- Users need a discoverable way to mark an exercise block finished before all planned sets are complete. +2. **Trailing set ambiguity** -- The auto-populated pending input row looks identical to a confirmed set row. Users can't tell if exiting/finishing will save or discard it. + +--- + +## Context (from code exploration) + +| Finding | Detail | +|---|---| +| Done button | `ExerciseBlock` already renders a "Done" button when `onSkipExercise` prop is provided (`exercise-block.tsx:208-228`). `StrengthWorkoutView` wires `handleMarkDone` → `skipActivity` which adds the activity ID to `skippedActivityIds`. Block then collapses. Users can re-expand via expand button. | +| Pending row rendering | A pending input row renders in `strength-workout-view.tsx:331-376` when (a) no confirmed sets exist yet, OR (b) `pendingInputs[activity.id]` is `true` (set by "Add set" button). | +| Trailing row on finish | `handleFinish` (log.$workoutId.tsx:280-338) clears `pendingInputs` before calling `finishWorkout()`. The unconfirmed row is discarded -- never persisted. No DB overcounting. | +| Discoverability gap | The "Done" button is at the bottom of an expanded exercise block. On a long exercise block with many sets, it may be below the fold on mobile. No visual cue distinguishes a pending (unconfirmed) input row from confirmed set rows except that it has editable fields. | + +**Net scope:** Gap 1 is largely solved at the code level -- the wiring exists. The work is UX polish: ensure the Done button is discoverable. Gap 2 needs a pending-row visual indicator and/or a finish-time guard when the user has typed values into a pending row. + +--- + +## Goal + +- Done button is clearly discoverable and accessible without scrolling on typical exercise blocks. +- Pending input rows are visually distinct from confirmed set rows so users know they are "not yet logged." +- If a user hits Finish with a non-empty pending row (weight or reps typed), surface a confirmation or auto-discard notice so nothing is silently lost. + +--- + +## Approach + +### 1. Pending row visual indicator (`exercise-block.tsx` + `set-row.tsx`) + +Add a visual "PENDING" or "UNLOGGED" state to the pending input row: + +- Add an `isPending?: boolean` prop to `SetRow`. +- When `isPending` is true, render the row with a muted/dimmed style (e.g., `opacity-60` or `bg-surface-pit/40`) and a small ALL-CAPS label (`PENDING` or `IN PROGRESS`) in the status column instead of the confirm checkmark column. +- Pass `isPending={true}` to the `SetRow` rendered for the pending input in `strength-workout-view.tsx:370-376`. + +This makes it immediately obvious which row is "not yet logged." + +### 2. Finish-time guard for non-empty pending rows (`strength-workout-view.tsx` + `log.$workoutId.tsx`) + +In `handleFinish` (log.$workoutId.tsx:280), before clearing `pendingInputs`, check if any activity has a pending row with non-empty weight or reps. If so, surface a brief confirmation dialog or toast: "You have an unconfirmed set -- it won't be saved. Finish anyway?" + +Implementation detail: +- `pendingInputs` is a `Record` -- it only tracks whether to show the row, not the current input values. +- The actual weight/reps input values live in local state inside each `SetRow` component, making them inaccessible from `handleFinish`. +- **Simpler approach:** Lift a `pendingSetValues` map (activityId → `{weight, reps}`) up to `StrengthWorkoutView` state, updated on every input change via a callback prop on `SetRow`. Then `handleFinish` can inspect it. +- If lifting state is too invasive, use a ref-based approach: a `pendingHasValues` Set stored in a ref, updated via an `onPendingChange` callback from `SetRow`. + +### 3. Done button discoverability (`exercise-block.tsx`) + +The "Done" button is already rendered at block bottom (line 208-228). Evaluate if it needs a sticky or pinned position when the block is long. Options: +- Keep current position (bottom of block) -- acceptable if most blocks have ≤ 5 sets. +- Add a secondary collapsed-header affordance: a small "DONE" badge/button in the exercise block header row that dismisses without scrolling. + +Recommendation: Start with option A (no change to position, verify in app). Only add header affordance if gym-floor testing shows it's hard to reach. + +--- + +## Files to Touch + +| File | Change | +|---|---| +| `src/components/workout/set-row.tsx` | Add `isPending?: boolean` prop; conditional dim + PENDING label in status column | +| `src/components/workout/exercise-block.tsx` | Pass `isPending` to the pending `SetRow`; optionally expose `onPendingChange` callback | +| `src/routes/_authenticated/log.$workoutId/strength-workout-view.tsx` | Track `pendingSetValues` state; pass `isPending` and `onPendingChange` to relevant rows | +| `src/routes/_authenticated/log.$workoutId.tsx` | Guard in `handleFinish`: if any pending row has values, show confirmation before finishing | + +--- + +## Verification + +- [ ] Pending input row is visually distinct from confirmed rows (muted styling + PENDING label visible) +- [ ] Confirming a set clears the PENDING state and shows the row as confirmed +- [ ] Finishing with a non-empty pending row shows a confirmation prompt +- [ ] Finishing with an empty pending row finishes immediately (no false positive prompt) +- [ ] Done button collapses the exercise block and it appears in collapsed/done state +- [ ] Collapsed block can be re-expanded to add more sets +- [ ] No regressions in set confirmation or workout finish flow + +--- + +## Risks + +- **Lifting pending values state** could be moderately invasive if `SetRow` doesn't currently support callbacks for every keystroke. Check current `SetRow` props and local state before deciding between lifted state vs ref approach. +- **False positive guard triggers** if the empty pending row pre-fills with prescribed weight/reps. Guard should check if values differ from the initial pre-filled defaults, OR check if the user has actually interacted with the row (dirty flag). + +--- + +## Execution + +`/impl` -- moderate scope, 3-4 files, no cross-stack coordination needed. diff --git a/Context/QuickPlans/Done/2026-04-21-p22-014-modal-state-encapsulation.md b/Context/QuickPlans/Done/2026-04-21-p22-014-modal-state-encapsulation.md new file mode 100644 index 0000000..749b92a --- /dev/null +++ b/Context/QuickPlans/Done/2026-04-21-p22-014-modal-state-encapsulation.md @@ -0,0 +1,65 @@ +# P22-014: Move View-Local Modal State into StrengthWorkoutView + +**Date:** 2026-04-21 +**Scope:** Refactor (2 files) +**Source:** P22 review finding from PR #115 + +--- + +## Task + +Move `showAddExercise`, `showDiscardDialog`, and `pageError` state from the parent route +(`ActiveWorkoutPage`) into `StrengthWorkoutView`, where they are exclusively consumed. + +--- + +## Goal + +Reduce prop-threading in `ActiveWorkoutPage`. These three state pairs are UI concerns +internal to `StrengthWorkoutView` -- the parent has no reason to own or read them. +The component's prop interface shrinks from 43 to 37 props after removal. + +--- + +## Approach + +### 1. Add local state to `StrengthWorkoutView` + +In `strength-workout-view.tsx`, add three `useState` declarations: + +```tsx +const [showAddExercise, setShowAddExercise] = useState(false) +const [showDiscardDialog, setShowDiscardDialog] = useState(false) +const [pageError, setPageError] = useState(null) +``` + +### 2. Remove the six props from `StrengthWorkoutViewProps` + +Delete from the interface: +- `showAddExercise: boolean` +- `setShowAddExercise: (v: boolean) => void` +- `showDiscardDialog: boolean` +- `setShowDiscardDialog: (v: boolean) => void` +- `pageError: string | null` +- `setPageError: (v: string | null) => void` + +### 3. Clean up `log.$workoutId.tsx` + +- Remove the three `useState` declarations (lines 113-114, 116) from `ActiveWorkoutPage` +- Remove the six prop assignments from the `` JSX (lines 549-554) +- Verify `showSummary` and `restMinimized` are untouched (they remain at parent level) + +--- + +## Verification + +- `bun run build` passes (TypeScript confirms prop interface is clean) +- `bun run lint` passes +- Manual smoke test: open an active strength workout, add an exercise, trigger discard dialog, verify both modals open/close correctly and page errors surface as before + +--- + +## Risks + +- `pageError` is set in several places inside `StrengthWorkoutView` (lines 147, 292, 325, 410, 419) -- all internal, so no external setter call sites should remain after removal. Confirm with a grep for `setPageError` in `log.$workoutId.tsx` after the change. +- `restMinimized` is a candidate for the same treatment but has been intentionally deferred -- do not include it in this task. From 931b6bde204a5c57ad72fb0ee163758662cc74b9 Mon Sep 17 00:00:00 2001 From: Robert Hamilton Date: Wed, 22 Apr 2026 18:10:32 -0500 Subject: [PATCH 6/7] fix(review): resolve P22 review findings (8 fixes, 4 tasks) Fixes: - P22-001: getFrequentExerciseIds throws on RPC error instead of returning [] - P22-002: useGymRoster throws when Supabase client is null (was returning degraded data) - P22-003: Remove duplicate console.error at 3 catch sites that violate hook log ownership - P22-005: unconfirmSet guard throws instead of silently returning (error now surfaces to user) - P22-006: Add .refine cross-field invariants to GymInvitation and GymOwnershipTransfer; token min(1) -> min(24) - P22-007: Remove unused _exercise/_groupType params from addExerciseToWorkout interface and impl - P22-008: Toast user when notification prefs fail to load during rest timer start - P22-009: Clear pendingDirty for activity when onSkipExercise fires in StrengthWorkoutView Tasks (S008-T through S011-T added to F023/Steps.md Phase 6): - P22-010: StrengthWorkoutView finish guard tests (S008-T) - P22-011: SetRow.onPendingDirty idempotency tests (S009-T) - P22-012: EventWorkoutView tests (S010-T) - P22-013: allActivitiesDone mixed CIRCUIT+STRENGTH banner test (S011-T) Note: P22-004 deferred -- finding invalid; existing test documents that startProgrammedWorkout intentionally accepts standalone template workouts without programContext. Review: Context/Reviews/0022-since-beta10-2026-04-22.md --- .../023-Exercise-Completion-Signal/Steps.md | 42 +++++++++++++++++ .../workout/strength-workout-view.tsx | 22 ++++----- src/domain/types/gym.ts | 46 ++++++++++++------- src/hooks/use-active-workout.ts | 7 +-- src/hooks/use-gym-members.ts | 3 +- src/lib/__tests__/supabase-adapter.test.ts | 9 ++-- src/lib/supabase-adapter.ts | 2 +- src/routes/_authenticated/log.$workoutId.tsx | 3 +- .../__tests__/active-workout-store.test.ts | 3 +- src/stores/active-workout-store.ts | 16 +------ 10 files changed, 96 insertions(+), 57 deletions(-) diff --git a/Context/Features/023-Exercise-Completion-Signal/Steps.md b/Context/Features/023-Exercise-Completion-Signal/Steps.md index 26643a3..e87f4b3 100644 --- a/Context/Features/023-Exercise-Completion-Signal/Steps.md +++ b/Context/Features/023-Exercise-Completion-Signal/Steps.md @@ -172,6 +172,48 @@ --- +### Phase 6: Review Follow-up Tasks (P22 batch) + +- [ ] S008-T: Add test file for `StrengthWorkoutView` covering the finish guard + behavior. Three paths required: (1) no dirty rows -- `handleFinish` called + directly without showing dialog; (2) dirty row present -- `showFinishDirtyDialog` + displayed; (3) dirty row confirmed via dialog -- subsequent Finish proceeds + without re-showing dialog. Also verify `pendingDirty` is cleared when + `onSkipExercise` fires (P22-009 regression guard). + - **Assigned:** builder-ui + - **Depends:** none + - **Parallel:** true + +- [ ] S009-T: Extend `src/components/workout/__tests__/set-row.test.tsx` with + `onPendingDirty` prop tests. Required assertions: (1) called exactly once on + first weight or reps edit of a pending row; (2) not called on a confirmed + (read-only) row; (3) not called a second time on subsequent field edits of the + same pending row. + - **Assigned:** builder-ui + - **Depends:** none + - **Parallel:** true + +- [ ] S010-T: Add test file for `EventWorkoutView`. Cover: (1) discard dialog + triggered via `WorkoutPausedBar.onDiscard`, Cancel path keeps the workout + active, Discard path calls `handleDiscard`; (2) `canFinish` is always `true` + (no set-count gate); (3) `pageError` set externally renders `ErrorBanner` and + is dismissible. + - **Assigned:** builder-ui + - **Depends:** none + - **Parallel:** true + +- [ ] S011-T: Add a mixed CIRCUIT + STRENGTH scenario to + `src/routes/_authenticated/__tests__/-log-workout-finish-banner.test.tsx`. + Scenario: one CIRCUIT group with incomplete activities plus one STRENGTH group + where all activities are in `skippedActivityIds`. Assert `allActivitiesDone` + is `true` and the "READY TO FINISH?" banner is visible. This guards against + regression of the CIRCUIT exclusion filter in `allActivitiesDone`. + - **Assigned:** builder-ui + - **Depends:** S007-T + - **Parallel:** true + +--- + ## Acceptance Criteria - [ ] All 8 testable assertions from Spec.md verified (A-001 through A-008) diff --git a/src/components/workout/strength-workout-view.tsx b/src/components/workout/strength-workout-view.tsx index d5d176a..66b9b8c 100644 --- a/src/components/workout/strength-workout-view.tsx +++ b/src/components/workout/strength-workout-view.tsx @@ -428,24 +428,22 @@ export function StrengthWorkoutView({ return next }) } else { - deleteSet(activity.id, setId).catch((err) => { - console.error('[workout-page] deleteSet failed:', { - activityId: activity.id, - setId, - err, - }) + deleteSet(activity.id, setId).catch(() => { setPageError('Failed to delete set. Please try again.') }) } }} onUnconfirmSet={handleUnconfirmSet} - onSkipExercise={() => handleMarkDone(activity.id)} + onSkipExercise={() => { + setPendingDirty((prev) => { + const next = new Set(prev) + next.delete(activity.id) + return next + }) + handleMarkDone(activity.id) + }} onRemoveExercise={() => { - removeActivity(activity.id).catch((err) => { - console.error('[workout-page] removeActivity failed:', { - activityId: activity.id, - err, - }) + removeActivity(activity.id).catch(() => { setPageError('Failed to remove exercise. Please try again.') }) }} diff --git a/src/domain/types/gym.ts b/src/domain/types/gym.ts index 9b64767..0dc6e12 100644 --- a/src/domain/types/gym.ts +++ b/src/domain/types/gym.ts @@ -68,16 +68,25 @@ export type GymMember = z.infer // conversion is handled in data-mapper, not here. // --------------------------------------------------------------------------- -export const gymInvitationSchema = z.object({ - id: entityId, - gymId: entityId, - token: z.string().min(1), - expiresAt: isoDateTime, - maxUses: z.number().int().positive(), - usesCount: z.number().int().nonnegative(), - createdBy: entityId, - createdAt: isoDateTime, -}) +export const gymInvitationSchema = z + .object({ + id: entityId, + gymId: entityId, + token: z.string().min(24), + expiresAt: isoDateTime, + maxUses: z.number().int().positive(), + usesCount: z.number().int().nonnegative(), + createdBy: entityId, + createdAt: isoDateTime, + }) + .refine((v) => v.usesCount <= v.maxUses, { + message: 'usesCount must not exceed maxUses', + path: ['usesCount'], + }) + .refine((v) => v.expiresAt > v.createdAt, { + message: 'expiresAt must be after createdAt', + path: ['expiresAt'], + }) export type GymInvitation = z.infer // --------------------------------------------------------------------------- @@ -87,12 +96,17 @@ export type GymInvitation = z.infer // primary key, enforcing the single-pending-transfer-per-gym invariant. // --------------------------------------------------------------------------- -export const gymOwnershipTransferSchema = z.object({ - gymId: entityId, - proposedBy: entityId, - proposedTo: entityId, - proposedAt: isoDateTime, -}) +export const gymOwnershipTransferSchema = z + .object({ + gymId: entityId, + proposedBy: entityId, + proposedTo: entityId, + proposedAt: isoDateTime, + }) + .refine((v) => v.proposedBy !== v.proposedTo, { + message: 'proposedBy and proposedTo must be different users', + path: ['proposedTo'], + }) export type GymOwnershipTransfer = z.infer // --------------------------------------------------------------------------- diff --git a/src/hooks/use-active-workout.ts b/src/hooks/use-active-workout.ts index 1f2d94e..5b2b759 100644 --- a/src/hooks/use-active-workout.ts +++ b/src/hooks/use-active-workout.ts @@ -1,4 +1,5 @@ import { useCallback, useEffect, useRef } from 'react' +import { toast } from 'sonner' import { useQueryClient } from '@tanstack/react-query' import { useActiveWorkoutStore, @@ -342,7 +343,7 @@ export function useActiveWorkout() { } // Update store with DB-assigned IDs - storeAddExercise(exercise, groupType, savedGroup, savedActivity) + storeAddExercise(savedGroup, savedActivity) return { group: savedGroup, activity: savedActivity } } catch (err) { @@ -417,9 +418,9 @@ export function useActiveWorkout() { }) }) .catch((err) => { - // If prefs fail to load, still start the timer without notification console.error('[workout] Failed to load notification preferences:', err) storeStartRestTimer(restSeconds, exerciseName, setNum) + toast('Rest notifications unavailable -- check notification permissions.') }) } @@ -492,7 +493,7 @@ export function useActiveWorkout() { const targetSet = findSetById(currentGroups, setId) if (!targetSet) { console.error('[workout] unconfirmSet: set not found in store', { setId }) - return + throw new Error('Set not found -- it may have already been removed.') } // The mutation hook's onError is the single log owner for this // rejection (see error-handling.md "Mutation Hook Log Ownership"). diff --git a/src/hooks/use-gym-members.ts b/src/hooks/use-gym-members.ts index 1b776fe..95bd858 100644 --- a/src/hooks/use-gym-members.ts +++ b/src/hooks/use-gym-members.ts @@ -65,8 +65,7 @@ export function useGymRoster(gymId: string | null | undefined) { const client = getSupabaseClient() if (!client) { - console.error('[gym-members] Supabase client not initialized for roster fetch') - return sorted.map((m) => ({ ...m, displayName: null })) + throw new Error('[gym-members] Supabase client not initialized') } const userIds = sorted.map((m) => m.userId) diff --git a/src/lib/__tests__/supabase-adapter.test.ts b/src/lib/__tests__/supabase-adapter.test.ts index 9d97ebb..fff81aa 100644 --- a/src/lib/__tests__/supabase-adapter.test.ts +++ b/src/lib/__tests__/supabase-adapter.test.ts @@ -1532,17 +1532,16 @@ describe('Utility operations', () => { expect(result).toEqual(['id-a', 'id-b', 'id-c']) }) - it('graceful degradation: returns [] and logs error when rpc fails', async () => { + it('throws and logs error when rpc fails', async () => { const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) try { + const rpcError = { message: 'rpc error' } mockClient.rpc.mockResolvedValue({ data: null, - error: { message: 'rpc error' }, + error: rpcError, }) - const result = await adapter.getFrequentExerciseIds('user-001') - - expect(result).toEqual([]) + await expect(adapter.getFrequentExerciseIds('user-001')).rejects.toEqual(rpcError) expect(errorSpy).toHaveBeenCalledWith( expect.stringContaining('[supabase-adapter]'), expect.anything(), diff --git a/src/lib/supabase-adapter.ts b/src/lib/supabase-adapter.ts index 2855585..3fe58b6 100644 --- a/src/lib/supabase-adapter.ts +++ b/src/lib/supabase-adapter.ts @@ -1637,7 +1637,7 @@ export class SupabaseAdapter implements DataAdapter { }) if (error) { console.error('[supabase-adapter] getFrequentExerciseIds failed:', { userId, error }) - return [] + throw error } return (data as Array<{ exercise_id: string }>).map((r) => r.exercise_id) } diff --git a/src/routes/_authenticated/log.$workoutId.tsx b/src/routes/_authenticated/log.$workoutId.tsx index 61071ab..0c52f1c 100644 --- a/src/routes/_authenticated/log.$workoutId.tsx +++ b/src/routes/_authenticated/log.$workoutId.tsx @@ -458,8 +458,7 @@ function ActiveWorkoutPage() { async (loggedActivityId: string, setId: string) => { try { await unconfirmSet(loggedActivityId, setId) - } catch (err) { - console.error('[workout-page] handleUnconfirmSet failed:', { loggedActivityId, setId, err }) + } catch { setPageError('Failed to undo set.') } }, diff --git a/src/stores/__tests__/active-workout-store.test.ts b/src/stores/__tests__/active-workout-store.test.ts index a72e2b5..53e0eb7 100644 --- a/src/stores/__tests__/active-workout-store.test.ts +++ b/src/stores/__tests__/active-workout-store.test.ts @@ -367,7 +367,6 @@ describe('nested updates', () => { undoAction: null, }) - const exercise = makeExercise({ id: 'ex-2', name: 'Squat' }) const newGroup = makeLoggedActivityGroup({ id: 'lag-new', ordinal: 2 }) const newActivity = makeLoggedActivity({ id: 'la-new', @@ -375,7 +374,7 @@ describe('nested updates', () => { exerciseId: 'ex-2', }) - getState().addExerciseToWorkout(exercise, 'STRAIGHT_SETS', newGroup, newActivity) + getState().addExerciseToWorkout(newGroup, newActivity) const state = getState() expect(state.loggedGroups).toHaveLength(2) diff --git a/src/stores/active-workout-store.ts b/src/stores/active-workout-store.ts index 97dfb58..bee37c1 100644 --- a/src/stores/active-workout-store.ts +++ b/src/stores/active-workout-store.ts @@ -7,8 +7,6 @@ import type { LoggedActivityGroup, LoggedActivity, LoggedSet, - Exercise, - GroupType, NoteContent, } from '@/domain/types' import { noteContentSchema } from '@/domain/types' @@ -142,12 +140,7 @@ interface ActiveWorkoutActions { unpauseWorkout(): void // Exercise management - addExerciseToWorkout( - exercise: Exercise, - groupType: GroupType, - newGroup: LoggedActivityGroup, - newActivity: LoggedActivity, - ): void + addExerciseToWorkout(newGroup: LoggedActivityGroup, newActivity: LoggedActivity): void skipActivity(activityId: string): void removeActivity(activityId: string): void @@ -331,12 +324,7 @@ export const useActiveWorkoutStore = create ({ loggedGroups: [ ...state.loggedGroups, From 0d60eab7e5a667bf37d693849644f9453bdea9b7 Mon Sep 17 00:00:00 2001 From: Robert Hamilton Date: Wed, 22 Apr 2026 18:29:38 -0500 Subject: [PATCH 7/7] test(workout): add P22 review test tasks (S008-T through S011-T) S008-T: StrengthWorkoutView test file (8 tests) - Finish guard: no dirty rows calls handleFinish directly - Finish guard: dirty row shows "Unconfirmed set" dialog - Finish guard: dialog confirm calls handleFinish; cancel does not - P22-009 regression: onSkipExercise clears pendingDirty so next Finish tap bypasses the dialog - Discard dialog: open, cancel, and confirm paths S009-T: SetRow onPendingDirty tests (3 tests added to set-row.test.tsx) - Called exactly once on first weight edit of a pending row - Not called on a confirmed (read-only) row - Not called a second time on subsequent field edits (isDirty guard) S010-T: EventWorkoutView test file (9 tests) - Discard dialog triggered by WorkoutPausedBar.onDiscard - Cancel path: setShowDiscardDialog(false), handleDiscard not called - Discard path: handleDiscard called - canFinish is always true (no set-count gate) - pageError renders ErrorBanner, onDismiss calls setPageError(null) S011-T: Verified covered by S007-T (no new test needed) - Mixed CIRCUIT+STRENGTH all-done banner scenario already exists at line 542 of -log-workout-finish-banner.test.tsx --- .../023-Exercise-Completion-Signal/Steps.md | 8 +- .../__tests__/event-workout-view.test.tsx | 218 +++++++++++ .../workout/__tests__/set-row.test.tsx | 58 +++ .../__tests__/strength-workout-view.test.tsx | 344 ++++++++++++++++++ 4 files changed, 624 insertions(+), 4 deletions(-) create mode 100644 src/components/workout/__tests__/event-workout-view.test.tsx create mode 100644 src/components/workout/__tests__/strength-workout-view.test.tsx diff --git a/Context/Features/023-Exercise-Completion-Signal/Steps.md b/Context/Features/023-Exercise-Completion-Signal/Steps.md index e87f4b3..7523c1c 100644 --- a/Context/Features/023-Exercise-Completion-Signal/Steps.md +++ b/Context/Features/023-Exercise-Completion-Signal/Steps.md @@ -174,7 +174,7 @@ ### Phase 6: Review Follow-up Tasks (P22 batch) -- [ ] S008-T: Add test file for `StrengthWorkoutView` covering the finish guard +- [x] S008-T: Add test file for `StrengthWorkoutView` covering the finish guard behavior. Three paths required: (1) no dirty rows -- `handleFinish` called directly without showing dialog; (2) dirty row present -- `showFinishDirtyDialog` displayed; (3) dirty row confirmed via dialog -- subsequent Finish proceeds @@ -184,7 +184,7 @@ - **Depends:** none - **Parallel:** true -- [ ] S009-T: Extend `src/components/workout/__tests__/set-row.test.tsx` with +- [x] S009-T: Extend `src/components/workout/__tests__/set-row.test.tsx` with `onPendingDirty` prop tests. Required assertions: (1) called exactly once on first weight or reps edit of a pending row; (2) not called on a confirmed (read-only) row; (3) not called a second time on subsequent field edits of the @@ -193,7 +193,7 @@ - **Depends:** none - **Parallel:** true -- [ ] S010-T: Add test file for `EventWorkoutView`. Cover: (1) discard dialog +- [x] S010-T: Add test file for `EventWorkoutView`. Cover: (1) discard dialog triggered via `WorkoutPausedBar.onDiscard`, Cancel path keeps the workout active, Discard path calls `handleDiscard`; (2) `canFinish` is always `true` (no set-count gate); (3) `pageError` set externally renders `ErrorBanner` and @@ -202,7 +202,7 @@ - **Depends:** none - **Parallel:** true -- [ ] S011-T: Add a mixed CIRCUIT + STRENGTH scenario to +- [x] S011-T: Add a mixed CIRCUIT + STRENGTH scenario to `src/routes/_authenticated/__tests__/-log-workout-finish-banner.test.tsx`. Scenario: one CIRCUIT group with incomplete activities plus one STRENGTH group where all activities are in `skippedActivityIds`. Assert `allActivitiesDone` diff --git a/src/components/workout/__tests__/event-workout-view.test.tsx b/src/components/workout/__tests__/event-workout-view.test.tsx new file mode 100644 index 0000000..5dba450 --- /dev/null +++ b/src/components/workout/__tests__/event-workout-view.test.tsx @@ -0,0 +1,218 @@ +// @vitest-environment happy-dom +import { render, screen, act } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { describe, it, expect, vi, beforeEach } from 'vitest' +import type { EventMetadata } from '@/domain/types' + +// --------------------------------------------------------------------------- +// Captured prop refs (vi.hoisted so mocks can reference them) +// --------------------------------------------------------------------------- +const { capturedPausedBarProps, capturedErrorBannerProps } = vi.hoisted(() => { + const capturedPausedBarProps = { current: null as any } + const capturedErrorBannerProps = { current: null as any } + return { capturedPausedBarProps, capturedErrorBannerProps } +}) + +// --------------------------------------------------------------------------- +// Mocks +// --------------------------------------------------------------------------- +vi.mock('@/components/workout/workout-header', () => ({ + WorkoutHeader: () =>
, +})) + +vi.mock('@/components/workout/workout-paused-bar', () => ({ + WorkoutPausedBar: (props: any) => { + capturedPausedBarProps.current = props + return
+ }, +})) + +vi.mock('@/components/workout/error-banner', () => ({ + ErrorBanner: (props: any) => { + capturedErrorBannerProps.current = props + return
{props.message}
+ }, +})) + +vi.mock('@/components/workout/workout-header-menu', () => ({ + WorkoutHeaderMenu: () => null, +})) + +vi.mock('@/components/event-builder/event-detail', () => ({ + EventDetail: () => null, +})) + +vi.mock('@/components/ui/button', () => ({ + Button: ({ children, onClick, disabled, ...rest }: any) => ( + + ), +})) + +vi.mock('@/components/ui/dialog', () => ({ + Dialog: ({ children, open }: any) => (open ?
{children}
: null), + DialogContent: ({ children }: any) =>
{children}
, + DialogHeader: ({ children }: any) =>
{children}
, + DialogTitle: ({ children }: any) =>
{children}
, + DialogDescription: ({ children }: any) =>
{children}
, + DialogFooter: ({ children }: any) =>
{children}
, +})) + +// Import after mocks +import { EventWorkoutView } from '../event-workout-view' + +// --------------------------------------------------------------------------- +// Test helpers +// --------------------------------------------------------------------------- +const fakeEventMetadata: EventMetadata = { + requirements: [], +} + +interface EventWorkoutViewProps { + workoutLog: { id: string; eventMetadata: EventMetadata } + elapsedSeconds: number + isPauseSupported: boolean + isPaused: boolean + handlePause: () => void + handleResume: () => void + handleFinish: () => Promise + handleDiscard: () => Promise + isBroadcasting: boolean + publishFocus: () => void + publishUnfocus: () => void + isFinishing: boolean + isDiscarding: boolean + pageError: string | null + setPageError: (error: string | null) => void + showDiscardDialog: boolean + setShowDiscardDialog: (open: boolean) => void +} + +function makeProps(overrides: Partial = {}): EventWorkoutViewProps { + return { + workoutLog: { id: 'wl-1', eventMetadata: fakeEventMetadata }, + elapsedSeconds: 0, + isPauseSupported: false, + isPaused: false, + handlePause: vi.fn(), + handleResume: vi.fn(), + handleFinish: vi.fn().mockResolvedValue(undefined), + handleDiscard: vi.fn().mockResolvedValue(undefined), + isBroadcasting: false, + publishFocus: vi.fn(), + publishUnfocus: vi.fn(), + isFinishing: false, + isDiscarding: false, + pageError: null, + setPageError: vi.fn(), + showDiscardDialog: false, + setShowDiscardDialog: vi.fn(), + ...overrides, + } +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- +describe('EventWorkoutView', () => { + beforeEach(() => { + capturedPausedBarProps.current = null + capturedErrorBannerProps.current = null + }) + + describe('discard dialog -- triggered by WorkoutPausedBar.onDiscard', () => { + it('calls setShowDiscardDialog(true) when onDiscard is invoked', () => { + const setShowDiscardDialog = vi.fn() + render() + + act(() => { + capturedPausedBarProps.current.onDiscard() + }) + + expect(setShowDiscardDialog).toHaveBeenCalledWith(true) + }) + }) + + describe('cancel path -- dialog closes, handleDiscard NOT called', () => { + it('calls setShowDiscardDialog(false) on Cancel and does not call handleDiscard', async () => { + const user = userEvent.setup() + const setShowDiscardDialog = vi.fn() + const handleDiscard = vi.fn().mockResolvedValue(undefined) + + render( + , + ) + + await user.click(screen.getByRole('button', { name: /cancel/i })) + + expect(setShowDiscardDialog).toHaveBeenCalledWith(false) + expect(handleDiscard).not.toHaveBeenCalled() + }) + }) + + describe('discard path -- handleDiscard IS called', () => { + it('calls handleDiscard when Discard button is clicked', async () => { + const user = userEvent.setup() + const handleDiscard = vi.fn().mockResolvedValue(undefined) + + render( + , + ) + + await user.click(screen.getByRole('button', { name: /^discard$/i })) + + expect(handleDiscard).toHaveBeenCalledTimes(1) + }) + }) + + describe('canFinish is always true', () => { + it('passes canFinish={true} to WorkoutPausedBar', () => { + render() + expect(capturedPausedBarProps.current.canFinish).toBe(true) + }) + }) + + describe('pageError renders ErrorBanner and is dismissible', () => { + it('renders the error message when pageError is set', () => { + render() + expect(screen.getByTestId('error-banner')).toBeInTheDocument() + expect(screen.getByText('Something went wrong')).toBeInTheDocument() + }) + + it('calls setPageError(null) when onDismiss is invoked', () => { + const setPageError = vi.fn() + render( + , + ) + + act(() => { + capturedErrorBannerProps.current.onDismiss() + }) + + expect(setPageError).toHaveBeenCalledWith(null) + }) + + it('does not render ErrorBanner when pageError is null', () => { + render() + expect(screen.queryByTestId('error-banner')).not.toBeInTheDocument() + }) + }) + + describe('discard dialog content', () => { + it('renders correct title and description when dialog is open', () => { + render() + expect(screen.getByText('Discard workout')).toBeInTheDocument() + expect( + screen.getByText(/all logged sets will be permanently deleted/i), + ).toBeInTheDocument() + }) + + it('does not render dialog content when showDiscardDialog is false', () => { + render() + expect(screen.queryByText('Discard workout')).not.toBeInTheDocument() + }) + }) +}) diff --git a/src/components/workout/__tests__/set-row.test.tsx b/src/components/workout/__tests__/set-row.test.tsx index 070df21..0c9bf78 100644 --- a/src/components/workout/__tests__/set-row.test.tsx +++ b/src/components/workout/__tests__/set-row.test.tsx @@ -139,4 +139,62 @@ describe('SetRow', () => { expect(onDelete).toHaveBeenCalledOnce() }) }) + + describe('onPendingDirty', () => { + it('is called exactly once on first weight edit of a pending row', async () => { + const user = userEvent.setup() + const onPendingDirty = vi.fn() + render( + , + ) + + const weightInput = screen.getByLabelText('Weight for set 1') + await user.type(weightInput, '135') + + expect(onPendingDirty).toHaveBeenCalledOnce() + }) + + it('is not called on a confirmed (read-only) row', () => { + const onPendingDirty = vi.fn() + render( + , + ) + + expect(onPendingDirty).not.toHaveBeenCalled() + }) + + it('is not called a second time on subsequent field edits', async () => { + const user = userEvent.setup() + const onPendingDirty = vi.fn() + render( + , + ) + + const weightInput = screen.getByLabelText('Weight for set 1') + await user.type(weightInput, '135') + expect(onPendingDirty).toHaveBeenCalledOnce() + + const repsInput = screen.getByLabelText('Reps for set 1') + await user.type(repsInput, '5') + expect(onPendingDirty).toHaveBeenCalledOnce() + }) + }) }) diff --git a/src/components/workout/__tests__/strength-workout-view.test.tsx b/src/components/workout/__tests__/strength-workout-view.test.tsx new file mode 100644 index 0000000..8253f87 --- /dev/null +++ b/src/components/workout/__tests__/strength-workout-view.test.tsx @@ -0,0 +1,344 @@ +// @vitest-environment happy-dom +import { render, screen, act } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import type { Exercise } from '@/domain/types' +import type { LoggedActivityGroupWithActivities } from '@/stores/active-workout-store' + +// --------------------------------------------------------------------------- +// Captured prop containers -- initialized via vi.hoisted so they exist before +// vi.mock factories execute. +// --------------------------------------------------------------------------- +const { capturedBlocks, capturedBar } = vi.hoisted(() => ({ + capturedBlocks: {} as Record void; onSkipExercise?: () => void }>, + capturedBar: {} as { onFinish?: () => void; onDiscard?: () => void }, +})) + +// --------------------------------------------------------------------------- +// Mocks +// --------------------------------------------------------------------------- + +vi.mock('@/components/workout/workout-header', () => ({ + WorkoutHeader: () =>
, +})) + +vi.mock('@/components/workout/workout-paused-bar', () => ({ + WorkoutPausedBar: (props: any) => { + capturedBar.onFinish = props.onFinish + capturedBar.onDiscard = props.onDiscard + return
+ }, +})) + +vi.mock('@/components/workout/error-banner', () => ({ + ErrorBanner: (props: any) =>
{props.message}
, +})) + +vi.mock('@/components/workout/workout-header-menu', () => ({ + WorkoutHeaderMenu: () => null, +})) + +vi.mock('@/components/workout/exercise-block', () => ({ + ExerciseBlock: (props: any) => { + capturedBlocks[props.loggedActivityId] = props + return
+ }, +})) + +vi.mock('@/components/workout/program-context-banner', () => ({ + ProgramContextBanner: () => null, +})) + +vi.mock('@/components/workout/rest-view', () => ({ + RestView: () => null, +})) + +vi.mock('@/components/workout/rest-timer-banner', () => ({ + RestTimerBanner: () => null, +})) + +vi.mock('@/components/workout/undo-banner', () => ({ + UndoBanner: () => null, +})) + +vi.mock('@/components/workout/add-exercise-sheet', () => ({ + AddExerciseSheet: () => null, +})) + +vi.mock('@/components/workout/cardio-panel', () => ({ + CardioPanel: () => null, +})) + +vi.mock('@/components/workout/ruck-panel', () => ({ + RuckPanel: () => null, +})) + +vi.mock('@/components/workout/circuit-panel', () => ({ + CircuitPanel: () => null, +})) + +vi.mock('@/components/onboarding/onboarding-hint', () => ({ + OnboardingHint: () => null, +})) + +vi.mock('@/components/ui/button', () => ({ + Button: ({ children, onClick, disabled, ...rest }: any) => ( + + ), +})) + +vi.mock('@/components/ui/dialog', () => ({ + Dialog: ({ open, children }: any) => (open ? <>{children} : null), + DialogContent: ({ children }: any) =>
{children}
, + DialogHeader: ({ children }: any) =>
{children}
, + DialogTitle: ({ children }: any) =>

{children}

, + DialogDescription: ({ children }: any) =>

{children}

, + DialogFooter: ({ children }: any) =>
{children}
, +})) + +vi.mock('@/lib/workout-utils', () => ({ + getExerciseModality: () => 'strength', + DEFAULT_CIRCUIT_REPS: 10, +})) + +// Import component after mocks are set up +import { StrengthWorkoutView } from '../strength-workout-view' + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function makeGroup(activityId = 'act-1'): LoggedActivityGroupWithActivities { + return { + id: 'group-1', + workoutLogId: 'wl-1', + groupType: 'STRAIGHT_SETS', + ordinal: 1, + activities: [ + { + id: activityId, + loggedGroupId: 'group-1', + exerciseId: 'ex-1', + ordinal: 1, + sets: [], + }, + ], + } +} + +function makeProps(overrides: Record = {}) { + return { + workoutLog: { id: 'wl-1', userId: 'user-1' }, + loggedGroups: [makeGroup()], + elapsedSeconds: 0, + isPauseSupported: false, + isPaused: false, + handlePause: vi.fn(), + handleResume: vi.fn(), + handleFinish: vi.fn().mockResolvedValue(undefined), + handleDiscard: vi.fn().mockResolvedValue(undefined), + handleAddExercise: vi.fn().mockResolvedValue(undefined), + handleConfirmSet: vi.fn(), + handleUndoSet: vi.fn().mockResolvedValue(undefined), + handleUnconfirmSet: vi.fn().mockResolvedValue(undefined), + handleMarkDone: vi.fn(), + handleExpandDone: vi.fn(), + isBroadcasting: false, + publishFocus: vi.fn(), + publishUnfocus: vi.fn(), + isFinishing: false, + isDiscarding: false, + isConfirmingSet: false, + isProgrammedWorkout: false, + firstWorkoutCompleted: true, + confirmedSetCount: 1, + activeFocusId: null, + allActivitiesDone: false, + skippedActivityIds: new Set(), + expandedDoneActivityIds: new Set(), + exerciseMap: { 'ex-1': { id: 'ex-1', name: 'Squat', category: 'BARBELL' } as Exercise }, + exerciseNames: { 'ex-1': 'Squat' }, + restTimer: null, + restMinimized: false, + setRestMinimized: vi.fn(), + undoAction: null, + pendingInputs: {}, + setPendingInputs: vi.fn(), + programBannerProps: null, + confirmSet: vi.fn().mockResolvedValue(undefined), + deleteSet: vi.fn().mockResolvedValue(undefined), + removeActivity: vi.fn().mockResolvedValue(undefined), + skipRest: vi.fn(), + adjustRest: vi.fn(), + ...overrides, + } +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe('StrengthWorkoutView', () => { + beforeEach(() => { + vi.clearAllMocks() + // Reset captured prop containers between tests + for (const key of Object.keys(capturedBlocks)) delete capturedBlocks[key] + capturedBar.onFinish = undefined + capturedBar.onDiscard = undefined + }) + + // ------------------------------------------------------------------------- + // Finish guard (handleFinishWithGuard) + // ------------------------------------------------------------------------- + + describe('finish guard', () => { + it('calls handleFinish directly when no dirty rows exist', async () => { + const handleFinish = vi.fn().mockResolvedValue(undefined) + render() + + act(() => { + capturedBar.onFinish?.() + }) + + expect(handleFinish).toHaveBeenCalledTimes(1) + expect(screen.queryByText('Unconfirmed set')).not.toBeInTheDocument() + }) + + it('shows the "Unconfirmed set" dialog instead of calling handleFinish when a dirty row exists', () => { + const handleFinish = vi.fn().mockResolvedValue(undefined) + render() + + // Dirty the activity + act(() => { + capturedBlocks['act-1'].onPendingDirty?.() + }) + + // Trigger finish + act(() => { + capturedBar.onFinish?.() + }) + + expect(screen.getByText('Unconfirmed set')).toBeInTheDocument() + expect(handleFinish).not.toHaveBeenCalled() + }) + + it('calls handleFinish when "Finish" inside the dirty dialog is clicked', async () => { + const user = userEvent.setup() + const handleFinish = vi.fn().mockResolvedValue(undefined) + render() + + act(() => { + capturedBlocks['act-1'].onPendingDirty?.() + }) + act(() => { + capturedBar.onFinish?.() + }) + + // Dialog is open -- click the Finish button inside it + const finishBtn = screen.getByRole('button', { name: /^Finish$/i }) + await user.click(finishBtn) + + expect(handleFinish).toHaveBeenCalledTimes(1) + expect(screen.queryByText('Unconfirmed set')).not.toBeInTheDocument() + }) + + it('does not call handleFinish when "Cancel" inside the dirty dialog is clicked', async () => { + const user = userEvent.setup() + const handleFinish = vi.fn().mockResolvedValue(undefined) + render() + + act(() => { + capturedBlocks['act-1'].onPendingDirty?.() + }) + act(() => { + capturedBar.onFinish?.() + }) + + const cancelBtn = screen.getByRole('button', { name: /^Cancel$/i }) + await user.click(cancelBtn) + + expect(handleFinish).not.toHaveBeenCalled() + expect(screen.queryByText('Unconfirmed set')).not.toBeInTheDocument() + }) + }) + + // ------------------------------------------------------------------------- + // P22-009 regression guard: pendingDirty cleared when skip fires + // ------------------------------------------------------------------------- + + describe('pendingDirty cleared on skip (P22-009 regression guard)', () => { + it('clears pendingDirty for an activity when onSkipExercise fires, so finish proceeds without dialog', () => { + const handleFinish = vi.fn().mockResolvedValue(undefined) + const handleMarkDone = vi.fn() + render() + + // Mark as dirty + act(() => { + capturedBlocks['act-1'].onPendingDirty?.() + }) + + // Skip the exercise -- should clear dirty state + act(() => { + capturedBlocks['act-1'].onSkipExercise?.() + }) + + expect(handleMarkDone).toHaveBeenCalledWith('act-1') + + // Finish should now call handleFinish directly without showing the dialog + act(() => { + capturedBar.onFinish?.() + }) + + expect(handleFinish).toHaveBeenCalledTimes(1) + expect(screen.queryByText('Unconfirmed set')).not.toBeInTheDocument() + }) + }) + + // ------------------------------------------------------------------------- + // Discard dialog + // ------------------------------------------------------------------------- + + describe('discard dialog', () => { + it('opens the discard dialog when WorkoutPausedBar.onDiscard fires', () => { + render() + + act(() => { + capturedBar.onDiscard?.() + }) + + expect(screen.getByText('Discard workout')).toBeInTheDocument() + }) + + it('closes the discard dialog without calling handleDiscard when "Cancel" is clicked', async () => { + const user = userEvent.setup() + const handleDiscard = vi.fn().mockResolvedValue(undefined) + render() + + act(() => { + capturedBar.onDiscard?.() + }) + + const cancelBtn = screen.getByRole('button', { name: /^Cancel$/i }) + await user.click(cancelBtn) + + expect(handleDiscard).not.toHaveBeenCalled() + expect(screen.queryByText('Discard workout')).not.toBeInTheDocument() + }) + + it('calls handleDiscard when "Discard" is clicked', async () => { + const user = userEvent.setup() + const handleDiscard = vi.fn().mockResolvedValue(undefined) + render() + + act(() => { + capturedBar.onDiscard?.() + }) + + const discardBtn = screen.getByRole('button', { name: /^Discard$/i }) + await user.click(discardBtn) + + expect(handleDiscard).toHaveBeenCalledTimes(1) + }) + }) +})