Permalink Tweaks (#2768)

* Revert back to creating a new flow when re-selecting the same room.

Fix the composer bug by adding IDs to the all of the module AnyViews.

* Don't show the live timeline when starting with a focussed event.

ElementX/Sources/Application/Navigation/NavigationCoordinators.swift

@@ -379,9 +379,11 @@ private struct NavigationSplitCoordinatorView: View {
         // through the NavigationSplitCoordinator as well.
         .sheet(item: $navigationSplitCoordinator.sheetModule) { module in
             module.coordinator?.toPresentable()
+                .id(module.id)
         }
         .fullScreenCover(item: $navigationSplitCoordinator.fullScreenCoverModule) { module in
             module.coordinator?.toPresentable()
+                .id(module.id)
         }
         // Handle `horizontalSizeClass` changes breaking the navigation bar
         // https://github.com/element-hq/element-x-ios/issues/617
@@ -408,8 +410,10 @@ private struct NavigationSplitCoordinatorView: View {
     var navigationStack: some View {
         NavigationStack(path: navigationSplitCoordinator.compactLayoutStackModulesBinding) {
             navigationSplitCoordinator.compactLayoutRootModule?.coordinator?.toPresentable()
+                .id(navigationSplitCoordinator.compactLayoutRootModule?.id) // Is a nil ID ok?
                 .navigationDestination(for: NavigationModule.self) { module in
                     module.coordinator?.toPresentable()
+                        .id(module.id)
                 }
         }
     }
@@ -419,19 +423,24 @@ private struct NavigationSplitCoordinatorView: View {
         NavigationSplitView(columnVisibility: $columnVisibility) {
             if let sidebarModule = navigationSplitCoordinator.sidebarModule {
                 sidebarModule.coordinator?.toPresentable()
+                    .id(sidebarModule.id)
             } else {
                 navigationSplitCoordinator.placeholderModule.coordinator?.toPresentable()
+                    .id(navigationSplitCoordinator.placeholderModule.id)
             }
         } detail: {
             if let detailModule = navigationSplitCoordinator.detailModule {
                 detailModule.coordinator?.toPresentable()
+                    .id(detailModule.id)
             } else {
                 navigationSplitCoordinator.placeholderModule.coordinator?.toPresentable()
+                    .id(navigationSplitCoordinator.placeholderModule.id)
             }
         }
         .navigationSplitViewStyle(.balanced)
         .navigationDestination(for: NavigationModule.self) { module in
             module.coordinator?.toPresentable()
+                .id(module.id)
         }
         .animation(.elementDefault, value: navigationSplitCoordinator.sidebarModule)
         .animation(.elementDefault, value: navigationSplitCoordinator.detailModule)
@@ -713,15 +722,19 @@ private struct NavigationStackCoordinatorView: View {
     var body: some View {
         NavigationStack(path: $navigationStackCoordinator.stackModules) {
             navigationStackCoordinator.rootModule?.coordinator?.toPresentable()
+                .id(navigationStackCoordinator.rootModule?.id) // Is a nil ID ok?
                 .navigationDestination(for: NavigationModule.self) { module in
                     module.coordinator?.toPresentable()
+                        .id(module.id)
                 }
         }
         .sheet(item: $navigationStackCoordinator.sheetModule) { module in
             module.coordinator?.toPresentable()
+                .id(module.id)
         }
         .fullScreenCover(item: $navigationStackCoordinator.fullScreenCoverModule) { module in
             module.coordinator?.toPresentable()
+                .id(module.id)
         }
         .animation(.elementDefault, value: navigationStackCoordinator.rootModule)
     }

ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift

@@ -47,6 +47,11 @@ enum RoomFlowCoordinatorEntryPoint: Hashable {
     case eventID(String)
     /// The flow will start by showing the room's details.
     case roomDetails
+    
+    var isEventID: Bool {
+        guard case .eventID = self else { return false }
+        return true
+    }
 }
 
 // swiftlint:disable:next type_body_length
@@ -508,7 +513,14 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
             case .initial, .roomDetails(isRoot: true), .joinRoomScreen:
                 break
             default:
-                return // The room is already on the stack, no need to present it again
+                // The room is already on the stack, no need to present it again
+                
+                // Check if we need to focus on an event
+                if let focussedEventID {
+                    roomScreenCoordinator?.focusOnEvent(eventID: focussedEventID)
+                }
+                
+                return
             }
         }
         
@@ -524,6 +536,7 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
                                                           stateEventStringBuilder: RoomStateEventStringBuilder(userID: userID))
                 
         let timelineController = roomTimelineControllerFactory.buildRoomTimelineController(roomProxy: roomProxy,
+                                                                                           initialFocussedEventID: focussedEventID,
                                                                                            timelineItemFactory: timelineItemFactory)
         self.timelineController = timelineController
         
@@ -1012,6 +1025,7 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
                                                           stateEventStringBuilder: RoomStateEventStringBuilder(userID: userID))
                 
         let roomTimelineController = roomTimelineControllerFactory.buildRoomTimelineController(roomProxy: roomProxy,
+                                                                                               initialFocussedEventID: nil,
                                                                                                timelineItemFactory: timelineItemFactory)
         
         let parameters = RoomPollsHistoryScreenCoordinatorParameters(roomProxy: roomProxy,

ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift

@@ -272,15 +272,15 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
                 presentHomeScreen()
                 attemptStartingOnboarding()
             case(.roomList(let selectedRoomID), .selectRoom(let roomID, let entryPoint), .roomList):
-                if selectedRoomID == roomID {
-                    if let roomFlowCoordinator {
-                        let route: AppRoute = switch entryPoint {
-                        case .room: .room(roomID: roomID)
-                        case .eventID(let eventID): .event(roomID: roomID, eventID: eventID)
-                        case .roomDetails: .roomDetails(roomID: roomID)
-                        }
-                        roomFlowCoordinator.handleAppRoute(route, animated: animated)
+                if selectedRoomID == roomID,
+                   !entryPoint.isEventID, // Don't reuse the existing room so the live timeline is hidden while the detached timeline is loading.
+                   let roomFlowCoordinator {
+                    let route: AppRoute = switch entryPoint {
+                    case .room: .room(roomID: roomID)
+                    case .roomDetails: .roomDetails(roomID: roomID)
+                    case .eventID(let eventID): .event(roomID: roomID, eventID: eventID) // ignored.
                     }
+                    roomFlowCoordinator.handleAppRoute(route, animated: animated)
                 } else {
                     Task { await self.startRoomFlow(roomID: roomID, entryPoint: entryPoint, animated: animated) }
                 }

ElementX/Sources/Mocks/Generated/GeneratedMocks.swift

@@ -9652,6 +9652,77 @@ class RoomSummaryProviderMock: RoomSummaryProviderProtocol {
         setFilterClosure?(filter)
     }
 }
+class RoomTimelineControllerFactoryMock: RoomTimelineControllerFactoryProtocol {
+
+    //MARK: - buildRoomTimelineController
+
+    var buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryUnderlyingCallsCount = 0
+    var buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryCallsCount: Int {
+        get {
+            if Thread.isMainThread {
+                return buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryUnderlyingCallsCount
+            } else {
+                var returnValue: Int? = nil
+                DispatchQueue.main.sync {
+                    returnValue = buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryUnderlyingCallsCount
+                }
+
+                return returnValue!
+            }
+        }
+        set {
+            if Thread.isMainThread {
+                buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryUnderlyingCallsCount = newValue
+            } else {
+                DispatchQueue.main.sync {
+                    buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryUnderlyingCallsCount = newValue
+                }
+            }
+        }
+    }
+    var buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryCalled: Bool {
+        return buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryCallsCount > 0
+    }
+    var buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryReceivedArguments: (roomProxy: RoomProxyProtocol, initialFocussedEventID: String?, timelineItemFactory: RoomTimelineItemFactoryProtocol)?
+    var buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryReceivedInvocations: [(roomProxy: RoomProxyProtocol, initialFocussedEventID: String?, timelineItemFactory: RoomTimelineItemFactoryProtocol)] = []
+
+    var buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryUnderlyingReturnValue: RoomTimelineControllerProtocol!
+    var buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryReturnValue: RoomTimelineControllerProtocol! {
+        get {
+            if Thread.isMainThread {
+                return buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryUnderlyingReturnValue
+            } else {
+                var returnValue: RoomTimelineControllerProtocol? = nil
+                DispatchQueue.main.sync {
+                    returnValue = buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryUnderlyingReturnValue
+                }
+
+                return returnValue!
+            }
+        }
+        set {
+            if Thread.isMainThread {
+                buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryUnderlyingReturnValue = newValue
+            } else {
+                DispatchQueue.main.sync {
+                    buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryUnderlyingReturnValue = newValue
+                }
+            }
+        }
+    }
+    var buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryClosure: ((RoomProxyProtocol, String?, RoomTimelineItemFactoryProtocol) -> RoomTimelineControllerProtocol)?
+
+    func buildRoomTimelineController(roomProxy: RoomProxyProtocol, initialFocussedEventID: String?, timelineItemFactory: RoomTimelineItemFactoryProtocol) -> RoomTimelineControllerProtocol {
+        buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryCallsCount += 1
+        buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryReceivedArguments = (roomProxy: roomProxy, initialFocussedEventID: initialFocussedEventID, timelineItemFactory: timelineItemFactory)
+        buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryReceivedInvocations.append((roomProxy: roomProxy, initialFocussedEventID: initialFocussedEventID, timelineItemFactory: timelineItemFactory))
+        if let buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryClosure = buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryClosure {
+            return buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryClosure(roomProxy, initialFocussedEventID, timelineItemFactory)
+        } else {
+            return buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryReturnValue
+        }
+    }
+}
 class RoomTimelineProviderMock: RoomTimelineProviderProtocol {
     var updatePublisher: AnyPublisher<([TimelineItemProxy], PaginationState), Never> {
         get { return underlyingUpdatePublisher }

ElementX/Sources/Mocks/RoomTimelineControllerFactoryMock.swift

@@ -16,11 +16,18 @@
 
 import Foundation
 
-struct MockRoomTimelineControllerFactory: RoomTimelineControllerFactoryProtocol {
-    func buildRoomTimelineController(roomProxy: RoomProxyProtocol,
-                                     timelineItemFactory: RoomTimelineItemFactoryProtocol) -> RoomTimelineControllerProtocol {
-        let timelineController = MockRoomTimelineController()
-        timelineController.timelineItems = RoomTimelineItemFixtures.largeChunk
-        return timelineController
+struct RoomTimelineControllerFactoryMockConfiguration {
+    var timelineController: MockRoomTimelineController?
+}
+
+extension RoomTimelineControllerFactoryMock {
+    convenience init(configuration: RoomTimelineControllerFactoryMockConfiguration) {
+        self.init()
+        
+        buildRoomTimelineControllerRoomProxyInitialFocussedEventIDTimelineItemFactoryReturnValue = configuration.timelineController ?? {
+            let timelineController = MockRoomTimelineController()
+            timelineController.timelineItems = RoomTimelineItemFixtures.largeChunk
+            return timelineController
+        }()
     }
 }

ElementX/Sources/Screens/RoomDetailsScreen/View/RoomDetailsScreen.swift

@@ -62,6 +62,7 @@ struct RoomDetailsScreen: View {
             }
         }
         .navigationTitle(L10n.screenRoomDetailsTitle)
+        .navigationBarTitleDisplayMode(.inline)
         .track(screen: .RoomDetails)
         .interactiveQuickLook(item: $context.mediaPreviewItem, shouldHideControls: true)
     }

ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift

@@ -95,28 +95,28 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
                                                          bindings: .init(reactionsCollapsed: [:])),
                    imageProvider: mediaProvider)
         
-        // This may change to load the detached timeline directly.
-        if let focussedEventID {
-            Task { await focusOnEvent(eventID: focussedEventID) }
+        if focussedEventID != nil {
+            // The timeline controller will start loading a detached timeline.
+            showFocusLoadingIndicator()
         }
         
         setupSubscriptions()
         setupDirectRoomSubscriptionsIfNeeded()
         
-        state.timelineItemMenuActionProvider = { [weak self] itemId -> TimelineItemMenuActions? in
+        state.timelineItemMenuActionProvider = { [weak self] itemID -> TimelineItemMenuActions? in
             guard let self else {
                 return nil
             }
             
-            return self.roomScreenInteractionHandler.timelineItemMenuActionsForItemId(itemId)
+            return self.roomScreenInteractionHandler.timelineItemMenuActionsForItemId(itemID)
         }
 
-        state.audioPlayerStateProvider = { [weak self] itemId -> AudioPlayerState? in
+        state.audioPlayerStateProvider = { [weak self] itemID -> AudioPlayerState? in
             guard let self else {
                 return nil
             }
             
-            return self.roomScreenInteractionHandler.audioPlayerState(for: itemId)
+            return self.roomScreenInteractionHandler.audioPlayerState(for: itemID)
         }
         
         buildTimelineViews()
@@ -191,8 +191,10 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
         case .focusLive:
             focusLive()
         case .scrolledToFocussedItem:
-            // Use a Task to mutate view state after the current view update.
-            Task { state.timelineViewState.focussedEventNeedsDisplay = false }
+            Task { // Use a Task to mutate view state after the current view update.
+                state.timelineViewState.focussedEventNeedsDisplay = false
+                hideFocusLoadingIndicator()
+            }
         }
     }
 

ElementX/Sources/Services/Timeline/TimelineController/RoomTimelineController.swift

@@ -44,6 +44,7 @@ class RoomTimelineController: RoomTimelineControllerProtocol {
     }
     
     init(roomProxy: RoomProxyProtocol,
+         initialFocussedEventID: String?,
          timelineItemFactory: RoomTimelineItemFactoryProtocol,
          appSettings: AppSettings) {
         self.roomProxy = roomProxy
@@ -54,9 +55,24 @@ class RoomTimelineController: RoomTimelineControllerProtocol {
         
         activeTimeline = roomProxy.timeline
         activeTimelineProvider = liveTimelineProvider
-        configureActiveTimelineProvider()
         
         NotificationCenter.default.addObserver(self, selector: #selector(contentSizeCategoryDidChange), name: UIContentSizeCategory.didChangeNotification, object: nil)
+        
+        guard let initialFocussedEventID else {
+            configureActiveTimelineProvider()
+            return
+        }
+        
+        Task {
+            callbacks.send(.paginationState(PaginationState(backward: .paginating, forward: .paginating)))
+            switch await focusOnEvent(initialFocussedEventID, timelineSize: 100) {
+            case .success:
+                break
+            case .failure:
+                // Setup the live timeline as a fallback.
+                configureActiveTimelineProvider()
+            }
+        }
     }
     
     func focusOnEvent(_ eventID: String, timelineSize: UInt16) async -> Result<Void, RoomTimelineControllerError> {
@@ -78,7 +94,6 @@ class RoomTimelineController: RoomTimelineControllerProtocol {
     func focusLive() {
         activeTimeline = roomProxy.timeline
         activeTimelineProvider = liveTimelineProvider
-        callbacks.send(.isLive(true))
     }
     
     func paginateBackwards(requestSize: UInt16) async -> Result<Void, RoomTimelineControllerError> {

ElementX/Sources/Services/Timeline/TimelineController/RoomTimelineControllerFactory.swift

@@ -18,8 +18,10 @@ import Foundation
 
 struct RoomTimelineControllerFactory: RoomTimelineControllerFactoryProtocol {
     func buildRoomTimelineController(roomProxy: RoomProxyProtocol,
+                                     initialFocussedEventID: String?,
                                      timelineItemFactory: RoomTimelineItemFactoryProtocol) -> RoomTimelineControllerProtocol {
         RoomTimelineController(roomProxy: roomProxy,
+                               initialFocussedEventID: initialFocussedEventID,
                                timelineItemFactory: timelineItemFactory,
                                appSettings: ServiceLocator.shared.settings)
     }

ElementX/Sources/Services/Timeline/TimelineController/RoomTimelineControllerFactoryProtocol.swift

@@ -19,5 +19,9 @@ import Foundation
 @MainActor
 protocol RoomTimelineControllerFactoryProtocol {
     func buildRoomTimelineController(roomProxy: RoomProxyProtocol,
+                                     initialFocussedEventID: String?,
                                      timelineItemFactory: RoomTimelineItemFactoryProtocol) -> RoomTimelineControllerProtocol
 }
+
+// sourcery: AutoMockable
+extension RoomTimelineControllerFactoryProtocol { }

ElementX/Sources/UITests/UITestsAppCoordinator.swift

@@ -503,7 +503,7 @@ class MockScreen: Identifiable {
                                                              appLockService: AppLockService(keychainController: KeychainControllerMock(),
                                                                                             appSettings: ServiceLocator.shared.settings),
                                                              bugReportService: BugReportServiceMock(),
-                                                             roomTimelineControllerFactory: MockRoomTimelineControllerFactory(),
+                                                             roomTimelineControllerFactory: RoomTimelineControllerFactoryMock(configuration: .init()),
                                                              appMediator: AppMediatorMock.default,
                                                              appSettings: appSettings,
                                                              analytics: ServiceLocator.shared.analytics,