From 8a3468658b1a63b8cfba7e78c301b9d65ca57eb9 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Fri, 21 Aug 2026 11:40:10 +0300 Subject: [PATCH] Address PR comments --- .../HomeScreen/HomeScreenViewModel.swift | 26 +++++++------------ .../HomeScreen/View/HomeScreenContent.swift | 3 --- Tools/Sources/Commands/CI/RunTests.swift | 3 +-- .../Sources/HomeScreenViewModelTests.swift | 8 +++--- .../RoomDetailsScreenViewModelTests.swift | 2 -- 5 files changed, 15 insertions(+), 27 deletions(-) diff --git a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift index 60c6a6d6f..d4e323aff 100644 --- a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift +++ b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift @@ -177,9 +177,6 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol updateRooms() - // The subscription above only delivers the current state through an async main queue - // hop, which can end up queued behind the first render and leave the skeletons up on a - // warm launch, where the provider has already published the cached rooms. if let roomSummaryProvider { updateRoomListMode(with: roomSummaryProvider.statePublisher.value, hasRooms: !roomSummaryProvider.roomListPublisher.value.isEmpty) @@ -337,19 +334,16 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol } private func updateRoomListMode(with roomSummaryProviderState: RoomSummaryProviderState, hasRooms: Bool) { - let isLoadingData = !roomSummaryProviderState.isLoaded - let hasNoRooms = roomSummaryProviderState.isLoaded && roomSummaryProviderState.totalNumberOfRooms == 0 - // Only the initial transition needs holding back until the rooms have published, an - // empty list later on is a filter or a search result. - let isInitialLoad = state.roomListMode == .skeletons - - var roomListMode = state.roomListMode - if isLoadingData { - roomListMode = .skeletons - } else if hasNoRooms { - roomListMode = .empty - } else if hasRooms || !isInitialLoad { - roomListMode = .rooms + let roomListMode: HomeScreenRoomListMode = if !roomSummaryProviderState.isLoaded { + .skeletons // Still loading. + } else if roomSummaryProviderState.totalNumberOfRooms == 0 { + .empty // Loaded, there are no rooms at all. + } else if hasRooms { + .rooms // Loaded and the summaries have published. + } else if state.roomListMode == .skeletons { + .skeletons // Loaded but nothing published yet, flipping to .rooms would flash an empty list. + } else { + .rooms } guard roomListMode != state.roomListMode else { diff --git a/ElementX/Sources/Screens/HomeScreen/View/HomeScreenContent.swift b/ElementX/Sources/Screens/HomeScreen/View/HomeScreenContent.swift index dce72e254..7d78d11c8 100644 --- a/ElementX/Sources/Screens/HomeScreen/View/HomeScreenContent.swift +++ b/ElementX/Sources/Screens/HomeScreen/View/HomeScreenContent.swift @@ -68,9 +68,6 @@ struct HomeScreenContent: View { scrollViewAdapter.scrollView = scrollView } .onReceive(scrollViewAdapter.didScroll) { _ in - // Live, undelayed updates while scrolling: pagination needs to see the - // range early enough to grow the list before the user reaches the bottom, - // and the scroll geometry is valid mid-scroll. sendVisibleRange() } .onReceive(scrollViewAdapter.isScrolling) { _ in diff --git a/Tools/Sources/Commands/CI/RunTests.swift b/Tools/Sources/Commands/CI/RunTests.swift index 8c090eec6..bd388659f 100644 --- a/Tools/Sources/Commands/CI/RunTests.swift +++ b/Tools/Sources/Commands/CI/RunTests.swift @@ -151,8 +151,7 @@ struct RunTests: AsyncParsableCommand { // -test-iterations is the total number of attempts (initial + retries) command += " -retry-tests-on-failure" command += " -test-iterations \(retries + 1)" - // Repetitions happen in the same process by default, so a runner that exits (e.g. code 75 - // after a failed app launch) takes its own retries down with it and the test gets skipped. + // Ensure retries still happen when a runner exits early. command += " -test-repetition-relaunch-enabled YES" } diff --git a/UnitTests/Sources/HomeScreenViewModelTests.swift b/UnitTests/Sources/HomeScreenViewModelTests.swift index 93fd80712..915deb332 100644 --- a/UnitTests/Sources/HomeScreenViewModelTests.swift +++ b/UnitTests/Sources/HomeScreenViewModelTests.swift @@ -399,9 +399,9 @@ final class HomeScreenViewModelTests { #expect(context.viewState.roomListMode == .skeletons) // The provider reports loaded before the first summaries have been published. + let failure = deferFailure(context.$viewState, timeout: .seconds(1)) { $0.roomListMode != .skeletons } stateSubject.send(.loaded(totalNumberOfRooms: 8)) - try await Task.sleep(for: .milliseconds(100)) - #expect(context.viewState.roomListMode == .skeletons) + try await failure.fulfill() let deferred = deferFulfillment(context.$viewState) { $0.roomListMode == .rooms } roomListSubject.send(.mockRooms) @@ -418,10 +418,10 @@ final class HomeScreenViewModelTests { try await deferred.fulfill() // A filter or a search without matches empties the list, the mode must not regress. + let failure = deferFailure(context.$viewState, timeout: .seconds(1)) { $0.roomListMode != .rooms } roomListSubject.send([]) stateSubject.send(.loaded(totalNumberOfRooms: 9)) - try await Task.sleep(for: .milliseconds(100)) - #expect(context.viewState.roomListMode == .rooms) + try await failure.fulfill() } private func setupViewModelWithManualProvider() -> (CurrentValueSubject<[RoomSummary], Never>, CurrentValueSubject) { diff --git a/UnitTests/Sources/RoomDetailsScreenViewModelTests.swift b/UnitTests/Sources/RoomDetailsScreenViewModelTests.swift index 857639211..9ebbfeb77 100644 --- a/UnitTests/Sources/RoomDetailsScreenViewModelTests.swift +++ b/UnitTests/Sources/RoomDetailsScreenViewModelTests.swift @@ -271,8 +271,6 @@ struct RoomDetailsScreenViewModelTests { #expect(context.viewState.dmRecipientInfo?.member == RoomMemberDetails(withProxy: recipient)) #expect(!context.viewState.isProcessingIgnoreRequest) - // Waiting on the transient isProcessingIgnoreRequest transition races the view model's task, - // which can complete before the observation starts. Wait on the alert instead. let deferredAlert = deferFulfillment(context.observe(\.alertInfo)) { $0 != nil } context.send(viewAction: .unignoreConfirmed)