New Stats: Improve Top List screen (#24868)

kean · web-flow · commit 3bf316da7af2 · 2025-09-25T09:50:35.000-04:00
* Show top 50 items in Top List

* Show top 10 in a separate list

* Show top 10

* Hardcode separate l10n values for top 10 and top 50
diff --git a/Modules/Sources/JetpackStats/Screens/TopListScreenView.swift b/Modules/Sources/JetpackStats/Screens/TopListScreenView.swift
@@ -1,9 +1,11 @@
 import SwiftUI
 import DesignSystem
 import UniformTypeIdentifiers
+import WordPressUI
 
 struct TopListScreenView: View {
     @StateObject private var viewModel: TopListViewModel
+    @State private var isShowingAllItems = false
 
     @Environment(\.router) var router
     @Environment(\.context) var context
@@ -33,32 +35,40 @@ struct TopListScreenView: View {
     }
 
     var body: some View {
-        ScrollView {
-            VStack(spacing: Constants.step4) {
+        List {
+            Group {
                 headerView
                     .background(Color(.secondarySystemBackground).opacity(0.7))
                     .cardStyle()
                     .dynamicTypeSize(...DynamicTypeSize.xLarge)
                     .accessibilityElement(children: .contain)
                     .padding(.horizontal, Constants.step1)
+                    .padding(.top, Constants.step1)
 
-                VStack {
-                    listHeaderView
-                        .padding(.horizontal, Constants.step1)
-                        .dynamicTypeSize(...DynamicTypeSize.xLarge)
-                    listContentView
-                        .grayscale(viewModel.isStale ? 1 : 0)
-                        .animation(.smooth, value: viewModel.isStale)
+                Group {
+                    if viewModel.isFirstLoad {
+                        listContent(data: mockData())
+                            .redacted(reason: .placeholder)
+                            .pulsating()
+                    } else if let data = viewModel.data {
+                        if data.items.isEmpty {
+                            makeEmptyStateView(message: Strings.Chart.empty)
+                        } else {
+                            listContent(data: data)
+                        }
+                    } else {
+                        makeEmptyStateView(message: viewModel.loadingError?.localizedDescription ?? Strings.Errors.generic)
+                    }
                 }
                 .padding(.horizontal, Constants.cardHorizontalInset(for: horizontalSizeClass))
             }
+            .listRowSeparator(.hidden)
+            .listRowInsets(.zero)
             .dynamicTypeSize(...DynamicTypeSize.xxxLarge)
-            .padding(.vertical, Constants.step2)
-            .frame(maxWidth: horizontalSizeClass == .regular ? Constants.maxHortizontalWidthPlainLists : .infinity)
-            .frame(maxWidth: .infinity)
-            .animation(.spring, value: viewModel.data.map(ObjectIdentifier.init))
         }
-        .background(Color(.systemBackground))
+        .animation(.default, value: viewModel.data.map(ObjectIdentifier.init))
+        .listStyle(.plain)
+        .environment(\.defaultMinListRowHeight, 1)
         .navigationTitle(viewModel.selection.item.localizedTitle)
         .navigationBarTitleDisplayMode(.inline)
         .toolbar {
@@ -106,9 +116,9 @@ struct TopListScreenView: View {
     @ViewBuilder
     private var headerView: some View {
         HStack(alignment: .center, spacing: Constants.step1) {
-            VStack(alignment: .leading, spacing: 4) {
-                Text(viewModel.selection.item.getTitle(for: viewModel.selection.metric))
-                    .font(.headline)
+            VStack(alignment: .leading, spacing: 2) {
+                Text(viewModel.selection.metric.localizedTitle)
+                    .font(.title3.weight(.medium))
                     .foregroundColor(.primary)
                 Text(context.formatters.dateRange.string(from: viewModel.dateRange.dateInterval))
                     .font(.subheadline)
@@ -118,7 +128,7 @@ struct TopListScreenView: View {
             Spacer()
 
             // Always show the metrics view to preserve identity
-            metricsOverviewView(data: viewModel.data ?? mockData)
+            metricsOverviewView(data: viewModel.data ?? mockData())
                 .redacted(reason: viewModel.isFirstLoad ? .placeholder : [])
                 .pulsating(viewModel.isFirstLoad)
                 .animation(.smooth, value: viewModel.isFirstLoad)
@@ -127,20 +137,6 @@ struct TopListScreenView: View {
         .padding(.horizontal, Constants.step3)
     }
 
-    private var listHeaderView: some View {
-        HStack {
-            Text(viewModel.selection.item.localizedTitle)
-                .font(.subheadline)
-                .fontWeight(.medium)
-
-            Spacer()
-
-            Text(viewModel.selection.metric.localizedTitle)
-                .font(.subheadline)
-                .fontWeight(.medium)
-        }
-    }
-
     @ViewBuilder
     private func metricsOverviewView(data: TopListData) -> some View {
         let formattedValue = StatsValueFormatter(metric: data.metric)
@@ -166,53 +162,140 @@ struct TopListScreenView: View {
         }
     }
 
+    // MARK: - Lists
+
+    enum ListSection {
+        case top10
+        case top50
+        case other
+
+        var title: String {
+            switch self {
+            case .top10: Strings.TopListTitles.top10
+            case .top50: Strings.TopListTitles.top50
+            case .other: ""
+            }
+        }
+    }
+
     @ViewBuilder
-    private var listContentView: some View {
-        if viewModel.isFirstLoad {
-            itemsListView(data: mockData)
-                .redacted(reason: .placeholder)
-                .pulsating()
-        } else if let data = viewModel.data {
-            if data.items.isEmpty {
-                makeEmptyStateView(message: Strings.Chart.empty)
+    private func listContent(data: TopListData) -> some View {
+        if data.items.count > 0 {
+            listSection(.top10, data: data)
+        }
+        if data.items.count > 10 {
+            listSection(.top50, data: data)
+        }
+        if data.items.count > 50 {
+            if isShowingAllItems {
+                listSection(.other, data: data)
             } else {
-                itemsListView(data: data)
+                showMoreButton
             }
+        }
+    }
+
+    @ViewBuilder
+    private func listSection(_ section: ListSection, data: TopListData) -> some View {
+        if section == .other {
+            VStack(spacing: 0) {
+                Spacer().frame(height: 20)
+                Divider()
+                Spacer().frame(height: 20)
+            }
+            .padding(.horizontal, Constants.step1)
         } else {
-            makeEmptyStateView(message: viewModel.loadingError?.localizedDescription ?? Strings.Errors.generic)
+            listHeaderView(title: section.title)
+                .padding(EdgeInsets(top: Constants.step3, leading: Constants.step1, bottom: Constants.step0_5, trailing: Constants.step1))
+                .dynamicTypeSize(...DynamicTypeSize.xLarge)
+        }
+        listForEach(for: section, data: data)
+            .grayscale(viewModel.isStale ? 1 : 0)
+            .animation(.smooth, value: viewModel.isStale)
+    }
+
+    private func listHeaderView(title: String) -> some View {
+        HStack {
+            Text(title)
+                .font(.subheadline)
+                .fontWeight(.medium)
+
+            Spacer()
+
+            Text(viewModel.selection.metric.localizedTitle)
+                .font(.subheadline)
+                .fontWeight(.medium)
         }
     }
 
-    private func itemsListView(data: TopListData) -> some View {
-        VStack(spacing: Constants.step0_5) {
-            ForEach(data.items, id: \.id) { item in
-                TopListItemView(
-                    item: item,
-                    previousValue: data.previousItem(for: item)?.metrics[viewModel.selection.metric],
-                    metric: viewModel.selection.metric,
-                    maxValue: data.metrics.maxValue,
-                    dateRange: viewModel.dateRange
-                )
-                .frame(height: TopListItemView.defaultCellHeight)
+    @ViewBuilder
+    private func listForEach(for section: ListSection, data: TopListData) -> some View {
+        let items = getDisplayedItems(from: data.items, section: section)
+        ForEach(items, id: \.element.id) { index, item in
+            TopListItemView(
+                index: index < 50 ? index : nil,
+                item: item,
+                previousValue: data.previousItem(for: item)?.metrics[viewModel.selection.metric],
+                metric: viewModel.selection.metric,
+                maxValue: data.metrics.maxValue,
+                dateRange: viewModel.dateRange
+            )
+            .frame(height: TopListItemView.defaultCellHeight)
+        }
+        .listRowInsets(EdgeInsets(top: Constants.step0_5 / 2, leading: 0, bottom: Constants.step0_5 / 2, trailing: 0))
+    }
+
+    private func getDisplayedItems(
+        from items: [any TopListItemProtocol],
+        section: ListSection
+    ) -> [(offset: Int, element: any TopListItemProtocol)] {
+        switch section {
+        case .top10:
+            return Array(items.enumerated().prefix(10))
+        case .top50:
+            return Array(items.enumerated().prefix(50).dropFirst(10))
+        case .other:
+            return Array(items.enumerated().dropFirst(50))
+        }
+    }
+
+    private var showMoreButton: some View {
+        Button {
+            withAnimation(.spring) {
+                isShowingAllItems = true
+            }
+        } label: {
+            HStack(spacing: 4) {
+                Text(Strings.Buttons.showMore)
+                    .font(.subheadline)
+                    .fontWeight(.medium)
+                Image(systemName: "chevron.down")
+                    .font(.caption)
+                    .fontWeight(.medium)
             }
         }
+        .buttonStyle(.plain)
+        .frame(maxWidth: .infinity)
+        .padding(.vertical, Constants.step2)
     }
 
     private func makeEmptyStateView(message: String) -> some View {
-        itemsListView(data: mockData)
-            .redacted(reason: .placeholder)
-            .grayscale(1)
-            .opacity(0.25)
-            .overlay {
-                SimpleErrorView(message: message)
-            }
+        VStack {
+            listContent(data: mockData(count: 6))
+        }
+        .redacted(reason: .placeholder)
+        .grayscale(1)
+        .opacity(0.25)
+        .overlay {
+            SimpleErrorView(message: message)
+        }
     }
 
-    private var mockData: TopListData {
+    private func mockData(count: Int = 10) -> TopListData {
         TopListData.mock(
             for: viewModel.selection.item,
             metric: viewModel.selection.metric,
-            itemCount: 10
+            itemCount: count
         )
     }
 
diff --git a/Modules/Sources/JetpackStats/Strings.swift b/Modules/Sources/JetpackStats/Strings.swift
@@ -122,6 +122,8 @@ enum Strings {
         static let highestBounceRate = AppLocalizedString("jetpackStats.topList.highestBounceRate", value: "Highest Bounce Rate", comment: "Title for items with highest bounce rate")
         static let longestTimeOnSite = AppLocalizedString("jetpackStats.topList.longestTimeOnSite", value: "Longest Time on Site", comment: "Title for items with longest time on site")
         static let mostDownloadeded = AppLocalizedString("jetpackStats.topList.mostDownloads", value: "Most Downloaded", comment: "Title for chart")
+        static let top10 = AppLocalizedString("jetpackStats.postDetails.top10", value: "Top 10", comment: "Section title")
+        static let top50 = AppLocalizedString("jetpackStats.postDetails.top50", value: "Top 50", comment: "Section title")
     }
 
     enum Errors {
diff --git a/Modules/Sources/JetpackStats/Views/TopList/TopListItemView.swift b/Modules/Sources/JetpackStats/Views/TopList/TopListItemView.swift
@@ -4,6 +4,7 @@ import DesignSystem
 struct TopListItemView: View {
     static let defaultCellHeight: CGFloat = 52
 
+    var index: Int?
     let item: any TopListItemProtocol
     let previousValue: Int?
     let metric: SiteMetric
@@ -52,6 +53,15 @@ struct TopListItemView: View {
 
     var content: some View {
         HStack(alignment: .center, spacing: 0) {
+            if let index {
+                Text("\(index + 1)")
+                    .font(.system(.subheadline, design: .rounded, weight: .medium))
+                    .frame(width: 22, alignment: .center)
+                    .lineLimit(1)
+                    .dynamicTypeSize(...DynamicTypeSize.large)
+                    .padding(.trailing, 8)
+            }
+
             // Content-specific view
             switch item {
             case let post as TopListItem.Post:
@@ -92,7 +102,7 @@ struct TopListItemView: View {
             .padding(.trailing, -3)
             .dynamicTypeSize(...DynamicTypeSize.xLarge)
         }
-        .dynamicTypeSize(...DynamicTypeSize.xxLarge)
+        .dynamicTypeSize(...DynamicTypeSize.xLarge)
         .padding(.horizontal, Constants.step1)
         .frame(height: cellHeight)
         .contentShape(Rectangle())
diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
@@ -3,6 +3,8 @@
 * [*] Fix a few minor visual issues on iOS 26 [#24863], [#24863], [#24865], [#24866], [#24867]
 * [*] [Intelligence] Add support for generating excerpts for posts [#24852]
 * [*] Fix performance issues with search in Reader Subscriptions [#24841]
+* [*] Improve performance and animations when showing a full Top List with a large number of items [#24868]
+* [*] Show the Top 10 and 50 items in the Top List screen in a dedicated section [#24868]
 
 26.3.1
 ------

Original file line number	Diff line number	Diff line change
`@@ -122,6 +122,8 @@ enum Strings {`
`122`	`122`	`static let highestBounceRate = AppLocalizedString("jetpackStats.topList.highestBounceRate", value: "Highest Bounce Rate", comment: "Title for items with highest bounce rate")`
`123`	`123`	`static let longestTimeOnSite = AppLocalizedString("jetpackStats.topList.longestTimeOnSite", value: "Longest Time on Site", comment: "Title for items with longest time on site")`
`124`	`124`	`static let mostDownloadeded = AppLocalizedString("jetpackStats.topList.mostDownloads", value: "Most Downloaded", comment: "Title for chart")`
	`125`	`+ static let top10 = AppLocalizedString("jetpackStats.postDetails.top10", value: "Top 10", comment: "Section title")`
	`126`	`+ static let top50 = AppLocalizedString("jetpackStats.postDetails.top50", value: "Top 50", comment: "Section title")`
`125`	`127`	`}`
`126`	`128`
`127`	`129`	`enum Errors {`