Fix Android profile chart touch UX: pan when zoomed, instant scrubbing, overlap cleanup - #920
Open
ericgriffin wants to merge 8 commits into
Open
Fix Android profile chart touch UX: pan when zoomed, instant scrubbing, overlap cleanup#920ericgriffin wants to merge 8 commits into
ericgriffin wants to merge 8 commits into
Conversation
One-finger drag now pans a zoomed chart (as the zoom hint promises) via an arena-claiming overlay recognizer; two-finger pinch/pan wins the arena even when the fingers land asynchronously; the DoubleTapGestureRecognizer and its 300 ms arena hold are replaced by timestamp-based detection, so taps resolve instantly and a fast tap-then-drag scrubs instead of dead-panning. Double-tap-hold pan is superseded by pan-while-zoomed.
Labels anchor below the profile depth at the event time instead of pinning to the plot top (the surface, where the ascent tail lives), flip inward at the plot edges, stagger on collision, and drop the less severe of two events landing within 24 px of each other.
The fixed top-right default sat exactly on the end-of-dive ascent tail. The default corner is now chosen by counting profile points in each corner window; a saved dragged position still always wins.
- resolve every pointer joining a multi-touch gesture, including into an already-claimed drag (each pointer has its own arena) - restore double-click-to-zoom for mouse/stylus (tap bookkeeping is now kind-agnostic; only the pinch machinery stays touch-only) - floor placeEventLabels' clamp bound so a plot shorter than the label text cannot throw during transient layouts
Contributor
There was a problem hiding this comment.
Pull request overview
Improves the dive-profile chart interaction model (primarily for Android touch) by making pan/pinch deterministic and responsive, removing the delayed double-tap recognizer behavior, and reducing overlay crowding via collision-aware event-label placement and smarter fullscreen readout-card default placement.
Changes:
- Introduces
ChartTouchClaimRecognizer+ Listener-driven gesture math for reliable touch pan/pinch and instant scrubbing (no 300ms gesture-arena hold). - Adds pure layout utilities for event-label placement (
placeEventLabels) and fullscreen readout-card corner selection (leastOccupiedReadoutCorner), and wires them into the chart/page. - Expands/updates widget & unit tests to cover the new touch UX, event labels, and readout-card placement.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/features/dive_log/presentation/widgets/readout_card_placement_test.dart | Unit tests for least-occupied-corner selection. |
| test/features/dive_log/presentation/widgets/profile_event_labels_test.dart | Unit tests for collision-aware event label placement rules. |
| test/features/dive_log/presentation/widgets/profile_chart_viewport_test.dart | Updates drag-intent tests to use isZoomed instead of doubleTapHold. |
| test/features/dive_log/presentation/widgets/dive_profile_chart_test.dart | Updates existing chart tests for the new “tap then drag pans when zoomed” behavior. |
| test/features/dive_log/presentation/widgets/dive_profile_chart_gestures_test.dart | New end-to-end widget tests for touch gestures (tap/scrub/pan/pinch/double-tap). |
| test/features/dive_log/presentation/widgets/dive_profile_chart_event_labels_test.dart | New widget tests validating on-chart event label positioning and overlap behavior. |
| test/features/dive_log/presentation/widgets/chart_touch_recognizer_test.dart | New tests for the custom touch-claim gesture recognizer and arena behavior. |
| test/features/dive_log/presentation/pages/fullscreen_profile_page_test.dart | Adds coverage for the new default readout-card corner behavior. |
| lib/features/dive_log/presentation/widgets/readout_card_placement.dart | Adds leastOccupiedReadoutCorner pure function for readout-card defaults. |
| lib/features/dive_log/presentation/widgets/profile_event_labels.dart | Adds placeEventLabels pure function for collision-aware event-label layout. |
| lib/features/dive_log/presentation/widgets/profile_chart_viewport.dart | Updates chartDragIntent API/behavior to key off zoom state. |
| lib/features/dive_log/presentation/widgets/dive_profile_chart.dart | Core integration: Listener-driven touch model, touch-claim overlay, event-label wiring, double-tap detection, touchCallback gating. |
| lib/features/dive_log/presentation/widgets/chart_touch_recognizer.dart | Implements ChartTouchClaimRecognizer to deterministically win/claim key touch gestures. |
| lib/features/dive_log/presentation/pages/fullscreen_profile_page.dart | Uses leastOccupiedReadoutCorner to seed readout-card default corner (memoized). |
| docs/superpowers/plans/2026-08-08-android-profile-chart-ux.md | Implementation plan documenting the new touch model and overlay/layout strategies. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1920
to
+1924
| final lastUp = _lastTapUpStamp; | ||
| _doubleTapArmed = | ||
| lastUp != null && | ||
| event.timeStamp - lastUp < kDoubleTapTimeout && | ||
| (event.localPosition - _lastTapUpPosition).distance <= |
Comment on lines
+85
to
+104
| final EventLabelAnchor anchor; | ||
| final double left; | ||
| if (spec.xPx + spec.textWidth / 2 > plotWidth) { | ||
| anchor = EventLabelAnchor.leftOfLine; | ||
| left = spec.xPx - spec.textWidth - _lineGap; | ||
| } else if (spec.xPx - spec.textWidth / 2 < 0) { | ||
| anchor = EventLabelAnchor.rightOfLine; | ||
| left = spec.xPx + _lineGap; | ||
| } else { | ||
| anchor = EventLabelAnchor.center; | ||
| left = spec.xPx - spec.textWidth / 2; | ||
| } | ||
|
|
||
| // Floor at 0: a transient layout can hand us a plot shorter than the | ||
| // text, and clamp() throws when its bounds are inverted. | ||
| final maxTop = math.max(0.0, plotHeight - spec.textHeight); | ||
| bool collides(double top) { | ||
| final rect = Rect.fromLTWH(left, top, spec.textWidth, spec.textHeight); | ||
| return placedRects.any(rect.overlaps); | ||
| } |
Comment on lines
+5303
to
+5306
| final painter = TextPainter( | ||
| text: TextSpan(text: kept[i].displayName, style: labelStyle), | ||
| textDirection: TextDirection.ltr, | ||
| )..layout(); |
Contributor
|
📦 Build artifacts for this PR · commit
Artifacts expire in 7 days. Downloading requires being signed in to GitHub. macOS needs two extractions: unzip the downloaded artifact, then unzip the Updated automatically on each push. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the rough dive-profile chart experience on Android touch:
PanGestureRecognizerin the gesture arena (innermost recognizer wins ties), so it failed whenever one finger moved before the second landed.DoubleTapGestureRecognizerheld every tap's arena for 300 ms (delayed tooltip), and a fast tap-then-drag set the old_doubleTapHoldflag, classifying the drag as a viewport pan - a no-op at 1x zoom, so the scrub just did not happen.Changes
ChartTouchClaimRecognizeron a translucent overlay stacked directly above theLineChart: hit-tested first, it joins each pointer's arena before fl_chart's recognizers and wins deterministically. It claims a second touch pointer immediately (pinch/two-finger pan works regardless of finger landing order) and claims a one-finger drag past slop while zoomed (drag-to-pan on touch, matching the hint). Taps and long-presses pass through, so tap tooltips and long-press scrubbing (including while zoomed) are preserved.DoubleTapGestureRecognizerand its 300 ms arena hold; double-tap/double-click zoom is detected manually fromPointerEvent.timeStampfor all pointer kinds. Fast tap-then-drag now scrubs; nested tap targets (photo markers, right-axis metric selector) respond instantly. The redundant double-tap-hold pan gesture was removed.Listener(cumulative against a gesture-start snapshot);touchCallbackis gated during claimed/multi-touch gestures so fl_chart cannot scrub under a pinch or leave a stale tooltip after a claimed pan.chartDragIntentnow keys on viewport zoom instead of the removed hold flag.placeEventLabels: event labels anchor just below the profile depth at the event time, flip inward at plot edges, stagger vertically on collision, hide when nothing fits, and events within 24 px keep only the most severe.leastOccupiedReadoutCorner: the fullscreen readout card defaults to the chart corner the profile occupies least; a saved dragged position still always wins.Test Plan
flutter testpasses (full suite; one pre-existing known-flaky backup test failed in the full run and passes in isolation)flutter analyzepassesScreenshots
Interaction-behavior changes only; no static visual changes beyond event-label positions, which depend on live profile data.