CMM-2198: Only drill down the Views bar chart on a deliberate tap - #23172
Merged
nbradbury merged 5 commits intoAug 4, 2026
Conversation
The bar chart's drill-down was triggered from the marker's onHidden callback. With the default show-on-press marker controller, the marker shows on pointer-down and hides on pointer-up, so any gesture that started on the chart -- including a vertical scroll -- ended in a "hidden" callback that was indistinguishable from a tap. That silently collapsed the selected period to a single day (or month), reloaded every Traffic card, and persisted the change across launches. Switch the bar chart to the toggle-on-tap marker controller, which only accepts Interaction.Tap. Vico validates those against touch slop and the long-press timeout, so a scroll no longer registers as a bar selection. The drill-down now fires from onShown, making the lastShownIndex bookkeeping unnecessary.
Collaborator
Generated by 🚫 Danger |
Contributor
|
|
Contributor
|
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #23172 +/- ##
=======================================
Coverage 37.84% 37.84%
=======================================
Files 2345 2345
Lines 127530 127530
Branches 17715 17716 +1
=======================================
Hits 48258 48258
Misses 75313 75313
Partials 3959 3959 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The previous commit switched the bar chart to the toggle-on-tap marker controller. That fixed the scroll case but broke two others, because Vico only calls onShown when previousMarkerTargetHashCode transitions from null and toggle-on-tap leaves the marker visible after a tap: - Only the first tap drilled down. Later taps changed the marker's targets, so they routed to onUpdated and did nothing. - markerX is a rememberSaveable but the chart's hash code is not, so recreating the activity replayed onShown and drilled down with no user interaction at all. Go back to the default show-on-press controller, which also restores press-and-hold tooltips, and use the marker callbacks only to record which bar the pointer is over. The drill-down now fires from detectTapGestures, which an enclosing scroll cancels by consuming the drag.
…tats-scrolling-changes-dates
pressedIndex was only ever written, never cleared, so it survived past the gesture that set it. The axis labels sit inside this composable but outside the chart's layer bounds, so tapping one produces no marker and no onShown -- yet detectTapGestures still fires, drilling into whichever bar was pressed last. The same stale index also outlived the data reload that a drill-down triggers, where it pointed at an unrelated bucket. Clear it in onHidden, which Vico dispatches from the draw that follows the pointer-up, after the tap has been handled.
nbradbury
marked this pull request as ready for review
August 4, 2026 13:05
dcalhoun
approved these changes
Aug 4, 2026
dcalhoun
left a comment
dcalhoun
left a comment
Member
There was a problem hiding this comment.
Changes fix the issue from my testing. However, the inline comment appears incorrect. We should address that.
The comment claimed onHidden stops a tap on the axis labels from drilling into a stale bar. That premise is wrong: pointerPositionToX maps only the pointer's x coordinate and ignores y, so the bottom-axis labels sit under their bars and hit-test to a valid target. onShown fires there like anywhere else in the box, with or without a prior interaction. Describe what actually happens instead. The onHidden reset stays -- it still clears state that would otherwise outlive its gesture -- but it is belt-and-braces, not what governs axis taps.
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
4 participants
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.
Description
The Views bar chart drilled down from the marker's
onHiddencallback. The marker shows on pointer-down and hides on pointer-up, so any gesture that started on the chart — including a vertical scroll — was indistinguishable from a tap. It silently collapsed the selected period to a single day, reloaded every Traffic card, and persisted across launches. Two users reported this in 27.0 Play Store reviews.The marker's visibility callbacks now only record which bar the pointer is over. The drill-down fires from
detectTapGestures, which an enclosing scroll cancels by consuming the drag. Tooltip behaviour is unchanged.Testing instructions
trunk, open stats for a site that has traffic