Skip to content

Android: Fixed the Deep Timeline zoom/pan "snap back" issue - #2382

Merged
ryanbr merged 1 commit into
ryanbr:mainfrom
andigandhi:deep-timeline
Sep 22, 2026
Merged

ryanbr merged 1 commit into
ryanbr:mainfrom
andigandhi:deep-timeline

Conversation

@andigandhi

@andigandhi andigandhi commented Sep 21, 2026

Copy link
Copy Markdown

What this PR does

In TimelineChart (Charts.kt:1232), the gesture handler captured windowStart and windowEnd values from the closure at composition time. When a zoom/pan gesture updated the window via onWindowChange, it triggered a recomposition with new values, but the gesture handler still referenced the stale captured values. This caused the graph to snap back because each gesture event computed the new window relative to outdated values.

Used rememberUpdatedState to create stable references that always hold the latest window values:

val currentWindowStart by rememberUpdatedState(windowStart)
val currentWindowEnd by rememberUpdatedState(windowEnd)

The gesture handler now reads from these state objects instead of capturing the values directly. This is the standard Compose pattern for accessing current state in long-lived closures like gesture handlers.

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Documentation
  • CI / tooling

How it was tested

Was tested on a real Android Device using real Whoop MG data.

Checklist

  • Swift package tests pass for any package I touched (swift test in Packages/<name>)
  • Android unit tests pass if I touched android/ (./gradlew testFullDebugUnitTest)
  • No new build warnings introduced
  • UI changes use only StrandDesign tokens — no hardcoded colors, fonts, or spacing
  • No hardcoded hex frame bytes; protocol facts live in the schema / decoders
  • Follows the conventions in docs/CONTRIBUTING.md
  • I did not commit generated output (Strand.xcodeproj/) or any secrets/keystores

Related issues

Refs #2368

@ryanbr

ryanbr commented Sep 22, 2026

Copy link
Copy Markdown
Owner

Reviewed at 5237b59. Correct, complete, and well diagnosed. Thank you for filing #2368 and then tracking it down yourself.

Checked

  • The diagnosis holds. .pointerInput(bounds) is keyed on bounds, which does not change when the window pans or zooms, so the lambda is never recreated and keeps the windowStart / windowEnd it captured at the composition that made it. Every gesture event then computed from the original window, which is exactly the snap-back.
  • rememberUpdatedState is the right instrument, not just a working one. Adding the window to pointerInput's keys would also read current values, but it restarts detectTransformGestures whenever they change, which cancels the pinch mid-gesture. Your description says that, and it is the part a reviewer most wants to see reasoned about.
  • The fix is complete. After the change the gesture block reads only the two updated states, bounds, and onWindowChange. That last one is captured too, but the single call site passes { window = it } writing to a state-backed variable, so a stale capture of it cannot misbehave.
  • No sibling left behind. The other three pointerInput blocks in this file are keyed on the data they actually read (cleanValues, xFracs and values, cleanSelectionLabels, axisWidth), so they restart when their inputs move. TimelineChart was the only one keyed on something that stands still while the values it reads change.

On testing

"Tested on a real Android device" is the right answer here and I am not going to ask for a unit test. This repository has no androidTest source set and no createComposeRule anywhere in it, so there is no harness a gesture test could run in. Asking for one would mean asking you to introduce Compose UI testing infrastructure to land a two-line fix, which is not a reasonable trade.

One housekeeping item, which I will handle

The last line of the description pairs a linking verb with the issue number. A description acts on merge whatever the squash message says, so I will rewrite it to a plain reference before taking this in, and retire the issue separately once this lands. Nothing for you to do.

Merging.

@ryanbr
ryanbr merged commit e9fe33d into ryanbr:main Sep 22, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants