feat: enhance workout with animated transitions and gestures#138
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @isotronic, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughWorkoutSessionScreen now uses Reanimated and gesture detection to enable swipe navigation between sets with animated previous/current/next overlay panels, an outgoing-snapshot overlay during set completion, shared-value worklets for gestures, and coordinated slide animations when completing or swiping between sets. ChangesAnimated swipe navigation and panel transitions
Sequence DiagramsequenceDiagram
participant User
participant GestureDetector
participant ReanimatedWorklet
participant WorkoutSessionScreen
participant Store
User->>GestureDetector: pan / swipe input
GestureDetector->>ReanimatedWorklet: invoke pan worklet (reads canGoPrev/canGoNext)
ReanimatedWorklet->>ReanimatedWorklet: animate offsetX / snapshotOffsetX
ReanimatedWorklet->>WorkoutSessionScreen: runOnJS(commitNext/commitPrevious) after animation
WorkoutSessionScreen->>Store: update indices / updateWeightAndReps / nextSet()
WorkoutSessionScreen->>ReanimatedWorklet: set outgoing snapshot shared values (for completion animation)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
app/(app)/(workout)/workout-session.tsx (4)
71-90: 💤 Low valueDuplicate noop-handler prop bag — extract a shared default.
The full set of noop handler defaults is declared twice (here and in
getPanelData). IfSessionSetInfo's prop surface changes, both call sites must be updated in lockstep, which is easy to miss. Hoist the noop handler block (and any other constant defaults likeanimatedUrl: undefined) into a singleconst READONLY_PANEL_DEFAULTS = { ... }and spread it into both.Also applies to: 459-495
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/`(app)/(workout)/workout-session.tsx around lines 71 - 90, The snapshotToProps function duplicates the same noop handler/default prop bag used elsewhere (see getPanelData); extract those shared defaults into a single constant (e.g. READONLY_PANEL_DEFAULTS) and then spread that constant into snapshotToProps and getPanelData (and any other call sites) so updates to SessionSetInfo's props only need to be made once; ensure the constant includes animatedUrl, animatedImageLoading, animatedImageError, isLastSetOfLastExercise, isFirstSetOfFirstExercise and all noop handlers (handleWeightInputChange, handleWeightChange, handleRepsInputChange, handleRepsChange, handleTimeInputChange, handlePreviousSet, handleNextSet, handleCompleteSet, removeSet, addSet) and replace the duplicated literal objects with a spread of READONLY_PANEL_DEFAULTS in snapshotToProps and getPanelData.
433-496: ⚖️ Poor tradeoff
getPanelDatais rebuilt and invoked at least twice per render with O(n·m) lookups.Each render constructs a fresh
getPanelDataclosure and runs it inline for the previous and next panels (lines 760-763 and 813). Internally, each call walkspreviousWorkoutDataand per-workout calls.find(...)overexercises. For modestly sized history this is fine, but it executes on every store/shared-value-driven re-render — including during rapid swipe-driven re-renders. Consider memoising the previous/next prop bags withuseMemokeyed on(workout, currentExerciseIndex, currentSetIndex, weightAndReps, completedSets, settings), and/or caching the previous-workout lookup byexercise_id.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/`(app)/(workout)/workout-session.tsx around lines 433 - 496, getPanelData is recreated and called multiple times per render causing O(n·m) lookups; memoize the panel props for previous/current/next panels and cache previous-workout lookups. Convert getPanelData into a pure helper (or keep it but don't recreate callers) and compute panel objects with useMemo keyed on [workout, currentExerciseIndex, currentSetIndex, weightAndReps, completedSets, settings]; inside the memo build a Map from exercise_id to previousWorkoutData to avoid repeated .find(...) calls (or memoize findLastAvailableSetData by exercise_id), then return the same panel objects for repeated renders instead of rerunning the O(n·m) traversal.
34-39: 💤 Low valueRemove the unnecessary
AnimatedViewshim; Reanimated 4 + React 19 handles children correctly.The comment references React 18, but the project uses React 19.1.0. More importantly, Reanimated 4 with React 19 natively exposes
childreninAnimated.Viewprop types (viacreateAnimatedComponent), making this shim redundant. Delete it and useAnimated.Viewdirectly, or if a shim is needed for other reasons, simplify the typing and drop the stale comment.If keeping the shim, prefer proper typing over
any:💡 Type improvement
-// React 18 + Reanimated 4: Animated.View types don't include children in strict TS -const AnimatedView = Animated.View as unknown as React.ComponentType<{ - style?: any; - pointerEvents?: string; - children?: React.ReactNode; -}>; +import type { StyleProp, ViewStyle } from "react-native"; +const AnimatedView = Animated.View as unknown as React.ComponentType<{ + style?: StyleProp<ViewStyle>; + children?: React.ReactNode; +}>;Also drop
pointerEvents(the prop form is deprecated; use style.pointerEvents instead).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/`(app)/(workout)/workout-session.tsx around lines 34 - 39, The current shim const AnimatedView = Animated.View as unknown as React.ComponentType<{...}> is stale—remove this shim and use Animated.View directly everywhere (search for AnimatedView usages) because Reanimated 4 + React 19 already includes children in its props; if you must keep a wrapper, replace the shim with a properly typed alias using React.ComponentProps<typeof Animated.View> (e.g., React.ComponentType<React.ComponentProps<typeof Animated.View>>) and remove the deprecated pointerEvents and the stale React 18 comment.
514-534: ⚡ Quick winMemoise
panGesture,handlePreviousSet, andhandleNextSetto prevent constant rebinding.These functions and gesture objects are recreated on every render (store updates, animation-frame-driven updates, etc.), forcing Reanimated/Gesture Handler to rebind them on each cycle. This is wasteful and can cause issues if a re-render occurs during an in-flight pan gesture. Wrap them in
useCallback/useMemoto keep the worklets stable.First, ensure
commitPreviousandcommitNextare wrapped inuseCallbackso they can be safely included in dependency arrays:const commitPrevious = useCallback(() => { if (previousExerciseIndex !== null && previousSetIndex !== null) { setCurrentExerciseIndex(previousExerciseIndex as number); setCurrentSetIndex(previousExerciseIndex as number, previousSetIndex as number); } }, [previousExerciseIndex, previousSetIndex, setCurrentExerciseIndex, setCurrentSetIndex]); const commitNext = useCallback(() => { setCurrentExerciseIndex(nextExerciseIndex); setCurrentSetIndex(nextExerciseIndex, nextSetIndex); }, [nextExerciseIndex, nextSetIndex, setCurrentExerciseIndex, setCurrentSetIndex]);Then memoise the handlers and gesture:
const handlePreviousSet = useCallback(() => { if (!hasPreviousSet) return; offsetX.value = withTiming(SCREEN_WIDTH, { duration: 250 }, (finished) => { "worklet"; if (finished) { offsetX.value = 0; runOnJS(commitPrevious)(); } }); }, [hasPreviousSet, commitPrevious]); const handleNextSet = useCallback(() => { if (!hasNextSet) return; offsetX.value = withTiming(-SCREEN_WIDTH, { duration: 250 }, (finished) => { "worklet"; if (finished) { offsetX.value = 0; runOnJS(commitNext)(); } }); }, [hasNextSet, commitNext]); const panGesture = useMemo( () => Gesture.Pan() .activeOffsetX([-10, 10]) // ... rest of gesture config .onEnd((e) => { "worklet"; // ... gesture logic using canGoPrev.value, canGoNext.value, etc. }), [commitPrevious, commitNext] );Note: Omit SharedValues (
offsetX,canGoPrev,canGoNext) from dependency arrays—they maintain stable references across renders.Also applies to: 655–703
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/`(app)/(workout)/workout-session.tsx around lines 514 - 534, The handlers and gesture are being recreated every render causing Reanimated/Gesture Handler rebinding; wrap commitPrevious and commitNext in useCallback (including their relevant state setters and index deps), then memoise handlePreviousSet and handleNextSet with useCallback (depend on hasPreviousSet/hasNextSet and the memoised commits) and memoise panGesture with useMemo (depend on the memoised commit functions and any non-shared-value deps); do not include SharedValues like offsetX, canGoPrev, or canGoNext in dependency arrays so worklets remain stable during in-flight gestures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/`(app)/(workout)/workout-session.tsx:
- Around line 144-151: The weightIncrement calculation can produce NaN when
settings.weightIncrement is missing or invalid; update the computation for
weightIncrement (used by SessionSetInfo and downstream arithmetic) to parse and
then fallback to 1 when parseFloat yields NaN or when settings is falsy—i.e.,
check settings?.weightIncrement, run parseFloat, and if Number.isNaN(result) use
1 as the default so weight stepping always receives a valid numeric increment.
- Around line 573-581: The code currently converts the time string to total
seconds via convertTimeStrToSeconds and writes that stringified seconds value
into the model using updateWeightAndReps, but SessionSetInfo.formatTimeInput
expects a display-style value (mm:ss) so storing seconds breaks display; either
preserve the original display string when calling updateWeightAndReps (pass
timeStr instead of validTimeNum.toString()) or convert seconds back to a mm:ss
string before storing (implement a secondsToDisplayString helper and use it when
writing into weightAndReps); update the call site around
convertTimeStrToSeconds/updateWeightAndReps and ensure formatTimeInput receives
the expected display format.
- Around line 755-758: Move the pointerEvents usage out of the AnimatedView prop
and into its style array: for each AnimatedView that currently has
pointerEvents="none" (e.g., the ones using prevPanelStyle, currentPanelStyle,
nextPanelStyle), add { pointerEvents: "none" } into the corresponding style
object or merge it into the style array and remove the pointerEvents prop from
the JSX; also remove pointerEvents?: string from the AnimatedView type cast so
the component type no longer expects a pointerEvents prop.
---
Nitpick comments:
In `@app/`(app)/(workout)/workout-session.tsx:
- Around line 71-90: The snapshotToProps function duplicates the same noop
handler/default prop bag used elsewhere (see getPanelData); extract those shared
defaults into a single constant (e.g. READONLY_PANEL_DEFAULTS) and then spread
that constant into snapshotToProps and getPanelData (and any other call sites)
so updates to SessionSetInfo's props only need to be made once; ensure the
constant includes animatedUrl, animatedImageLoading, animatedImageError,
isLastSetOfLastExercise, isFirstSetOfFirstExercise and all noop handlers
(handleWeightInputChange, handleWeightChange, handleRepsInputChange,
handleRepsChange, handleTimeInputChange, handlePreviousSet, handleNextSet,
handleCompleteSet, removeSet, addSet) and replace the duplicated literal objects
with a spread of READONLY_PANEL_DEFAULTS in snapshotToProps and getPanelData.
- Around line 433-496: getPanelData is recreated and called multiple times per
render causing O(n·m) lookups; memoize the panel props for previous/current/next
panels and cache previous-workout lookups. Convert getPanelData into a pure
helper (or keep it but don't recreate callers) and compute panel objects with
useMemo keyed on [workout, currentExerciseIndex, currentSetIndex, weightAndReps,
completedSets, settings]; inside the memo build a Map from exercise_id to
previousWorkoutData to avoid repeated .find(...) calls (or memoize
findLastAvailableSetData by exercise_id), then return the same panel objects for
repeated renders instead of rerunning the O(n·m) traversal.
- Around line 34-39: The current shim const AnimatedView = Animated.View as
unknown as React.ComponentType<{...}> is stale—remove this shim and use
Animated.View directly everywhere (search for AnimatedView usages) because
Reanimated 4 + React 19 already includes children in its props; if you must keep
a wrapper, replace the shim with a properly typed alias using
React.ComponentProps<typeof Animated.View> (e.g.,
React.ComponentType<React.ComponentProps<typeof Animated.View>>) and remove the
deprecated pointerEvents and the stale React 18 comment.
- Around line 514-534: The handlers and gesture are being recreated every render
causing Reanimated/Gesture Handler rebinding; wrap commitPrevious and commitNext
in useCallback (including their relevant state setters and index deps), then
memoise handlePreviousSet and handleNextSet with useCallback (depend on
hasPreviousSet/hasNextSet and the memoised commits) and memoise panGesture with
useMemo (depend on the memoised commit functions and any non-shared-value deps);
do not include SharedValues like offsetX, canGoPrev, or canGoNext in dependency
arrays so worklets remain stable during in-flight gestures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c5154ebb-484f-4110-b4a0-0560a37a6231
📒 Files selected for processing (1)
app/(app)/(workout)/workout-session.tsx
There was a problem hiding this comment.
Pull request overview
This PR enhances the workout session experience by adding gesture-driven navigation and animated transitions between sets, including an animated “complete set” panel transition.
Changes:
- Added horizontal swipe (pan) gesture support to navigate to previous/next sets.
- Implemented animated sliding transitions for set navigation and for completing a set (outgoing snapshot + incoming panel).
- Pre-rendered adjacent (previous/next) set panels to improve perceived smoothness during transitions.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/`(app)/(workout)/workout-session.tsx:
- Around line 441-442: getPanelData currently returns null when settings is
falsy, which causes adjacent panel overlays to render blank during
swipe/transition; update getPanelData (and the other similar check near the
second occurrence) to avoid returning null when settings is missing by deriving
and returning panel data using available workout information and sensible
defaults (or the same fallback logic used by the main panel) instead of bailing
out—modify the functions that reference getPanelData to expect that it always
returns a panel object (or a minimal fallback panel) so previous/next overlays
can render correctly even when settings is undefined.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8b71584c-b714-46b5-9b14-bdc466a1991f
📒 Files selected for processing (1)
app/(app)/(workout)/workout-session.tsx
Summary by CodeRabbit
New Features
Refactor
Improvements