From b14c8ce470c7a41d6f87ec5c863d5295e1730250 Mon Sep 17 00:00:00 2001 From: adalpari Date: Fri, 3 Jul 2026 11:23:55 +0200 Subject: [PATCH 01/13] Add expand/collapse child referrers to new stats Referrer groups can contain child referrers (e.g. "Search Engines"), but the new stats Referrers card discarded them and rendered a flat list. Thread a children list (name, url, views) from the datasource through the repository, UI state and ViewModel, and render referrer groups with children as expand/collapse rows in both the card and the detail screen, following the UTM card pattern. Tapping a child opens its URL in a Chrome Custom Tab. CMM-2133 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../android/ui/newstats/NewStatsActivity.kt | 9 +- .../ui/newstats/datasource/StatsDataSource.kt | 10 + .../datasource/StatsDataSourceImpl.kt | 16 +- .../ui/newstats/mostviewed/MostViewedCard.kt | 178 +++++++++++++---- .../mostviewed/MostViewedCardUiState.kt | 20 +- .../mostviewed/MostViewedDetailActivity.kt | 182 ++++++++++++++---- .../mostviewed/MostViewedViewModel.kt | 13 +- .../ui/newstats/repository/StatsRepository.kt | 21 +- .../repository/StatsRepositoryTest.kt | 29 +++ 9 files changed, 399 insertions(+), 79 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/NewStatsActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/NewStatsActivity.kt index 04d6d79ef287..c8f936daecb1 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/NewStatsActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/NewStatsActivity.kt @@ -1,5 +1,6 @@ package org.wordpress.android.ui.newstats +import android.app.Activity import android.content.Context import android.content.Intent import android.os.Bundle @@ -64,6 +65,7 @@ import dagger.hilt.android.AndroidEntryPoint import kotlinx.coroutines.launch import org.wordpress.android.R import org.wordpress.android.ui.ActivityLauncher +import org.wordpress.android.ui.ActivityNavigator import org.wordpress.android.ui.compose.theme.AppThemeM3 import org.wordpress.android.ui.main.BaseAppCompatActivity import org.wordpress.android.ui.newstats.components.AddCardBottomSheet @@ -409,6 +411,10 @@ private fun TrafficTabContent( newStatsViewModel: NewStatsViewModel = viewModel() ) { val context = LocalContext.current + val activityNavigator = remember { ActivityNavigator() } + val onReferrerChildClick: (String) -> Unit = { url -> + (context as? Activity)?.let { activityNavigator.openInCustomTab(it, url) } + } val todaysStatsUiState by todaysStatsViewModel.uiState.collectAsState() val viewsStatsUiState by viewsStatsViewModel.uiState.collectAsState() val postsUiState by mostViewedViewModel.postsUiState.collectAsState() @@ -696,7 +702,8 @@ private fun TrafficTabContent( onMoveUp = { newStatsViewModel.moveCardUp(cardType) }, onMoveToTop = { newStatsViewModel.moveCardToTop(cardType) }, onMoveDown = { newStatsViewModel.moveCardDown(cardType) }, - onMoveToBottom = { newStatsViewModel.moveCardToBottom(cardType) } + onMoveToBottom = { newStatsViewModel.moveCardToBottom(cardType) }, + onChildClick = onReferrerChildClick ) StatsCardType.LOCATIONS -> LocationsCard( uiState = locationsUiState, diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/datasource/StatsDataSource.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/datasource/StatsDataSource.kt index c0c6cda7060c..8f3b415b531d 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/datasource/StatsDataSource.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/datasource/StatsDataSource.kt @@ -418,6 +418,16 @@ sealed class ReferrersDataResult { */ data class ReferrerDataItem( val name: String, + val views: Long, + val children: List = emptyList() +) + +/** + * A child referrer nested under a referrer group (e.g. a specific search engine). + */ +data class ReferrerChildDataItem( + val name: String, + val url: String?, val views: Long ) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/datasource/StatsDataSourceImpl.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/datasource/StatsDataSourceImpl.kt index d5f7c02a0489..4f45bbd9698e 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/datasource/StatsDataSourceImpl.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/datasource/StatsDataSourceImpl.kt @@ -14,6 +14,7 @@ import uniffi.wp_api.StatsFileDownloadsParams import uniffi.wp_api.StatsFileDownloadsPeriod import uniffi.wp_api.StatsReferrersParams import uniffi.wp_api.StatsReferrersPeriod +import uniffi.wp_api.StatsReferrersResults import uniffi.wp_api.StatsRegionViewsParams import uniffi.wp_api.StatsRegionViewsPeriod import uniffi.wp_api.StatsDevicesParams @@ -238,7 +239,8 @@ class StatsDataSourceImpl @Inject constructor( groups.map { group -> ReferrerDataItem( name = group.name.orEmpty(), - views = group.total?.toLong() ?: 0L + views = group.total?.toLong() ?: 0L, + children = group.results.toChildren() ) } ) @@ -249,6 +251,18 @@ class StatsDataSourceImpl @Inject constructor( } } + private fun StatsReferrersResults?.toChildren(): List = + when (this) { + is StatsReferrersResults.Referrers -> v1.map { child -> + ReferrerChildDataItem( + name = child.name.orEmpty(), + url = child.url, + views = child.views?.toLong() ?: 0L + ) + } + else -> emptyList() + } + private fun buildCountryViewsParams(dateRange: StatsDateRange, max: Int) = when (dateRange) { is StatsDateRange.Preset -> StatsCountryViewsParams( period = StatsCountryViewsPeriod.DAY, diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt index 38312fd0d414..56a68411f7b8 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt @@ -1,7 +1,9 @@ package org.wordpress.android.ui.newstats.mostviewed +import androidx.compose.animation.AnimatedVisibility import androidx.compose.foundation.background import androidx.compose.foundation.border +import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -17,11 +19,17 @@ import androidx.compose.foundation.layout.width import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.ChevronRight +import androidx.compose.material.icons.filled.KeyboardArrowDown +import androidx.compose.material.icons.filled.KeyboardArrowUp import androidx.compose.material3.Button import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +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.clip @@ -60,7 +68,8 @@ fun MostViewedCard( onMoveToTop: (() -> Unit)? = null, onMoveDown: (() -> Unit)? = null, onMoveToBottom: (() -> Unit)? = null, - onOpenWpAdmin: (() -> Unit)? = null + onOpenWpAdmin: (() -> Unit)? = null, + onChildClick: (String) -> Unit = {} ) { val borderColor = MaterialTheme.colorScheme.outlineVariant @@ -80,7 +89,8 @@ fun MostViewedCard( is MostViewedCardUiState.Loading -> LoadingContent() is MostViewedCardUiState.Loaded -> LoadedContent( uiState, cardType, onShowAllClick, onRemoveCard, - cardPosition, onMoveUp, onMoveToTop, onMoveDown, onMoveToBottom + cardPosition, onMoveUp, onMoveToTop, onMoveDown, onMoveToBottom, + onChildClick ) is MostViewedCardUiState.Error -> ErrorContent( uiState, cardType, onRetry, onRemoveCard, @@ -173,7 +183,8 @@ private fun LoadedContent( onMoveUp: (() -> Unit)?, onMoveToTop: (() -> Unit)?, onMoveDown: (() -> Unit)?, - onMoveToBottom: (() -> Unit)? + onMoveToBottom: (() -> Unit)?, + onChildClick: (String) -> Unit ) { Column( modifier = Modifier @@ -199,7 +210,7 @@ private fun LoadedContent( val percentage = if (state.maxViewsForBar > 0) { item.views.toFloat() / state.maxViewsForBar.toFloat() } else 0f - MostViewedItemRow(item = item, percentage = percentage) + MostViewedItemRow(item = item, percentage = percentage, onChildClick = onChildClick) if (index < state.items.lastIndex) { Spacer(modifier = Modifier.height(4.dp)) } @@ -283,62 +294,159 @@ private fun ColumnHeadersRow(cardType: StatsCardType) { } @Composable -private fun MostViewedItemRow(item: MostViewedItem, percentage: Float) { +private fun MostViewedItemRow( + item: MostViewedItem, + percentage: Float, + onChildClick: (String) -> Unit +) { val barColor = MaterialTheme.colorScheme.primary.copy(alpha = HIGHLIGHTED_ITEM_BACKGROUND_ALPHA) + val hasChildren = item.children.isNotEmpty() + var expanded by remember { mutableStateOf(false) } + val maxChildViews = item.children.maxOfOrNull { it.views } ?: 0L + + Column { + Box( + modifier = Modifier + .fillMaxWidth() + .height(IntrinsicSize.Min) + .clip(RoundedCornerShape(8.dp)) + .then( + if (hasChildren) Modifier.clickable { expanded = !expanded } else Modifier + ) + ) { + // Background bar representing the percentage + Box( + modifier = Modifier + .fillMaxWidth(fraction = percentage) + .fillMaxHeight() + .background(barColor) + ) + + // Content + Row( + modifier = Modifier + .fillMaxWidth() + .padding(vertical = 12.dp, horizontal = 16.dp), + horizontalArrangement = Arrangement.SpaceBetween, + verticalAlignment = Alignment.CenterVertically + ) { + Text( + text = item.title, + style = MaterialTheme.typography.bodyLarge, + color = MaterialTheme.colorScheme.onSurface, + maxLines = 2, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.weight(1f) + ) + + Spacer(modifier = Modifier.width(16.dp)) + + Row(verticalAlignment = Alignment.CenterVertically) { + Column(horizontalAlignment = Alignment.End) { + Row(verticalAlignment = Alignment.CenterVertically) { + Text( + text = formatStatValue(item.views), + style = MaterialTheme.typography.bodyLarge, + fontWeight = FontWeight.SemiBold, + color = MaterialTheme.colorScheme.onSurface + ) + Spacer(modifier = Modifier.width(4.dp)) + ExpandChevron( + hasChildren = hasChildren, + expanded = expanded + ) + } + ChangeIndicator(change = item.change) + } + } + } + } + + if (hasChildren) { + AnimatedVisibility(visible = expanded) { + Column(modifier = Modifier.padding(start = 24.dp)) { + item.children.forEach { child -> + val childPercentage = if (maxChildViews > 0) { + child.views.toFloat() / maxChildViews.toFloat() + } else 0f + MostViewedChildRow( + child = child, + percentage = childPercentage, + onChildClick = onChildClick + ) + } + } + } + } + } +} + +@Composable +private fun ExpandChevron(hasChildren: Boolean, expanded: Boolean) { + val imageVector = when { + !hasChildren -> Icons.Default.ChevronRight + expanded -> Icons.Default.KeyboardArrowUp + else -> Icons.Default.KeyboardArrowDown + } + val contentDescription = if (hasChildren) { + stringResource( + if (expanded) R.string.stats_collapse_group else R.string.stats_expand_group + ) + } else null + Icon( + imageVector = imageVector, + contentDescription = contentDescription, + modifier = Modifier.size(16.dp), + tint = MaterialTheme.colorScheme.onSurfaceVariant + ) +} + +@Composable +private fun MostViewedChildRow( + child: MostViewedChildItem, + percentage: Float, + onChildClick: (String) -> Unit +) { + val barColor = MaterialTheme.colorScheme.primary.copy(alpha = HIGHLIGHTED_ITEM_BACKGROUND_ALPHA) + val url = child.url + val clickModifier = if (!url.isNullOrBlank()) { + Modifier.clickable { onChildClick(url) } + } else Modifier Box( modifier = Modifier .fillMaxWidth() .height(IntrinsicSize.Min) .clip(RoundedCornerShape(8.dp)) + .then(clickModifier) ) { - // Background bar representing the percentage Box( modifier = Modifier .fillMaxWidth(fraction = percentage) .fillMaxHeight() .background(barColor) ) - - // Content Row( modifier = Modifier .fillMaxWidth() - .padding(vertical = 12.dp, horizontal = 16.dp), + .padding(vertical = 10.dp, horizontal = 16.dp), horizontalArrangement = Arrangement.SpaceBetween, verticalAlignment = Alignment.CenterVertically ) { Text( - text = item.title, - style = MaterialTheme.typography.bodyLarge, + text = child.name, + style = MaterialTheme.typography.bodyMedium, color = MaterialTheme.colorScheme.onSurface, - maxLines = 2, + maxLines = 1, overflow = TextOverflow.Ellipsis, modifier = Modifier.weight(1f) ) - - Spacer(modifier = Modifier.width(16.dp)) - - Row(verticalAlignment = Alignment.CenterVertically) { - Column(horizontalAlignment = Alignment.End) { - Row(verticalAlignment = Alignment.CenterVertically) { - Text( - text = formatStatValue(item.views), - style = MaterialTheme.typography.bodyLarge, - fontWeight = FontWeight.SemiBold, - color = MaterialTheme.colorScheme.onSurface - ) - Spacer(modifier = Modifier.width(4.dp)) - Icon( - imageVector = Icons.Default.ChevronRight, - contentDescription = null, - modifier = Modifier.size(16.dp), - tint = MaterialTheme.colorScheme.onSurfaceVariant - ) - } - ChangeIndicator(change = item.change) - } - } + Spacer(modifier = Modifier.width(8.dp)) + Text( + text = formatStatValue(child.views), + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurface + ) } } } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCardUiState.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCardUiState.kt index e053da3de913..46f11d9f9132 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCardUiState.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCardUiState.kt @@ -42,9 +42,24 @@ data class MostViewedItem( val id: Long, val title: String, val views: Long, - val change: MostViewedChange + val change: MostViewedChange, + val children: List = emptyList() ) +/** + * A child item nested under a Most Viewed item (e.g. a referrer under a referrer group). + * + * @param name The name of the child item + * @param url The URL opened when the child row is tapped (null if not linkable) + * @param views The number of views + */ +@Parcelize +data class MostViewedChildItem( + val name: String, + val url: String?, + val views: Long +) : Parcelable + /** * Represents the change in views compared to the previous period. */ @@ -84,5 +99,6 @@ data class MostViewedDetailItem( val id: Long, val title: String, val views: Long, - val change: MostViewedChange + val change: MostViewedChange, + val children: List = emptyList() ) : Parcelable diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt index ea51e19f7216..62fee6737cb0 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt @@ -4,7 +4,9 @@ import android.content.Context import android.content.Intent import android.os.Bundle import androidx.activity.compose.setContent +import androidx.compose.animation.AnimatedVisibility import androidx.compose.foundation.background +import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -24,6 +26,8 @@ import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.ArrowBack import androidx.compose.material.icons.filled.ChevronRight +import androidx.compose.material.icons.filled.KeyboardArrowDown +import androidx.compose.material.icons.filled.KeyboardArrowUp import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.IconButton @@ -32,6 +36,10 @@ import androidx.compose.material3.Scaffold import androidx.compose.material3.Text import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +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.clip @@ -42,6 +50,7 @@ import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp import dagger.hilt.android.AndroidEntryPoint import org.wordpress.android.R +import org.wordpress.android.ui.ActivityNavigator import org.wordpress.android.ui.compose.theme.AppThemeM3 import org.wordpress.android.ui.main.BaseAppCompatActivity import org.wordpress.android.ui.newstats.StatsCardType @@ -49,6 +58,7 @@ import org.wordpress.android.ui.newstats.components.StatsSummaryCard import org.wordpress.android.ui.newstats.util.formatStatValue import org.wordpress.android.util.extensions.getParcelableArrayListCompat import org.wordpress.android.util.extensions.getSerializableCompat +import javax.inject.Inject private const val EXTRA_CARD_TYPE = "extra_card_type" private const val EXTRA_ITEMS = "extra_items" @@ -60,6 +70,8 @@ private const val EXTRA_VALUE_HEADER_RES_ID = "extra_value_header_res_id" @AndroidEntryPoint class MostViewedDetailActivity : BaseAppCompatActivity() { + @Inject lateinit var activityNavigator: ActivityNavigator + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) @@ -88,7 +100,8 @@ class MostViewedDetailActivity : BaseAppCompatActivity() { totalViewsChangePercent = totalViewsChangePercent, dateRange = dateRange, valueHeaderResId = valueHeaderResId, - onBackPressed = onBackPressedDispatcher::onBackPressed + onBackPressed = onBackPressedDispatcher::onBackPressed, + onChildClick = { url -> activityNavigator.openInCustomTab(this, url) } ) } } @@ -136,7 +149,8 @@ private fun MostViewedDetailScreen( totalViewsChangePercent: Double, dateRange: String, valueHeaderResId: Int = R.string.stats_views, - onBackPressed: () -> Unit + onBackPressed: () -> Unit, + onChildClick: (String) -> Unit = {} ) { val title = stringResource(cardType.displayNameResId) @@ -184,7 +198,8 @@ private fun MostViewedDetailScreen( DetailItemRow( position = index + 1, item = item, - maxViewsForBar = maxViewsForBar + maxViewsForBar = maxViewsForBar, + onChildClick = onChildClick ) if (index < items.lastIndex) { Spacer(modifier = Modifier.height(4.dp)) @@ -226,18 +241,135 @@ private fun ColumnHeaders( private fun DetailItemRow( position: Int, item: MostViewedDetailItem, - maxViewsForBar: Long + maxViewsForBar: Long, + onChildClick: (String) -> Unit ) { val percentage = if (maxViewsForBar > 0) item.views.toFloat() / maxViewsForBar.toFloat() else 0f val barColor = MaterialTheme.colorScheme.primary.copy( alpha = HIGHLIGHTED_ITEM_BACKGROUND_ALPHA ) + val hasChildren = item.children.isNotEmpty() + var expanded by remember { mutableStateOf(false) } + val maxChildViews = item.children.maxOfOrNull { it.views } ?: 0L + + Column { + Box( + modifier = Modifier + .fillMaxWidth() + .height(IntrinsicSize.Min) + .clip(RoundedCornerShape(8.dp)) + .then( + if (hasChildren) Modifier.clickable { expanded = !expanded } else Modifier + ) + ) { + Box( + modifier = Modifier + .fillMaxWidth(fraction = percentage) + .fillMaxHeight() + .background(barColor) + ) + + Row( + modifier = Modifier + .fillMaxWidth() + .padding(vertical = 12.dp, horizontal = 16.dp), + verticalAlignment = Alignment.CenterVertically + ) { + Text( + text = position.toString(), + style = MaterialTheme.typography.bodyLarge, + fontWeight = FontWeight.SemiBold, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.width(32.dp) + ) + + Text( + text = item.title, + style = MaterialTheme.typography.bodyLarge, + maxLines = 2, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.weight(1f) + ) + + Spacer(modifier = Modifier.width(8.dp)) + + Column(horizontalAlignment = Alignment.End) { + Row(verticalAlignment = Alignment.CenterVertically) { + Text( + text = formatStatValue(item.views), + style = MaterialTheme.typography.bodyLarge, + fontWeight = FontWeight.SemiBold + ) + Spacer(modifier = Modifier.width(4.dp)) + DetailExpandChevron( + hasChildren = hasChildren, + expanded = expanded + ) + } + ChangeIndicator(change = item.change) + } + } + } + + if (hasChildren) { + AnimatedVisibility(visible = expanded) { + Column(modifier = Modifier.padding(start = 32.dp)) { + item.children.forEach { child -> + val childPercentage = if (maxChildViews > 0) { + child.views.toFloat() / maxChildViews.toFloat() + } else 0f + DetailChildRow( + child = child, + percentage = childPercentage, + onChildClick = onChildClick + ) + } + } + } + } + } +} + +@Composable +private fun DetailExpandChevron(hasChildren: Boolean, expanded: Boolean) { + val imageVector = when { + !hasChildren -> Icons.Default.ChevronRight + expanded -> Icons.Default.KeyboardArrowUp + else -> Icons.Default.KeyboardArrowDown + } + val contentDescription = if (hasChildren) { + stringResource( + if (expanded) R.string.stats_collapse_group else R.string.stats_expand_group + ) + } else null + Icon( + imageVector = imageVector, + contentDescription = contentDescription, + modifier = Modifier.size(16.dp), + tint = MaterialTheme.colorScheme.onSurfaceVariant + ) +} + +@Composable +private fun DetailChildRow( + child: MostViewedChildItem, + percentage: Float, + onChildClick: (String) -> Unit +) { + val barColor = MaterialTheme.colorScheme.primary.copy( + alpha = HIGHLIGHTED_ITEM_BACKGROUND_ALPHA + ) + val url = child.url + val clickModifier = if (!url.isNullOrBlank()) { + Modifier.clickable { onChildClick(url) } + } else Modifier Box( modifier = Modifier .fillMaxWidth() .height(IntrinsicSize.Min) .clip(RoundedCornerShape(8.dp)) + .then(clickModifier) ) { Box( modifier = Modifier @@ -245,48 +377,26 @@ private fun DetailItemRow( .fillMaxHeight() .background(barColor) ) - Row( modifier = Modifier .fillMaxWidth() - .padding(vertical = 12.dp, horizontal = 16.dp), + .padding(vertical = 10.dp, horizontal = 16.dp), verticalAlignment = Alignment.CenterVertically ) { Text( - text = position.toString(), - style = MaterialTheme.typography.bodyLarge, - fontWeight = FontWeight.SemiBold, - color = MaterialTheme.colorScheme.onSurfaceVariant, - modifier = Modifier.width(32.dp) - ) - - Text( - text = item.title, - style = MaterialTheme.typography.bodyLarge, - maxLines = 2, + text = child.name, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurface, + maxLines = 1, overflow = TextOverflow.Ellipsis, modifier = Modifier.weight(1f) ) - Spacer(modifier = Modifier.width(8.dp)) - - Column(horizontalAlignment = Alignment.End) { - Row(verticalAlignment = Alignment.CenterVertically) { - Text( - text = formatStatValue(item.views), - style = MaterialTheme.typography.bodyLarge, - fontWeight = FontWeight.SemiBold - ) - Spacer(modifier = Modifier.width(4.dp)) - Icon( - imageVector = Icons.Default.ChevronRight, - contentDescription = null, - modifier = Modifier.size(16.dp), - tint = MaterialTheme.colorScheme.onSurfaceVariant - ) - } - ChangeIndicator(change = item.change) - } + Text( + text = formatStatValue(child.views), + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurface + ) } } } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedViewModel.kt index a87ddb02f60d..29b6c51264b6 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedViewModel.kt @@ -236,7 +236,8 @@ class MostViewedViewModel @Inject constructor( id = item.id, title = item.title, views = item.views, - change = item.change + change = item.change, + children = item.children ) }, maxViewsForBar = cardItems.firstOrNull()?.views ?: 1L @@ -342,5 +343,13 @@ private fun MostViewedItemData.toDetailItem(): MostViewedDetailItem { viewsChange < 0 -> MostViewedChange.Negative(abs(viewsChange), abs(viewsChangePercent)) else -> MostViewedChange.NoChange } - return MostViewedDetailItem(id = id, title = title, views = views, change = change) + return MostViewedDetailItem( + id = id, + title = title, + views = views, + change = change, + children = children.map { child -> + MostViewedChildItem(name = child.name, url = child.url, views = child.views) + } + ) } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/repository/StatsRepository.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/repository/StatsRepository.kt index 6cea8670b6cd..af6e7c812240 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/repository/StatsRepository.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/repository/StatsRepository.kt @@ -690,7 +690,14 @@ class StatsRepository @Inject constructor( title = item.name, views = item.views, previousViews = previousViews, - isFirst = index == 0 + isFirst = index == 0, + children = item.children.map { child -> + MostViewedChildData( + name = child.name, + url = child.url, + views = child.views + ) + } ) }, totalViews = totalViews, @@ -1869,13 +1876,23 @@ data class MostViewedItemData( val title: String, val views: Long, val previousViews: Long, - val isFirst: Boolean + val isFirst: Boolean, + val children: List = emptyList() ) { val viewsChange: Long get() = views - previousViews val viewsChangePercent: Double get() = calculateItemChangePercent(views, previousViews) } +/** + * A child item nested under a most viewed item (e.g. a referrer under a referrer group). + */ +data class MostViewedChildData( + val name: String, + val url: String?, + val views: Long +) + /** * Result wrapper for country views fetch operation. */ diff --git a/WordPress/src/test/java/org/wordpress/android/ui/newstats/repository/StatsRepositoryTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/newstats/repository/StatsRepositoryTest.kt index 8445ed76e203..848903b6a958 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/newstats/repository/StatsRepositoryTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/newstats/repository/StatsRepositoryTest.kt @@ -5,6 +5,7 @@ import org.wordpress.android.ui.newstats.StatsPeriod import org.wordpress.android.ui.newstats.datasource.CommentsDataPoint import org.wordpress.android.ui.newstats.datasource.LikesDataPoint import org.wordpress.android.ui.newstats.datasource.PostsDataPoint +import org.wordpress.android.ui.newstats.datasource.ReferrerChildDataItem import org.wordpress.android.ui.newstats.datasource.ReferrerDataItem import org.wordpress.android.ui.newstats.datasource.ReferrersDataResult import org.wordpress.android.ui.newstats.datasource.StatsDataSource @@ -566,6 +567,34 @@ class StatsRepositoryTest : BaseUnitTest() { assertThat(success.items[1].views).isEqualTo(TEST_REFERRER_VIEWS_2) } + @Test + fun `given referrer group with children, when fetchMostViewed with REFERRERS, then children propagate`() = test { + val referrersData = listOf( + ReferrerDataItem( + name = "Search Engines", + views = 23, + children = listOf( + ReferrerChildDataItem(name = "Google Search", url = "http://google.com/", views = 23) + ) + ) + ) + whenever(statsDataSource.fetchReferrers(any(), any(), any())) + .thenReturn(ReferrersDataResult.Success(referrersData)) + + val result = repository.fetchMostViewed( + TEST_SITE_ID, + StatsPeriod.Last7Days, + MostViewedDataSource.REFERRERS + ) + + assertThat(result).isInstanceOf(MostViewedResult.Success::class.java) + val success = result as MostViewedResult.Success + assertThat(success.items[0].children).hasSize(1) + assertThat(success.items[0].children[0].name).isEqualTo("Google Search") + assertThat(success.items[0].children[0].url).isEqualTo("http://google.com/") + assertThat(success.items[0].children[0].views).isEqualTo(23) + } + @Test fun `given successful response, when fetchMostViewed, then totalViews is calculated correctly`() = test { whenever(statsDataSource.fetchTopPostsAndPages(any(), any(), any())) From 20a356c38058b232d50c7d9fb8ec220abbc5426f Mon Sep 17 00:00:00 2001 From: adalpari Date: Fri, 3 Jul 2026 11:38:26 +0200 Subject: [PATCH 02/13] Refine referrer row chevrons Remove the decorative right arrow from Most Viewed rows (shared by posts, referrers, clicks, search terms, video plays and file downloads) since it did not navigate anywhere. Move the expand/collapse dropdown arrow before the views count on referrer groups with children, and show a right arrow only on clickable URL child rows to signal they open a link. CMM-2133 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ui/newstats/mostviewed/MostViewedCard.kt | 39 ++++++++++--------- .../mostviewed/MostViewedDetailActivity.kt | 39 ++++++++++--------- 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt index 56a68411f7b8..8c6020f80b40 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt @@ -344,17 +344,16 @@ private fun MostViewedItemRow( Row(verticalAlignment = Alignment.CenterVertically) { Column(horizontalAlignment = Alignment.End) { Row(verticalAlignment = Alignment.CenterVertically) { + if (hasChildren) { + ExpandChevron(expanded = expanded) + Spacer(modifier = Modifier.width(4.dp)) + } Text( text = formatStatValue(item.views), style = MaterialTheme.typography.bodyLarge, fontWeight = FontWeight.SemiBold, color = MaterialTheme.colorScheme.onSurface ) - Spacer(modifier = Modifier.width(4.dp)) - ExpandChevron( - hasChildren = hasChildren, - expanded = expanded - ) } ChangeIndicator(change = item.change) } @@ -382,20 +381,12 @@ private fun MostViewedItemRow( } @Composable -private fun ExpandChevron(hasChildren: Boolean, expanded: Boolean) { - val imageVector = when { - !hasChildren -> Icons.Default.ChevronRight - expanded -> Icons.Default.KeyboardArrowUp - else -> Icons.Default.KeyboardArrowDown - } - val contentDescription = if (hasChildren) { - stringResource( - if (expanded) R.string.stats_collapse_group else R.string.stats_expand_group - ) - } else null +private fun ExpandChevron(expanded: Boolean) { Icon( - imageVector = imageVector, - contentDescription = contentDescription, + imageVector = if (expanded) Icons.Default.KeyboardArrowUp else Icons.Default.KeyboardArrowDown, + contentDescription = stringResource( + if (expanded) R.string.stats_collapse_group else R.string.stats_expand_group + ), modifier = Modifier.size(16.dp), tint = MaterialTheme.colorScheme.onSurfaceVariant ) @@ -409,7 +400,8 @@ private fun MostViewedChildRow( ) { val barColor = MaterialTheme.colorScheme.primary.copy(alpha = HIGHLIGHTED_ITEM_BACKGROUND_ALPHA) val url = child.url - val clickModifier = if (!url.isNullOrBlank()) { + val isClickable = !url.isNullOrBlank() + val clickModifier = if (isClickable) { Modifier.clickable { onChildClick(url) } } else Modifier @@ -447,6 +439,15 @@ private fun MostViewedChildRow( style = MaterialTheme.typography.bodyMedium, color = MaterialTheme.colorScheme.onSurface ) + if (isClickable) { + Spacer(modifier = Modifier.width(4.dp)) + Icon( + imageVector = Icons.Default.ChevronRight, + contentDescription = null, + modifier = Modifier.size(16.dp), + tint = MaterialTheme.colorScheme.onSurfaceVariant + ) + } } } } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt index 62fee6737cb0..00812d0d18e2 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt @@ -295,16 +295,15 @@ private fun DetailItemRow( Column(horizontalAlignment = Alignment.End) { Row(verticalAlignment = Alignment.CenterVertically) { + if (hasChildren) { + DetailExpandChevron(expanded = expanded) + Spacer(modifier = Modifier.width(4.dp)) + } Text( text = formatStatValue(item.views), style = MaterialTheme.typography.bodyLarge, fontWeight = FontWeight.SemiBold ) - Spacer(modifier = Modifier.width(4.dp)) - DetailExpandChevron( - hasChildren = hasChildren, - expanded = expanded - ) } ChangeIndicator(change = item.change) } @@ -331,20 +330,12 @@ private fun DetailItemRow( } @Composable -private fun DetailExpandChevron(hasChildren: Boolean, expanded: Boolean) { - val imageVector = when { - !hasChildren -> Icons.Default.ChevronRight - expanded -> Icons.Default.KeyboardArrowUp - else -> Icons.Default.KeyboardArrowDown - } - val contentDescription = if (hasChildren) { - stringResource( - if (expanded) R.string.stats_collapse_group else R.string.stats_expand_group - ) - } else null +private fun DetailExpandChevron(expanded: Boolean) { Icon( - imageVector = imageVector, - contentDescription = contentDescription, + imageVector = if (expanded) Icons.Default.KeyboardArrowUp else Icons.Default.KeyboardArrowDown, + contentDescription = stringResource( + if (expanded) R.string.stats_collapse_group else R.string.stats_expand_group + ), modifier = Modifier.size(16.dp), tint = MaterialTheme.colorScheme.onSurfaceVariant ) @@ -360,7 +351,8 @@ private fun DetailChildRow( alpha = HIGHLIGHTED_ITEM_BACKGROUND_ALPHA ) val url = child.url - val clickModifier = if (!url.isNullOrBlank()) { + val isClickable = !url.isNullOrBlank() + val clickModifier = if (isClickable) { Modifier.clickable { onChildClick(url) } } else Modifier @@ -397,6 +389,15 @@ private fun DetailChildRow( style = MaterialTheme.typography.bodyMedium, color = MaterialTheme.colorScheme.onSurface ) + if (isClickable) { + Spacer(modifier = Modifier.width(4.dp)) + Icon( + imageVector = Icons.Default.ChevronRight, + contentDescription = null, + modifier = Modifier.size(16.dp), + tint = MaterialTheme.colorScheme.onSurfaceVariant + ) + } } } } From 3ad112dff7e973220627d897cc5fc398af6c452d Mon Sep 17 00:00:00 2001 From: adalpari Date: Fri, 3 Jul 2026 11:40:18 +0200 Subject: [PATCH 03/13] Add top margin above expanded children Expanded child rows sat flush against their parent row in the referrers and UTM cards and detail screens. Add a small top margin so children are visually separated when a group is expanded. CMM-2133 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt | 2 +- .../android/ui/newstats/mostviewed/MostViewedDetailActivity.kt | 2 +- .../wordpress/android/ui/newstats/utm/UtmCommonComposables.kt | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt index 8c6020f80b40..e297971c63d7 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt @@ -363,7 +363,7 @@ private fun MostViewedItemRow( if (hasChildren) { AnimatedVisibility(visible = expanded) { - Column(modifier = Modifier.padding(start = 24.dp)) { + Column(modifier = Modifier.padding(start = 24.dp, top = 4.dp)) { item.children.forEach { child -> val childPercentage = if (maxChildViews > 0) { child.views.toFloat() / maxChildViews.toFloat() diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt index 00812d0d18e2..721ae4692cc6 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt @@ -312,7 +312,7 @@ private fun DetailItemRow( if (hasChildren) { AnimatedVisibility(visible = expanded) { - Column(modifier = Modifier.padding(start = 32.dp)) { + Column(modifier = Modifier.padding(start = 32.dp, top = 4.dp)) { item.children.forEach { child -> val childPercentage = if (maxChildViews > 0) { child.views.toFloat() / maxChildViews.toFloat() diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/utm/UtmCommonComposables.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/utm/UtmCommonComposables.kt index 36977acee8b8..7e85371e3ba3 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/utm/UtmCommonComposables.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/utm/UtmCommonComposables.kt @@ -120,7 +120,8 @@ fun UtmExpandableRow( AnimatedVisibility(visible = expanded) { Column( modifier = Modifier.padding( - start = 24.dp + start = 24.dp, + top = 4.dp ) ) { item.topPosts.forEach { post -> From 39eb8b8fef894e7b85e6864b1dc204a8207b56f0 Mon Sep 17 00:00:00 2001 From: adalpari Date: Fri, 3 Jul 2026 11:44:05 +0200 Subject: [PATCH 04/13] Add spacing between expanded children When a group had more than one child, the child rows were rendered flush against each other. Space them evenly in the referrers and UTM cards and detail screens. CMM-2133 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../android/ui/newstats/mostviewed/MostViewedCard.kt | 5 ++++- .../ui/newstats/mostviewed/MostViewedDetailActivity.kt | 5 ++++- .../android/ui/newstats/utm/UtmCommonComposables.kt | 4 +++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt index e297971c63d7..a0da019be044 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt @@ -363,7 +363,10 @@ private fun MostViewedItemRow( if (hasChildren) { AnimatedVisibility(visible = expanded) { - Column(modifier = Modifier.padding(start = 24.dp, top = 4.dp)) { + Column( + modifier = Modifier.padding(start = 24.dp, top = 4.dp), + verticalArrangement = Arrangement.spacedBy(4.dp) + ) { item.children.forEach { child -> val childPercentage = if (maxChildViews > 0) { child.views.toFloat() / maxChildViews.toFloat() diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt index 721ae4692cc6..0c1b9fd5c160 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt @@ -312,7 +312,10 @@ private fun DetailItemRow( if (hasChildren) { AnimatedVisibility(visible = expanded) { - Column(modifier = Modifier.padding(start = 32.dp, top = 4.dp)) { + Column( + modifier = Modifier.padding(start = 32.dp, top = 4.dp), + verticalArrangement = Arrangement.spacedBy(4.dp) + ) { item.children.forEach { child -> val childPercentage = if (maxChildViews > 0) { child.views.toFloat() / maxChildViews.toFloat() diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/utm/UtmCommonComposables.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/utm/UtmCommonComposables.kt index 7e85371e3ba3..408528cba354 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/utm/UtmCommonComposables.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/utm/UtmCommonComposables.kt @@ -2,6 +2,7 @@ package org.wordpress.android.ui.newstats.utm import androidx.compose.animation.AnimatedVisibility import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer @@ -122,7 +123,8 @@ fun UtmExpandableRow( modifier = Modifier.padding( start = 24.dp, top = 4.dp - ) + ), + verticalArrangement = Arrangement.spacedBy(4.dp) ) { item.topPosts.forEach { post -> val postPct = From 5065846759ada6648cdca4aecbd4e7ed3b6edc41 Mon Sep 17 00:00:00 2001 From: adalpari Date: Fri, 3 Jul 2026 12:04:25 +0200 Subject: [PATCH 05/13] Show all referrers in the detail screen The referrers endpoint returns only 10 items when max is unset. Send an explicit max=0 (which the server treats as unlimited) so it returns the full referrer list, matching how Posts & Pages behaves: the card still shows the first CARD_MAX_ITEMS entries while the detail screen shows all of them. CMM-2133 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../android/ui/newstats/datasource/StatsDataSourceImpl.kt | 4 ++-- .../android/ui/newstats/repository/StatsRepository.kt | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/datasource/StatsDataSourceImpl.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/datasource/StatsDataSourceImpl.kt index 4f45bbd9698e..64e5a0eac555 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/datasource/StatsDataSourceImpl.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/datasource/StatsDataSourceImpl.kt @@ -210,14 +210,14 @@ class StatsDataSourceImpl @Inject constructor( period = StatsReferrersPeriod.DAY, date = dateRange.date, num = dateRange.num.toUInt(), - max = max.coerceAtLeast(1).toUInt(), + max = max.coerceAtLeast(0).toUInt(), locale = wpComLanguage ) is StatsDateRange.Custom -> StatsReferrersParams( period = StatsReferrersPeriod.DAY, date = dateRange.date, startDate = dateRange.startDate, - max = max.coerceAtLeast(1).toUInt(), + max = max.coerceAtLeast(0).toUInt(), locale = wpComLanguage ) } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/repository/StatsRepository.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/repository/StatsRepository.kt index af6e7c812240..18c0e436d9c4 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/repository/StatsRepository.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/repository/StatsRepository.kt @@ -662,8 +662,11 @@ class StatsRepository @Inject constructor( currentDateRange: StatsDateRange, previousDateRange: StatsDateRange ): MostViewedResult = coroutineScope { - val currentDeferred = async { statsDataSource.fetchReferrers(siteId, currentDateRange) } - val previousDeferred = async { statsDataSource.fetchReferrers(siteId, previousDateRange) } + // Request all referrers (max = 0 = unlimited) so the detail screen can show more than the + // card. The server defaults to 10 when max is unset, so an explicit 0 is sent instead. The + // card itself still shows only the first CARD_MAX_ITEMS entries. + val currentDeferred = async { statsDataSource.fetchReferrers(siteId, currentDateRange, max = 0) } + val previousDeferred = async { statsDataSource.fetchReferrers(siteId, previousDateRange, max = 0) } val currentResult = currentDeferred.await() val previousResult = previousDeferred.await() From 36588969dd79d79c918f28f73e06aa1084c42e83 Mon Sep 17 00:00:00 2001 From: adalpari Date: Fri, 3 Jul 2026 12:38:18 +0200 Subject: [PATCH 06/13] Simplify referrer expand/collapse composables Extract the duplicated chevron, child row and expandable-children block from MostViewedCard and MostViewedDetailActivity into a shared MostViewedCommonComposables file, reusing StatsListRowContainer for the child row bar. Inject ActivityNavigator into NewStatsActivity and thread the child-click callback down instead of constructing the navigator in a composable and casting the context to Activity. Compute maxChildViews only when a group is expanded, and document the referrers max=0 semantics. CMM-2133 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../android/ui/newstats/NewStatsActivity.kt | 26 ++-- .../datasource/StatsDataSourceImpl.kt | 7 +- .../ui/newstats/mostviewed/MostViewedCard.kt | 105 +--------------- .../mostviewed/MostViewedCommonComposables.kt | 116 ++++++++++++++++++ .../mostviewed/MostViewedDetailActivity.kt | 104 +--------------- 5 files changed, 149 insertions(+), 209 deletions(-) create mode 100644 WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCommonComposables.kt diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/NewStatsActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/NewStatsActivity.kt index c8f936daecb1..594099ba6f2a 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/NewStatsActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/NewStatsActivity.kt @@ -1,6 +1,5 @@ package org.wordpress.android.ui.newstats -import android.app.Activity import android.content.Context import android.content.Intent import android.os.Bundle @@ -139,6 +138,9 @@ class NewStatsActivity : BaseAppCompatActivity() { @Inject lateinit var newStatsFeatureConfig: NewStatsFeatureConfig + @Inject + lateinit var activityNavigator: ActivityNavigator + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) val shouldShowIntro = @@ -155,6 +157,9 @@ class NewStatsActivity : BaseAppCompatActivity() { onIntroDismissed = { appPrefsWrapper .setNewStatsIntroShown(true) + }, + onReferrerChildClick = { url -> + activityNavigator.openInCustomTab(this, url) } ) } @@ -230,7 +235,8 @@ private fun NewStatsScreen( showSwitchToOldStats: Boolean = false, onSwitchToOldStats: () -> Unit = {}, showIntroBottomSheet: Boolean = false, - onIntroDismissed: () -> Unit = {} + onIntroDismissed: () -> Unit = {}, + onReferrerChildClick: (String) -> Unit = {} ) { val viewsStatsViewModel: ViewsStatsViewModel = viewModel() val selectedPeriod by viewsStatsViewModel.selectedPeriod.collectAsState() @@ -372,7 +378,8 @@ private fun NewStatsScreen( ) { page -> StatsTabContent( tab = tabs[page], - viewsStatsViewModel = viewsStatsViewModel + viewsStatsViewModel = viewsStatsViewModel, + onReferrerChildClick = onReferrerChildClick ) } } @@ -382,11 +389,13 @@ private fun NewStatsScreen( @Composable private fun StatsTabContent( tab: StatsTab, - viewsStatsViewModel: ViewsStatsViewModel + viewsStatsViewModel: ViewsStatsViewModel, + onReferrerChildClick: (String) -> Unit = {} ) { when (tab) { StatsTab.TRAFFIC -> TrafficTabContent( - viewsStatsViewModel = viewsStatsViewModel + viewsStatsViewModel = viewsStatsViewModel, + onReferrerChildClick = onReferrerChildClick ) StatsTab.INSIGHTS -> InsightsTabContent() StatsTab.SUBSCRIBERS -> SubscribersTabContent() @@ -408,13 +417,10 @@ private fun TrafficTabContent( fileDownloadsViewModel: FileDownloadsViewModel = viewModel(), devicesViewModel: DevicesViewModel = viewModel(), utmViewModel: UtmViewModel = viewModel(), - newStatsViewModel: NewStatsViewModel = viewModel() + newStatsViewModel: NewStatsViewModel = viewModel(), + onReferrerChildClick: (String) -> Unit = {} ) { val context = LocalContext.current - val activityNavigator = remember { ActivityNavigator() } - val onReferrerChildClick: (String) -> Unit = { url -> - (context as? Activity)?.let { activityNavigator.openInCustomTab(it, url) } - } val todaysStatsUiState by todaysStatsViewModel.uiState.collectAsState() val viewsStatsUiState by viewsStatsViewModel.uiState.collectAsState() val postsUiState by mostViewedViewModel.postsUiState.collectAsState() diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/datasource/StatsDataSourceImpl.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/datasource/StatsDataSourceImpl.kt index 64e5a0eac555..bb56d064e5f5 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/datasource/StatsDataSourceImpl.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/datasource/StatsDataSourceImpl.kt @@ -205,19 +205,22 @@ class StatsDataSourceImpl @Inject constructor( dateRange: StatsDateRange, max: Int ): ReferrersDataResult { + // Referrers-specific max semantics: the server treats max=0 as "all" but an unset max as + // "default 10", so 0 is sent explicitly (unlike other endpoints that map 0 -> null). + val maxParam = max.coerceAtLeast(0).toUInt() val params = when (dateRange) { is StatsDateRange.Preset -> StatsReferrersParams( period = StatsReferrersPeriod.DAY, date = dateRange.date, num = dateRange.num.toUInt(), - max = max.coerceAtLeast(0).toUInt(), + max = maxParam, locale = wpComLanguage ) is StatsDateRange.Custom -> StatsReferrersParams( period = StatsReferrersPeriod.DAY, date = dateRange.date, startDate = dateRange.startDate, - max = max.coerceAtLeast(0).toUInt(), + max = maxParam, locale = wpComLanguage ) } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt index a0da019be044..2de0c1d214d9 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt @@ -1,6 +1,5 @@ package org.wordpress.android.ui.newstats.mostviewed -import androidx.compose.animation.AnimatedVisibility import androidx.compose.foundation.background import androidx.compose.foundation.border import androidx.compose.foundation.clickable @@ -14,15 +13,9 @@ import androidx.compose.foundation.layout.fillMaxHeight import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width import androidx.compose.foundation.shape.RoundedCornerShape -import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.filled.ChevronRight -import androidx.compose.material.icons.filled.KeyboardArrowDown -import androidx.compose.material.icons.filled.KeyboardArrowUp import androidx.compose.material3.Button -import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable @@ -302,7 +295,6 @@ private fun MostViewedItemRow( val barColor = MaterialTheme.colorScheme.primary.copy(alpha = HIGHLIGHTED_ITEM_BACKGROUND_ALPHA) val hasChildren = item.children.isNotEmpty() var expanded by remember { mutableStateOf(false) } - val maxChildViews = item.children.maxOfOrNull { it.views } ?: 0L Column { Box( @@ -345,7 +337,7 @@ private fun MostViewedItemRow( Column(horizontalAlignment = Alignment.End) { Row(verticalAlignment = Alignment.CenterVertically) { if (hasChildren) { - ExpandChevron(expanded = expanded) + MostViewedExpandChevron(expanded = expanded) Spacer(modifier = Modifier.width(4.dp)) } Text( @@ -361,97 +353,12 @@ private fun MostViewedItemRow( } } - if (hasChildren) { - AnimatedVisibility(visible = expanded) { - Column( - modifier = Modifier.padding(start = 24.dp, top = 4.dp), - verticalArrangement = Arrangement.spacedBy(4.dp) - ) { - item.children.forEach { child -> - val childPercentage = if (maxChildViews > 0) { - child.views.toFloat() / maxChildViews.toFloat() - } else 0f - MostViewedChildRow( - child = child, - percentage = childPercentage, - onChildClick = onChildClick - ) - } - } - } - } - } -} - -@Composable -private fun ExpandChevron(expanded: Boolean) { - Icon( - imageVector = if (expanded) Icons.Default.KeyboardArrowUp else Icons.Default.KeyboardArrowDown, - contentDescription = stringResource( - if (expanded) R.string.stats_collapse_group else R.string.stats_expand_group - ), - modifier = Modifier.size(16.dp), - tint = MaterialTheme.colorScheme.onSurfaceVariant - ) -} - -@Composable -private fun MostViewedChildRow( - child: MostViewedChildItem, - percentage: Float, - onChildClick: (String) -> Unit -) { - val barColor = MaterialTheme.colorScheme.primary.copy(alpha = HIGHLIGHTED_ITEM_BACKGROUND_ALPHA) - val url = child.url - val isClickable = !url.isNullOrBlank() - val clickModifier = if (isClickable) { - Modifier.clickable { onChildClick(url) } - } else Modifier - - Box( - modifier = Modifier - .fillMaxWidth() - .height(IntrinsicSize.Min) - .clip(RoundedCornerShape(8.dp)) - .then(clickModifier) - ) { - Box( - modifier = Modifier - .fillMaxWidth(fraction = percentage) - .fillMaxHeight() - .background(barColor) + MostViewedExpandableChildren( + children = item.children, + expanded = expanded, + startPadding = 24.dp, + onChildClick = onChildClick ) - Row( - modifier = Modifier - .fillMaxWidth() - .padding(vertical = 10.dp, horizontal = 16.dp), - horizontalArrangement = Arrangement.SpaceBetween, - verticalAlignment = Alignment.CenterVertically - ) { - Text( - text = child.name, - style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onSurface, - maxLines = 1, - overflow = TextOverflow.Ellipsis, - modifier = Modifier.weight(1f) - ) - Spacer(modifier = Modifier.width(8.dp)) - Text( - text = formatStatValue(child.views), - style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onSurface - ) - if (isClickable) { - Spacer(modifier = Modifier.width(4.dp)) - Icon( - imageVector = Icons.Default.ChevronRight, - contentDescription = null, - modifier = Modifier.size(16.dp), - tint = MaterialTheme.colorScheme.onSurfaceVariant - ) - } - } } } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCommonComposables.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCommonComposables.kt new file mode 100644 index 000000000000..8eb9c2e01762 --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCommonComposables.kt @@ -0,0 +1,116 @@ +package org.wordpress.android.ui.newstats.mostviewed + +import androidx.compose.animation.AnimatedVisibility +import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.layout.width +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.ChevronRight +import androidx.compose.material.icons.filled.KeyboardArrowDown +import androidx.compose.material.icons.filled.KeyboardArrowUp +import androidx.compose.material3.Icon +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.unit.Dp +import androidx.compose.ui.unit.dp +import org.wordpress.android.R +import org.wordpress.android.ui.newstats.components.StatsListRowContainer +import org.wordpress.android.ui.newstats.util.formatStatValue + +/** + * Expand/collapse chevron shown on a Most Viewed row that has children. + * Shared by the card ([MostViewedCard]) and the detail screen ([MostViewedDetailActivity]). + */ +@Composable +internal fun MostViewedExpandChevron(expanded: Boolean) { + Icon( + imageVector = if (expanded) Icons.Default.KeyboardArrowUp else Icons.Default.KeyboardArrowDown, + contentDescription = stringResource( + if (expanded) R.string.stats_collapse_group else R.string.stats_expand_group + ), + modifier = Modifier.size(16.dp), + tint = MaterialTheme.colorScheme.onSurfaceVariant + ) +} + +/** + * Indented, animated list of child rows shown when a Most Viewed group is expanded. + * Shared by the card and the detail screen; only the [startPadding] indent differs. + */ +@Composable +internal fun MostViewedExpandableChildren( + children: List, + expanded: Boolean, + startPadding: Dp, + onChildClick: (String) -> Unit +) { + AnimatedVisibility(visible = expanded) { + val maxChildViews = children.maxOfOrNull { it.views } ?: 0L + Column( + modifier = Modifier.padding(start = startPadding, top = 4.dp), + verticalArrangement = Arrangement.spacedBy(4.dp) + ) { + children.forEach { child -> + val childPercentage = if (maxChildViews > 0) { + child.views.toFloat() / maxChildViews.toFloat() + } else 0f + MostViewedChildRow(child = child, percentage = childPercentage, onChildClick = onChildClick) + } + } + } +} + +@Composable +private fun MostViewedChildRow( + child: MostViewedChildItem, + percentage: Float, + onChildClick: (String) -> Unit +) { + val url = child.url + val isClickable = !url.isNullOrBlank() + val clickModifier = if (isClickable) Modifier.clickable { onChildClick(url) } else Modifier + + StatsListRowContainer(percentage = percentage, modifier = clickModifier) { + Row( + modifier = Modifier + .fillMaxWidth() + .padding(vertical = 10.dp, horizontal = 16.dp), + verticalAlignment = Alignment.CenterVertically + ) { + Text( + text = child.name, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurface, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.weight(1f) + ) + Spacer(modifier = Modifier.width(8.dp)) + Text( + text = formatStatValue(child.views), + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurface + ) + if (isClickable) { + Spacer(modifier = Modifier.width(4.dp)) + Icon( + imageVector = Icons.Default.ChevronRight, + contentDescription = null, + modifier = Modifier.size(16.dp), + tint = MaterialTheme.colorScheme.onSurfaceVariant + ) + } + } + } +} diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt index 0c1b9fd5c160..4686e4bee5f6 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt @@ -4,7 +4,6 @@ import android.content.Context import android.content.Intent import android.os.Bundle import androidx.activity.compose.setContent -import androidx.compose.animation.AnimatedVisibility import androidx.compose.foundation.background import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Arrangement @@ -18,16 +17,12 @@ import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.itemsIndexed import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.ArrowBack -import androidx.compose.material.icons.filled.ChevronRight -import androidx.compose.material.icons.filled.KeyboardArrowDown -import androidx.compose.material.icons.filled.KeyboardArrowUp import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.IconButton @@ -250,7 +245,6 @@ private fun DetailItemRow( ) val hasChildren = item.children.isNotEmpty() var expanded by remember { mutableStateOf(false) } - val maxChildViews = item.children.maxOfOrNull { it.views } ?: 0L Column { Box( @@ -296,7 +290,7 @@ private fun DetailItemRow( Column(horizontalAlignment = Alignment.End) { Row(verticalAlignment = Alignment.CenterVertically) { if (hasChildren) { - DetailExpandChevron(expanded = expanded) + MostViewedExpandChevron(expanded = expanded) Spacer(modifier = Modifier.width(4.dp)) } Text( @@ -310,98 +304,12 @@ private fun DetailItemRow( } } - if (hasChildren) { - AnimatedVisibility(visible = expanded) { - Column( - modifier = Modifier.padding(start = 32.dp, top = 4.dp), - verticalArrangement = Arrangement.spacedBy(4.dp) - ) { - item.children.forEach { child -> - val childPercentage = if (maxChildViews > 0) { - child.views.toFloat() / maxChildViews.toFloat() - } else 0f - DetailChildRow( - child = child, - percentage = childPercentage, - onChildClick = onChildClick - ) - } - } - } - } - } -} - -@Composable -private fun DetailExpandChevron(expanded: Boolean) { - Icon( - imageVector = if (expanded) Icons.Default.KeyboardArrowUp else Icons.Default.KeyboardArrowDown, - contentDescription = stringResource( - if (expanded) R.string.stats_collapse_group else R.string.stats_expand_group - ), - modifier = Modifier.size(16.dp), - tint = MaterialTheme.colorScheme.onSurfaceVariant - ) -} - -@Composable -private fun DetailChildRow( - child: MostViewedChildItem, - percentage: Float, - onChildClick: (String) -> Unit -) { - val barColor = MaterialTheme.colorScheme.primary.copy( - alpha = HIGHLIGHTED_ITEM_BACKGROUND_ALPHA - ) - val url = child.url - val isClickable = !url.isNullOrBlank() - val clickModifier = if (isClickable) { - Modifier.clickable { onChildClick(url) } - } else Modifier - - Box( - modifier = Modifier - .fillMaxWidth() - .height(IntrinsicSize.Min) - .clip(RoundedCornerShape(8.dp)) - .then(clickModifier) - ) { - Box( - modifier = Modifier - .fillMaxWidth(fraction = percentage) - .fillMaxHeight() - .background(barColor) + MostViewedExpandableChildren( + children = item.children, + expanded = expanded, + startPadding = 32.dp, + onChildClick = onChildClick ) - Row( - modifier = Modifier - .fillMaxWidth() - .padding(vertical = 10.dp, horizontal = 16.dp), - verticalAlignment = Alignment.CenterVertically - ) { - Text( - text = child.name, - style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onSurface, - maxLines = 1, - overflow = TextOverflow.Ellipsis, - modifier = Modifier.weight(1f) - ) - Spacer(modifier = Modifier.width(8.dp)) - Text( - text = formatStatValue(child.views), - style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onSurface - ) - if (isClickable) { - Spacer(modifier = Modifier.width(4.dp)) - Icon( - imageVector = Icons.Default.ChevronRight, - contentDescription = null, - modifier = Modifier.size(16.dp), - tint = MaterialTheme.colorScheme.onSurfaceVariant - ) - } - } } } From 423f06f32fed94e4ae6ca3358496c4fc22fd69f8 Mon Sep 17 00:00:00 2001 From: adalpari Date: Fri, 3 Jul 2026 12:54:43 +0200 Subject: [PATCH 07/13] Preserve referrer expanded state when scrolling detail screen The detail screen renders rows in a LazyColumn, so the per-row expanded state held in a plain remember was discarded when a row scrolled off screen and reset to collapsed when it returned. Use rememberSaveable and key the list by item id so an expanded group stays expanded across scrolling. CMM-2133 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ui/newstats/mostviewed/MostViewedDetailActivity.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt index 4686e4bee5f6..8fc6d1d55d6d 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt @@ -33,7 +33,7 @@ import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.remember +import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -189,7 +189,7 @@ private fun MostViewedDetailScreen( Spacer(modifier = Modifier.height(8.dp)) } - itemsIndexed(items) { index, item -> + itemsIndexed(items, key = { _, item -> item.id }) { index, item -> DetailItemRow( position = index + 1, item = item, @@ -244,7 +244,7 @@ private fun DetailItemRow( alpha = HIGHLIGHTED_ITEM_BACKGROUND_ALPHA ) val hasChildren = item.children.isNotEmpty() - var expanded by remember { mutableStateOf(false) } + var expanded by rememberSaveable { mutableStateOf(false) } Column { Box( From ea52e247f8f33ed25ec9452cab19f6da5bfd4477 Mon Sep 17 00:00:00 2001 From: adalpari Date: Fri, 3 Jul 2026 13:20:25 +0200 Subject: [PATCH 08/13] Self-fetch referrers detail and fix detail list key The referrers card fetched the full (max=0) list and passed it to the detail screen through the launching Intent, and the detail LazyColumn was keyed by item.id (derived from name.hashCode()). On sites with many referrers this risked a TransactionTooLargeException, and empty/duplicate referrer names could collide into duplicate keys and crash the list. Bound the card request to REFERRERS_CARD_MAX and add fetchReferrersDetail (max=0) so the detail screen self-fetches its own unbounded list via a new MostViewedDetailViewModel (Loading/Error/Loaded), passing only the period through the Intent. Key the detail list by index instead of item.id. CMM-2133 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../android/ui/newstats/NewStatsActivity.kt | 12 +- .../mostviewed/MostViewedDetailActivity.kt | 257 +++++++++++++----- .../mostviewed/MostViewedDetailViewModel.kt | 109 ++++++++ .../mostviewed/MostViewedViewModel.kt | 28 +- .../ui/newstats/repository/StatsRepository.kt | 34 ++- .../mostviewed/MostViewedViewModelTest.kt | 16 -- .../repository/StatsRepositoryTest.kt | 26 ++ 7 files changed, 356 insertions(+), 126 deletions(-) create mode 100644 WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailViewModel.kt diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/NewStatsActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/NewStatsActivity.kt index 594099ba6f2a..e9c24172f624 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/NewStatsActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/NewStatsActivity.kt @@ -691,15 +691,11 @@ private fun TrafficTabContent( uiState = referrersUiState, cardType = cardType, onShowAllClick = { - val detailData = mostViewedViewModel.getReferrersDetailData() - MostViewedDetailActivity.start( + // The referrers detail screen self-fetches the full (unbounded) list, so + // only the selected period is passed instead of the whole item list. + MostViewedDetailActivity.startReferrers( context = context, - cardType = detailData.cardType, - items = detailData.items, - totalViews = detailData.totalViews, - totalViewsChange = detailData.totalViewsChange, - totalViewsChangePercent = detailData.totalViewsChangePercent, - dateRange = detailData.dateRange + period = mostViewedViewModel.getCurrentPeriod() ) }, onRetry = mostViewedViewModel::onRetryReferrers, diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt index 8fc6d1d55d6d..20c2f04cb07d 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt @@ -4,6 +4,7 @@ import android.content.Context import android.content.Intent import android.os.Bundle import androidx.activity.compose.setContent +import androidx.activity.viewModels import androidx.compose.foundation.background import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Arrangement @@ -23,6 +24,8 @@ import androidx.compose.foundation.lazy.itemsIndexed import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.ArrowBack +import androidx.compose.material3.Button +import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.IconButton @@ -31,8 +34,10 @@ import androidx.compose.material3.Scaffold import androidx.compose.material3.Text import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable +import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment @@ -49,6 +54,7 @@ import org.wordpress.android.ui.ActivityNavigator import org.wordpress.android.ui.compose.theme.AppThemeM3 import org.wordpress.android.ui.main.BaseAppCompatActivity import org.wordpress.android.ui.newstats.StatsCardType +import org.wordpress.android.ui.newstats.StatsPeriod import org.wordpress.android.ui.newstats.components.StatsSummaryCard import org.wordpress.android.ui.newstats.util.formatStatValue import org.wordpress.android.util.extensions.getParcelableArrayListCompat @@ -63,45 +69,80 @@ private const val EXTRA_TOTAL_VIEWS_CHANGE_PERCENT = "extra_total_views_change_p private const val EXTRA_DATE_RANGE = "extra_date_range" private const val EXTRA_VALUE_HEADER_RES_ID = "extra_value_header_res_id" +// Self-fetch mode (referrers): the detail screen re-fetches its own unbounded data for this period +// instead of receiving the item list through the Intent. When these are absent the screen renders +// the items passed via EXTRA_ITEMS (posts, clicks, search terms, etc.). +private const val EXTRA_PERIOD_TYPE = "extra_period_type" +private const val EXTRA_PERIOD_CUSTOM_START = "extra_period_custom_start" +private const val EXTRA_PERIOD_CUSTOM_END = "extra_period_custom_end" +private const val NO_EPOCH_DAY = -1L + @AndroidEntryPoint class MostViewedDetailActivity : BaseAppCompatActivity() { @Inject lateinit var activityNavigator: ActivityNavigator + private val viewModel: MostViewedDetailViewModel by viewModels() + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) val cardType = intent.extras?.getSerializableCompat(EXTRA_CARD_TYPE) ?: StatsCardType.MOST_VIEWED_POSTS_AND_PAGES - val items = intent.extras?.getParcelableArrayListCompat(EXTRA_ITEMS) - ?: arrayListOf() - val totalViews = intent.getLongExtra(EXTRA_TOTAL_VIEWS, 0L) - val totalViewsChange = intent.getLongExtra(EXTRA_TOTAL_VIEWS_CHANGE, 0L) - val totalViewsChangePercent = intent.getDoubleExtra(EXTRA_TOTAL_VIEWS_CHANGE_PERCENT, 0.0) - val dateRange = intent.getStringExtra(EXTRA_DATE_RANGE) ?: "" - val valueHeaderResId = intent.getIntExtra( - EXTRA_VALUE_HEADER_RES_ID, R.string.stats_views - ) - // Calculate maxViewsForBar once (list is sorted by views descending) - val maxViewsForBar = items.firstOrNull()?.views ?: 1L + val valueHeaderResId = intent.getIntExtra(EXTRA_VALUE_HEADER_RES_ID, R.string.stats_views) + val selfFetchPeriod = readSelfFetchPeriod() + + if (selfFetchPeriod != null) { + viewModel.loadReferrers(selfFetchPeriod) + } setContent { AppThemeM3 { + val uiState = if (selfFetchPeriod != null) { + viewModel.uiState.collectAsState().value + } else { + remember { intent.toPassedItemsState() } + } MostViewedDetailScreen( cardType = cardType, - items = items, - maxViewsForBar = maxViewsForBar, - totalViews = totalViews, - totalViewsChange = totalViewsChange, - totalViewsChangePercent = totalViewsChangePercent, - dateRange = dateRange, + uiState = uiState, valueHeaderResId = valueHeaderResId, onBackPressed = onBackPressedDispatcher::onBackPressed, + onRetry = { viewModel.retry() }, onChildClick = { url -> activityNavigator.openInCustomTab(this, url) } ) } } } + /** + * Reads the period for self-fetch mode, or null when the screen should render the items passed + * via the Intent (posts, clicks, search terms, etc.). + */ + private fun readSelfFetchPeriod(): StatsPeriod? { + val periodType = intent.getStringExtra(EXTRA_PERIOD_TYPE) ?: return null + val customStart = intent.getLongExtra(EXTRA_PERIOD_CUSTOM_START, NO_EPOCH_DAY) + val customEnd = intent.getLongExtra(EXTRA_PERIOD_CUSTOM_END, NO_EPOCH_DAY) + return StatsPeriod.fromTypeString( + type = periodType, + customStartEpochDay = customStart.takeIf { it != NO_EPOCH_DAY }, + customEndEpochDay = customEnd.takeIf { it != NO_EPOCH_DAY } + ) + } + + private fun Intent.toPassedItemsState(): MostViewedDetailUiState.Loaded { + val items = extras?.getParcelableArrayListCompat(EXTRA_ITEMS) + ?: arrayListOf() + return MostViewedDetailUiState.Loaded( + items = items, + // Calculate maxViewsForBar once (list is sorted by views descending) + maxViewsForBar = items.firstOrNull()?.views ?: 1L, + totalViews = getLongExtra(EXTRA_TOTAL_VIEWS, 0L), + totalViewsChange = getLongExtra(EXTRA_TOTAL_VIEWS_CHANGE, 0L), + totalViewsChangePercent = getDoubleExtra(EXTRA_TOTAL_VIEWS_CHANGE_PERCENT, 0.0), + dateRange = getStringExtra(EXTRA_DATE_RANGE) ?: "" + ) + } + companion object { @Suppress("LongParameterList") fun start( @@ -130,6 +171,22 @@ class MostViewedDetailActivity : BaseAppCompatActivity() { } context.startActivity(intent) } + + /** + * Launches the referrers detail screen in self-fetch mode: only the [period] is passed and + * the screen re-fetches the full referrer list itself (see [MostViewedDetailViewModel]). + */ + fun startReferrers(context: Context, period: StatsPeriod) { + val intent = Intent(context, MostViewedDetailActivity::class.java).apply { + putExtra(EXTRA_CARD_TYPE, StatsCardType.MOST_VIEWED_REFERRERS) + putExtra(EXTRA_PERIOD_TYPE, period.toTypeString()) + if (period is StatsPeriod.Custom) { + putExtra(EXTRA_PERIOD_CUSTOM_START, period.startDate.toEpochDay()) + putExtra(EXTRA_PERIOD_CUSTOM_END, period.endDate.toEpochDay()) + } + } + context.startActivity(intent) + } } } @@ -137,14 +194,10 @@ class MostViewedDetailActivity : BaseAppCompatActivity() { @Composable private fun MostViewedDetailScreen( cardType: StatsCardType, - items: List, - maxViewsForBar: Long, - totalViews: Long, - totalViewsChange: Long, - totalViewsChangePercent: Double, - dateRange: String, + uiState: MostViewedDetailUiState, valueHeaderResId: Int = R.string.stats_views, onBackPressed: () -> Unit, + onRetry: () -> Unit = {}, onChildClick: (String) -> Unit = {} ) { val title = stringResource(cardType.displayNameResId) @@ -164,47 +217,99 @@ private fun MostViewedDetailScreen( ) } ) { contentPadding -> - LazyColumn( - modifier = Modifier - .fillMaxSize() - .padding(contentPadding) - .padding(horizontal = 16.dp) - ) { - item { - Spacer(modifier = Modifier.height(8.dp)) - StatsSummaryCard( - totalViews = totalViews, - dateRange = dateRange, - totalViewsChange = totalViewsChange, - totalViewsChangePercent = totalViewsChangePercent - ) - Spacer(modifier = Modifier.height(16.dp)) - } + val contentModifier = Modifier + .fillMaxSize() + .padding(contentPadding) + when (uiState) { + is MostViewedDetailUiState.Loading -> DetailLoadingContent(contentModifier) + is MostViewedDetailUiState.Error -> DetailErrorContent(uiState.message, onRetry, contentModifier) + is MostViewedDetailUiState.Loaded -> DetailLoadedContent( + state = uiState, + valueHeaderResId = valueHeaderResId, + onChildClick = onChildClick, + modifier = contentModifier + ) + } + } +} - item { - ColumnHeaders( - itemCount = items.size, - valueHeaderResId = valueHeaderResId - ) - Spacer(modifier = Modifier.height(8.dp)) - } +@Composable +private fun DetailLoadedContent( + state: MostViewedDetailUiState.Loaded, + valueHeaderResId: Int, + onChildClick: (String) -> Unit, + modifier: Modifier = Modifier +) { + LazyColumn( + modifier = modifier.padding(horizontal = 16.dp) + ) { + item { + Spacer(modifier = Modifier.height(8.dp)) + StatsSummaryCard( + totalViews = state.totalViews, + dateRange = state.dateRange, + totalViewsChange = state.totalViewsChange, + totalViewsChangePercent = state.totalViewsChangePercent + ) + Spacer(modifier = Modifier.height(16.dp)) + } - itemsIndexed(items, key = { _, item -> item.id }) { index, item -> - DetailItemRow( - position = index + 1, - item = item, - maxViewsForBar = maxViewsForBar, - onChildClick = onChildClick - ) - if (index < items.lastIndex) { - Spacer(modifier = Modifier.height(4.dp)) - } - } + item { + ColumnHeaders( + itemCount = state.items.size, + valueHeaderResId = valueHeaderResId + ) + Spacer(modifier = Modifier.height(8.dp)) + } - item { - Spacer(modifier = Modifier.height(16.dp)) + // Key by index rather than item.id: referrer ids are derived from name.hashCode(), + // which is not guaranteed unique (empty/duplicate names collide) and would crash the + // LazyColumn. The list is a static snapshot, so the index is a stable, unique key. + itemsIndexed(state.items, key = { index, _ -> index }) { index, item -> + DetailItemRow( + position = index + 1, + item = item, + maxViewsForBar = state.maxViewsForBar, + onChildClick = onChildClick + ) + if (index < state.items.lastIndex) { + Spacer(modifier = Modifier.height(4.dp)) } } + + item { + Spacer(modifier = Modifier.height(16.dp)) + } + } +} + +@Composable +private fun DetailLoadingContent(modifier: Modifier = Modifier) { + Box(modifier = modifier, contentAlignment = Alignment.Center) { + CircularProgressIndicator() + } +} + +@Composable +private fun DetailErrorContent( + message: String, + onRetry: () -> Unit, + modifier: Modifier = Modifier +) { + Column( + modifier = modifier.padding(16.dp), + horizontalAlignment = Alignment.CenterHorizontally, + verticalArrangement = Arrangement.Center + ) { + Text( + text = message, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.error + ) + Spacer(modifier = Modifier.height(16.dp)) + Button(onClick = onRetry) { + Text(text = stringResource(R.string.retry)) + } } } @@ -319,23 +424,25 @@ private fun MostViewedDetailScreenPreview() { AppThemeM3 { MostViewedDetailScreen( cardType = StatsCardType.MOST_VIEWED_POSTS_AND_PAGES, - items = listOf( - MostViewedDetailItem(1, "Welcome to Automattic", 998, - MostViewedChange.Positive(41, 4.3)), - MostViewedDetailItem(2, "Travel Guidelines", 111, - MostViewedChange.Positive(22, 24.7)), - MostViewedDetailItem(3, "LibreChat", 93, - MostViewedChange.Positive(21, 29.2)), - MostViewedDetailItem(4, "Getting Started with Claude Code: A Comprehensive Tutorial", 91, - MostViewedChange.Positive(47, 106.8)), - MostViewedDetailItem(5, "AI Tools & Resource Hub", 72, - MostViewedChange.Positive(31, 75.6)) + uiState = MostViewedDetailUiState.Loaded( + items = listOf( + MostViewedDetailItem(1, "Welcome to Automattic", 998, + MostViewedChange.Positive(41, 4.3)), + MostViewedDetailItem(2, "Travel Guidelines", 111, + MostViewedChange.Positive(22, 24.7)), + MostViewedDetailItem(3, "LibreChat", 93, + MostViewedChange.Positive(21, 29.2)), + MostViewedDetailItem(4, "Getting Started with Claude Code: A Comprehensive Tutorial", 91, + MostViewedChange.Positive(47, 106.8)), + MostViewedDetailItem(5, "AI Tools & Resource Hub", 72, + MostViewedChange.Positive(31, 75.6)) + ), + maxViewsForBar = 998, + totalViews = 5400, + totalViewsChange = 69, + totalViewsChangePercent = 1.3, + dateRange = "21-27 Jan" ), - maxViewsForBar = 998, - totalViews = 5400, - totalViewsChange = 69, - totalViewsChangePercent = 1.3, - dateRange = "21-27 Jan", onBackPressed = {} ) } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailViewModel.kt new file mode 100644 index 000000000000..8db136bd99d1 --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailViewModel.kt @@ -0,0 +1,109 @@ +package org.wordpress.android.ui.newstats.mostviewed + +import androidx.lifecycle.ViewModel +import androidx.lifecycle.viewModelScope +import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.launch +import org.wordpress.android.R +import org.wordpress.android.fluxc.store.AccountStore +import org.wordpress.android.ui.mysite.SelectedSiteRepository +import org.wordpress.android.ui.newstats.StatsPeriod +import org.wordpress.android.ui.newstats.repository.MostViewedResult +import org.wordpress.android.ui.newstats.repository.StatsRepository +import org.wordpress.android.ui.newstats.util.toDateRangeString +import org.wordpress.android.viewmodel.ResourceProvider +import javax.inject.Inject + +/** + * Backs [MostViewedDetailActivity] when it self-fetches its data (currently the referrers detail + * screen). The referrers card is bounded to [StatsRepository]'s card max, so the detail screen + * re-requests the full, unbounded list (max = 0) instead of receiving it through the Intent — this + * keeps the card request small and avoids passing a large parcelable list across the process + * boundary (which could exceed the Binder transaction limit). + */ +@HiltViewModel +class MostViewedDetailViewModel @Inject constructor( + private val selectedSiteRepository: SelectedSiteRepository, + private val accountStore: AccountStore, + private val statsRepository: StatsRepository, + private val resourceProvider: ResourceProvider +) : ViewModel() { + private val _uiState = MutableStateFlow(MostViewedDetailUiState.Loading) + val uiState: StateFlow = _uiState.asStateFlow() + + private var period: StatsPeriod? = null + private var hasStartedLoading = false + + /** + * Loads the referrers detail data for [period]. Safe to call on every [MostViewedDetailActivity] + * creation: the fetch only runs once (subsequent calls after e.g. a rotation are ignored while a + * result is already present). Use [retry] to force a reload. + */ + fun loadReferrers(period: StatsPeriod) { + this.period = period + if (hasStartedLoading) return + load() + } + + fun retry() = load() + + private fun load() { + val period = period ?: return + val site = selectedSiteRepository.getSelectedSite() + val accessToken = accountStore.accessToken + if (site == null || accessToken.isNullOrEmpty()) { + _uiState.value = MostViewedDetailUiState.Error(resourceProvider.getString(R.string.stats_error_api)) + return + } + hasStartedLoading = true + statsRepository.init(accessToken) + _uiState.value = MostViewedDetailUiState.Loading + viewModelScope.launch { + _uiState.value = fetchReferrers(site.siteId, period) + } + } + + @Suppress("TooGenericExceptionCaught") + private suspend fun fetchReferrers(siteId: Long, period: StatsPeriod): MostViewedDetailUiState = + try { + val result = statsRepository.fetchReferrersDetail(siteId = siteId, period = period) + when (result) { + is MostViewedResult.Success -> { + val items = result.items.map { it.toDetailItem() } + MostViewedDetailUiState.Loaded( + items = items, + maxViewsForBar = items.firstOrNull()?.views ?: 1L, + totalViews = result.totalViews, + totalViewsChange = result.totalViewsChange, + totalViewsChangePercent = result.totalViewsChangePercent, + dateRange = period.toDateRangeString(resourceProvider) + ) + } + is MostViewedResult.Error -> MostViewedDetailUiState.Error( + resourceProvider.getString(R.string.stats_error_api) + ) + } + } catch (e: Exception) { + MostViewedDetailUiState.Error( + e.message ?: resourceProvider.getString(R.string.stats_error_unknown) + ) + } +} + +sealed interface MostViewedDetailUiState { + data object Loading : MostViewedDetailUiState + + data class Loaded( + val items: List, + val maxViewsForBar: Long, + val totalViews: Long, + val totalViewsChange: Long, + val totalViewsChangePercent: Double, + val dateRange: String + ) : MostViewedDetailUiState + + data class Error(val message: String) : MostViewedDetailUiState +} diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedViewModel.kt index 29b6c51264b6..97698a9d1d54 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedViewModel.kt @@ -53,11 +53,6 @@ class MostViewedViewModel @Inject constructor( private var postsCachedTotalViewsChange: Long = 0L private var postsCachedTotalViewsChangePercent: Double = 0.0 - private var referrersAllItems: List = emptyList() - private var referrersCachedTotalViews: Long = 0L - private var referrersCachedTotalViewsChange: Long = 0L - private var referrersCachedTotalViewsChangePercent: Double = 0.0 - fun onPeriodChanged(period: StatsPeriod) { currentPeriod = period onPeriodChangedPosts(period) @@ -144,16 +139,11 @@ class MostViewedViewModel @Inject constructor( ) } - fun getReferrersDetailData(): MostViewedDetailData { - return MostViewedDetailData( - cardType = StatsCardType.MOST_VIEWED_REFERRERS, - items = referrersAllItems, - totalViews = referrersCachedTotalViews, - totalViewsChange = referrersCachedTotalViewsChange, - totalViewsChangePercent = referrersCachedTotalViewsChangePercent, - dateRange = currentPeriod.toDateRangeString(resourceProvider) - ) - } + /** + * The period currently selected for the referrers card. The referrers detail screen re-fetches + * its own (unbounded) data for this period rather than receiving the full list via the Intent. + */ + fun getCurrentPeriod(): StatsPeriod = currentPeriod fun loadData() { loadPosts() @@ -260,10 +250,8 @@ class MostViewedViewModel @Inject constructor( _postsUiState.value = loadedState } MostViewedDataSource.REFERRERS -> { - referrersAllItems = allItems - referrersCachedTotalViews = result.totalViews - referrersCachedTotalViewsChange = result.totalViewsChange - referrersCachedTotalViewsChangePercent = result.totalViewsChangePercent + // The referrers detail screen self-fetches (see MostViewedDetailViewModel), so unlike + // posts we don't cache the full item list / totals here. _referrersUiState.value = loadedState } } @@ -337,7 +325,7 @@ data class MostViewedDetailData( val dateRange: String ) -private fun MostViewedItemData.toDetailItem(): MostViewedDetailItem { +internal fun MostViewedItemData.toDetailItem(): MostViewedDetailItem { val change = when { viewsChange > 0 -> MostViewedChange.Positive(viewsChange, abs(viewsChangePercent)) viewsChange < 0 -> MostViewedChange.Negative(abs(viewsChange), abs(viewsChangePercent)) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/repository/StatsRepository.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/repository/StatsRepository.kt index 18c0e436d9c4..5cedee2d3fa1 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/repository/StatsRepository.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/repository/StatsRepository.kt @@ -81,6 +81,11 @@ internal fun calculateItemChangePercent( private const val NUM_DAYS_TODAY = 1 private const val SUBSCRIBERS_DEFAULT_MAX = 10 +// Number of referrers requested for the card. The detail screen requests all of them (max = 0, +// which the server treats as "unlimited"). +private const val REFERRERS_CARD_MAX = 10 +private const val REFERRERS_DETAIL_MAX = 0 + /** * Repository for fetching stats data using the wordpress-rs API. * Handles hourly visits/views data for the Today's Stats card chart. @@ -590,11 +595,24 @@ class StatsRepository @Inject constructor( fetchTopPostsWithComparison(siteId, currentDateRange, previousDateRange) } MostViewedDataSource.REFERRERS -> { - fetchReferrersWithComparison(siteId, currentDateRange, previousDateRange) + fetchReferrersWithComparison(siteId, currentDateRange, previousDateRange, REFERRERS_CARD_MAX) } } } + /** + * Fetches the full referrer list (max = 0 = unlimited) for the referrers detail screen. Kept + * separate from [fetchMostViewed] (which bounds the card to [REFERRERS_CARD_MAX]) so the detail + * screen can show more entries than the card without inflating the card request. + */ + suspend fun fetchReferrersDetail( + siteId: Long, + period: StatsPeriod + ): MostViewedResult = withContext(ioDispatcher) { + val (currentDateRange, previousDateRange) = calculateComparisonDateRanges(period) + fetchReferrersWithComparison(siteId, currentDateRange, previousDateRange, REFERRERS_DETAIL_MAX) + } + private suspend fun fetchTopPostsWithComparison( siteId: Long, currentDateRange: StatsDateRange, @@ -660,13 +678,15 @@ class StatsRepository @Inject constructor( private suspend fun fetchReferrersWithComparison( siteId: Long, currentDateRange: StatsDateRange, - previousDateRange: StatsDateRange + previousDateRange: StatsDateRange, + max: Int ): MostViewedResult = coroutineScope { - // Request all referrers (max = 0 = unlimited) so the detail screen can show more than the - // card. The server defaults to 10 when max is unset, so an explicit 0 is sent instead. The - // card itself still shows only the first CARD_MAX_ITEMS entries. - val currentDeferred = async { statsDataSource.fetchReferrers(siteId, currentDateRange, max = 0) } - val previousDeferred = async { statsDataSource.fetchReferrers(siteId, previousDateRange, max = 0) } + // The card requests REFERRERS_CARD_MAX items; the detail screen requests all of them by + // passing max = 0 (the server treats 0 as "unlimited", vs. an unset max that defaults to 10). + // Fetching all only when the detail screen opens keeps the card request small and avoids + // passing a large list across the process boundary. + val currentDeferred = async { statsDataSource.fetchReferrers(siteId, currentDateRange, max = max) } + val previousDeferred = async { statsDataSource.fetchReferrers(siteId, previousDateRange, max = max) } val currentResult = currentDeferred.await() val previousResult = previousDeferred.await() diff --git a/WordPress/src/test/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedViewModelTest.kt index d99b5fdba58d..da0c4cac684b 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedViewModelTest.kt @@ -509,22 +509,6 @@ class MostViewedViewModelTest : BaseUnitTest() { assertThat(detailData.totalViewsChangePercent).isEqualTo(TEST_TOTAL_VIEWS_CHANGE_PERCENT) } - @Test - fun `when getReferrersDetailData is called, then returns cached referrers data`() = test { - whenever(statsRepository.fetchMostViewed(any(), any(), any())) - .thenReturn(createSuccessResult()) - whenever(resourceProvider.getString(R.string.stats_period_last_7_days)) - .thenReturn("Last 7 days") - - initViewModel() - advanceUntilIdle() - - val detailData = viewModel.getReferrersDetailData() - - assertThat(detailData.cardType).isEqualTo(StatsCardType.MOST_VIEWED_REFERRERS) - assertThat(detailData.items).hasSize(2) - } - @Test fun `when getPostsDetailData is called, then all items are returned not just card items`() = test { val manyItems = (1..15).mapIndexed { idx, index -> diff --git a/WordPress/src/test/java/org/wordpress/android/ui/newstats/repository/StatsRepositoryTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/newstats/repository/StatsRepositoryTest.kt index 848903b6a958..72776a094158 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/newstats/repository/StatsRepositoryTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/newstats/repository/StatsRepositoryTest.kt @@ -595,6 +595,28 @@ class StatsRepositoryTest : BaseUnitTest() { assertThat(success.items[0].children[0].views).isEqualTo(23) } + @Test + fun `when fetchMostViewed with REFERRERS, then card-sized max is requested`() = test { + whenever(statsDataSource.fetchReferrers(any(), any(), any())) + .thenReturn(ReferrersDataResult.Success(createReferrersData())) + + repository.fetchMostViewed(TEST_SITE_ID, StatsPeriod.Last7Days, MostViewedDataSource.REFERRERS) + + // Card request is bounded (current + previous period). + verify(statsDataSource, times(2)).fetchReferrers(any(), any(), eq(REFERRERS_CARD_MAX)) + } + + @Test + fun `when fetchReferrersDetail, then unlimited max is requested`() = test { + whenever(statsDataSource.fetchReferrers(any(), any(), any())) + .thenReturn(ReferrersDataResult.Success(createReferrersData())) + + repository.fetchReferrersDetail(TEST_SITE_ID, StatsPeriod.Last7Days) + + // Detail request asks for all referrers (max = 0), for current + previous period. + verify(statsDataSource, times(2)).fetchReferrers(any(), any(), eq(REFERRERS_DETAIL_MAX)) + } + @Test fun `given successful response, when fetchMostViewed, then totalViews is calculated correctly`() = test { whenever(statsDataSource.fetchTopPostsAndPages(any(), any(), any())) @@ -790,6 +812,10 @@ class StatsRepositoryTest : BaseUnitTest() { private const val TEST_ACCESS_TOKEN = "test_access_token" private val TEST_ERROR_TYPE = StatsErrorType.NETWORK_ERROR + // Mirrors StatsRepository's private referrer max values (card vs. detail). + private const val REFERRERS_CARD_MAX = 10 + private const val REFERRERS_DETAIL_MAX = 0 + private const val TEST_PERIOD_1 = "2024-01-15" private const val TEST_PERIOD_2 = "2024-01-16" From 80e56d7352e0fab3793ffbe919374ea08c172a2f Mon Sep 17 00:00:00 2001 From: adalpari Date: Fri, 3 Jul 2026 13:58:47 +0200 Subject: [PATCH 09/13] Key referrer expanded state on item id and add detail VM tests Reset the expanded state when a row is reused for a different referrer, and add unit tests covering MostViewedDetailViewModel load/error/retry behavior. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ui/newstats/mostviewed/MostViewedCard.kt | 4 +- .../MostViewedDetailViewModelTest.kt | 204 ++++++++++++++++++ 2 files changed, 207 insertions(+), 1 deletion(-) create mode 100644 WordPress/src/test/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailViewModelTest.kt diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt index 2de0c1d214d9..b08ee9ae541f 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt @@ -294,7 +294,9 @@ private fun MostViewedItemRow( ) { val barColor = MaterialTheme.colorScheme.primary.copy(alpha = HIGHLIGHTED_ITEM_BACKGROUND_ALPHA) val hasChildren = item.children.isNotEmpty() - var expanded by remember { mutableStateOf(false) } + // Key the expanded state on the item id so it resets when the row is reused for a different + // referrer (e.g. after the period changes and the list reloads at the same position). + var expanded by remember(item.id) { mutableStateOf(false) } Column { Box( diff --git a/WordPress/src/test/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailViewModelTest.kt new file mode 100644 index 000000000000..177d5cf7a7c4 --- /dev/null +++ b/WordPress/src/test/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailViewModelTest.kt @@ -0,0 +1,204 @@ +package org.wordpress.android.ui.newstats.mostviewed + +import kotlinx.coroutines.ExperimentalCoroutinesApi +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Test +import org.mockito.Mock +import org.mockito.kotlin.any +import org.mockito.kotlin.never +import org.mockito.kotlin.times +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import org.wordpress.android.BaseUnitTest +import org.wordpress.android.R +import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.fluxc.store.AccountStore +import org.wordpress.android.ui.mysite.SelectedSiteRepository +import org.wordpress.android.ui.newstats.StatsPeriod +import org.wordpress.android.ui.newstats.repository.MostViewedChildData +import org.wordpress.android.ui.newstats.repository.MostViewedItemData +import org.wordpress.android.ui.newstats.repository.MostViewedResult +import org.wordpress.android.ui.newstats.repository.StatsRepository +import org.wordpress.android.viewmodel.ResourceProvider + +@ExperimentalCoroutinesApi +class MostViewedDetailViewModelTest : BaseUnitTest() { + @Mock + private lateinit var selectedSiteRepository: SelectedSiteRepository + + @Mock + private lateinit var accountStore: AccountStore + + @Mock + private lateinit var statsRepository: StatsRepository + + @Mock + private lateinit var resourceProvider: ResourceProvider + + private lateinit var viewModel: MostViewedDetailViewModel + + private val testSite = SiteModel().apply { + id = 1 + siteId = TEST_SITE_ID + name = "Test Site" + } + + @Before + fun setUp() { + whenever(selectedSiteRepository.getSelectedSite()).thenReturn(testSite) + whenever(accountStore.accessToken).thenReturn(TEST_ACCESS_TOKEN) + whenever(resourceProvider.getString(R.string.stats_period_last_7_days)).thenReturn(DATE_RANGE) + whenever(resourceProvider.getString(R.string.stats_error_api)).thenReturn(API_ERROR) + whenever(resourceProvider.getString(R.string.stats_error_unknown)).thenReturn(UNKNOWN_ERROR) + viewModel = MostViewedDetailViewModel( + selectedSiteRepository, + accountStore, + statsRepository, + resourceProvider + ) + } + + @Test + fun `when loadReferrers succeeds, then loaded state with items and totals is emitted`() = test { + whenever(statsRepository.fetchReferrersDetail(any(), any())).thenReturn(createSuccessResult()) + + viewModel.loadReferrers(StatsPeriod.Last7Days) + advanceUntilIdle() + + val state = viewModel.uiState.value + assertThat(state).isInstanceOf(MostViewedDetailUiState.Loaded::class.java) + val loaded = state as MostViewedDetailUiState.Loaded + assertThat(loaded.items).hasSize(1) + assertThat(loaded.items[0].title).isEqualTo("Search Engines") + assertThat(loaded.maxViewsForBar).isEqualTo(TEST_VIEWS) + assertThat(loaded.totalViews).isEqualTo(TEST_TOTAL_VIEWS) + assertThat(loaded.totalViewsChange).isEqualTo(TEST_TOTAL_VIEWS_CHANGE) + assertThat(loaded.totalViewsChangePercent).isEqualTo(TEST_TOTAL_VIEWS_CHANGE_PERCENT) + assertThat(loaded.dateRange).isEqualTo(DATE_RANGE) + } + + @Test + fun `when loadReferrers succeeds, then child items propagate to the loaded state`() = test { + whenever(statsRepository.fetchReferrersDetail(any(), any())).thenReturn(createSuccessResult()) + + viewModel.loadReferrers(StatsPeriod.Last7Days) + advanceUntilIdle() + + val loaded = viewModel.uiState.value as MostViewedDetailUiState.Loaded + assertThat(loaded.items[0].children).hasSize(1) + assertThat(loaded.items[0].children[0].name).isEqualTo("Google Search") + assertThat(loaded.items[0].children[0].url).isEqualTo(CHILD_URL) + assertThat(loaded.items[0].children[0].views).isEqualTo(TEST_VIEWS) + } + + @Test + fun `when no site is selected, then error state is emitted and no fetch happens`() = test { + whenever(selectedSiteRepository.getSelectedSite()).thenReturn(null) + + viewModel.loadReferrers(StatsPeriod.Last7Days) + advanceUntilIdle() + + val state = viewModel.uiState.value + assertThat(state).isInstanceOf(MostViewedDetailUiState.Error::class.java) + assertThat((state as MostViewedDetailUiState.Error).message).isEqualTo(API_ERROR) + verify(statsRepository, never()).fetchReferrersDetail(any(), any()) + } + + @Test + fun `when access token is empty, then error state is emitted and no fetch happens`() = test { + whenever(accountStore.accessToken).thenReturn("") + + viewModel.loadReferrers(StatsPeriod.Last7Days) + advanceUntilIdle() + + val state = viewModel.uiState.value + assertThat(state).isInstanceOf(MostViewedDetailUiState.Error::class.java) + assertThat((state as MostViewedDetailUiState.Error).message).isEqualTo(API_ERROR) + verify(statsRepository, never()).fetchReferrersDetail(any(), any()) + } + + @Test + fun `when the repository returns an error, then the api error state is emitted`() = test { + whenever(statsRepository.fetchReferrersDetail(any(), any())) + .thenReturn(MostViewedResult.Error("Network error")) + + viewModel.loadReferrers(StatsPeriod.Last7Days) + advanceUntilIdle() + + val state = viewModel.uiState.value + assertThat(state).isInstanceOf(MostViewedDetailUiState.Error::class.java) + assertThat((state as MostViewedDetailUiState.Error).message).isEqualTo(API_ERROR) + } + + @Test + fun `when the fetch throws, then the error state carries the exception message`() = test { + whenever(statsRepository.fetchReferrersDetail(any(), any())) + .thenThrow(RuntimeException(EXCEPTION_MESSAGE)) + + viewModel.loadReferrers(StatsPeriod.Last7Days) + advanceUntilIdle() + + val state = viewModel.uiState.value + assertThat(state).isInstanceOf(MostViewedDetailUiState.Error::class.java) + assertThat((state as MostViewedDetailUiState.Error).message).isEqualTo(EXCEPTION_MESSAGE) + } + + @Test + fun `when loadReferrers is called twice, then the data is fetched only once`() = test { + whenever(statsRepository.fetchReferrersDetail(any(), any())).thenReturn(createSuccessResult()) + + viewModel.loadReferrers(StatsPeriod.Last7Days) + advanceUntilIdle() + viewModel.loadReferrers(StatsPeriod.Last7Days) + advanceUntilIdle() + + verify(statsRepository, times(1)).fetchReferrersDetail(any(), any()) + } + + @Test + fun `when retry is called after a load, then the data is fetched again`() = test { + whenever(statsRepository.fetchReferrersDetail(any(), any())).thenReturn(createSuccessResult()) + + viewModel.loadReferrers(StatsPeriod.Last7Days) + advanceUntilIdle() + viewModel.retry() + advanceUntilIdle() + + verify(statsRepository, times(2)).fetchReferrersDetail(any(), any()) + } + + private fun createSuccessResult() = MostViewedResult.Success( + items = listOf( + MostViewedItemData( + id = 1, + title = "Search Engines", + views = TEST_VIEWS, + previousViews = TEST_PREVIOUS_VIEWS, + isFirst = true, + children = listOf( + MostViewedChildData(name = "Google Search", url = CHILD_URL, views = TEST_VIEWS) + ) + ) + ), + totalViews = TEST_TOTAL_VIEWS, + totalViewsChange = TEST_TOTAL_VIEWS_CHANGE, + totalViewsChangePercent = TEST_TOTAL_VIEWS_CHANGE_PERCENT + ) + + companion object { + private const val TEST_SITE_ID = 123L + private const val TEST_ACCESS_TOKEN = "test_access_token" + private const val DATE_RANGE = "Last 7 days" + private const val API_ERROR = "Failed to load stats" + private const val UNKNOWN_ERROR = "Something went wrong" + private const val EXCEPTION_MESSAGE = "boom" + private const val CHILD_URL = "http://google.com/" + + private const val TEST_VIEWS = 23L + private const val TEST_PREVIOUS_VIEWS = 10L + private const val TEST_TOTAL_VIEWS = 23L + private const val TEST_TOTAL_VIEWS_CHANGE = 13L + private const val TEST_TOTAL_VIEWS_CHANGE_PERCENT = 130.0 + } +} From 649c80209bf16685c7cc342342d08d53b05c6424 Mon Sep 17 00:00:00 2001 From: adalpari Date: Fri, 3 Jul 2026 14:18:01 +0200 Subject: [PATCH 10/13] Show localized error and make card expand state rotation-safe The referrers detail ViewModel surfaced raw exception messages to users; log the exception and show the localized stats_error_unknown string instead, and rethrow CancellationException. Make the referrers card's expand/collapse state survive configuration changes by keying each row on its item id and holding the expanded flag in rememberSaveable, so an expanded group stays expanded after rotation (matching the detail screen) while still resetting when the list reloads for a different referrer. CMM-2133 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ui/newstats/mostviewed/MostViewedCard.kt | 15 ++++++++++----- .../mostviewed/MostViewedDetailViewModel.kt | 9 ++++++--- .../mostviewed/MostViewedDetailViewModelTest.kt | 4 ++-- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt index b08ee9ae541f..909c81692d2b 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt @@ -20,8 +20,9 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue +import androidx.compose.runtime.key import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.remember +import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -203,7 +204,11 @@ private fun LoadedContent( val percentage = if (state.maxViewsForBar > 0) { item.views.toFloat() / state.maxViewsForBar.toFloat() } else 0f - MostViewedItemRow(item = item, percentage = percentage, onChildClick = onChildClick) + // Key on the item id so the row's saved expanded state stays with its referrer + // (and resets to collapsed) when the list reloads after a period change. + key(item.id) { + MostViewedItemRow(item = item, percentage = percentage, onChildClick = onChildClick) + } if (index < state.items.lastIndex) { Spacer(modifier = Modifier.height(4.dp)) } @@ -294,9 +299,9 @@ private fun MostViewedItemRow( ) { val barColor = MaterialTheme.colorScheme.primary.copy(alpha = HIGHLIGHTED_ITEM_BACKGROUND_ALPHA) val hasChildren = item.children.isNotEmpty() - // Key the expanded state on the item id so it resets when the row is reused for a different - // referrer (e.g. after the period changes and the list reloads at the same position). - var expanded by remember(item.id) { mutableStateOf(false) } + // The caller wraps each row in key(item.id), so this saved state is scoped to (and resets with) + // the referrer, and survives configuration changes (e.g. rotation) via rememberSaveable. + var expanded by rememberSaveable { mutableStateOf(false) } Column { Box( diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailViewModel.kt index 8db136bd99d1..ca12429c3428 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailViewModel.kt @@ -5,6 +5,7 @@ import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.launch import org.wordpress.android.R @@ -14,6 +15,7 @@ import org.wordpress.android.ui.newstats.StatsPeriod import org.wordpress.android.ui.newstats.repository.MostViewedResult import org.wordpress.android.ui.newstats.repository.StatsRepository import org.wordpress.android.ui.newstats.util.toDateRangeString +import org.wordpress.android.util.AppLog import org.wordpress.android.viewmodel.ResourceProvider import javax.inject.Inject @@ -86,10 +88,11 @@ class MostViewedDetailViewModel @Inject constructor( resourceProvider.getString(R.string.stats_error_api) ) } + } catch (e: CancellationException) { + throw e } catch (e: Exception) { - MostViewedDetailUiState.Error( - e.message ?: resourceProvider.getString(R.string.stats_error_unknown) - ) + AppLog.e(AppLog.T.STATS, "Error fetching referrers detail: ${e.message}", e) + MostViewedDetailUiState.Error(resourceProvider.getString(R.string.stats_error_unknown)) } } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailViewModelTest.kt index 177d5cf7a7c4..532fab9e941e 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailViewModelTest.kt @@ -132,7 +132,7 @@ class MostViewedDetailViewModelTest : BaseUnitTest() { } @Test - fun `when the fetch throws, then the error state carries the exception message`() = test { + fun `when the fetch throws, then the generic unknown error state is emitted`() = test { whenever(statsRepository.fetchReferrersDetail(any(), any())) .thenThrow(RuntimeException(EXCEPTION_MESSAGE)) @@ -141,7 +141,7 @@ class MostViewedDetailViewModelTest : BaseUnitTest() { val state = viewModel.uiState.value assertThat(state).isInstanceOf(MostViewedDetailUiState.Error::class.java) - assertThat((state as MostViewedDetailUiState.Error).message).isEqualTo(EXCEPTION_MESSAGE) + assertThat((state as MostViewedDetailUiState.Error).message).isEqualTo(UNKNOWN_ERROR) } @Test From c32d1d54aabce53c8e7605e4d1e9f5824fbe6e78 Mon Sep 17 00:00:00 2001 From: adalpari Date: Fri, 3 Jul 2026 14:40:34 +0200 Subject: [PATCH 11/13] Split referrer tests into StatsRepositoryReferrersTest to fix LargeClass StatsRepositoryTest tripped detekt's LargeClass rule after the referrer tests were added. Extract them into a dedicated test class instead of baselining, keeping the referrer tests grouped in one place. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../StatsRepositoryReferrersTest.kt | 145 ++++++++++++++++++ .../repository/StatsRepositoryTest.kt | 102 ------------ 2 files changed, 145 insertions(+), 102 deletions(-) create mode 100644 WordPress/src/test/java/org/wordpress/android/ui/newstats/repository/StatsRepositoryReferrersTest.kt diff --git a/WordPress/src/test/java/org/wordpress/android/ui/newstats/repository/StatsRepositoryReferrersTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/newstats/repository/StatsRepositoryReferrersTest.kt new file mode 100644 index 000000000000..55784967b85e --- /dev/null +++ b/WordPress/src/test/java/org/wordpress/android/ui/newstats/repository/StatsRepositoryReferrersTest.kt @@ -0,0 +1,145 @@ +package org.wordpress.android.ui.newstats.repository + +import kotlinx.coroutines.ExperimentalCoroutinesApi +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Test +import org.mockito.Mock +import org.mockito.kotlin.any +import org.mockito.kotlin.eq +import org.mockito.kotlin.times +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import org.wordpress.android.BaseUnitTest +import org.wordpress.android.fluxc.utils.AppLogWrapper +import org.wordpress.android.ui.newstats.StatsPeriod +import org.wordpress.android.ui.newstats.datasource.ReferrerChildDataItem +import org.wordpress.android.ui.newstats.datasource.ReferrerDataItem +import org.wordpress.android.ui.newstats.datasource.ReferrersDataResult +import org.wordpress.android.ui.newstats.datasource.StatsDataSource +import org.wordpress.android.ui.newstats.datasource.StatsErrorType +import org.wordpress.android.ui.newstats.mostviewed.MostViewedDataSource + +@ExperimentalCoroutinesApi +class StatsRepositoryReferrersTest : BaseUnitTest() { + @Mock + private lateinit var statsDataSource: StatsDataSource + + @Mock + private lateinit var appLogWrapper: AppLogWrapper + + private lateinit var repository: StatsRepository + + @Before + fun setUp() { + repository = StatsRepository( + statsDataSource = statsDataSource, + appLogWrapper = appLogWrapper, + ioDispatcher = testDispatcher() + ) + } + + @Test + fun `given successful response, when fetchMostViewed with REFERRERS, then success result is returned`() = test { + whenever(statsDataSource.fetchReferrers(any(), any(), any())) + .thenReturn(ReferrersDataResult.Success(createReferrersData())) + + val result = repository.fetchMostViewed( + TEST_SITE_ID, + StatsPeriod.Last7Days, + MostViewedDataSource.REFERRERS + ) + + assertThat(result).isInstanceOf(MostViewedResult.Success::class.java) + val success = result as MostViewedResult.Success + assertThat(success.items).hasSize(2) + assertThat(success.items[0].title).isEqualTo(TEST_REFERRER_NAME_1) + assertThat(success.items[0].views).isEqualTo(TEST_REFERRER_VIEWS_1) + assertThat(success.items[1].title).isEqualTo(TEST_REFERRER_NAME_2) + assertThat(success.items[1].views).isEqualTo(TEST_REFERRER_VIEWS_2) + } + + @Test + fun `given referrer group with children, when fetchMostViewed with REFERRERS, then children propagate`() = test { + val referrersData = listOf( + ReferrerDataItem( + name = "Search Engines", + views = 23, + children = listOf( + ReferrerChildDataItem(name = "Google Search", url = "http://google.com/", views = 23) + ) + ) + ) + whenever(statsDataSource.fetchReferrers(any(), any(), any())) + .thenReturn(ReferrersDataResult.Success(referrersData)) + + val result = repository.fetchMostViewed( + TEST_SITE_ID, + StatsPeriod.Last7Days, + MostViewedDataSource.REFERRERS + ) + + assertThat(result).isInstanceOf(MostViewedResult.Success::class.java) + val success = result as MostViewedResult.Success + assertThat(success.items[0].children).hasSize(1) + assertThat(success.items[0].children[0].name).isEqualTo("Google Search") + assertThat(success.items[0].children[0].url).isEqualTo("http://google.com/") + assertThat(success.items[0].children[0].views).isEqualTo(23) + } + + @Test + fun `when fetchMostViewed with REFERRERS, then card-sized max is requested`() = test { + whenever(statsDataSource.fetchReferrers(any(), any(), any())) + .thenReturn(ReferrersDataResult.Success(createReferrersData())) + + repository.fetchMostViewed(TEST_SITE_ID, StatsPeriod.Last7Days, MostViewedDataSource.REFERRERS) + + // Card request is bounded (current + previous period). + verify(statsDataSource, times(2)).fetchReferrers(any(), any(), eq(REFERRERS_CARD_MAX)) + } + + @Test + fun `when fetchReferrersDetail, then unlimited max is requested`() = test { + whenever(statsDataSource.fetchReferrers(any(), any(), any())) + .thenReturn(ReferrersDataResult.Success(createReferrersData())) + + repository.fetchReferrersDetail(TEST_SITE_ID, StatsPeriod.Last7Days) + + // Detail request asks for all referrers (max = 0), for current + previous period. + verify(statsDataSource, times(2)).fetchReferrers(any(), any(), eq(REFERRERS_DETAIL_MAX)) + } + + @Test + fun `given error response, when fetchMostViewed with REFERRERS, then error result is returned`() = test { + whenever(statsDataSource.fetchReferrers(any(), any(), any())) + .thenReturn(ReferrersDataResult.Error(TEST_ERROR_TYPE)) + + val result = repository.fetchMostViewed( + TEST_SITE_ID, + StatsPeriod.Last7Days, + MostViewedDataSource.REFERRERS + ) + + assertThat(result).isInstanceOf(MostViewedResult.Error::class.java) + assertThat((result as MostViewedResult.Error).message).isEqualTo(TEST_ERROR_TYPE.name) + } + + private fun createReferrersData() = listOf( + ReferrerDataItem(name = TEST_REFERRER_NAME_1, views = TEST_REFERRER_VIEWS_1), + ReferrerDataItem(name = TEST_REFERRER_NAME_2, views = TEST_REFERRER_VIEWS_2) + ) + + companion object { + private const val TEST_SITE_ID = 123L + private val TEST_ERROR_TYPE = StatsErrorType.NETWORK_ERROR + + // Mirrors StatsRepository's private referrer max values (card vs. detail). + private const val REFERRERS_CARD_MAX = 10 + private const val REFERRERS_DETAIL_MAX = 0 + + private const val TEST_REFERRER_NAME_1 = "google.com" + private const val TEST_REFERRER_NAME_2 = "twitter.com" + private const val TEST_REFERRER_VIEWS_1 = 200L + private const val TEST_REFERRER_VIEWS_2 = 150L + } +} diff --git a/WordPress/src/test/java/org/wordpress/android/ui/newstats/repository/StatsRepositoryTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/newstats/repository/StatsRepositoryTest.kt index 72776a094158..f0ad9af260b8 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/newstats/repository/StatsRepositoryTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/newstats/repository/StatsRepositoryTest.kt @@ -5,9 +5,6 @@ import org.wordpress.android.ui.newstats.StatsPeriod import org.wordpress.android.ui.newstats.datasource.CommentsDataPoint import org.wordpress.android.ui.newstats.datasource.LikesDataPoint import org.wordpress.android.ui.newstats.datasource.PostsDataPoint -import org.wordpress.android.ui.newstats.datasource.ReferrerChildDataItem -import org.wordpress.android.ui.newstats.datasource.ReferrerDataItem -import org.wordpress.android.ui.newstats.datasource.ReferrersDataResult import org.wordpress.android.ui.newstats.datasource.StatsDataSource import org.wordpress.android.ui.newstats.datasource.StatsErrorType import org.wordpress.android.ui.newstats.datasource.StatsUnit @@ -547,76 +544,6 @@ class StatsRepositoryTest : BaseUnitTest() { assertThat(success.items[1].views).isEqualTo(TEST_POST_VIEWS_2) } - @Test - fun `given successful response, when fetchMostViewed with REFERRERS, then success result is returned`() = test { - whenever(statsDataSource.fetchReferrers(any(), any(), any())) - .thenReturn(ReferrersDataResult.Success(createReferrersData())) - - val result = repository.fetchMostViewed( - TEST_SITE_ID, - StatsPeriod.Last7Days, - MostViewedDataSource.REFERRERS - ) - - assertThat(result).isInstanceOf(MostViewedResult.Success::class.java) - val success = result as MostViewedResult.Success - assertThat(success.items).hasSize(2) - assertThat(success.items[0].title).isEqualTo(TEST_REFERRER_NAME_1) - assertThat(success.items[0].views).isEqualTo(TEST_REFERRER_VIEWS_1) - assertThat(success.items[1].title).isEqualTo(TEST_REFERRER_NAME_2) - assertThat(success.items[1].views).isEqualTo(TEST_REFERRER_VIEWS_2) - } - - @Test - fun `given referrer group with children, when fetchMostViewed with REFERRERS, then children propagate`() = test { - val referrersData = listOf( - ReferrerDataItem( - name = "Search Engines", - views = 23, - children = listOf( - ReferrerChildDataItem(name = "Google Search", url = "http://google.com/", views = 23) - ) - ) - ) - whenever(statsDataSource.fetchReferrers(any(), any(), any())) - .thenReturn(ReferrersDataResult.Success(referrersData)) - - val result = repository.fetchMostViewed( - TEST_SITE_ID, - StatsPeriod.Last7Days, - MostViewedDataSource.REFERRERS - ) - - assertThat(result).isInstanceOf(MostViewedResult.Success::class.java) - val success = result as MostViewedResult.Success - assertThat(success.items[0].children).hasSize(1) - assertThat(success.items[0].children[0].name).isEqualTo("Google Search") - assertThat(success.items[0].children[0].url).isEqualTo("http://google.com/") - assertThat(success.items[0].children[0].views).isEqualTo(23) - } - - @Test - fun `when fetchMostViewed with REFERRERS, then card-sized max is requested`() = test { - whenever(statsDataSource.fetchReferrers(any(), any(), any())) - .thenReturn(ReferrersDataResult.Success(createReferrersData())) - - repository.fetchMostViewed(TEST_SITE_ID, StatsPeriod.Last7Days, MostViewedDataSource.REFERRERS) - - // Card request is bounded (current + previous period). - verify(statsDataSource, times(2)).fetchReferrers(any(), any(), eq(REFERRERS_CARD_MAX)) - } - - @Test - fun `when fetchReferrersDetail, then unlimited max is requested`() = test { - whenever(statsDataSource.fetchReferrers(any(), any(), any())) - .thenReturn(ReferrersDataResult.Success(createReferrersData())) - - repository.fetchReferrersDetail(TEST_SITE_ID, StatsPeriod.Last7Days) - - // Detail request asks for all referrers (max = 0), for current + previous period. - verify(statsDataSource, times(2)).fetchReferrers(any(), any(), eq(REFERRERS_DETAIL_MAX)) - } - @Test fun `given successful response, when fetchMostViewed, then totalViews is calculated correctly`() = test { whenever(statsDataSource.fetchTopPostsAndPages(any(), any(), any())) @@ -677,21 +604,6 @@ class StatsRepositoryTest : BaseUnitTest() { assertThat((result as MostViewedResult.Error).message).isEqualTo(TEST_ERROR_TYPE.name) } - @Test - fun `given error response, when fetchMostViewed with REFERRERS, then error result is returned`() = test { - whenever(statsDataSource.fetchReferrers(any(), any(), any())) - .thenReturn(ReferrersDataResult.Error(TEST_ERROR_TYPE)) - - val result = repository.fetchMostViewed( - TEST_SITE_ID, - StatsPeriod.Last7Days, - MostViewedDataSource.REFERRERS - ) - - assertThat(result).isInstanceOf(MostViewedResult.Error::class.java) - assertThat((result as MostViewedResult.Error).message).isEqualTo(TEST_ERROR_TYPE.name) - } - @Test fun `given item in both periods, when fetchMostViewed, then viewsChange is calculated for item`() = test { val currentData = listOf( @@ -800,11 +712,6 @@ class StatsRepositoryTest : BaseUnitTest() { TopPostDataItem(id = 1, title = TEST_POST_TITLE_1, views = TEST_POST_VIEWS_1), TopPostDataItem(id = 2, title = TEST_POST_TITLE_2, views = TEST_POST_VIEWS_2) ) - - private fun createReferrersData() = listOf( - ReferrerDataItem(name = TEST_REFERRER_NAME_1, views = TEST_REFERRER_VIEWS_1), - ReferrerDataItem(name = TEST_REFERRER_NAME_2, views = TEST_REFERRER_VIEWS_2) - ) // endregion companion object { @@ -812,10 +719,6 @@ class StatsRepositoryTest : BaseUnitTest() { private const val TEST_ACCESS_TOKEN = "test_access_token" private val TEST_ERROR_TYPE = StatsErrorType.NETWORK_ERROR - // Mirrors StatsRepository's private referrer max values (card vs. detail). - private const val REFERRERS_CARD_MAX = 10 - private const val REFERRERS_DETAIL_MAX = 0 - private const val TEST_PERIOD_1 = "2024-01-15" private const val TEST_PERIOD_2 = "2024-01-16" @@ -840,10 +743,5 @@ class StatsRepositoryTest : BaseUnitTest() { private const val TEST_POST_TITLE_2 = "Test Post 2" private const val TEST_POST_VIEWS_1 = 500L private const val TEST_POST_VIEWS_2 = 300L - - private const val TEST_REFERRER_NAME_1 = "google.com" - private const val TEST_REFERRER_NAME_2 = "twitter.com" - private const val TEST_REFERRER_VIEWS_1 = 200L - private const val TEST_REFERRER_VIEWS_2 = 150L } } From 7765b0571d08deba392a9f9c93c445a3ca366c7c Mon Sep 17 00:00:00 2001 From: adalpari Date: Fri, 3 Jul 2026 16:32:40 +0200 Subject: [PATCH 12/13] Key referrer card rows by index to avoid hashCode collisions Referrer ids are derived from name.hashCode(), which can collide on empty/duplicate names. Two colliding rows sharing a key() could misattribute the per-row rememberSaveable expanded state. Add the index to the key to guarantee uniqueness while keeping the item id so the saved state still resets when the referrer at a position changes after a period reload. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../android/ui/newstats/mostviewed/MostViewedCard.kt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt index 909c81692d2b..21de375c50a2 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt @@ -204,9 +204,11 @@ private fun LoadedContent( val percentage = if (state.maxViewsForBar > 0) { item.views.toFloat() / state.maxViewsForBar.toFloat() } else 0f - // Key on the item id so the row's saved expanded state stays with its referrer - // (and resets to collapsed) when the list reloads after a period change. - key(item.id) { + // Key on the index plus the item id: the index guarantees uniqueness (referrer ids + // are derived from name.hashCode() and can collide on empty/duplicate names), while + // the id resets the row's saved expanded state when a different referrer lands at + // this position after a period reload. rememberSaveable keeps it across rotation. + key(index, item.id) { MostViewedItemRow(item = item, percentage = percentage, onChildClick = onChildClick) } if (index < state.items.lastIndex) { From c9097379e054fcc8f7c256dca4750e3264fda604 Mon Sep 17 00:00:00 2001 From: adalpari Date: Fri, 3 Jul 2026 16:43:47 +0200 Subject: [PATCH 13/13] Extract shared MostViewedExpandableRow and use OpenInNew for child links The card (MostViewedItemRow) and detail screen (DetailItemRow) each carried a near-identical ~80-line expandable-row body that differed only by the leading position number and the child indent. Consolidate them into a single MostViewedExpandableRow in MostViewedCommonComposables.kt, built on StatsListRowContainer and parameterized with position/child indent (mirroring the UtmExpandableRow pattern), so future changes to expand/collapse behavior live in one place. Also swap the clickable child row's ChevronRight for the standard AutoMirrored OpenInNew icon (with an "Open link" content description) to make it clearer the row opens an external URL. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ui/newstats/mostviewed/MostViewedCard.kt | 95 ++------------- .../mostviewed/MostViewedCommonComposables.kt | 97 +++++++++++++++- .../mostviewed/MostViewedDetailActivity.kt | 108 ++---------------- WordPress/src/main/res/values/strings.xml | 1 + 4 files changed, 114 insertions(+), 187 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt index 21de375c50a2..42f16b1f3682 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCard.kt @@ -2,14 +2,11 @@ package org.wordpress.android.ui.newstats.mostviewed import androidx.compose.foundation.background import androidx.compose.foundation.border -import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.IntrinsicSize import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer -import androidx.compose.foundation.layout.fillMaxHeight import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding @@ -21,15 +18,12 @@ import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.runtime.key -import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.font.FontWeight -import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp import org.wordpress.android.R @@ -40,13 +34,11 @@ import org.wordpress.android.ui.newstats.components.CardPosition import org.wordpress.android.ui.newstats.components.ShowAllFooter import org.wordpress.android.ui.newstats.components.StatsCardMenu import org.wordpress.android.ui.newstats.util.ShimmerBox -import org.wordpress.android.ui.newstats.util.formatStatValue import java.util.Locale private val CardCornerRadius = 10.dp private val CardPadding = 16.dp private val CardMargin = 16.dp -internal const val HIGHLIGHTED_ITEM_BACKGROUND_ALPHA = 0.08f private const val LOADING_SHIMMER_ITEM_COUNT = 5 @Composable @@ -209,7 +201,14 @@ private fun LoadedContent( // the id resets the row's saved expanded state when a different referrer lands at // this position after a period reload. rememberSaveable keeps it across rotation. key(index, item.id) { - MostViewedItemRow(item = item, percentage = percentage, onChildClick = onChildClick) + MostViewedExpandableRow( + title = item.title, + views = item.views, + change = item.change, + children = item.children, + percentage = percentage, + onChildClick = onChildClick + ) } if (index < state.items.lastIndex) { Spacer(modifier = Modifier.height(4.dp)) @@ -293,84 +292,6 @@ private fun ColumnHeadersRow(cardType: StatsCardType) { } } -@Composable -private fun MostViewedItemRow( - item: MostViewedItem, - percentage: Float, - onChildClick: (String) -> Unit -) { - val barColor = MaterialTheme.colorScheme.primary.copy(alpha = HIGHLIGHTED_ITEM_BACKGROUND_ALPHA) - val hasChildren = item.children.isNotEmpty() - // The caller wraps each row in key(item.id), so this saved state is scoped to (and resets with) - // the referrer, and survives configuration changes (e.g. rotation) via rememberSaveable. - var expanded by rememberSaveable { mutableStateOf(false) } - - Column { - Box( - modifier = Modifier - .fillMaxWidth() - .height(IntrinsicSize.Min) - .clip(RoundedCornerShape(8.dp)) - .then( - if (hasChildren) Modifier.clickable { expanded = !expanded } else Modifier - ) - ) { - // Background bar representing the percentage - Box( - modifier = Modifier - .fillMaxWidth(fraction = percentage) - .fillMaxHeight() - .background(barColor) - ) - - // Content - Row( - modifier = Modifier - .fillMaxWidth() - .padding(vertical = 12.dp, horizontal = 16.dp), - horizontalArrangement = Arrangement.SpaceBetween, - verticalAlignment = Alignment.CenterVertically - ) { - Text( - text = item.title, - style = MaterialTheme.typography.bodyLarge, - color = MaterialTheme.colorScheme.onSurface, - maxLines = 2, - overflow = TextOverflow.Ellipsis, - modifier = Modifier.weight(1f) - ) - - Spacer(modifier = Modifier.width(16.dp)) - - Row(verticalAlignment = Alignment.CenterVertically) { - Column(horizontalAlignment = Alignment.End) { - Row(verticalAlignment = Alignment.CenterVertically) { - if (hasChildren) { - MostViewedExpandChevron(expanded = expanded) - Spacer(modifier = Modifier.width(4.dp)) - } - Text( - text = formatStatValue(item.views), - style = MaterialTheme.typography.bodyLarge, - fontWeight = FontWeight.SemiBold, - color = MaterialTheme.colorScheme.onSurface - ) - } - ChangeIndicator(change = item.change) - } - } - } - } - - MostViewedExpandableChildren( - children = item.children, - expanded = expanded, - startPadding = 24.dp, - onChildClick = onChildClick - ) - } -} - @Composable internal fun ChangeIndicator(change: MostViewedChange) { val (text, color) = when (change) { diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCommonComposables.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCommonComposables.kt index 8eb9c2e01762..0d8a67786e14 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCommonComposables.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedCommonComposables.kt @@ -11,16 +11,21 @@ import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.filled.ChevronRight +import androidx.compose.material.icons.automirrored.filled.OpenInNew import androidx.compose.material.icons.filled.KeyboardArrowDown import androidx.compose.material.icons.filled.KeyboardArrowUp import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.saveable.rememberSaveable +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp @@ -28,6 +33,92 @@ import org.wordpress.android.R import org.wordpress.android.ui.newstats.components.StatsListRowContainer import org.wordpress.android.ui.newstats.util.formatStatValue +/** + * Expandable Most Viewed row (group title, views, change badge, and an optional expand/collapse + * chevron that reveals indented child rows). Shared by the card ([MostViewedCard]) and the detail + * screen ([MostViewedDetailActivity]); they differ only by the leading [position] number (detail + * only) and the child [childStartPadding] indent. + * + * The caller is expected to wrap this row in a `key(...)` so the [rememberSaveable] expanded state + * is scoped to the item and survives configuration changes (e.g. rotation). + * + * @param title The group title + * @param views The group's view count + * @param change The change badge shown under the views + * @param children The child rows revealed when expanded (empty = no chevron, not expandable) + * @param percentage Bar fill percentage (0f..1f) + * @param onChildClick Invoked with a child's URL when a linkable child row is tapped + * @param position Optional 1-based position number shown on the detail screen + * @param childStartPadding Indent applied to the expanded child rows + */ +@Composable +internal fun MostViewedExpandableRow( + title: String, + views: Long, + change: MostViewedChange, + children: List, + percentage: Float, + onChildClick: (String) -> Unit, + position: Int? = null, + childStartPadding: Dp = 24.dp +) { + val hasChildren = children.isNotEmpty() + var expanded by rememberSaveable { mutableStateOf(false) } + val clickModifier = if (hasChildren) Modifier.clickable { expanded = !expanded } else Modifier + + Column { + StatsListRowContainer(percentage = percentage, modifier = clickModifier) { + Row( + modifier = Modifier + .fillMaxWidth() + .padding(vertical = 12.dp, horizontal = 16.dp), + verticalAlignment = Alignment.CenterVertically + ) { + if (position != null) { + Text( + text = position.toString(), + style = MaterialTheme.typography.bodyLarge, + fontWeight = FontWeight.SemiBold, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.width(32.dp) + ) + } + Text( + text = title, + style = MaterialTheme.typography.bodyLarge, + color = MaterialTheme.colorScheme.onSurface, + maxLines = 2, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.weight(1f) + ) + Spacer(modifier = Modifier.width(16.dp)) + Column(horizontalAlignment = Alignment.End) { + Row(verticalAlignment = Alignment.CenterVertically) { + if (hasChildren) { + MostViewedExpandChevron(expanded = expanded) + Spacer(modifier = Modifier.width(4.dp)) + } + Text( + text = formatStatValue(views), + style = MaterialTheme.typography.bodyLarge, + fontWeight = FontWeight.SemiBold, + color = MaterialTheme.colorScheme.onSurface + ) + } + ChangeIndicator(change = change) + } + } + } + + MostViewedExpandableChildren( + children = children, + expanded = expanded, + startPadding = childStartPadding, + onChildClick = onChildClick + ) + } +} + /** * Expand/collapse chevron shown on a Most Viewed row that has children. * Shared by the card ([MostViewedCard]) and the detail screen ([MostViewedDetailActivity]). @@ -105,8 +196,8 @@ private fun MostViewedChildRow( if (isClickable) { Spacer(modifier = Modifier.width(4.dp)) Icon( - imageVector = Icons.Default.ChevronRight, - contentDescription = null, + imageVector = Icons.AutoMirrored.Filled.OpenInNew, + contentDescription = stringResource(R.string.stats_open_referrer_link), modifier = Modifier.size(16.dp), tint = MaterialTheme.colorScheme.onSurfaceVariant ) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt index 20c2f04cb07d..f38e59098cca 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/newstats/mostviewed/MostViewedDetailActivity.kt @@ -5,23 +5,17 @@ import android.content.Intent import android.os.Bundle import androidx.activity.compose.setContent import androidx.activity.viewModels -import androidx.compose.foundation.background -import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.IntrinsicSize import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer -import androidx.compose.foundation.layout.fillMaxHeight import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.layout.width import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.itemsIndexed -import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.ArrowBack import androidx.compose.material3.Button @@ -36,16 +30,11 @@ import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember -import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier -import androidx.compose.ui.draw.clip import androidx.compose.ui.res.stringResource -import androidx.compose.ui.text.font.FontWeight -import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp import dagger.hilt.android.AndroidEntryPoint @@ -56,7 +45,6 @@ import org.wordpress.android.ui.main.BaseAppCompatActivity import org.wordpress.android.ui.newstats.StatsCardType import org.wordpress.android.ui.newstats.StatsPeriod import org.wordpress.android.ui.newstats.components.StatsSummaryCard -import org.wordpress.android.ui.newstats.util.formatStatValue import org.wordpress.android.util.extensions.getParcelableArrayListCompat import org.wordpress.android.util.extensions.getSerializableCompat import javax.inject.Inject @@ -266,11 +254,18 @@ private fun DetailLoadedContent( // which is not guaranteed unique (empty/duplicate names collide) and would crash the // LazyColumn. The list is a static snapshot, so the index is a stable, unique key. itemsIndexed(state.items, key = { index, _ -> index }) { index, item -> - DetailItemRow( + val percentage = if (state.maxViewsForBar > 0) { + item.views.toFloat() / state.maxViewsForBar.toFloat() + } else 0f + MostViewedExpandableRow( + title = item.title, + views = item.views, + change = item.change, + children = item.children, + percentage = percentage, + onChildClick = onChildClick, position = index + 1, - item = item, - maxViewsForBar = state.maxViewsForBar, - onChildClick = onChildClick + childStartPadding = 32.dp ) if (index < state.items.lastIndex) { Spacer(modifier = Modifier.height(4.dp)) @@ -337,87 +332,6 @@ private fun ColumnHeaders( } } -@Composable -private fun DetailItemRow( - position: Int, - item: MostViewedDetailItem, - maxViewsForBar: Long, - onChildClick: (String) -> Unit -) { - val percentage = if (maxViewsForBar > 0) item.views.toFloat() / maxViewsForBar.toFloat() else 0f - val barColor = MaterialTheme.colorScheme.primary.copy( - alpha = HIGHLIGHTED_ITEM_BACKGROUND_ALPHA - ) - val hasChildren = item.children.isNotEmpty() - var expanded by rememberSaveable { mutableStateOf(false) } - - Column { - Box( - modifier = Modifier - .fillMaxWidth() - .height(IntrinsicSize.Min) - .clip(RoundedCornerShape(8.dp)) - .then( - if (hasChildren) Modifier.clickable { expanded = !expanded } else Modifier - ) - ) { - Box( - modifier = Modifier - .fillMaxWidth(fraction = percentage) - .fillMaxHeight() - .background(barColor) - ) - - Row( - modifier = Modifier - .fillMaxWidth() - .padding(vertical = 12.dp, horizontal = 16.dp), - verticalAlignment = Alignment.CenterVertically - ) { - Text( - text = position.toString(), - style = MaterialTheme.typography.bodyLarge, - fontWeight = FontWeight.SemiBold, - color = MaterialTheme.colorScheme.onSurfaceVariant, - modifier = Modifier.width(32.dp) - ) - - Text( - text = item.title, - style = MaterialTheme.typography.bodyLarge, - maxLines = 2, - overflow = TextOverflow.Ellipsis, - modifier = Modifier.weight(1f) - ) - - Spacer(modifier = Modifier.width(8.dp)) - - Column(horizontalAlignment = Alignment.End) { - Row(verticalAlignment = Alignment.CenterVertically) { - if (hasChildren) { - MostViewedExpandChevron(expanded = expanded) - Spacer(modifier = Modifier.width(4.dp)) - } - Text( - text = formatStatValue(item.views), - style = MaterialTheme.typography.bodyLarge, - fontWeight = FontWeight.SemiBold - ) - } - ChangeIndicator(change = item.change) - } - } - } - - MostViewedExpandableChildren( - children = item.children, - expanded = expanded, - startPadding = 32.dp, - onChildClick = onChildClick - ) - } -} - @Preview(showBackground = true) @Composable private fun MostViewedDetailScreenPreview() { diff --git a/WordPress/src/main/res/values/strings.xml b/WordPress/src/main/res/values/strings.xml index 2cb702d1ac13..8993c5f7bf39 100644 --- a/WordPress/src/main/res/values/strings.xml +++ b/WordPress/src/main/res/values/strings.xml @@ -1586,6 +1586,7 @@ Tag Expand Collapse + Open link Check back when you\'ve published your first post! It\'s been %1$s since %2$s was published. Get the ball rolling and increase your post views by sharing your post: It’s been %1$s since %2$s was published. Here’s how the post performed so far: