From 0c2e59b7a0c398d59282154017e26369dc904fa5 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Mon, 3 Aug 2026 14:56:33 -0400 Subject: [PATCH 1/4] CMM-2198: Only drill down the Views bar chart on a deliberate tap 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. --- .../ui/newstats/viewsstats/ViewsStatsCard.kt | 28 ++++++++----------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/viewsstats/ViewsStatsCard.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/viewsstats/ViewsStatsCard.kt index 33949c34504a..05acaed37ecf 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/viewsstats/ViewsStatsCard.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/viewsstats/ViewsStatsCard.kt @@ -33,10 +33,7 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect -import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.remember -import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.alpha @@ -65,6 +62,7 @@ import com.patrykandpatrick.vico.compose.cartesian.layer.LineCartesianLayer import com.patrykandpatrick.vico.compose.cartesian.layer.rememberColumnCartesianLayer import com.patrykandpatrick.vico.compose.cartesian.layer.rememberLineCartesianLayer import com.patrykandpatrick.vico.compose.cartesian.marker.CartesianMarker +import com.patrykandpatrick.vico.compose.cartesian.marker.CartesianMarkerController import com.patrykandpatrick.vico.compose.cartesian.marker.CartesianMarkerVisibilityListener import com.patrykandpatrick.vico.compose.cartesian.marker.DefaultCartesianMarker import com.patrykandpatrick.vico.compose.cartesian.marker.rememberDefaultCartesianMarker @@ -682,25 +680,20 @@ private fun ViewsStatsChart( ) } ChartType.BAR -> { - var lastShownIndex by remember { mutableIntStateOf(-1) } + // Drill down only on a deliberate tap. The marker controller decides which pointer + // interactions toggle the marker, and `rememberToggleOnTap` reacts solely to + // `Interaction.Tap`, which Vico validates against touch slop and the long-press + // timeout. The default (show-on-press) controller instead shows the marker on + // pointer-down and hides it on pointer-up, so a vertical scroll that started on the + // chart ended in a "hidden" callback that was indistinguishable from a tap and + // silently changed the selected period. val barTapListener = remember(onBarTapped) { object : CartesianMarkerVisibilityListener { override fun onShown( marker: CartesianMarker, targets: List ) { - lastShownIndex = targets.firstOrNull() - ?.x?.toInt() ?: -1 - } - - override fun onHidden( - marker: CartesianMarker - ) { - val index = lastShownIndex - if (index >= 0) { - lastShownIndex = -1 - onBarTapped(index) - } + targets.firstOrNull()?.x?.toInt()?.let(onBarTapped) } } } @@ -742,7 +735,8 @@ private fun ViewsStatsChart( ), marker = marker, markerVisibilityListener = barTapListener, - decorations = listOf(averageLine) + decorations = listOf(averageLine), + markerController = CartesianMarkerController.rememberToggleOnTap() ), modelProducer = modelProducer, scrollState = rememberVicoScrollState( From af190d28ad57c75e13100b3000d2d4061c49101b Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Mon, 3 Aug 2026 15:41:33 -0400 Subject: [PATCH 2/4] CMM-2198: Gate the bar chart drill-down on detectTapGestures 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. --- .../ui/newstats/viewsstats/ViewsStatsCard.kt | 37 +++++++++++++------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/viewsstats/ViewsStatsCard.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/viewsstats/ViewsStatsCard.kt index 05acaed37ecf..dcb68f5982ad 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/viewsstats/ViewsStatsCard.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/viewsstats/ViewsStatsCard.kt @@ -2,6 +2,7 @@ package org.wordpress.android.ui.newstats.viewsstats import androidx.compose.foundation.background import androidx.compose.foundation.border +import androidx.compose.foundation.gestures.detectTapGestures import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -33,13 +34,17 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.alpha import androidx.compose.ui.draw.clip import androidx.compose.ui.graphics.Brush import androidx.compose.ui.graphics.Color +import androidx.compose.ui.input.pointer.pointerInput import org.wordpress.android.ui.newstats.StatsColors import androidx.compose.ui.platform.LocalConfiguration import androidx.compose.ui.res.stringResource @@ -62,7 +67,6 @@ import com.patrykandpatrick.vico.compose.cartesian.layer.LineCartesianLayer import com.patrykandpatrick.vico.compose.cartesian.layer.rememberColumnCartesianLayer import com.patrykandpatrick.vico.compose.cartesian.layer.rememberLineCartesianLayer import com.patrykandpatrick.vico.compose.cartesian.marker.CartesianMarker -import com.patrykandpatrick.vico.compose.cartesian.marker.CartesianMarkerController import com.patrykandpatrick.vico.compose.cartesian.marker.CartesianMarkerVisibilityListener import com.patrykandpatrick.vico.compose.cartesian.marker.DefaultCartesianMarker import com.patrykandpatrick.vico.compose.cartesian.marker.rememberDefaultCartesianMarker @@ -680,20 +684,25 @@ private fun ViewsStatsChart( ) } ChartType.BAR -> { - // Drill down only on a deliberate tap. The marker controller decides which pointer - // interactions toggle the marker, and `rememberToggleOnTap` reacts solely to - // `Interaction.Tap`, which Vico validates against touch slop and the long-press - // timeout. The default (show-on-press) controller instead shows the marker on - // pointer-down and hides it on pointer-up, so a vertical scroll that started on the - // chart ended in a "hidden" callback that was indistinguishable from a tap and - // silently changed the selected period. - val barTapListener = remember(onBarTapped) { + // The marker shows on pointer-down and hides on pointer-up, so its visibility + // callbacks cannot tell a tap from the start of a scroll. Track which bar the + // pointer is over here, and drill down only once detectTapGestures confirms the + // gesture was a tap -- an enclosing scroll consumes the drag, which cancels it. + var pressedIndex by remember { mutableIntStateOf(-1) } + val barTapListener = remember { object : CartesianMarkerVisibilityListener { override fun onShown( marker: CartesianMarker, targets: List ) { - targets.firstOrNull()?.x?.toInt()?.let(onBarTapped) + pressedIndex = targets.firstOrNull()?.x?.toInt() ?: -1 + } + + override fun onUpdated( + marker: CartesianMarker, + targets: List + ) { + pressedIndex = targets.firstOrNull()?.x?.toInt() ?: -1 } } } @@ -735,8 +744,7 @@ private fun ViewsStatsChart( ), marker = marker, markerVisibilityListener = barTapListener, - decorations = listOf(averageLine), - markerController = CartesianMarkerController.rememberToggleOnTap() + decorations = listOf(averageLine) ), modelProducer = modelProducer, scrollState = rememberVicoScrollState( @@ -745,6 +753,11 @@ private fun ViewsStatsChart( modifier = Modifier .fillMaxWidth() .height(ChartHeight) + .pointerInput(onBarTapped) { + detectTapGestures { + if (pressedIndex >= 0) onBarTapped(pressedIndex) + } + } ) } } From 46589732099aaaee25732e5002d1730708c8343b Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Tue, 4 Aug 2026 07:42:02 -0400 Subject: [PATCH 3/4] CMM-2198: Clear the pressed bar index when the marker hides 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. --- .../android/ui/newstats/viewsstats/ViewsStatsCard.kt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/viewsstats/ViewsStatsCard.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/viewsstats/ViewsStatsCard.kt index dcb68f5982ad..3faadd67df85 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/viewsstats/ViewsStatsCard.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/viewsstats/ViewsStatsCard.kt @@ -688,6 +688,9 @@ private fun ViewsStatsChart( // callbacks cannot tell a tap from the start of a scroll. Track which bar the // pointer is over here, and drill down only once detectTapGestures confirms the // gesture was a tap -- an enclosing scroll consumes the drag, which cancels it. + // onHidden clears the index so that a tap landing outside the plotted area (the + // axis labels are inside this composable but outside the chart's layer bounds) + // can't drill into whichever bar happened to be pressed last. var pressedIndex by remember { mutableIntStateOf(-1) } val barTapListener = remember { object : CartesianMarkerVisibilityListener { @@ -704,6 +707,10 @@ private fun ViewsStatsChart( ) { pressedIndex = targets.firstOrNull()?.x?.toInt() ?: -1 } + + override fun onHidden(marker: CartesianMarker) { + pressedIndex = -1 + } } } CartesianChartHost( From b7239729034c1b0b6cd669a4ca8995134cf896d1 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Tue, 4 Aug 2026 11:07:53 -0400 Subject: [PATCH 4/4] CMM-2198: Correct the bar tap comment 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. --- .../android/ui/newstats/viewsstats/ViewsStatsCard.kt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/viewsstats/ViewsStatsCard.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/viewsstats/ViewsStatsCard.kt index 3faadd67df85..a88be0696417 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/viewsstats/ViewsStatsCard.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/viewsstats/ViewsStatsCard.kt @@ -688,9 +688,8 @@ private fun ViewsStatsChart( // callbacks cannot tell a tap from the start of a scroll. Track which bar the // pointer is over here, and drill down only once detectTapGestures confirms the // gesture was a tap -- an enclosing scroll consumes the drag, which cancels it. - // onHidden clears the index so that a tap landing outside the plotted area (the - // axis labels are inside this composable but outside the chart's layer bounds) - // can't drill into whichever bar happened to be pressed last. + // Hit-testing is horizontal only, so a tap anywhere in this box selects the + // nearest bar, matching the marker's existing behavior. var pressedIndex by remember { mutableIntStateOf(-1) } val barTapListener = remember { object : CartesianMarkerVisibilityListener {