Skip to content

Commit fdd4c16

Browse files
authored
Merge pull request #7650 from woocommerce/fix/7639-swipe-to-switch-time-range-tab
Dashboard: sync stats after swiping to another time range tab
2 parents 1f41687 + a6fd2f1 commit fdd4c16

File tree

2 files changed

+52
-17
lines changed

2 files changed

+52
-17
lines changed

RELEASE-NOTES.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
10.3
44
-----
55
- [*] Dashboard: the last selected time range tab (Today/This Week/This Month/This Year) is persisted for the site and shown on the next site launch (app launch or switching stores). [https://github.com/woocommerce/woocommerce-ios/pull/7638]
6+
- [*] Dashboard: swiping to another time range tab now triggers syncing for the target tab. Previously, the stats on the target tab aren't synced from the swipe gesture. [https://github.com/woocommerce/woocommerce-ios/pull/7650]
67
- [*] In-Person Payments: Fixed an issue where the Pay in Person toggle could be out of sync with the setting on the website. [https://github.com/woocommerce/woocommerce-ios/pull/7656]
78

89
10.2

WooCommerce/Classes/ViewRelated/Dashboard/Stats v4/StoreStatsAndTopPerformersViewController.swift

Lines changed: 51 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,18 @@ final class StoreStatsAndTopPerformersViewController: ButtonBarPagerTabStripView
3232

3333
private var periodVCs = [StoreStatsAndTopPerformersPeriodViewController]()
3434
private let siteID: Int64
35-
private var isSyncing = false
35+
// A set of syncing time ranges is tracked instead of a single boolean so that the stats for each time range
36+
// can be synced when swiping or tapping to change the time range tab before the syncing finishes for the previously selected tab.
37+
private var syncingTimeRanges: Set<StatsTimeRangeV4> = []
3638
private let usageTracksEventEmitter = StoreStatsUsageTracksEventEmitter()
3739
private let dashboardViewModel: DashboardViewModel
3840
private let timeRanges: [StatsTimeRangeV4] = [.today, .thisWeek, .thisMonth, .thisYear]
3941

40-
/// Because loading the last selected time range tab is async, we need to make sure any call to the public `reloadData` is after the selected time range
41-
/// is ready to avoid making unnecessary API requests for the non-selected tab.
42-
@Published private var isSelectedTimeRangeReady: Bool = false
42+
/// Because loading the last selected time range tab is async, the selected tab index is initially `nil` and set after the last selected value is loaded.
43+
/// We need to make sure any call to the public `reloadData` is after the selected time range is set to avoid making unnecessary API requests
44+
/// for the non-selected tab.
45+
@Published private var selectedTimeRangeIndex: Int?
46+
private var selectedTimeRangeIndexSubscription: AnyCancellable?
4347
private var reloadDataAfterSelectedTimeRangeSubscriptions: Set<AnyCancellable> = []
4448

4549
// MARK: - View Lifecycle
@@ -62,20 +66,21 @@ final class StoreStatsAndTopPerformersViewController: ButtonBarPagerTabStripView
6266
let selectedTimeRange = await loadLastTimeRange() ?? .today
6367
guard let selectedTabIndex = timeRanges.firstIndex(of: selectedTimeRange),
6468
selectedTabIndex != currentIndex else {
65-
isSelectedTimeRangeReady = true
69+
selectedTimeRangeIndex = currentIndex
6670
return
6771
}
6872
// There is currently no straightforward way to set a different default tab using `XLPagerTabStrip` without forking.
6973
// This is a workaround following https://github.com/xmartlabs/XLPagerTabStrip/issues/537#issuecomment-534903598
7074
moveToViewController(at: selectedTabIndex, animated: false)
7175
reloadPagerTabStripView()
72-
isSelectedTimeRangeReady = true
76+
selectedTimeRangeIndex = selectedTabIndex
7377
}
7478

7579
// 👆 must be called before super.viewDidLoad()
7680

7781
super.viewDidLoad()
7882
configureView()
83+
observeSelectedTimeRangeIndex()
7984
}
8085

8186
override func viewWillAppear(_ animated: Bool) {
@@ -101,23 +106,51 @@ final class StoreStatsAndTopPerformersViewController: ButtonBarPagerTabStripView
101106
}
102107
}
103108

104-
override func collectionView(_ collectionView: UICollectionView, didSelectItemAt indexPath: IndexPath) {
105-
super.collectionView(collectionView, didSelectItemAt: indexPath)
106-
let timeRangeTabIndex = indexPath.item
107-
guard let periodViewController = viewControllers[timeRangeTabIndex] as? StoreStatsAndTopPerformersPeriodViewController,
108-
timeRangeTabIndex != currentIndex else {
109+
override func updateIndicator(for viewController: PagerTabStripViewController,
110+
fromIndex: Int,
111+
toIndex: Int,
112+
withProgressPercentage progressPercentage: CGFloat,
113+
indexWasChanged: Bool) {
114+
super.updateIndicator(for: viewController,
115+
fromIndex: fromIndex,
116+
toIndex: toIndex,
117+
withProgressPercentage: progressPercentage,
118+
indexWasChanged: indexWasChanged)
119+
// The initially selected tab should be ignored because it should be set after `loadLastTimeRange` in `viewDidLoad`.
120+
guard selectedTimeRangeIndex != nil else {
109121
return
110122
}
111-
saveLastTimeRange(periodViewController.timeRange)
112-
syncStats(forced: false, viewControllerToSync: periodViewController)
123+
selectedTimeRangeIndex = toIndex
124+
}
125+
126+
func observeSelectedTimeRangeIndex() {
127+
let timeRangeCount = timeRanges.count
128+
selectedTimeRangeIndexSubscription = $selectedTimeRangeIndex
129+
.compactMap { $0 }
130+
// It's possible to reach an out-of-bound index by swipe gesture, thus checking the index range here.
131+
.filter { $0 >= 0 && $0 < timeRangeCount }
132+
.removeDuplicates()
133+
// Tapping to change to a farther tab could result in `updateIndicator` callback to be triggered for the middle tabs.
134+
// A short debounce workaround is applied here to avoid making API requests for the middle tabs.
135+
.debounce(for: .seconds(0.3), scheduler: DispatchQueue.main)
136+
.sink { [weak self] timeRangeTabIndex in
137+
guard let self = self else { return }
138+
guard let periodViewController = self.viewControllers[timeRangeTabIndex] as? StoreStatsAndTopPerformersPeriodViewController else {
139+
return
140+
}
141+
self.saveLastTimeRange(periodViewController.timeRange)
142+
self.syncStats(forced: false, viewControllerToSync: periodViewController)
143+
}
113144
}
114145
}
115146

116147
extension StoreStatsAndTopPerformersViewController: DashboardUI {
117148
@MainActor
118149
func reloadData(forced: Bool) async {
119150
await withCheckedContinuation { continuation in
120-
$isSelectedTimeRangeReady.filter { $0 == true }.first()
151+
$selectedTimeRangeIndex
152+
.compactMap { $0 }
153+
.first()
121154
.sink { [weak self] _ in
122155
self?.syncAllStats(forced: forced) { _ in
123156
continuation.resume(returning: ())
@@ -140,12 +173,13 @@ private extension StoreStatsAndTopPerformersViewController {
140173
}
141174

142175
func syncStats(forced: Bool, viewControllerToSync: StoreStatsAndTopPerformersPeriodViewController, onCompletion: ((Result<Void, Error>) -> Void)? = nil) {
143-
guard !isSyncing else {
176+
let timeRange = viewControllerToSync.timeRange
177+
guard !syncingTimeRanges.contains(timeRange) else {
144178
onCompletion?(.success(()))
145179
return
146180
}
147181

148-
isSyncing = true
182+
syncingTimeRanges.insert(timeRange)
149183

150184
let group = DispatchGroup()
151185

@@ -156,7 +190,7 @@ private extension StoreStatsAndTopPerformersViewController {
156190

157191
defer {
158192
group.notify(queue: .main) { [weak self] in
159-
self?.isSyncing = false
193+
self?.syncingTimeRanges.remove(timeRange)
160194
self?.showSpinner(for: viewControllerToSync, shouldShowSpinner: false)
161195
if let error = syncError {
162196
DDLogError("⛔️ Error loading dashboard: \(error)")

0 commit comments

Comments
 (0)