Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Context/Backlog/Ideas.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@ skill. Prioritize via the backlog-prioritize skill.

<!-- Add new ideas below this line -->

## 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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
88 changes: 88 additions & 0 deletions Context/QuickPlans/Done/2026-04-20-split-log-workout-route.md
Original file line number Diff line number Diff line change
@@ -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.
Loading
Loading