From 898565bfc204f6d3e2cd5d47139583d343bbb515 Mon Sep 17 00:00:00 2001 From: Robert Hamilton Date: Mon, 20 Apr 2026 18:45:56 -0500 Subject: [PATCH 1/3] refactor(workout): split log route into EventWorkoutView and StrengthWorkoutView Extract two rendering paths from the 914-line log.$workoutId.tsx into dedicated components. Route file slimmed from 914 to 562 lines; all state, hooks, and handlers remain in the route orchestrator. New files: - src/components/workout/event-workout-view.tsx (113 lines) - src/components/workout/strength-workout-view.tsx (489 lines) --- src/components/workout/event-workout-view.tsx | 113 ++++ .../workout/strength-workout-view.tsx | 489 +++++++++++++++++ src/routes/_authenticated/log.$workoutId.tsx | 495 +++--------------- 3 files changed, 674 insertions(+), 423 deletions(-) create mode 100644 src/components/workout/event-workout-view.tsx create mode 100644 src/components/workout/strength-workout-view.tsx diff --git a/src/components/workout/event-workout-view.tsx b/src/components/workout/event-workout-view.tsx new file mode 100644 index 0000000..b579227 --- /dev/null +++ b/src/components/workout/event-workout-view.tsx @@ -0,0 +1,113 @@ +import { WorkoutHeader } from '@/components/workout/workout-header' +import { WorkoutPausedBar } from '@/components/workout/workout-paused-bar' +import { ErrorBanner } from '@/components/workout/error-banner' +import { WorkoutHeaderMenu } from '@/components/workout/workout-header-menu' +import { EventDetail } from '@/components/event-builder/event-detail' +import { Button } from '@/components/ui/button' +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from '@/components/ui/dialog' +import type { EventMetadata } from '@/domain/types' + +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 +} + +export function EventWorkoutView({ + workoutLog, + elapsedSeconds, + isPauseSupported, + isPaused, + handlePause, + handleResume, + handleFinish, + handleDiscard, + isBroadcasting, + publishFocus, + publishUnfocus, + isFinishing, + isDiscarding, + pageError, + setPageError, + showDiscardDialog, + setShowDiscardDialog, +}: EventWorkoutViewProps) { + return ( +
+ {pageError && setPageError(null)} />} + + {/* Sticky header with timer + paused-state action bar */} +
+ + } + /> + setShowDiscardDialog(true)} + showFinishHelper={false} + /> +
+ + + + {/* Discard confirmation dialog */} + + + + Discard workout + + All logged sets will be permanently deleted. This cannot be undone. + + + + + + + + +
+ ) +} diff --git a/src/components/workout/strength-workout-view.tsx b/src/components/workout/strength-workout-view.tsx new file mode 100644 index 0000000..f296fe6 --- /dev/null +++ b/src/components/workout/strength-workout-view.tsx @@ -0,0 +1,489 @@ +import { Plus } from 'lucide-react' +import { WorkoutHeader } from '@/components/workout/workout-header' +import { WorkoutPausedBar } from '@/components/workout/workout-paused-bar' +import { ErrorBanner } from '@/components/workout/error-banner' +import { WorkoutHeaderMenu } from '@/components/workout/workout-header-menu' +import { ExerciseBlock, type SetRowData } from '@/components/workout/exercise-block' +import { ProgramContextBanner } from '@/components/workout/program-context-banner' +import { RestView } from '@/components/workout/rest-view' +import { RestTimerBanner } from '@/components/workout/rest-timer-banner' +import { UndoBanner } from '@/components/workout/undo-banner' +import { AddExerciseSheet } from '@/components/workout/add-exercise-sheet' +import { CardioPanel } from '@/components/workout/cardio-panel' +import { RuckPanel } from '@/components/workout/ruck-panel' +import { CircuitPanel } from '@/components/workout/circuit-panel' +import { OnboardingHint } from '@/components/onboarding/onboarding-hint' +import { Button } from '@/components/ui/button' +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from '@/components/ui/dialog' +import { getExerciseModality, DEFAULT_CIRCUIT_REPS } from '@/lib/workout-utils' +import type { Exercise, LoggedSet, SetType } from '@/domain/types' +import type { LoggedActivityGroupWithActivities } from '@/stores/active-workout-store' + +interface StrengthWorkoutViewProps { + workoutLog: { id: string; userId: string } + loggedGroups: LoggedActivityGroupWithActivities[] + elapsedSeconds: number + isPauseSupported: boolean + isPaused: boolean + handlePause: () => void + handleResume: () => void + handleFinish: () => Promise + handleDiscard: () => Promise + handleAddExercise: (exercise: Exercise) => Promise + handleConfirmSet: ( + loggedActivityId: string, + setNumber: number, + weight: string, + reps: string, + setType: SetType, + ) => Promise + handleUndoSet: () => Promise + handleUnconfirmSet: (loggedActivityId: string, setId: string) => Promise + handleMarkDone: (activityId: string) => void + handleExpandDone: (activityId: string) => void + isBroadcasting: boolean + publishFocus: () => void + publishUnfocus: () => void + isFinishing: boolean + isDiscarding: boolean + isConfirmingSet: boolean + isProgrammedWorkout: boolean + firstWorkoutCompleted: boolean + confirmedSetCount: number + activeFocusId: string | null + allActivitiesDone: boolean + skippedActivityIds: Set + expandedDoneActivityIds: Set + exerciseMap: Record + exerciseNames: Record + restTimer: { remaining: number; total: number } | null + restMinimized: boolean + setRestMinimized: (minimized: boolean) => void + undoAction: { setId: string; expiresAt: number } | null + pendingInputs: Record + setPendingInputs: React.Dispatch>> + programBannerProps: { + programName?: string + blockName?: string + weekNumber?: number + dayLabel?: string + } | 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, + restSeconds?: number, + ) => Promise + deleteSet: (activityId: string, setId: string) => Promise + removeActivity: (activityId: string) => Promise + skipRest: () => void + adjustRest: (delta: number) => void +} + +export function StrengthWorkoutView({ + workoutLog, + loggedGroups, + elapsedSeconds, + isPauseSupported, + isPaused, + handlePause, + handleResume, + handleFinish, + handleDiscard, + handleAddExercise, + handleConfirmSet, + handleUndoSet, + handleUnconfirmSet, + handleMarkDone, + handleExpandDone, + isBroadcasting, + publishFocus, + publishUnfocus, + isFinishing, + isDiscarding, + isConfirmingSet, + isProgrammedWorkout, + firstWorkoutCompleted, + confirmedSetCount, + activeFocusId, + allActivitiesDone, + skippedActivityIds, + expandedDoneActivityIds, + exerciseMap, + exerciseNames, + restTimer, + restMinimized, + setRestMinimized, + undoAction, + pendingInputs, + setPendingInputs, + programBannerProps, + showAddExercise, + setShowAddExercise, + showDiscardDialog, + setShowDiscardDialog, + pageError, + setPageError, + confirmSet, + deleteSet, + removeActivity, + skipRest, + adjustRest, +}: StrengthWorkoutViewProps) { + return ( +
+ {pageError && setPageError(null)} />} + + {/* Sticky header with timer + paused-state action bar */} +
+ + } + /> + 0} + onDiscard={() => setShowDiscardDialog(true)} + showFinishHelper={confirmedSetCount === 0} + /> +
+ + {/* Program context banner -- only visible in SET mode, and only + until the first confirmed set orients the user. */} + {!restTimer && programBannerProps && confirmedSetCount === 0 && ( + + )} + + {/* REST mode (full-page): shown when rest is active and not minimized. */} + {restTimer && !restMinimized && ( + setRestMinimized(true)} + /> + )} + + {/* REST mode (minimized): compact sticky banner with expand + skip. */} + {restTimer && restMinimized && ( + setRestMinimized(false)} + onSkip={skipRest} + /> + )} + + {/* SET mode: only the active exercise block renders (hard focus). */} + {(!restTimer || restMinimized) && ( + <> +
+ {loggedGroups.map((group) => { + // Group-level rendering: circuits render once per group + if (group.groupType === 'CIRCUIT') { + const circuitExercises = group.activities.map((a) => ({ + name: exerciseNames[a.exerciseId] ?? 'Unknown', + targetReps: DEFAULT_CIRCUIT_REPS, + })) + // Hard focus: only render the active block in SET mode. + if (group.id !== activeFocusId) return null + return ( +
+ { + const activity = group.activities[exerciseIndex] + if (!activity) return + try { + // Pass restSeconds: 0 -- CircuitPanel runs its own inter-exercise + // and inter-round rest, so the global rest timer must not fire. + await confirmSet( + activity.id, + { + loggedActivityId: activity.id, + setNumber: round, + setType: 'WORKING', + completed: true, + actualReps, + }, + 0, + ) + } catch (err) { + console.error('[workout-log] Failed to log circuit set:', err) + setPageError('Failed to save circuit set.') + } + }} + onComplete={() => { + // Per-set logging happens in onExerciseDone; nothing to do here. + }} + /> +
+ ) + } + + return group.activities.map((activity) => { + const exercise = exerciseMap[activity.exerciseId] + const modality = getExerciseModality(exercise, group.groupType) + + // Cardio and ruck activities do not have a collapsed done state -- + // hide them when skipped. + if ( + skippedActivityIds.has(activity.id) && + (modality === 'cardio' || modality === 'ruck') + ) { + return null + } + + // Cardio panel + if (modality === 'cardio' && exercise) { + return ( +
+ { + try { + await confirmSet(activity.id, { + loggedActivityId: activity.id, + setNumber: 1, + setType: 'WORKING', + completed: true, + actualDuration: { seconds: data.durationSeconds }, + actualDistance: data.distance + ? { value: parseFloat(data.distance), unit: 'mi' } + : undefined, + actualHeartRate: data.heartRate + ? parseInt(data.heartRate, 10) + : undefined, + }) + } catch (err) { + console.error('[workout-page] cardio confirmSet:', err) + setPageError('Failed to save cardio session.') + } + }} + /> +
+ ) + } + + // Ruck panel + if (modality === 'ruck') { + return ( +
+ { + try { + await confirmSet(activity.id, { + loggedActivityId: activity.id, + setNumber: 1, + setType: 'WORKING', + completed: true, + actualDuration: { seconds: data.durationSeconds }, + actualDistance: data.distance + ? { value: parseFloat(data.distance), unit: 'mi' } + : undefined, + ruckLoad: data.loadWeight + ? { value: parseFloat(data.loadWeight), unit: 'lb' } + : undefined, + elevationGain: data.elevation + ? { value: parseFloat(data.elevation), unit: 'm' } + : undefined, + }) + } catch (err) { + console.error('[workout-page] ruck confirmSet:', err) + setPageError('Failed to save ruck session.') + } + }} + /> +
+ ) + } + + // Standard strength exercise block + const confirmedSets = activity.sets.filter((s) => s.completed) + const lastConfirmedSet = + confirmedSets.length > 0 ? confirmedSets[confirmedSets.length - 1] : undefined + + // Build set row data: confirmed sets + one empty row for the next set + const setRows: SetRowData[] = [ + ...activity.sets.map((set, idx) => ({ + id: set.id, + setNumber: idx + 1, + weight: set.actualWeight?.value?.toString(), + reps: set.actualReps?.toString(), + confirmed: set.completed, + prescribedWeight: set.prescribed?.weight ?? undefined, + prescribedReps: set.prescribed?.reps ?? undefined, + })), + ] + + // Carry-forward prescribed values for the pending input row. + const lastSetWithPrescription = [...activity.sets] + .reverse() + .find((s) => s.prescribed != null) + const nextPrescribedWeight = + lastSetWithPrescription?.prescribed?.weight ?? undefined + const nextPrescribedReps = lastSetWithPrescription?.prescribed?.reps ?? undefined + + // Show a pending input row by default for the first set; subsequent + // rows require an explicit ADD SET tap. Don't add a pending row if + // there's already an unconfirmed (completed: false) set in the list -- + // that happens when the user un-confirms a set. + const hasUncompletedSets = activity.sets.some((s) => !s.completed) + if ( + (confirmedSets.length === 0 && !hasUncompletedSets) || + pendingInputs[activity.id] + ) { + const nextSetNumber = setRows.length + 1 + setRows.push({ + id: `pending-${activity.id}-${nextSetNumber}`, + setNumber: nextSetNumber, + weight: + lastConfirmedSet?.actualWeight?.value?.toString() ?? + nextPrescribedWeight?.value?.toString(), + reps: + lastConfirmedSet?.actualReps?.toString() ?? + (nextPrescribedReps != null ? String(nextPrescribedReps) : undefined), + confirmed: false, + prescribedWeight: nextPrescribedWeight, + prescribedReps: nextPrescribedReps, + isPending: true, + }) + } + + const isDone = + skippedActivityIds.has(activity.id) && + !expandedDoneActivityIds.has(activity.id) + + return ( +
+ handleExpandDone(activity.id)} + onAddSet={() => + setPendingInputs((prev) => ({ ...prev, [activity.id]: true })) + } + onDeleteSet={(setId) => { + if (setId.startsWith('pending-')) { + setPendingInputs((prev) => ({ ...prev, [activity.id]: false })) + } else { + deleteSet(activity.id, setId).catch((err) => { + console.error('[workout-page] deleteSet failed:', { activityId: activity.id, setId, err }) + setPageError('Failed to delete set. Please try again.') + }) + } + }} + onUnconfirmSet={handleUnconfirmSet} + onSkipExercise={() => handleMarkDone(activity.id)} + onRemoveExercise={() => { + removeActivity(activity.id).catch((err) => { + console.error('[workout-page] removeActivity failed:', { activityId: activity.id, err }) + setPageError('Failed to remove exercise. Please try again.') + }) + }} + /> +
+ ) + }) + })} +
+ + {/* Sticky footer -- single forward action for free-form workouts. + Programmed workouts render no footer (the flow is predetermined). */} + {!isProgrammedWorkout && ( +
+ {allActivitiesDone && ( +

+ ALL EXERCISES DONE -- READY TO FINISH? +

+ )} + {loggedGroups.length === 0 && !firstWorkoutCompleted && ( + + Tap below to add your first exercise. + + )} + +
+ )} + + )} + + {/* Undo banner (shown when rest timer is NOT active) */} + {!restTimer && } + + {/* Add exercise sheet */} + + + {/* Discard confirmation dialog */} + + + + Discard workout + + All logged sets will be permanently deleted. This cannot be undone. + + + + + + + + +
+ ) +} diff --git a/src/routes/_authenticated/log.$workoutId.tsx b/src/routes/_authenticated/log.$workoutId.tsx index 6cea41c..83deaad 100644 --- a/src/routes/_authenticated/log.$workoutId.tsx +++ b/src/routes/_authenticated/log.$workoutId.tsx @@ -1,6 +1,5 @@ import { createFileRoute, useNavigate } from '@tanstack/react-router' import { useCallback, useEffect, useMemo, useState } from 'react' -import { Plus } from 'lucide-react' import { isTauri } from '@tauri-apps/api/core' import { useActiveWorkout } from '@/hooks/use-active-workout' import { useActiveWorkoutStore } from '@/stores/active-workout-store' @@ -8,36 +7,14 @@ import { useExercises } from '@/hooks/use-exercises' import { useUserProfile } from '@/hooks/use-user-profile' import { useOnboarding } from '@/hooks/use-onboarding' import { useOnboardingStore } from '@/stores/onboarding-store' -import { OnboardingHint } from '@/components/onboarding/onboarding-hint' import { useProgramFull } from '@/hooks/use-programs' import { detectPersonalRecords } from '@/lib/pr-detection' import { useDisplayBroadcast } from '@/hooks/use-display-broadcast' import { getActiveGymId } from '@/lib/display-realtime' -import { WorkoutHeader } from '@/components/workout/workout-header' -import { WorkoutPausedBar } from '@/components/workout/workout-paused-bar' -import { ErrorBanner } from '@/components/workout/error-banner' -import { WorkoutHeaderMenu } from '@/components/workout/workout-header-menu' -import { ExerciseBlock, type SetRowData } from '@/components/workout/exercise-block' -import { ProgramContextBanner } from '@/components/workout/program-context-banner' -import { RestView } from '@/components/workout/rest-view' -import { RestTimerBanner } from '@/components/workout/rest-timer-banner' -import { UndoBanner } from '@/components/workout/undo-banner' -import { AddExerciseSheet } from '@/components/workout/add-exercise-sheet' -import { CardioPanel } from '@/components/workout/cardio-panel' -import { RuckPanel } from '@/components/workout/ruck-panel' -import { CircuitPanel } from '@/components/workout/circuit-panel' import { WorkoutSummary } from '@/components/workout/workout-summary' -import { EventDetail } from '@/components/event-builder/event-detail' -import { Button } from '@/components/ui/button' -import { - Dialog, - DialogContent, - DialogDescription, - DialogFooter, - DialogHeader, - DialogTitle, -} from '@/components/ui/dialog' -import { getExerciseModality, parseNumericInput, DEFAULT_CIRCUIT_REPS } from '@/lib/workout-utils' +import { EventWorkoutView } from '@/components/workout/event-workout-view' +import { StrengthWorkoutView } from '@/components/workout/strength-workout-view' +import { parseNumericInput } from '@/lib/workout-utils' import type { Exercise, PersonalRecord, SetType } from '@/domain/types' import type { LoggedActivityGroupWithActivities } from '@/stores/active-workout-store' @@ -508,406 +485,78 @@ function ActiveWorkoutPage() { if (workoutLog.eventMetadata) { return ( -
- {pageError && setPageError(null)} />} - - {/* Sticky header with timer + paused-state action bar */} -
- - } - /> - setShowDiscardDialog(true)} - showFinishHelper={false} - /> -
- - - - {/* Discard confirmation dialog */} - - - - Discard workout - - All logged sets will be permanently deleted. This cannot be undone. - - - - - - - - -
+ ) } return ( -
- {pageError && setPageError(null)} />} - - {/* Sticky header with timer + paused-state action bar */} -
- - } - /> - 0} - onDiscard={() => setShowDiscardDialog(true)} - showFinishHelper={confirmedSetCount === 0} - /> -
- - {/* Program context banner -- only visible in SET mode, and only - until the first confirmed set orients the user. */} - {!restTimer && programBannerProps && confirmedSetCount === 0 && ( - - )} - - {/* REST mode (full-page): shown when rest is active and not minimized. */} - {restTimer && !restMinimized && ( - setRestMinimized(true)} - /> - )} - - {/* REST mode (minimized): compact sticky banner with expand + skip. */} - {restTimer && restMinimized && ( - setRestMinimized(false)} - onSkip={skipRest} - /> - )} - - {/* SET mode: only the active exercise block renders (hard focus). */} - {(!restTimer || restMinimized) && ( - <> -
- {loggedGroups.map((group) => { - // Group-level rendering: circuits render once per group - if (group.groupType === 'CIRCUIT') { - const circuitExercises = group.activities.map((a) => ({ - name: exerciseNames[a.exerciseId] ?? 'Unknown', - targetReps: DEFAULT_CIRCUIT_REPS, - })) - // Hard focus: only render the active block in SET mode. - if (group.id !== activeFocusId) return null - return ( -
- { - const activity = group.activities[exerciseIndex] - if (!activity) return - try { - // Pass restSeconds: 0 -- CircuitPanel runs its own inter-exercise - // and inter-round rest, so the global rest timer must not fire. - await confirmSet( - activity.id, - { - loggedActivityId: activity.id, - setNumber: round, - setType: 'WORKING', - completed: true, - actualReps, - }, - 0, - ) - } catch (err) { - console.error('[workout-log] Failed to log circuit set:', err) - setPageError('Failed to save circuit set.') - } - }} - onComplete={() => { - // Per-set logging happens in onExerciseDone; nothing to do here. - }} - /> -
- ) - } - - return group.activities.map((activity) => { - const exercise = exerciseMap[activity.exerciseId] - const modality = getExerciseModality(exercise, group.groupType) - - // Cardio and ruck activities do not have a collapsed done state -- - // hide them when skipped. - if ( - skippedActivityIds.has(activity.id) && - (modality === 'cardio' || modality === 'ruck') - ) { - return null - } - - // Cardio panel - if (modality === 'cardio' && exercise) { - return ( -
- { - try { - await confirmSet(activity.id, { - loggedActivityId: activity.id, - setNumber: 1, - setType: 'WORKING', - completed: true, - actualDuration: { seconds: data.durationSeconds }, - actualDistance: data.distance - ? { value: parseFloat(data.distance), unit: 'mi' } - : undefined, - actualHeartRate: data.heartRate - ? parseInt(data.heartRate, 10) - : undefined, - }) - } catch (err) { - console.error('[workout-page] cardio confirmSet:', err) - setPageError('Failed to save cardio session.') - } - }} - /> -
- ) - } - - // Ruck panel - if (modality === 'ruck') { - return ( -
- { - try { - await confirmSet(activity.id, { - loggedActivityId: activity.id, - setNumber: 1, - setType: 'WORKING', - completed: true, - actualDuration: { seconds: data.durationSeconds }, - actualDistance: data.distance - ? { value: parseFloat(data.distance), unit: 'mi' } - : undefined, - ruckLoad: data.loadWeight - ? { value: parseFloat(data.loadWeight), unit: 'lb' } - : undefined, - elevationGain: data.elevation - ? { value: parseFloat(data.elevation), unit: 'm' } - : undefined, - }) - } catch (err) { - console.error('[workout-page] ruck confirmSet:', err) - setPageError('Failed to save ruck session.') - } - }} - /> -
- ) - } - - // Standard strength exercise block - const confirmedSets = activity.sets.filter((s) => s.completed) - const lastConfirmedSet = - confirmedSets.length > 0 ? confirmedSets[confirmedSets.length - 1] : undefined - - // Build set row data: confirmed sets + one empty row for the next set - const setRows: SetRowData[] = [ - ...activity.sets.map((set, idx) => ({ - id: set.id, - setNumber: idx + 1, - weight: set.actualWeight?.value?.toString(), - reps: set.actualReps?.toString(), - confirmed: set.completed, - prescribedWeight: set.prescribed?.weight ?? undefined, - prescribedReps: set.prescribed?.reps ?? undefined, - })), - ] - - // Carry-forward prescribed values for the pending input row. - const lastSetWithPrescription = [...activity.sets] - .reverse() - .find((s) => s.prescribed != null) - const nextPrescribedWeight = - lastSetWithPrescription?.prescribed?.weight ?? undefined - const nextPrescribedReps = lastSetWithPrescription?.prescribed?.reps ?? undefined - - // Show a pending input row by default for the first set; subsequent - // rows require an explicit ADD SET tap. Don't add a pending row if - // there's already an unconfirmed (completed: false) set in the list -- - // that happens when the user un-confirms a set. - const hasUncompletedSets = activity.sets.some((s) => !s.completed) - if ( - (confirmedSets.length === 0 && !hasUncompletedSets) || - pendingInputs[activity.id] - ) { - const nextSetNumber = setRows.length + 1 - setRows.push({ - id: `pending-${activity.id}-${nextSetNumber}`, - setNumber: nextSetNumber, - weight: - lastConfirmedSet?.actualWeight?.value?.toString() ?? - nextPrescribedWeight?.value?.toString(), - reps: - lastConfirmedSet?.actualReps?.toString() ?? - (nextPrescribedReps != null ? String(nextPrescribedReps) : undefined), - confirmed: false, - prescribedWeight: nextPrescribedWeight, - prescribedReps: nextPrescribedReps, - isPending: true, - }) - } - - const isDone = - skippedActivityIds.has(activity.id) && - !expandedDoneActivityIds.has(activity.id) - - return ( -
- handleExpandDone(activity.id)} - onAddSet={() => - setPendingInputs((prev) => ({ ...prev, [activity.id]: true })) - } - onDeleteSet={(setId) => { - if (setId.startsWith('pending-')) { - setPendingInputs((prev) => ({ ...prev, [activity.id]: false })) - } else { - deleteSet(activity.id, setId).catch((err) => { - console.error('[workout-page] deleteSet failed:', { activityId: activity.id, setId, err }) - setPageError('Failed to delete set. Please try again.') - }) - } - }} - onUnconfirmSet={handleUnconfirmSet} - onSkipExercise={() => handleMarkDone(activity.id)} - onRemoveExercise={() => { - removeActivity(activity.id).catch((err) => { - console.error('[workout-page] removeActivity failed:', { activityId: activity.id, err }) - setPageError('Failed to remove exercise. Please try again.') - }) - }} - /> -
- ) - }) - })} -
- - {/* Sticky footer -- single forward action for free-form workouts. - Programmed workouts render no footer (the flow is predetermined). */} - {!isProgrammedWorkout && ( -
- {allActivitiesDone && ( -

- ALL EXERCISES DONE -- READY TO FINISH? -

- )} - {loggedGroups.length === 0 && !firstWorkoutCompleted && ( - - Tap below to add your first exercise. - - )} - -
- )} - - )} - - {/* Undo banner (shown when rest timer is NOT active) */} - {!restTimer && } - - {/* Add exercise sheet */} - - - {/* Discard confirmation dialog */} - - - - Discard workout - - All logged sets will be permanently deleted. This cannot be undone. - - - - - - - - -
+ ) } From 5c40c5eeec9f324b7437229e2782caf46b97db90 Mon Sep 17 00:00:00 2001 From: Robert Hamilton Date: Mon, 20 Apr 2026 19:31:13 -0500 Subject: [PATCH 2/3] fix(workout): resolve PR #115 review findings P22-001 through P22-015 FIX (Critical) - P22-001: computeElapsed catch now calls setPageError on corrupt timer data - P22-002: circuit onExerciseDone guard logs + setPageError instead of silent return FIX (High) - P22-003: workoutLog null guard returns surface div + error log instead of blank null - P22-004: handleMarkDone/handleExpandDone guard empty activityId with log + setPageError - P22-005: confirmSet prop adds exerciseName? 4th parameter to StrengthWorkoutViewProps FIX (Medium) - P22-006: circuit catch prefix [workout-log] -> [strength-workout-view] - P22-007: CardioPanel/RuckPanel catch prefix [workout-page] -> [strength-workout-view] - P22-008: handleDiscard awaits navigate with .catch error + setPageError - P22-013: handleSummaryDone navigate chains .catch with log + setPageError FIX (Low) - P22-009: remove exerciseNames prop from StrengthWorkoutView; derive from exerciseMap - P22-010: export UndoAction from active-workout-store; use in StrengthWorkoutViewProps - P22-011: extract ProgramBannerProps named type with weekNumber: number required - P22-012: export EventWorkoutLog type from event-workout-view TASK: P22-014 added to Ideas backlog (move modal state into StrengthWorkoutView) ADR: P22-015 -> ADR-022 (mutation orchestration boundary in StrengthWorkoutView) Review: Context/Reviews/0022-pr115-split-log-workout-route-2026-04-20.md --- Context/Backlog/Ideas.md | 8 + ...th-view-mutation-orchestration-boundary.md | 54 ++++++ ...r115-split-log-workout-route-2026-04-20.md | 177 ++++++++++++++++++ src/components/workout/event-workout-view.tsx | 4 +- .../workout/strength-workout-view.tsx | 35 ++-- src/routes/_authenticated/log.$workoutId.tsx | 27 ++- src/stores/active-workout-store.ts | 2 +- 7 files changed, 287 insertions(+), 20 deletions(-) create mode 100644 Context/Decisions/ADR-022-strength-view-mutation-orchestration-boundary.md create mode 100644 Context/Reviews/0022-pr115-split-log-workout-route-2026-04-20.md diff --git a/Context/Backlog/Ideas.md b/Context/Backlog/Ideas.md index bc2e205..760113c 100644 --- a/Context/Backlog/Ideas.md +++ b/Context/Backlog/Ideas.md @@ -5,6 +5,14 @@ skill. Prioritize via the backlog-prioritize skill. +## Move view-local modal state into StrengthWorkoutView (P22-014) + +**Added:** 2026-04-20 +**Source:** PR #115 review finding P22-014 +**Priority:** Low + +`showAddExercise`, `showDiscardDialog`, `pendingInputs`, and `restMinimized` (4 `useState` calls) are used exclusively inside `StrengthWorkoutView` but are held in the parent route because the extraction was a pure mechanical lift. Moving these into the component cuts the prop interface from ~44 to ~36 props and removes shared-state ownership with no behavioral change. Out of scope for the extraction PR -- do as a standalone refactor. + ## ~~P17 Review: Adapter test coverage gaps (PR #107)~~ (Resolved 2026-04-15) **Added:** 2026-04-12 diff --git a/Context/Decisions/ADR-022-strength-view-mutation-orchestration-boundary.md b/Context/Decisions/ADR-022-strength-view-mutation-orchestration-boundary.md new file mode 100644 index 0000000..4b135e0 --- /dev/null +++ b/Context/Decisions/ADR-022-strength-view-mutation-orchestration-boundary.md @@ -0,0 +1,54 @@ +# ADR-022: Mutation Orchestration Boundary in StrengthWorkoutView + +**Date:** 2026-04-20 +**Status:** Proposed +**Context:** PR #115 review finding P22-015 + +--- + +## Context + +After extracting `StrengthWorkoutView` from `log.$workoutId.tsx`, the component receives both a raw hook operation (`confirmSet`) and a parent-wrapped handler (`handleConfirmSet`). The two are not interchangeable: + +- `handleConfirmSet` (strength sets): calls `setRestMinimized(false)` before confirming, normalizes raw string inputs via `parseNumericInput`, then calls `confirmSet` +- `confirmSet` (circuit, cardio, ruck sets): called directly, bypassing rest-timer minimization and input normalization + +This means the same physical action -- completing a set -- takes two different code paths depending on modality. The inconsistency is invisible from the component's prop interface. + +## Decision Needed + +Where does mutation orchestration belong: in the route or in the view? + +### Option A: Orchestration stays in the route (current state) + +The route wraps `confirmSet` into modality-specific handlers (`handleConfirmSet`, and in future `handleCardioConfirmSet`, etc.) and passes each to the view. The view is a pure presentation component that calls whatever handler it receives. + +**Pros:** View has no orchestration logic; route owns all side effects. +**Cons:** Props proliferate as modalities grow; view cannot change its own orchestration; the "two paths" bug can silently re-emerge on new modalities. + +### Option B: Orchestration moves into the view + +The view receives only `confirmSet` (raw) plus the data it needs (`setRestMinimized`). The view owns the normalization and side-effect logic per modality. + +**Pros:** All set-completion paths live together; adding a modality cannot silently bypass orchestration. +**Cons:** View has business logic; harder to test orchestration in isolation. + +### Option C: Extract a per-modality orchestration hook + +A `useSetConfirmation` hook (or similar) wraps `confirmSet` and `setRestMinimized`, encapsulates normalization, and returns modality-aware handlers. Both the route and view call into the hook rather than each other. + +**Pros:** Clean separation; testable; view and route stay thin. +**Cons:** Additional abstraction layer; only justified if orchestration complexity grows. + +## Recommendation + +**Option B** is the right default for current complexity: move `handleConfirmSet` logic into the view so all set-completion paths are colocated. Revisit Option C if a third or fourth modality requires materially different orchestration. + +This decision should be resolved before the next feature that adds a new workout modality or modifies the set-confirmation flow, to prevent the inconsistency from hardening further. + +## Consequences + +- `handleConfirmSet` is removed from the route's prop set passed to `StrengthWorkoutView` +- `setRestMinimized` remains a prop (used for rest-timer minimization side effect) +- Normalization logic (`parseNumericInput`) moves into the view's strength-set path +- The `confirmSet` prop type may need `exerciseName?: string` threading for the strength path diff --git a/Context/Reviews/0022-pr115-split-log-workout-route-2026-04-20.md b/Context/Reviews/0022-pr115-split-log-workout-route-2026-04-20.md new file mode 100644 index 0000000..72217fd --- /dev/null +++ b/Context/Reviews/0022-pr115-split-log-workout-route-2026-04-20.md @@ -0,0 +1,177 @@ +# PR Review: refactor/split-log-workout-route → main + +**Date:** 2026-04-20 +**Feature:** N/A (pure extraction refactor, no feature spec) +**Branch:** refactor/split-log-workout-route +**Reviewers:** pr-review-toolkit (code-reviewer, silent-failure-hunter, type-design-analyzer) +**Status:** ✅ Resolved + +## Summary + +15 findings total across 3 files (`event-workout-view.tsx`, `strength-workout-view.tsx`, +`log.$workoutId.tsx`). The extraction is mechanically faithful — no logic leaked into views, +no Iron & Ember regressions, no silent-import drift. Primary issues are error handling gaps +carried forward from the original file (including one data-loss path in circuit completion), +log prefix inconsistencies introduced by the split, and type drift on the `confirmSet` prop. +One architectural concern about dual mutation paths and one follow-up task for state ownership. + +**Breakdown:** 13 [FIX] · 1 [TASK] · 1 [ADR] + +--- + +## Findings + +### Fix-Now + +#### [FIX] P22-001: `computeElapsed` catch logs but never calls `setPageError` +- **File:** `src/routes/_authenticated/log.$workoutId.tsx:208-211` +- **Severity:** Critical +- **Detail:** The catch block returns `0` and logs the error but does not call `setPageError`. If `startedAt` is corrupt, this fires on every interval tick and the user sees a frozen 00:00 timer with no explanation. The `Number.isFinite` guard above it (which does call `setPageError`) only catches the non-finite case — a JS exception in `computeElapsed` takes a separate code path. +- **Fix:** Add `setPageError('Workout timer data is corrupt. Please discard or reload.')` inside the catch before `return 0`. +- **Status:** ✅ Fixed +- **Resolution:** Added `setPageError('Workout timer data is corrupt. Please discard or reload.')` inside the catch before `return 0`. + +#### [FIX] P22-002: Silent data loss in circuit `onExerciseDone` guard +- **File:** `src/components/workout/strength-workout-view.tsx:227-228` +- **Severity:** Critical +- **Detail:** `if (!activity) return` when `exerciseIndex` is out of bounds silently drops the circuit rep. The user has no indication the save was skipped — their workout data is silently incomplete. Must log with `[strength-workout-view]` prefix and call `setPageError`. +- **Fix:** + ```ts + if (!activity) { + console.error('[strength-workout-view] onExerciseDone: no activity at index', { exerciseIndex, round, groupId: group.id }) + setPageError('Failed to record circuit set. Please log it manually.') + return + } + ``` +- **Status:** ✅ Fixed +- **Resolution:** Added error log and `setPageError` call before returning; guard now surfaces the failure. + +#### [FIX] P22-003: Bare `return null` guard renders blank screen +- **File:** `src/routes/_authenticated/log.$workoutId.tsx:480` +- **Severity:** High +- **Detail:** `if (!workoutLog) return null` renders a completely blank screen with no log entry and no user message. The `useEffect` redirect fires after the render, leaving a visible void window. Per error-handling conventions, guard clauses in rendering paths that can produce user-visible failure states must log. +- **Fix:** Replace with a minimal logged indicator: `console.error('[workout-page] Rendered without active workoutLog'); return
` +- **Status:** ✅ Fixed +- **Resolution:** Replaced `return null` with `console.error` + minimal surface div `
`. + +#### [FIX] P22-004: `handleMarkDone` and `handleExpandDone` — silent user-action bare returns +- **File:** `src/routes/_authenticated/log.$workoutId.tsx:441-456` +- **Severity:** High +- **Detail:** Both handlers accept `activityId: string` with no guard. If `activityId` is empty or stale, `skipActivity` and `setExpandedDoneActivityIds` mutate silently with no log and no user feedback. Per error-handling conventions, user-action handlers must never silently return. +- **Fix:** Add `if (!activityId) { console.error('[workout-page] handleMarkDone called with empty activityId'); setPageError('Could not mark exercise done. Please reload.'); return }` at entry. +- **Status:** ✅ Fixed +- **Resolution:** Both handlers guard `!activityId` with `console.error` + `setPageError` before proceeding. + +#### [FIX] P22-005: `confirmSet` prop drops `exerciseName?: string` 4th parameter +- **File:** `src/components/workout/strength-workout-view.tsx:84-88` +- **Severity:** High +- **Detail:** The hook signature is `(loggedActivityId, setData, restSeconds?, exerciseName?)` but the prop interface declares only 3 parameters. Any future caller inside the view that needs to forward `exerciseName` cannot — the type silently truncates it. The return type is also widened to `Promise` rather than the hook's typed return. +- **Fix:** Add `exerciseName?: string` as 4th parameter to the prop type. Propagate the hook's actual return type if known. +- **Status:** ✅ Fixed +- **Resolution:** Added `exerciseName?: string` as 4th parameter to `confirmSet` prop type in `StrengthWorkoutViewProps`. + +#### [FIX] P22-006: Log prefix `[workout-log]` inconsistent with rest of file +- **File:** `src/components/workout/strength-workout-view.tsx:244` +- **Severity:** Medium +- **Detail:** Circuit set catch emits `[workout-log]` while every other log in both extracted components uses `[workout-page]`. This inconsistency breaks grep searches in production logs when tracing circuit failures. +- **Fix:** Change `[workout-log]` → `[workout-page]` at line 244. +- **Status:** ✅ Fixed +- **Resolution:** Changed `[workout-log]` → `[strength-workout-view]` (used consistent prefix with P22-007 rather than `[workout-page]` as specified; `[strength-workout-view]` is the correct origin module). + +#### [FIX] P22-007: CardioPanel/RuckPanel catch blocks use wrong module prefix +- **File:** `src/components/workout/strength-workout-view.tsx:291, 323` +- **Severity:** Medium +- **Detail:** Both catch blocks emit `[workout-page]` but they live inside `strength-workout-view.tsx`. Module prefixes should reflect where the log originates. Using the route's prefix for a component makes failures harder to locate in production log search. +- **Fix:** Change both to `[strength-workout-view]`. +- **Status:** ✅ Fixed +- **Resolution:** Changed both CardioPanel and RuckPanel catch prefixes from `[workout-page]` to `[strength-workout-view]`. + +#### [FIX] P22-008: `handleDiscard` — `navigate` not awaited after destructive operation +- **File:** `src/routes/_authenticated/log.$workoutId.tsx:341-350` +- **Severity:** Medium +- **Detail:** `discardWorkout()` is awaited and the dialog closes, then `navigate({ to: '/' })` is called fire-and-forget. If navigation fails, the user is stranded on a cleared-state page: workout discarded, dialog gone, no active session, no recovery path. +- **Fix:** `await navigate({ to: '/' }).catch((err) => { console.error('[workout-page] handleDiscard navigation failed:', err); setPageError('Workout discarded but could not navigate home.') })` +- **Status:** ✅ Fixed +- **Resolution:** `navigate` is now awaited with a `.catch` that logs and calls `setPageError` on navigation failure. + +#### [FIX] P22-009: Remove `exerciseNames` prop — redundant with `exerciseMap` +- **File:** `src/components/workout/strength-workout-view.tsx:65` +- **Severity:** Low +- **Detail:** `exerciseNames: Record` is fully derivable from `exerciseMap` inside the component. Both records must share the same key universe — passing them separately creates a consistency dependency the type system cannot enforce. +- **Fix:** Remove `exerciseNames` from `StrengthWorkoutViewProps`; derive inside the component: `const exerciseNames = Object.fromEntries(Object.entries(exerciseMap).map(([k, v]) => [k, v.name]))`. Alternatively, thread `exerciseMap` into the child components that currently accept `Record`. +- **Status:** ✅ Fixed +- **Resolution:** Removed `exerciseNames` from `StrengthWorkoutViewProps` and call site; added `const exerciseNames = Object.fromEntries(...)` derivation inside the component. + +#### [FIX] P22-010: Name the anonymous `undoAction` inline type +- **File:** `src/components/workout/strength-workout-view.tsx:69` +- **Severity:** Low +- **Detail:** `{ setId: string; expiresAt: number }` is a domain concept (an undo window) declared as an anonymous inline type. If the store already names this type, import it; if not, export `type UndoAction = { setId: string; expiresAt: number }` from the store and reference it here. +- **Status:** ✅ Fixed +- **Resolution:** Exported `UndoAction` from `active-workout-store.ts`; imported and used it in `StrengthWorkoutViewProps`. + +#### [FIX] P22-011: `programBannerProps` allows an all-optional empty object +- **File:** `src/components/workout/strength-workout-view.tsx:72-77` +- **Severity:** Low +- **Detail:** The inline type has four optional fields, meaning `{}` is structurally valid. A caller could pass `programBannerProps: {}` (non-null), enter the `ProgramContextBanner` render path, and display a blank banner with no type error. +- **Fix:** Extract to a named type with at least one required field (e.g., `programName: string`) so the "has a program but no data" case is unrepresentable. +- **Status:** ✅ Fixed +- **Resolution:** Extracted to named `ProgramBannerProps` type with `weekNumber: number` required (always present from `programContext`); replaces inline type in `StrengthWorkoutViewProps`. + +#### [FIX] P22-012: Extract `EventWorkoutViewProps.workoutLog` to a named type +- **File:** `src/components/workout/event-workout-view.tsx:18` +- **Severity:** Low +- **Detail:** The inline `{ id: string; eventMetadata: EventMetadata }` achieves correct narrowing but is anonymous. Tests, stories, or future sibling components must re-type it structurally. A named export (`EventWorkoutLog`) gives it a stable identity. +- **Fix:** `export type EventWorkoutLog = { id: string; eventMetadata: EventMetadata }` at module scope, then use in the props interface. +- **Status:** ✅ Fixed +- **Resolution:** Exported `EventWorkoutLog = { id: string; eventMetadata: EventMetadata }` from `event-workout-view.tsx`; used in `EventWorkoutViewProps`. + +#### [FIX] P22-013: `handleSummaryDone` — uncaught navigation failure leaves user stranded +- **File:** `src/routes/_authenticated/log.$workoutId.tsx:434-439` +- **Severity:** Medium +- **Detail:** `navigate({ to: '/' })` is called after clearing `summaryData`, `showSummary`, and `detectedPrs`. If navigation fails, the user is on a blank post-summary state with no recovery path and no feedback. +- **Fix:** Chain `.catch((err) => { console.error('[workout-page] handleSummaryDone navigation failed:', err); setPageError('Could not return to home. Please navigate manually.') })` on the navigate call. +- **Status:** ✅ Fixed +- **Resolution:** Chained `.catch` on `navigate({ to: '/' })` with `console.error` + `setPageError`. + +--- + +### Missing Tasks + +#### [TASK] P22-014: Move view-local modal state into `StrengthWorkoutView` +- **File:** `src/components/workout/strength-workout-view.tsx:29-93` / `src/routes/_authenticated/log.$workoutId.tsx:113-123` +- **Severity:** Low +- **Detail:** `showAddExercise`, `setShowAddExercise`, `showDiscardDialog`, `setShowDiscardDialog`, `pendingInputs`, `setPendingInputs`, `restMinimized`, `setRestMinimized` are all used exclusively inside `StrengthWorkoutView`. The parent holds them only because this was a pure extraction. Moving these 4 `useState` calls into the view cuts the prop interface from ~44 to ~36 props and removes shared state ownership. This is out of scope for a pure-extraction PR — track as a follow-up. +- **Relates to:** — +- **Status:** ✅ Task created +- **Resolution:** Added to `Context/Backlog/Ideas.md` as a standalone refactor item. + +--- + +### Architectural Concerns + +#### [ADR] P22-015: Dual mutation paths — `confirmSet` (raw) and `handleConfirmSet` (wrapped) both passed to `StrengthWorkoutView` +- **File:** `src/components/workout/strength-workout-view.tsx:36-48, 84-90` +- **Severity:** Medium +- **Detail:** The view receives both the raw hook operation (`confirmSet`) and the parent-wrapped handler (`handleConfirmSet`). Strength sets use `handleConfirmSet` (which calls `setRestMinimized(false)` before confirming and normalizes inputs via `parseNumericInput`). Circuit, cardio, and ruck sets call raw `confirmSet` directly, bypassing the rest-timer minimization and normalization. This is a behavioral inconsistency invisible from the prop interface: the same physical action (completing a set) takes two different code paths depending on modality. The root question is whether mutation orchestration (wrapping, normalizing, side-effecting) belongs in the route or in the view — and the current split answers "both", which is the wrong answer. Requires an explicit design decision before the next feature builds on top of this component. +- **Relates to:** — +- **Status:** ✅ ADR created +- **Resolution:** ADR-022 — `Context/Decisions/ADR-022-strength-view-mutation-orchestration-boundary.md` + +--- + +## Resolution Checklist +- [x] All [FIX] findings resolved (P22-001 through P22-013) +- [x] All [TASK] findings added to Steps.md or backlog (P22-014) +- [x] All [ADR] findings have ADRs created or dismissed (P22-015) +- [ ] Review verified by review-verify agent + +## Resolution Summary +**Resolved at:** 2026-04-20 +**Session:** PR #115 review resolve + +| Category | Total | Resolved | +|---|---|---| +| [FIX] | 13 | 13 | +| [TASK] | 1 | 1 | +| [ADR] | 1 | 1 | +| **Total** | **15** | **15** | diff --git a/src/components/workout/event-workout-view.tsx b/src/components/workout/event-workout-view.tsx index b579227..5155925 100644 --- a/src/components/workout/event-workout-view.tsx +++ b/src/components/workout/event-workout-view.tsx @@ -14,8 +14,10 @@ import { } from '@/components/ui/dialog' import type { EventMetadata } from '@/domain/types' +export type EventWorkoutLog = { id: string; eventMetadata: EventMetadata } + interface EventWorkoutViewProps { - workoutLog: { id: string; eventMetadata: EventMetadata } + workoutLog: EventWorkoutLog elapsedSeconds: number isPauseSupported: boolean isPaused: boolean diff --git a/src/components/workout/strength-workout-view.tsx b/src/components/workout/strength-workout-view.tsx index f296fe6..2aeba67 100644 --- a/src/components/workout/strength-workout-view.tsx +++ b/src/components/workout/strength-workout-view.tsx @@ -24,7 +24,14 @@ import { } from '@/components/ui/dialog' import { getExerciseModality, DEFAULT_CIRCUIT_REPS } from '@/lib/workout-utils' import type { Exercise, LoggedSet, SetType } from '@/domain/types' -import type { LoggedActivityGroupWithActivities } from '@/stores/active-workout-store' +import type { LoggedActivityGroupWithActivities, UndoAction } from '@/stores/active-workout-store' + +type ProgramBannerProps = { + programName?: string + blockName?: string + weekNumber: number + dayLabel?: string +} interface StrengthWorkoutViewProps { workoutLog: { id: string; userId: string } @@ -62,19 +69,13 @@ interface StrengthWorkoutViewProps { skippedActivityIds: Set expandedDoneActivityIds: Set exerciseMap: Record - exerciseNames: Record restTimer: { remaining: number; total: number } | null restMinimized: boolean setRestMinimized: (minimized: boolean) => void - undoAction: { setId: string; expiresAt: number } | null + undoAction: UndoAction | null pendingInputs: Record setPendingInputs: React.Dispatch>> - programBannerProps: { - programName?: string - blockName?: string - weekNumber?: number - dayLabel?: string - } | null + programBannerProps: ProgramBannerProps | null showAddExercise: boolean setShowAddExercise: (open: boolean) => void showDiscardDialog: boolean @@ -85,6 +86,7 @@ interface StrengthWorkoutViewProps { loggedActivityId: string, setData: Omit, restSeconds?: number, + exerciseName?: string, ) => Promise deleteSet: (activityId: string, setId: string) => Promise removeActivity: (activityId: string) => Promise @@ -122,7 +124,6 @@ export function StrengthWorkoutView({ skippedActivityIds, expandedDoneActivityIds, exerciseMap, - exerciseNames, restTimer, restMinimized, setRestMinimized, @@ -142,6 +143,8 @@ export function StrengthWorkoutView({ skipRest, adjustRest, }: StrengthWorkoutViewProps) { + const exerciseNames = Object.fromEntries(Object.entries(exerciseMap).map(([k, v]) => [k, v.name])) + return (
{pageError && setPageError(null)} />} @@ -225,7 +228,11 @@ export function StrengthWorkoutView({ rounds={3} onExerciseDone={async (exerciseIndex, round, actualReps) => { const activity = group.activities[exerciseIndex] - if (!activity) return + if (!activity) { + console.error('[strength-workout-view] onExerciseDone: no activity at index', { exerciseIndex, round, groupId: group.id }) + setPageError('Failed to record circuit set. Please log it manually.') + return + } try { // Pass restSeconds: 0 -- CircuitPanel runs its own inter-exercise // and inter-round rest, so the global rest timer must not fire. @@ -241,7 +248,7 @@ export function StrengthWorkoutView({ 0, ) } catch (err) { - console.error('[workout-log] Failed to log circuit set:', err) + console.error('[strength-workout-view] Failed to log circuit set:', err) setPageError('Failed to save circuit set.') } }} @@ -288,7 +295,7 @@ export function StrengthWorkoutView({ : undefined, }) } catch (err) { - console.error('[workout-page] cardio confirmSet:', err) + console.error('[strength-workout-view] cardio confirmSet:', err) setPageError('Failed to save cardio session.') } }} @@ -321,7 +328,7 @@ export function StrengthWorkoutView({ : undefined, }) } catch (err) { - console.error('[workout-page] ruck confirmSet:', err) + console.error('[strength-workout-view] ruck confirmSet:', err) setPageError('Failed to save ruck session.') } }} diff --git a/src/routes/_authenticated/log.$workoutId.tsx b/src/routes/_authenticated/log.$workoutId.tsx index 83deaad..6c752dd 100644 --- a/src/routes/_authenticated/log.$workoutId.tsx +++ b/src/routes/_authenticated/log.$workoutId.tsx @@ -207,6 +207,7 @@ function ActiveWorkoutPage() { return Math.max(0, Math.round(elapsedMs / 1000)) } catch (err) { console.error('[workout-log] Failed to compute elapsed:', err) + setPageError('Workout timer data is corrupt. Please discard or reload.') return 0 } } @@ -342,7 +343,10 @@ function ActiveWorkoutPage() { try { await discardWorkout() setShowDiscardDialog(false) - navigate({ to: '/' }) + await navigate({ to: '/' }).catch((err) => { + console.error('[workout-page] handleDiscard navigation failed:', err) + setPageError('Workout discarded but could not navigate home.') + }) } catch (err) { console.error('[workout-page] handleDiscard:', err) setPageError('Failed to discard workout.') @@ -435,11 +439,19 @@ function ActiveWorkoutPage() { setShowSummary(false) setSummaryData(null) setDetectedPrs([]) - navigate({ to: '/' }) + navigate({ to: '/' }).catch((err) => { + console.error('[workout-page] handleSummaryDone navigation failed:', err) + setPageError('Could not return to home. Please navigate manually.') + }) }, [navigate]) const handleMarkDone = useCallback( (activityId: string) => { + if (!activityId) { + console.error('[workout-page] handleMarkDone called with empty activityId') + setPageError('Could not mark exercise done. Please reload.') + return + } skipActivity(activityId) setPendingInputs((prev) => ({ ...prev, [activityId]: false })) setExpandedDoneActivityIds((prev) => { @@ -452,6 +464,11 @@ function ActiveWorkoutPage() { ) const handleExpandDone = useCallback((activityId: string) => { + if (!activityId) { + console.error('[workout-page] handleExpandDone called with empty activityId') + setPageError('Could not expand exercise. Please reload.') + return + } setExpandedDoneActivityIds((prev) => new Set(prev).add(activityId)) }, []) @@ -477,7 +494,10 @@ function ActiveWorkoutPage() { // Guard: no active workout // ----------------------------------------------------------------------- - if (!workoutLog) return null + if (!workoutLog) { + console.error('[workout-page] Rendered without active workoutLog') + return
+ } // ----------------------------------------------------------------------- // Event log: render EventDetail instead of exercise/set UI @@ -538,7 +558,6 @@ function ActiveWorkoutPage() { skippedActivityIds={skippedActivityIds} expandedDoneActivityIds={expandedDoneActivityIds} exerciseMap={exerciseMap} - exerciseNames={exerciseNames} restTimer={restTimer} restMinimized={restMinimized} setRestMinimized={setRestMinimized} diff --git a/src/stores/active-workout-store.ts b/src/stores/active-workout-store.ts index e00a1b3..e2297a0 100644 --- a/src/stores/active-workout-store.ts +++ b/src/stores/active-workout-store.ts @@ -33,7 +33,7 @@ export interface LoggedActivityGroupWithActivities extends LoggedActivityGroup { // Undo action -- tracks the last confirmed set for 10s reversal // --------------------------------------------------------------------------- -interface UndoAction { +export interface UndoAction { setId: string loggedActivityId: string expiresAt: number From 54ee7049a15318d878ac65ff018feeb67c9e4ff0 Mon Sep 17 00:00:00 2001 From: Robert Hamilton Date: Mon, 20 Apr 2026 20:11:48 -0500 Subject: [PATCH 3/3] chore(review): mark PR #115 review verified and archive QuickPlan --- .../2026-04-20-split-log-workout-route.md | 88 +++++++++++++++++++ ...r115-split-log-workout-route-2026-04-20.md | 2 +- 2 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 Context/QuickPlans/Done/2026-04-20-split-log-workout-route.md diff --git a/Context/QuickPlans/Done/2026-04-20-split-log-workout-route.md b/Context/QuickPlans/Done/2026-04-20-split-log-workout-route.md new file mode 100644 index 0000000..40ceac2 --- /dev/null +++ b/Context/QuickPlans/Done/2026-04-20-split-log-workout-route.md @@ -0,0 +1,88 @@ +# Quick Plan: Split log.$workoutId Route into Three Components + +**Task:** Refactor `src/routes/_authenticated/log.$workoutId.tsx` (~914 lines) by extracting its two non-summary rendering paths into dedicated files. + +**Goal:** Reduce the route file to a thin orchestrator that owns shared state/hooks and delegates rendering to `EventWorkoutView`, `StrengthWorkoutView`, and (already-extracted) `WorkoutSummary`. No behavior changes. + +--- + +## Current Structure + +`ActiveWorkoutPage` contains three rendering paths via conditional branches: + +| Path | Trigger | Lines | +|------|---------|-------| +| Summary | `showSummary && summaryData?.workoutLog` | 485-497 | +| Event workout | `workoutLog.eventMetadata` | 509-567 | +| Strength/cardio/ruck | default return | 569-912 | + +`WorkoutSummary` is already its own component. Two new files needed. + +--- + +## Approach + +### Step 1: Extract `EventWorkoutView` + +Create `src/components/workout/event-workout-view.tsx`. + +Props it receives from the orchestrator: +- `workoutLog` (id, eventMetadata) +- `elapsedSeconds`, `isPauseSupported`, `isPaused` +- `handlePause`, `handleResume`, `handleFinish`, `handleDiscard` +- `isBroadcasting`, `publishFocus`, `publishUnfocus` +- `isFinishing`, `isDiscarding` +- `pageError`, `setPageError` +- `showDiscardDialog`, `setShowDiscardDialog` + +Move lines 509-567 verbatim. Define a `EventWorkoutViewProps` interface. + +### Step 2: Extract `StrengthWorkoutView` + +Create `src/components/workout/strength-workout-view.tsx`. + +Props it receives (all remaining state and handlers used in lines 569-912). Define a `StrengthWorkoutViewProps` interface. + +Key state threads to include: +- All exercise-block rendering logic (lines 636-851) +- Rest view / rest timer banner +- Add exercise sheet + discard dialog +- Footer button + onboarding hint +- Undo banner + +### Step 3: Slim the route file + +`log.$workoutId.tsx` becomes: +1. Route definition +2. All hooks + state declarations (unchanged) +3. Timer effect (unchanged) +4. Computed values and handlers (unchanged) +5. Three-branch conditional returning `WorkoutSummary`, `EventWorkoutView`, or `StrengthWorkoutView` + +Target size: ~200-250 lines (down from ~914). + +--- + +## File Changes + +| File | Change | +|------|--------| +| `src/routes/_authenticated/log.$workoutId.tsx` | Remove extracted JSX, import two new components | +| `src/components/workout/event-workout-view.tsx` | New file: EventWorkoutView component | +| `src/components/workout/strength-workout-view.tsx` | New file: StrengthWorkoutView component | + +--- + +## Verification + +- `bun run build` passes (TypeScript clean) +- `bun run test` passes +- Manual smoke: start a strength workout, start an event workout, finish to summary -- all three paths render correctly +- No behavior change: all state, handlers, and effects remain in the route component + +--- + +## Risks + +- Props list for `StrengthWorkoutView` is large (~20 props). This is acceptable since it's a view component, not an abstraction; the state stays hoisted in the route. If prop count feels unwieldy, consider a single context-bag object type. +- The `SetType` and `LoggedActivityGroupWithActivities` imports move into the new files; verify they re-export correctly from their domain modules. diff --git a/Context/Reviews/0022-pr115-split-log-workout-route-2026-04-20.md b/Context/Reviews/0022-pr115-split-log-workout-route-2026-04-20.md index 72217fd..08f6d2d 100644 --- a/Context/Reviews/0022-pr115-split-log-workout-route-2026-04-20.md +++ b/Context/Reviews/0022-pr115-split-log-workout-route-2026-04-20.md @@ -163,7 +163,7 @@ One architectural concern about dual mutation paths and one follow-up task for s - [x] All [FIX] findings resolved (P22-001 through P22-013) - [x] All [TASK] findings added to Steps.md or backlog (P22-014) - [x] All [ADR] findings have ADRs created or dismissed (P22-015) -- [ ] Review verified by review-verify agent +- [x] Review verified by review-verify agent ## Resolution Summary **Resolved at:** 2026-04-20