Skip to content

Commit 432c448

Browse files
adalpariclaude
adalpari
and
claude
authored
CMM-1973: Views card period selection and chart improvements (#22726)
* CMM-1973: Persist Views card chart type selection across sessions Save the user's line/bar chart preference via SavedStateHandle (process death) and SharedPreferences (app restart), mirroring the existing period persistence pattern. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * CMM-1973: Show only current period in bar chart with wider bars Remove previous period series from bar chart and increase bar thickness from 8dp to 16dp for better readability. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * CMM-1973: Drill down period when tapping a bar in the chart Tapping a bar changes the period selector to the tapped period: daily bars drill into that single day, monthly bars into the full month. Hourly bars (smallest granularity) are ignored. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * CMM-1973: Fix custom period granularity for drill-down Single-day custom periods now use hourly granularity (24 data points) and month-long periods use daily granularity. Threshold bumped from 30 to 31 so 31-day months don't fall into the monthly bucket. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * CMM-1973: Show previous period as grey line on bar chart Overlay the previous period data as a grey line (no area fill) on the bar chart so users can compare current vs previous at a glance. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * CMM-1973: Use straight lines and dashed style for previous period Replace cubic curves with sharp connectors in both line and bar chart modes. Add dashed stroke to the previous period overlay line in bar mode to match the line mode style. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * CMM-1973: Fix monthly bar tap to drill down into full month Use the current period type to determine granularity instead of guessing from the raw period format. The API returns daily-format dates (YYYY-MM-DD) even for monthly data, so monthly taps now correctly create a full-month Custom period (first to last day). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * CMM-1973: Add loading spinner on bar tap and fix custom period drill-down Show a loading spinner overlay (with dimmed content) when tapping a bar instead of replacing the card with a full shimmer skeleton. Also fix drill-down for custom periods with monthly granularity using the same daysBetween > 31 threshold as the repository. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * CMM-1973: Clean up duplicated constant and verbose modifier Reuse existing isCustomPeriodMonthly() in drillDownPeriod() instead of duplicating the MAX_DAYS_IN_MONTH threshold. Simplify the conditional alpha modifier to idiomatic Compose style. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * CMM-1973: Add unit tests and consolidate chart type persistence Move storage key conversion into ChartType enum (storageKey / fromStorageKey), removing duplicated string constants and when blocks. Add guard to ignore bar taps while a drill-down is already loading. Add 10 unit tests covering chart type persistence, drill-down edge cases, and the loading guard. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * CMM-1973: Fix double-load race, fragile date parsing, and reflection in test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * CMM-1973: Fix detekt issues (ReturnCount suppress, unused variable) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * CMM-1973: Fix crash when drilling down to hourly chart data The Vico chart library crashes when CartesianValueFormatter returns an empty string. Return a space for out-of-range x-axis indices. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * CMM-1973: Code review fixes — imports, formatter, parsing, cleanup - Fix import ordering (CircularProgressIndicator in material3 group) - Replace space workaround in x-axis formatter with merged labels from both periods and toString() fallback - Wrap date parsing in drillDownPeriod with try-catch for safety - Extract drillDownMonth helper for clarity - Remove stale test comment and identical if/else branches - Remove extra blank line before @hiltviewmodel Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * CMM-1973: Fire bar tap drill-down on marker hide instead of show Store the tapped index in onShown and trigger the period change in onHidden, so the user sees the marker tooltip before the chart reloads. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * CMM-1973: Widen catch in drillDownPeriod to handle all parse errors NumberFormatException from drillDownMonth() was not caught by the DateTimeParseException-only catch block. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * CMM-1973: Catch specific exceptions instead of generic Exception CI detekt flags TooGenericExceptionCaught. Catch both DateTimeParseException and NumberFormatException explicitly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * CMM-1973: Suppress TooGenericExceptionCaught in drillDownPeriod Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * CMM-1973: Move TooGenericExceptionCaught suppress to function level Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 6c32946 commit 432c448

File tree

7 files changed

+538
-68
lines changed

7 files changed

+538
-68
lines changed

‎WordPress/src/main/java/org/wordpress/android/ui/newstats/NewStatsActivity.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,7 @@ private fun TrafficTabContent(
524524
StatsCardType.VIEWS_STATS -> ViewsStatsCard(
525525
uiState = viewsStatsUiState,
526526
onChartTypeChanged = viewsStatsViewModel::onChartTypeChanged,
527+
onBarTapped = viewsStatsViewModel::onBarTapped,
527528
onRetry = viewsStatsViewModel::onRetry,
528529
onRemoveCard = { newStatsViewModel.removeCard(cardType) },
529530
cardPosition = cardPosition,

‎WordPress/src/main/java/org/wordpress/android/ui/newstats/StatsCardsConfiguration.kt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ data class StatsCardsConfiguration(
88
val visibleCards: List<StatsCardType> = StatsCardType.defaultCards(),
99
val selectedPeriodType: String? = null,
1010
val customPeriodStartDate: Long? = null,
11-
val customPeriodEndDate: Long? = null
11+
val customPeriodEndDate: Long? = null,
12+
val selectedChartType: String? = null
1213
) {
1314
/**
1415
* Returns card types that are not currently visible (available to add).

‎WordPress/src/main/java/org/wordpress/android/ui/newstats/repository/StatsRepository.kt‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ private const val DAYS_IN_6_MONTHS = 182
5454
private const val DAYS_IN_12_MONTHS = 365
5555
private const val MONTHS_IN_6_MONTHS = 6
5656
private const val MONTHS_IN_12_MONTHS = 12
57+
private const val MAX_DAYS_IN_MONTH = 31
5758
private const val PERCENTAGE_MULTIPLIER = 100.0
5859
private const val PERCENTAGE_NO_CHANGE = 0.0
5960

@@ -512,12 +513,28 @@ class StatsRepository @Inject constructor(
512513
private fun calculateCustomPeriodDates(startDate: LocalDate, endDate: LocalDate): PeriodDateRange {
513514
val daysBetween = ChronoUnit.DAYS.between(startDate, endDate).toInt() + 1
514515

516+
// Single day → hourly granularity (like Today)
517+
if (daysBetween == 1) {
518+
val previousDate = startDate.minusDays(1)
519+
return PeriodDateRange(
520+
currentStart = startDate,
521+
currentEnd = startDate.plusDays(1),
522+
previousStart = previousDate,
523+
previousEnd = startDate,
524+
quantity = HOURLY_QUANTITY,
525+
unit = StatsUnit.HOUR,
526+
currentDisplayDate = startDate,
527+
previousDisplayDate = previousDate
528+
)
529+
}
530+
515531
val previousEnd = startDate.minusDays(1)
516532
val previousStart = previousEnd.minusDays(daysBetween.toLong() - 1)
517533

518-
// Determine unit based on range
534+
// Determine unit based on range — use daily for up to a full
535+
// month (31 days), monthly beyond that
519536
val unit = when {
520-
daysBetween <= DAYS_IN_30_DAYS -> StatsUnit.DAY
537+
daysBetween <= MAX_DAYS_IN_MONTH -> StatsUnit.DAY
521538
else -> StatsUnit.MONTH
522539
}
523540

‎WordPress/src/main/java/org/wordpress/android/ui/newstats/viewsstats/ViewsStatsCard.kt‎

Lines changed: 127 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,20 @@ import androidx.compose.material.icons.filled.PersonOutline
3232
import androidx.compose.material.icons.outlined.BarChart
3333
import androidx.compose.material.icons.outlined.Visibility
3434
import androidx.compose.material3.Button
35+
import androidx.compose.material3.CircularProgressIndicator
3536
import androidx.compose.material3.DropdownMenuItem
3637
import androidx.compose.material3.Icon
3738
import androidx.compose.material3.MaterialTheme
3839
import androidx.compose.material3.Text
3940
import androidx.compose.runtime.Composable
4041
import androidx.compose.runtime.LaunchedEffect
42+
import androidx.compose.runtime.getValue
43+
import androidx.compose.runtime.mutableIntStateOf
4144
import androidx.compose.runtime.remember
45+
import androidx.compose.runtime.setValue
4246
import androidx.compose.ui.Alignment
4347
import androidx.compose.ui.Modifier
48+
import androidx.compose.ui.draw.alpha
4449
import androidx.compose.ui.draw.clip
4550
import androidx.compose.ui.geometry.Offset
4651
import androidx.compose.ui.graphics.Brush
@@ -70,6 +75,7 @@ import com.patrykandpatrick.vico.core.cartesian.decoration.HorizontalLine
7075
import com.patrykandpatrick.vico.core.cartesian.layer.ColumnCartesianLayer
7176
import com.patrykandpatrick.vico.core.cartesian.layer.LineCartesianLayer
7277
import com.patrykandpatrick.vico.core.cartesian.marker.CartesianMarker
78+
import com.patrykandpatrick.vico.core.cartesian.marker.CartesianMarkerVisibilityListener
7379
import com.patrykandpatrick.vico.core.cartesian.marker.DefaultCartesianMarker
7480
import com.patrykandpatrick.vico.compose.cartesian.layer.rememberColumnCartesianLayer
7581
import com.patrykandpatrick.vico.compose.cartesian.layer.rememberLineCartesianLayer
@@ -111,6 +117,7 @@ private val SAMPLE_PREVIOUS_PERIOD_DATA = listOf(1000L, 1400L, 1150L, 1200L, 135
111117
fun ViewsStatsCard(
112118
uiState: ViewsStatsCardUiState,
113119
onChartTypeChanged: (ChartType) -> Unit,
120+
onBarTapped: (Int) -> Unit,
114121
onRetry: () -> Unit,
115122
onRemoveCard: () -> Unit,
116123
modifier: Modifier = Modifier,
@@ -137,7 +144,7 @@ fun ViewsStatsCard(
137144
when (uiState) {
138145
is ViewsStatsCardUiState.Loading -> LoadingContent()
139146
is ViewsStatsCardUiState.Loaded -> LoadedContent(
140-
uiState, onChartTypeChanged, onRemoveCard,
147+
uiState, onChartTypeChanged, onBarTapped, onRemoveCard,
141148
cardPosition, onMoveUp, onMoveToTop, onMoveDown, onMoveToBottom
142149
)
143150
is ViewsStatsCardUiState.Error -> ErrorContent(
@@ -256,39 +263,50 @@ private fun LoadingContent() {
256263
private fun LoadedContent(
257264
state: ViewsStatsCardUiState.Loaded,
258265
onChartTypeChanged: (ChartType) -> Unit,
266+
onBarTapped: (Int) -> Unit,
259267
onRemoveCard: () -> Unit,
260268
cardPosition: CardPosition?,
261269
onMoveUp: (() -> Unit)?,
262270
onMoveToTop: (() -> Unit)?,
263271
onMoveDown: (() -> Unit)?,
264272
onMoveToBottom: (() -> Unit)?
265273
) {
266-
Column(
267-
modifier = Modifier
268-
.fillMaxWidth()
269-
.padding(CardPadding)
270-
) {
271-
// Header Section
272-
HeaderSection(
273-
state = state,
274-
onChartTypeChanged = onChartTypeChanged,
275-
onRemoveCard = onRemoveCard,
276-
cardPosition = cardPosition,
277-
onMoveUp = onMoveUp,
278-
onMoveToTop = onMoveToTop,
279-
onMoveDown = onMoveDown,
280-
onMoveToBottom = onMoveToBottom
281-
)
282-
Spacer(modifier = Modifier.height(8.dp))
283-
// Chart Section
284-
ViewsStatsChart(
285-
chartData = state.chartData,
286-
periodAverage = state.periodAverage,
287-
chartType = state.chartType
288-
)
289-
Spacer(modifier = Modifier.height(16.dp))
290-
// Bottom Stats Row
291-
BottomStatsRow(stats = state.bottomStats)
274+
Box(modifier = Modifier.fillMaxWidth()) {
275+
Column(
276+
modifier = Modifier
277+
.fillMaxWidth()
278+
.padding(CardPadding)
279+
.alpha(if (state.isLoadingNewPeriod) 0.5f else 1f)
280+
) {
281+
// Header Section
282+
HeaderSection(
283+
state = state,
284+
onChartTypeChanged = onChartTypeChanged,
285+
onRemoveCard = onRemoveCard,
286+
cardPosition = cardPosition,
287+
onMoveUp = onMoveUp,
288+
onMoveToTop = onMoveToTop,
289+
onMoveDown = onMoveDown,
290+
onMoveToBottom = onMoveToBottom
291+
)
292+
Spacer(modifier = Modifier.height(8.dp))
293+
// Chart Section
294+
ViewsStatsChart(
295+
chartData = state.chartData,
296+
periodAverage = state.periodAverage,
297+
chartType = state.chartType,
298+
onBarTapped = onBarTapped
299+
)
300+
Spacer(modifier = Modifier.height(16.dp))
301+
// Bottom Stats Row
302+
BottomStatsRow(stats = state.bottomStats)
303+
}
304+
if (state.isLoadingNewPeriod) {
305+
CircularProgressIndicator(
306+
modifier = Modifier.align(Alignment.Center),
307+
color = MaterialTheme.colorScheme.primary
308+
)
309+
}
292310
}
293311
}
294312

@@ -480,7 +498,8 @@ private fun AverageRow(average: Long) {
480498
private fun ViewsStatsChart(
481499
chartData: ViewsStatsChartData,
482500
periodAverage: Long,
483-
chartType: ChartType
501+
chartType: ChartType,
502+
onBarTapped: (Int) -> Unit = {}
484503
) {
485504
// Key the model producer on chartType so it gets recreated when chart type changes
486505
val modelProducer = remember(chartType) { CartesianChartModelProducer() }
@@ -501,11 +520,14 @@ private fun ViewsStatsChart(
501520
}
502521
ChartType.BAR -> modelProducer.runTransaction {
503522
columnSeries {
504-
// Current period first (primary color)
505523
series(chartData.currentPeriod.map { it.views.toInt() })
506-
// Previous period second (grey)
507-
if (hasPreviousPeriod) {
508-
series(chartData.previousPeriod.map { it.views.toInt() })
524+
}
525+
if (hasPreviousPeriod) {
526+
lineSeries {
527+
series(
528+
chartData.previousPeriod
529+
.map { it.views.toInt() }
530+
)
509531
}
510532
}
511533
}
@@ -534,11 +556,17 @@ private fun ViewsStatsChart(
534556
val primaryColor = MaterialTheme.colorScheme.primary
535557
val secondaryColor = MaterialTheme.colorScheme.outline.copy(alpha = 0.5f)
536558

537-
// X-axis formatter to show date labels from current period data
538-
val dateLabels = chartData.currentPeriod.map { it.label }
559+
// X-axis labels from both series so the formatter covers the
560+
// full range even when the previous period has more data points
561+
val currentLabels = chartData.currentPeriod.map { it.label }
562+
val previousLabels = chartData.previousPeriod.map { it.label }
563+
val dateLabels = if (previousLabels.size > currentLabels.size) {
564+
currentLabels + previousLabels.drop(currentLabels.size)
565+
} else {
566+
currentLabels
567+
}
539568
val bottomAxisValueFormatter = CartesianValueFormatter { _, value, _ ->
540-
val index = value.toInt()
541-
if (index in dateLabels.indices) dateLabels[index] else ""
569+
dateLabels.getOrElse(value.toInt()) { value.toInt().toString() }
542570
}
543571

544572
// Marker value formatter to show date and views on touch
@@ -594,12 +622,12 @@ private fun ViewsStatsChart(
594622
LineCartesianLayer.Line(
595623
fill = LineCartesianLayer.LineFill.single(fill(primaryColor)),
596624
areaFill = LineCartesianLayer.AreaFill.single(fill(areaGradient)),
597-
pointConnector = LineCartesianLayer.PointConnector.cubic()
625+
pointConnector = LineCartesianLayer.PointConnector.Sharp
598626
),
599627
LineCartesianLayer.Line(
600628
fill = LineCartesianLayer.LineFill.single(fill(secondaryColor)),
601629
stroke = LineCartesianLayer.LineStroke.Dashed(),
602-
pointConnector = LineCartesianLayer.PointConnector.cubic()
630+
pointConnector = LineCartesianLayer.PointConnector.Sharp
603631
)
604632
)
605633
),
@@ -618,32 +646,72 @@ private fun ViewsStatsChart(
618646
)
619647
}
620648
ChartType.BAR -> {
649+
var lastShownIndex by remember { mutableIntStateOf(-1) }
650+
val barTapListener = remember(onBarTapped) {
651+
object : CartesianMarkerVisibilityListener {
652+
override fun onShown(
653+
marker: CartesianMarker,
654+
targets: List<CartesianMarker.Target>
655+
) {
656+
lastShownIndex = targets.firstOrNull()
657+
?.x?.toInt() ?: -1
658+
}
659+
660+
override fun onHidden(
661+
marker: CartesianMarker
662+
) {
663+
val index = lastShownIndex
664+
if (index >= 0) {
665+
lastShownIndex = -1
666+
onBarTapped(index)
667+
}
668+
}
669+
}
670+
}
621671
CartesianChartHost(
622672
chart = rememberCartesianChart(
623673
rememberColumnCartesianLayer(
624-
columnProvider = ColumnCartesianLayer.ColumnProvider.series(
625-
// Current period (primary color)
626-
LineComponent(
627-
fill = fill(primaryColor),
628-
thicknessDp = 8f,
629-
shape = CorneredShape.rounded(allPercent = 40)
630-
),
631-
// Previous period (grey)
632-
LineComponent(
633-
fill = fill(secondaryColor),
634-
thicknessDp = 8f,
635-
shape = CorneredShape.rounded(allPercent = 40)
674+
columnProvider = ColumnCartesianLayer
675+
.ColumnProvider.series(
676+
LineComponent(
677+
fill = fill(primaryColor),
678+
thicknessDp = 16f,
679+
shape = CorneredShape.rounded(
680+
allPercent = 40
681+
)
682+
)
636683
)
637-
),
638-
mergeMode = { ColumnCartesianLayer.MergeMode.Grouped(4f) }
639684
),
640-
startAxis = VerticalAxis.rememberStart(line = null),
641-
bottomAxis = HorizontalAxis.rememberBottom(valueFormatter = bottomAxisValueFormatter),
685+
rememberLineCartesianLayer(
686+
lineProvider = LineCartesianLayer
687+
.LineProvider.series(
688+
LineCartesianLayer.Line(
689+
fill = LineCartesianLayer
690+
.LineFill.single(
691+
fill(secondaryColor)
692+
),
693+
stroke = LineCartesianLayer
694+
.LineStroke.Dashed(),
695+
pointConnector =
696+
LineCartesianLayer
697+
.PointConnector.Sharp
698+
)
699+
)
700+
),
701+
startAxis = VerticalAxis.rememberStart(
702+
line = null
703+
),
704+
bottomAxis = HorizontalAxis.rememberBottom(
705+
valueFormatter = bottomAxisValueFormatter
706+
),
642707
marker = marker,
708+
markerVisibilityListener = barTapListener,
643709
decorations = listOf(averageLine)
644710
),
645711
modelProducer = modelProducer,
646-
scrollState = rememberVicoScrollState(scrollEnabled = false),
712+
scrollState = rememberVicoScrollState(
713+
scrollEnabled = false
714+
),
647715
modifier = Modifier
648716
.fillMaxWidth()
649717
.height(ChartHeight)
@@ -821,6 +889,7 @@ private fun ViewsStatsCardLoadingPreview() {
821889
ViewsStatsCard(
822890
uiState = ViewsStatsCardUiState.Loading,
823891
onChartTypeChanged = {},
892+
onBarTapped = {},
824893
onRetry = {},
825894
onRemoveCard = {}
826895
)
@@ -864,6 +933,7 @@ private fun ViewsStatsCardLoadedPreview() {
864933
ViewsStatsCard(
865934
uiState = sampleLoadedState(),
866935
onChartTypeChanged = {},
936+
onBarTapped = {},
867937
onRetry = {},
868938
onRemoveCard = {}
869939
)
@@ -879,6 +949,7 @@ private fun ViewsStatsCardErrorPreview() {
879949
message = stringResource(R.string.stats_error_api)
880950
),
881951
onChartTypeChanged = {},
952+
onBarTapped = {},
882953
onRetry = {},
883954
onRemoveCard = {}
884955
)
@@ -892,6 +963,7 @@ private fun ViewsStatsCardLoadedDarkPreview() {
892963
ViewsStatsCard(
893964
uiState = sampleLoadedState(),
894965
onChartTypeChanged = {},
966+
onBarTapped = {},
895967
onRetry = {},
896968
onRemoveCard = {}
897969
)

0 commit comments

Comments
 (0)
⚡