From 115c0af9c565175ad56f0cf6b389818c3ac2e95d Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Thu, 22 Aug 2024 16:47:05 +0200 Subject: [PATCH] PR comments --- .../xcshareddata/swiftpm/Package.resolved | 3 +- .../PinnedEventsTimelineFlowCoordinator.swift | 4 +- .../RoomFlowCoordinator.swift | 40 +++++++------- ...innedEventsTimelineScreenCoordinator.swift | 2 +- .../RoomScreen/RoomScreenCoordinator.swift | 25 +++++---- .../Screens/RoomScreen/RoomScreenModels.swift | 52 ++++++++++--------- .../RoomScreen/RoomScreenViewModel.swift | 22 ++++---- .../RoomScreenViewModelProtocol.swift | 2 +- .../PinnedItemsBannerView.swift | 8 +-- .../UITests/UITestsAppCoordinator.swift | 2 +- .../PinnedEventsBannerStateTests.swift | 50 +++++++++--------- .../Sources/RoomScreenViewModelTests.swift | 22 ++++---- 12 files changed, 118 insertions(+), 114 deletions(-) diff --git a/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 495894bf96..760bbaa7ef 100644 --- a/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -1,4 +1,5 @@ { + "originHash" : "1b1fb14b1ac1438c44ffd2f0151af65c83018d71b8674cd7f13ec21abeb31bcd", "pins" : [ { "identity" : "compound-design-tokens", @@ -288,5 +289,5 @@ } } ], - "version" : 2 + "version" : 3 } diff --git a/ElementX/Sources/FlowCoordinators/PinnedEventsTimelineFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/PinnedEventsTimelineFlowCoordinator.swift index 4bb173d8bf..96d0372006 100644 --- a/ElementX/Sources/FlowCoordinators/PinnedEventsTimelineFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/PinnedEventsTimelineFlowCoordinator.swift @@ -20,7 +20,7 @@ import Foundation enum PinnedEventsTimelineFlowCoordinatorAction { case finished case displayUser(userID: String) - case startChildRoomFlow(roomID: String) + case forwardedMessageToRoom(roomID: String) case displayRoomScreenWithFocussedPin(eventID: String) } @@ -149,7 +149,7 @@ class PinnedEventsTimelineFlowCoordinator: FlowCoordinatorProtocol { navigationStackCoordinator.setSheetCoordinator(nil) case .sent(let roomID): navigationStackCoordinator.setSheetCoordinator(nil) - actionsSubject.send(.startChildRoomFlow(roomID: roomID)) + actionsSubject.send(.forwardedMessageToRoom(roomID: roomID)) } } .store(in: &cancellables) diff --git a/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift index 5f16f4029d..fe4add059e 100644 --- a/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift @@ -72,9 +72,9 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { // periphery:ignore - used to avoid deallocation private var rolesAndPermissionsFlowCoordinator: RoomRolesAndPermissionsFlowCoordinator? // periphery:ignore - used to avoid deallocation - private var childRoomFlowCoordinator: RoomFlowCoordinator? - // periphery:ignore - used to avoid deallocation private var pinnedEventsTimelineFlowCoordinator: PinnedEventsTimelineFlowCoordinator? + // periphery:ignore - used to avoid deallocation + private var childRoomFlowCoordinator: RoomFlowCoordinator? private let stateMachine: StateMachine = .init(state: .initial) @@ -168,7 +168,7 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { } else if roomID != roomProxy.id { stateMachine.tryEvent(.startChildFlow(roomID: roomID, via: via, entryPoint: .eventID(eventID)), userInfo: EventUserInfo(animated: animated)) } else { - roomScreenCoordinator?.focusOnEvent(focusEvent: .init(eventID: eventID, shouldSetPin: false)) + roomScreenCoordinator?.focusOnEvent(.init(eventID: eventID, shouldSetPin: false)) } case .roomAlias, .childRoomAlias, .eventOnRoomAlias, .childEventOnRoomAlias: break // These are converted to a room ID route one level above. @@ -196,8 +196,8 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { switch room { case .joined(let roomProxy): await storeAndSubscribeToRoomProxy(roomProxy) - let focusEvent = focussedEventID.map { FocusEvent(eventID: $0, shouldSetPin: false) } - stateMachine.tryEvent(.presentRoom(focusEvent: focusEvent), userInfo: EventUserInfo(animated: animated)) + let focussedEvent = focussedEventID.map { FocusEvent(eventID: $0, shouldSetPin: false) } + stateMachine.tryEvent(.presentRoom(focussedEvent: focussedEvent), userInfo: EventUserInfo(animated: animated)) default: stateMachine.tryEvent(.presentJoinRoomScreen(via: via), userInfo: EventUserInfo(animated: animated)) } @@ -381,8 +381,8 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { case (_, .dismissJoinRoomScreen, .complete): dismissFlow(animated: animated) - case (_, .presentRoom(let focusEvent), .room): - Task { await self.presentRoom(fromState: context.fromState, focusEvent: focusEvent, animated: animated) } + case (_, .presentRoom(let focussedEvent), .room): + Task { await self.presentRoom(fromState: context.fromState, focussedEvent: focussedEvent, animated: animated) } case (_, .dismissFlow, .complete): dismissFlow(animated: animated) @@ -539,9 +539,9 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { /// Updates the navigation stack so it displays the timeline for the given room /// - Parameters: /// - fromState: The state that asked for the room presentation. - /// - focusEvent: An (optional) struct that contains the event ID that the timeline should be focussed around, and a boolean telling if such event should update the pinned events banner + /// - focussedEvent: An (optional) struct that contains the event ID that the timeline should be focussed around, and a boolean telling if such event should update the pinned events banner /// - animated: whether it should animate the transition - private func presentRoom(fromState: State, focusEvent: FocusEvent?, animated: Bool) async { + private func presentRoom(fromState: State, focussedEvent: FocusEvent?, animated: Bool) async { // If any sheets are presented dismiss them, rely on their dismissal callbacks to transition the state machine // through the correct states before presenting the room navigationStackCoordinator.setSheetCoordinator(nil) @@ -559,8 +559,8 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { // The room is already on the stack, no need to present it again // Check if we need to focus on an event - if let focusEvent { - roomScreenCoordinator?.focusOnEvent(focusEvent: focusEvent) + if let focussedEvent { + roomScreenCoordinator?.focusOnEvent(focussedEvent) } return @@ -579,7 +579,7 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { stateEventStringBuilder: RoomStateEventStringBuilder(userID: userID)) let timelineController = roomTimelineControllerFactory.buildRoomTimelineController(roomProxy: roomProxy, - initialFocussedEventID: focusEvent?.eventID, + initialFocussedEventID: focussedEvent?.eventID, timelineItemFactory: timelineItemFactory) self.timelineController = timelineController @@ -590,7 +590,7 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { let composerDraftService = ComposerDraftService(roomProxy: roomProxy, timelineItemfactory: timelineItemFactory) let parameters = RoomScreenCoordinatorParameters(roomProxy: roomProxy, - focusEvent: focusEvent, + focussedEvent: focussedEvent, timelineController: timelineController, mediaProvider: userSession.mediaProvider, mediaPlayerProvider: MediaPlayerProvider(), @@ -674,7 +674,7 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { if case let .joined(roomProxy) = await userSession.clientProxy.roomForIdentifier(roomID) { await storeAndSubscribeToRoomProxy(roomProxy) - stateMachine.tryEvent(.presentRoom(focusEvent: nil), userInfo: EventUserInfo(animated: animated)) + stateMachine.tryEvent(.presentRoom(focussedEvent: nil), userInfo: EventUserInfo(animated: animated)) analytics.trackJoinedRoom(isDM: roomProxy.isDirect, isSpace: roomProxy.isSpace, activeMemberCount: UInt(roomProxy.activeMembersCount)) } else { @@ -1337,16 +1337,18 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { return } - navigationStackCoordinator.setSheetCoordinator(nil) switch action { case .finished: - break + navigationStackCoordinator.setSheetCoordinator(nil) case .displayUser(let userID): + navigationStackCoordinator.setSheetCoordinator(nil) stateMachine.tryEvent(.presentRoomMemberDetails(userID: userID)) - case .startChildRoomFlow(let roomID): + case .forwardedMessageToRoom(let roomID): + navigationStackCoordinator.setSheetCoordinator(nil) stateMachine.tryEvent(.startChildFlow(roomID: roomID, via: [], entryPoint: .room)) case .displayRoomScreenWithFocussedPin(let eventID): - stateMachine.tryEvent(.presentRoom(focusEvent: .init(eventID: eventID, shouldSetPin: true))) + navigationStackCoordinator.setSheetCoordinator(nil) + stateMachine.tryEvent(.presentRoom(focussedEvent: .init(eventID: eventID, shouldSetPin: true))) } } .store(in: &cancellables) @@ -1447,7 +1449,7 @@ private extension RoomFlowCoordinator { case presentJoinRoomScreen(via: [String]) case dismissJoinRoomScreen - case presentRoom(focusEvent: FocusEvent?) + case presentRoom(focussedEvent: FocusEvent?) case dismissFlow case presentReportContent(itemID: TimelineItemIdentifier, senderID: String) diff --git a/ElementX/Sources/Screens/PinnedEventsTimelineScreen/PinnedEventsTimelineScreenCoordinator.swift b/ElementX/Sources/Screens/PinnedEventsTimelineScreen/PinnedEventsTimelineScreenCoordinator.swift index 8195a6ed51..fdc9daef7f 100644 --- a/ElementX/Sources/Screens/PinnedEventsTimelineScreen/PinnedEventsTimelineScreenCoordinator.swift +++ b/ElementX/Sources/Screens/PinnedEventsTimelineScreen/PinnedEventsTimelineScreenCoordinator.swift @@ -89,7 +89,7 @@ final class PinnedEventsTimelineScreenCoordinator: CoordinatorProtocol { actionsSubject.send(.displayRoomScreenWithFocussedPin(eventID: eventID)) // These other actions will not be handled in this view case .displayEmojiPicker, .displayReportContent, .displayCameraPicker, .displayMediaPicker, .displayDocumentPicker, .displayLocationPicker, .displayPollForm, .displayMediaUploadPreviewScreen, .composer, .hasScrolled: - break + fatalError("The action: \(action) should not be handled in this coordinator") } } .store(in: &cancellables) diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenCoordinator.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenCoordinator.swift index 1a971e671e..6f6435da78 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenCoordinator.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenCoordinator.swift @@ -21,7 +21,7 @@ import WysiwygComposer struct RoomScreenCoordinatorParameters { let roomProxy: JoinedRoomProxyProtocol - var focusEvent: FocusEvent? + var focussedEvent: FocusEvent? let timelineController: RoomTimelineControllerProtocol let mediaProvider: MediaProviderProtocol let mediaPlayerProvider: MediaPlayerProviderProtocol @@ -63,13 +63,13 @@ final class RoomScreenCoordinator: CoordinatorProtocol { } init(parameters: RoomScreenCoordinatorParameters) { - var selectedPinEventID: String? - if let focusEvent = parameters.focusEvent { - selectedPinEventID = focusEvent.shouldSetPin ? focusEvent.eventID : nil + var selectedPinnedEventID: String? + if let focussedEvent = parameters.focussedEvent { + selectedPinnedEventID = focussedEvent.shouldSetPin ? focussedEvent.eventID : nil } roomViewModel = RoomScreenViewModel(roomProxy: parameters.roomProxy, - initialSelectedPinEventID: selectedPinEventID, + initialSelectedPinnedEventID: selectedPinnedEventID, mediaProvider: parameters.mediaProvider, ongoingCallRoomIDPublisher: parameters.ongoingCallRoomIDPublisher, appMediator: parameters.appMediator, @@ -77,7 +77,7 @@ final class RoomScreenCoordinator: CoordinatorProtocol { analyticsService: ServiceLocator.shared.analytics) timelineViewModel = TimelineViewModel(roomProxy: parameters.roomProxy, - focussedEventID: parameters.focusEvent?.eventID, + focussedEventID: parameters.focussedEvent?.eventID, timelineController: parameters.timelineController, isPinnedEventsTimeline: false, mediaProvider: parameters.mediaProvider, @@ -141,8 +141,7 @@ final class RoomScreenCoordinator: CoordinatorProtocol { case .hasScrolled(direction: let direction): roomViewModel.timelineHasScrolled(direction: direction) case .viewInRoomTimeline: - // Not handled in this screen - break + fatalError("The action: \(action) should not be handled in this coordinator") } } .store(in: &cancellables) @@ -161,7 +160,7 @@ final class RoomScreenCoordinator: CoordinatorProtocol { switch actions { case .focusEvent(eventID: let eventID): - focusOnEvent(focusEvent: FocusEvent(eventID: eventID, shouldSetPin: false)) + focusOnEvent(FocusEvent(eventID: eventID, shouldSetPin: false)) case .displayPinnedEventsTimeline: actionsSubject.send(.presentPinnedEventsTimeline) case .displayRoomDetails: @@ -178,10 +177,10 @@ final class RoomScreenCoordinator: CoordinatorProtocol { composerViewModel.loadDraft() } - func focusOnEvent(focusEvent: FocusEvent) { - let eventID = focusEvent.eventID - if focusEvent.shouldSetPin { - roomViewModel.setSelectedPinEventID(eventID) + func focusOnEvent(_ focussedEvent: FocusEvent) { + let eventID = focussedEvent.eventID + if focussedEvent.shouldSetPin { + roomViewModel.setSelectedPinnedEventID(eventID) } Task { await timelineViewModel.focusOnEvent(eventID: eventID) } } diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift index 8e11e3106d..0430752f16 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift @@ -75,10 +75,10 @@ enum PinnedEventsBannerState: Equatable { } } - var selectedPinEventID: String? { + var selectedPinnedEventID: String? { switch self { case .loaded(let state): - return state.selectedPinEventID + return state.selectedPinnedEventID default: return nil } @@ -93,10 +93,10 @@ enum PinnedEventsBannerState: Equatable { } } - var selectedPinIndex: Int { + var selectedPinnedIndex: Int { switch self { case .loaded(let state): - return state.selectedPinIndex + return state.selectedPinnedIndex case .loading(let numbersOfEvents): // We always want the index to be the last one when loading, since is the default one. return numbersOfEvents - 1 @@ -108,12 +108,12 @@ enum PinnedEventsBannerState: Equatable { case .loading: return AttributedString(L10n.screenRoomPinnedBannerLoadingDescription) case .loaded(let state): - return state.selectedPinContent + return state.selectedPinnedContent } } var bannerIndicatorDescription: AttributedString { - let index = selectedPinIndex + 1 + let index = selectedPinnedIndex + 1 let boldPlaceholder = "{bold}" var finalString = AttributedString(L10n.screenRoomPinnedBannerIndicatorDescription(boldPlaceholder)) var boldString = AttributedString(L10n.screenRoomPinnedBannerIndicator(index, count)) @@ -136,18 +136,20 @@ enum PinnedEventsBannerState: Equatable { switch self { case .loading: // The default selected event should always be the last one. - self = .loaded(state: .init(pinnedEventContents: pinnedEventContents, selectedPinEventID: pinnedEventContents.keys.last)) + self = .loaded(state: .init(pinnedEventContents: pinnedEventContents, selectedPinnedEventID: pinnedEventContents.keys.last)) case .loaded(var state): state.pinnedEventContents = pinnedEventContents self = .loaded(state: state) } } - // Note that if we are setting this value, this is definitely sent from the pinned events timeline so we can assume that the pinned events timeline is already loaded, so we only need to set the selection for the loaded state - mutating func setSelectedPinEventID(_ eventID: String) { + // Note that if we are setting this value, this is definitely sent from the pinned events timeline + // so we can assume that the pinned events timeline is already loaded and we only need to set the + // selection for the loaded state + mutating func setSelectedPinnedEventID(_ eventID: String) { switch self { case .loaded(var state): - state.selectedPinEventID = eventID + state.selectedPinnedEventID = eventID self = .loaded(state: state) case .loading: break @@ -158,31 +160,31 @@ enum PinnedEventsBannerState: Equatable { struct PinnedEventsState: Equatable { var pinnedEventContents: OrderedDictionary = [:] { didSet { - if selectedPinEventID == nil, !pinnedEventContents.keys.isEmpty { + if selectedPinnedEventID == nil, !pinnedEventContents.keys.isEmpty { // The default selected event should always be the last one. - selectedPinEventID = pinnedEventContents.keys.last + selectedPinnedEventID = pinnedEventContents.keys.last } else if pinnedEventContents.isEmpty { - selectedPinEventID = nil - } else if let selectedPinEventID, !pinnedEventContents.keys.set.contains(selectedPinEventID) { - self.selectedPinEventID = pinnedEventContents.keys.last + selectedPinnedEventID = nil + } else if let selectedPinnedEventID, !pinnedEventContents.keys.set.contains(selectedPinnedEventID) { + self.selectedPinnedEventID = pinnedEventContents.keys.last } } } - var selectedPinEventID: String? + var selectedPinnedEventID: String? - var selectedPinIndex: Int { + var selectedPinnedIndex: Int { let defaultValue = pinnedEventContents.isEmpty ? 0 : pinnedEventContents.count - 1 - guard let selectedPinEventID else { + guard let selectedPinnedEventID else { return defaultValue } - return pinnedEventContents.keys.firstIndex(of: selectedPinEventID) ?? defaultValue + return pinnedEventContents.keys.firstIndex(of: selectedPinnedEventID) ?? defaultValue } - var selectedPinContent: AttributedString { + var selectedPinnedContent: AttributedString { var content = AttributedString(" ") - if let selectedPinEventID, - let pinnedEventContent = pinnedEventContents[selectedPinEventID] { + if let selectedPinnedEventID, + let pinnedEventContent = pinnedEventContents[selectedPinnedEventID] { content = pinnedEventContent } content.font = .compound.bodyMD @@ -194,12 +196,12 @@ struct PinnedEventsState: Equatable { guard !pinnedEventContents.isEmpty else { return } - let currentIndex = selectedPinIndex + let currentIndex = selectedPinnedIndex let nextIndex = currentIndex - 1 if nextIndex == -1 { - selectedPinEventID = pinnedEventContents.keys.last + selectedPinnedEventID = pinnedEventContents.keys.last } else { - selectedPinEventID = pinnedEventContents.keys[nextIndex % pinnedEventContents.count] + selectedPinnedEventID = pinnedEventContents.keys[nextIndex % pinnedEventContents.count] } } } diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift index 276b7036fa..b6083dce94 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift @@ -27,7 +27,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol private let appSettings: AppSettings private let analyticsService: AnalyticsService private let pinnedEventStringBuilder: RoomEventStringBuilder - private var initialSelectedPinEventID: String? + private var initialSelectedPinnedEventID: String? private let actionsSubject: PassthroughSubject = .init() var actions: AnyPublisher { @@ -53,7 +53,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol } init(roomProxy: JoinedRoomProxyProtocol, - initialSelectedPinEventID: String?, + initialSelectedPinnedEventID: String?, mediaProvider: MediaProviderProtocol, ongoingCallRoomIDPublisher: CurrentValuePublisher, appMediator: AppMediatorProtocol, @@ -63,7 +63,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol self.appMediator = appMediator self.appSettings = appSettings self.analyticsService = analyticsService - self.initialSelectedPinEventID = initialSelectedPinEventID + self.initialSelectedPinnedEventID = initialSelectedPinnedEventID pinnedEventStringBuilder = .pinnedEventStringBuilder(userID: roomProxy.ownUserID) super.init(initialViewState: .init(roomTitle: roomProxy.roomTitle, @@ -82,7 +82,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol override func process(viewAction: RoomScreenViewAction) { switch viewAction { case .tappedPinnedEventsBanner: - if let eventID = state.pinnedEventsBannerState.selectedPinEventID { + if let eventID = state.pinnedEventsBannerState.selectedPinnedEventID { actionsSubject.send(.focusEvent(eventID: eventID)) } state.pinnedEventsBannerState.previousPin() @@ -101,8 +101,8 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol state.lastScrollDirection = direction } - func setSelectedPinEventID(_ eventID: String) { - state.pinnedEventsBannerState.setSelectedPinEventID(eventID) + func setSelectedPinnedEventID(_ eventID: String) { + state.pinnedEventsBannerState.setSelectedPinnedEventID(eventID) } private func setupSubscriptions(ongoingCallRoomIDPublisher: CurrentValuePublisher) { @@ -169,10 +169,10 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol state.pinnedEventsBannerState.setPinnedEventContents(pinnedEventContents) - // If is the first time we are setting the pinned events, we should select the initial event if available. - if let initialSelectedPinEventID { - state.pinnedEventsBannerState.setSelectedPinEventID(initialSelectedPinEventID) - self.initialSelectedPinEventID = nil + // If it's the first time we are setting the pinned events, we should select the initial event if available. + if let initialSelectedPinnedEventID { + state.pinnedEventsBannerState.setSelectedPinnedEventID(initialSelectedPinnedEventID) + self.initialSelectedPinnedEventID = nil } } @@ -209,7 +209,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol extension RoomScreenViewModel { static func mock(roomProxyMock: JoinedRoomProxyMock) -> RoomScreenViewModel { RoomScreenViewModel(roomProxy: roomProxyMock, - initialSelectedPinEventID: nil, + initialSelectedPinnedEventID: nil, mediaProvider: MockMediaProvider(), ongoingCallRoomIDPublisher: .init(.init(nil)), appMediator: AppMediatorMock.default, diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModelProtocol.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModelProtocol.swift index 5e638fe212..899da6c4c3 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModelProtocol.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModelProtocol.swift @@ -22,5 +22,5 @@ protocol RoomScreenViewModelProtocol { var context: RoomScreenViewModel.Context { get } func timelineHasScrolled(direction: ScrollDirection) - func setSelectedPinEventID(_ eventID: String) + func setSelectedPinnedEventID(_ eventID: String) } diff --git a/ElementX/Sources/Screens/RoomScreen/View/PinnedItemsBanner/PinnedItemsBannerView.swift b/ElementX/Sources/Screens/RoomScreen/View/PinnedItemsBanner/PinnedItemsBannerView.swift index 8cb77c8b0f..38c2bcd57b 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/PinnedItemsBanner/PinnedItemsBannerView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/PinnedItemsBanner/PinnedItemsBannerView.swift @@ -38,7 +38,7 @@ struct PinnedItemsBannerView: View { Button { onMainButtonTap() } label: { HStack(spacing: 0) { HStack(spacing: 10) { - PinnedItemsIndicatorView(pinIndex: state.selectedPinIndex, pinsCount: state.count) + PinnedItemsIndicatorView(pinIndex: state.selectedPinnedIndex, pinsCount: state.count) .accessibilityHidden(true) CompoundIcon(\.pinSolid, size: .small, relativeTo: .compound.bodyMD) .foregroundColor(Color.compound.iconSecondaryAlpha) @@ -98,16 +98,16 @@ struct PinnedItemsBannerView_Previews: PreviewProvider, TestablePreview { PinnedItemsBannerView(state: .loaded(state: .init(pinnedEventContents: ["1": "Content", "2": "2", "3": "3"], - selectedPinEventID: "1")), + selectedPinnedEventID: "1")), onMainButtonTap: { }, onViewAllButtonTap: { }) PinnedItemsBannerView(state: .loaded(state: .init(pinnedEventContents: ["1": "Very very very very long content here", "2": "2"], - selectedPinEventID: "1")), + selectedPinnedEventID: "1")), onMainButtonTap: { }, onViewAllButtonTap: { }) PinnedItemsBannerView(state: .loaded(state: .init(pinnedEventContents: ["1": attributedContent], - selectedPinEventID: "1")), + selectedPinnedEventID: "1")), onMainButtonTap: { }, onViewAllButtonTap: { }) PinnedItemsBannerView(state: .loading(numbersOfEvents: 5), diff --git a/ElementX/Sources/UITests/UITestsAppCoordinator.swift b/ElementX/Sources/UITests/UITestsAppCoordinator.swift index 03bf186cbf..ec808c5c58 100644 --- a/ElementX/Sources/UITests/UITestsAppCoordinator.swift +++ b/ElementX/Sources/UITests/UITestsAppCoordinator.swift @@ -446,7 +446,7 @@ class MockScreen: Identifiable { let client = try UITestsSignalling.Client(mode: .app) client.signals.sink { [weak self] signal in guard case .timeline(.focusOnEvent(let eventID)) = signal else { return } - coordinator.focusOnEvent(focusEvent: .init(eventID: eventID, shouldSetPin: false)) + coordinator.focusOnEvent(.init(eventID: eventID, shouldSetPin: false)) try? client.send(.success) } .store(in: &cancellables) diff --git a/UnitTests/Sources/PinnedEventsBannerStateTests.swift b/UnitTests/Sources/PinnedEventsBannerStateTests.swift index 9e35c5664f..c4d22d1626 100644 --- a/UnitTests/Sources/PinnedEventsBannerStateTests.swift +++ b/UnitTests/Sources/PinnedEventsBannerStateTests.swift @@ -37,9 +37,9 @@ class PinnedEventsBannerStateTests: XCTestCase { XCTAssertTrue(state.isLoading) XCTAssertFalse(state.isEmpty) - XCTAssertNil(state.selectedPinEventID) + XCTAssertNil(state.selectedPinnedEventID) XCTAssertEqual(state.displayedMessage.string, L10n.screenRoomPinnedBannerLoadingDescription) - XCTAssertEqual(state.selectedPinIndex, 4) + XCTAssertEqual(state.selectedPinnedIndex, 4) XCTAssertEqual(state.count, 5) XCTAssertEqual(state.bannerIndicatorDescription.string, L10n.screenRoomPinnedBannerIndicatorDescription(L10n.screenRoomPinnedBannerIndicator(5, 5))) } @@ -48,42 +48,42 @@ class PinnedEventsBannerStateTests: XCTestCase { var state = PinnedEventsBannerState.loading(numbersOfEvents: 2) XCTAssertTrue(state.isLoading) state.setPinnedEventContents(["1": "test1", "2": "test2"]) - XCTAssertEqual(state, .loaded(state: .init(pinnedEventContents: ["1": "test1", "2": "test2"], selectedPinEventID: "2"))) + XCTAssertEqual(state, .loaded(state: .init(pinnedEventContents: ["1": "test1", "2": "test2"], selectedPinnedEventID: "2"))) XCTAssertFalse(state.isLoading) } func testLoaded() { - let state = PinnedEventsBannerState.loaded(state: .init(pinnedEventContents: ["1": "test1", "2": "test2"], selectedPinEventID: "2")) + let state = PinnedEventsBannerState.loaded(state: .init(pinnedEventContents: ["1": "test1", "2": "test2"], selectedPinnedEventID: "2")) XCTAssertFalse(state.isLoading) XCTAssertFalse(state.isEmpty) - XCTAssertEqual(state.selectedPinEventID, "2") + XCTAssertEqual(state.selectedPinnedEventID, "2") XCTAssertEqual(state.displayedMessage.string, "test2") - XCTAssertEqual(state.selectedPinIndex, 1) + XCTAssertEqual(state.selectedPinnedIndex, 1) XCTAssertEqual(state.count, 2) XCTAssertEqual(state.bannerIndicatorDescription.string, L10n.screenRoomPinnedBannerIndicatorDescription(L10n.screenRoomPinnedBannerIndicator(2, 2))) } func testPreviousPin() { - var state = PinnedEventsBannerState.loaded(state: .init(pinnedEventContents: ["1": "test1", "2": "test2", "3": "test3"], selectedPinEventID: "1")) - XCTAssertEqual(state.selectedPinEventID, "1") - XCTAssertEqual(state.selectedPinIndex, 0) + var state = PinnedEventsBannerState.loaded(state: .init(pinnedEventContents: ["1": "test1", "2": "test2", "3": "test3"], selectedPinnedEventID: "1")) + XCTAssertEqual(state.selectedPinnedEventID, "1") + XCTAssertEqual(state.selectedPinnedIndex, 0) XCTAssertEqual(state.displayedMessage.string, "test1") state.previousPin() - XCTAssertEqual(state.selectedPinEventID, "3") - XCTAssertEqual(state.selectedPinIndex, 2) + XCTAssertEqual(state.selectedPinnedEventID, "3") + XCTAssertEqual(state.selectedPinnedIndex, 2) XCTAssertEqual(state.displayedMessage.string, "test3") state.previousPin() - XCTAssertEqual(state.selectedPinEventID, "2") - XCTAssertEqual(state.selectedPinIndex, 1) + XCTAssertEqual(state.selectedPinnedEventID, "2") + XCTAssertEqual(state.selectedPinnedIndex, 1) XCTAssertEqual(state.displayedMessage.string, "test2") } func testSetContent() { - var state = PinnedEventsBannerState.loaded(state: .init(pinnedEventContents: ["1": "test1", "2": "test2", "3": "test3", "4": "test4"], selectedPinEventID: "2")) - XCTAssertEqual(state.selectedPinEventID, "2") - XCTAssertEqual(state.selectedPinIndex, 1) + var state = PinnedEventsBannerState.loaded(state: .init(pinnedEventContents: ["1": "test1", "2": "test2", "3": "test3", "4": "test4"], selectedPinnedEventID: "2")) + XCTAssertEqual(state.selectedPinnedEventID, "2") + XCTAssertEqual(state.selectedPinnedIndex, 1) XCTAssertEqual(state.displayedMessage.string, "test2") XCTAssertEqual(state.count, 4) XCTAssertFalse(state.isEmpty) @@ -91,8 +91,8 @@ class PinnedEventsBannerStateTests: XCTestCase { // let's remove the selected item state.setPinnedEventContents(["1": "test1", "3": "test3", "4": "test4"]) // new selected item is the new latest - XCTAssertEqual(state.selectedPinEventID, "4") - XCTAssertEqual(state.selectedPinIndex, 2) + XCTAssertEqual(state.selectedPinnedEventID, "4") + XCTAssertEqual(state.selectedPinnedIndex, 2) XCTAssertEqual(state.displayedMessage.string, "test4") XCTAssertEqual(state.count, 3) XCTAssertFalse(state.isEmpty) @@ -100,9 +100,9 @@ class PinnedEventsBannerStateTests: XCTestCase { // let's add a new item at the top state.setPinnedEventContents(["0": "test0", "1": "test1", "3": "test3", "4": "test4"]) // selected item doesn't change - XCTAssertEqual(state.selectedPinEventID, "4") + XCTAssertEqual(state.selectedPinnedEventID, "4") // but the index is updated - XCTAssertEqual(state.selectedPinIndex, 3) + XCTAssertEqual(state.selectedPinnedIndex, 3) XCTAssertEqual(state.displayedMessage.string, "test4") XCTAssertEqual(state.count, 4) XCTAssertFalse(state.isEmpty) @@ -110,9 +110,9 @@ class PinnedEventsBannerStateTests: XCTestCase { // let's add a new item at the bottom state.setPinnedEventContents(["0": "test0", "1": "test1", "3": "test3", "4": "test4", "5": "test5"]) // selected item doesn't change - XCTAssertEqual(state.selectedPinEventID, "4") + XCTAssertEqual(state.selectedPinnedEventID, "4") // and index stays the same - XCTAssertEqual(state.selectedPinIndex, 3) + XCTAssertEqual(state.selectedPinnedIndex, 3) XCTAssertEqual(state.displayedMessage.string, "test4") XCTAssertEqual(state.count, 5) XCTAssertFalse(state.isEmpty) @@ -120,12 +120,12 @@ class PinnedEventsBannerStateTests: XCTestCase { // set to tempty state.setPinnedEventContents([:]) XCTAssertTrue(state.isEmpty) - XCTAssertNil(state.selectedPinEventID) + XCTAssertNil(state.selectedPinnedEventID) // set to one item state.setPinnedEventContents(["6": "test6", "7": "test7"]) - XCTAssertEqual(state.selectedPinEventID, "7") - XCTAssertEqual(state.selectedPinIndex, 1) + XCTAssertEqual(state.selectedPinnedEventID, "7") + XCTAssertEqual(state.selectedPinnedIndex, 1) XCTAssertEqual(state.displayedMessage.string, "test7") XCTAssertEqual(state.count, 2) XCTAssertFalse(state.isEmpty) diff --git a/UnitTests/Sources/RoomScreenViewModelTests.swift b/UnitTests/Sources/RoomScreenViewModelTests.swift index 65f10532b3..bcc41d7cd6 100644 --- a/UnitTests/Sources/RoomScreenViewModelTests.swift +++ b/UnitTests/Sources/RoomScreenViewModelTests.swift @@ -43,7 +43,7 @@ class RoomScreenViewModelTests: XCTestCase { // setup the room proxy actions publisher roomProxyMock.underlyingActionsPublisher = updateSubject.eraseToAnyPublisher() let viewModel = RoomScreenViewModel(roomProxy: roomProxyMock, - initialSelectedPinEventID: nil, + initialSelectedPinnedEventID: nil, mediaProvider: MockMediaProvider(), ongoingCallRoomIDPublisher: .init(.init(nil)), appMediator: AppMediatorMock.default, @@ -68,7 +68,7 @@ class RoomScreenViewModelTests: XCTestCase { try await deferred.fulfill() XCTAssertTrue(viewModel.context.viewState.pinnedEventsBannerState.isLoading) XCTAssertTrue(viewModel.context.viewState.shouldShowPinnedEventsBanner) - XCTAssertEqual(viewModel.context.viewState.pinnedEventsBannerState.selectedPinIndex, 1) + XCTAssertEqual(viewModel.context.viewState.pinnedEventsBannerState.selectedPinnedIndex, 1) // setup the loaded pinned events injection in the timeline let pinnedTimelineMock = TimelineProxyMock() @@ -87,7 +87,7 @@ class RoomScreenViewModelTests: XCTestCase { try await deferred.fulfill() XCTAssertEqual(viewModel.context.viewState.pinnedEventsBannerState.count, 2) XCTAssertTrue(viewModel.context.viewState.shouldShowPinnedEventsBanner) - XCTAssertEqual(viewModel.context.viewState.pinnedEventsBannerState.selectedPinIndex, 1) + XCTAssertEqual(viewModel.context.viewState.pinnedEventsBannerState.selectedPinnedIndex, 1) // check if the banner is updating alongside the timeline deferred = deferFulfillment(viewModel.context.$viewState) { viewState in @@ -99,7 +99,7 @@ class RoomScreenViewModelTests: XCTestCase { try await deferred.fulfill() XCTAssertFalse(viewModel.context.viewState.pinnedEventsBannerState.isLoading) XCTAssertTrue(viewModel.context.viewState.shouldShowPinnedEventsBanner) - XCTAssertEqual(viewModel.context.viewState.pinnedEventsBannerState.selectedPinIndex, 1) + XCTAssertEqual(viewModel.context.viewState.pinnedEventsBannerState.selectedPinnedIndex, 1) // check how the scrolling changes the banner visibility viewModel.timelineHasScrolled(direction: .top) @@ -124,7 +124,7 @@ class RoomScreenViewModelTests: XCTestCase { .event(.init(item: EventTimelineItemSDKMock(configuration: .init(eventID: "test3")), id: "3"))] roomProxyMock.underlyingPinnedEventsTimeline = pinnedTimelineMock let viewModel = RoomScreenViewModel(roomProxy: roomProxyMock, - initialSelectedPinEventID: "test1", + initialSelectedPinnedEventID: "test1", mediaProvider: MockMediaProvider(), ongoingCallRoomIDPublisher: .init(.init(nil)), appMediator: AppMediatorMock.default, @@ -140,11 +140,11 @@ class RoomScreenViewModelTests: XCTestCase { XCTAssertEqual(viewModel.context.viewState.pinnedEventsBannerState.count, 3) XCTAssertTrue(viewModel.context.viewState.shouldShowPinnedEventsBanner) // And that is actually displaying the `initialSelectedPinEventID` which is gthe first one in the list - XCTAssertEqual(viewModel.context.viewState.pinnedEventsBannerState.selectedPinIndex, 0) + XCTAssertEqual(viewModel.context.viewState.pinnedEventsBannerState.selectedPinnedIndex, 0) // check if the banner scrolls when tapping the previous pin deferred = deferFulfillment(viewModel.context.$viewState) { viewState in - viewState.pinnedEventsBannerState.selectedPinIndex == 2 + viewState.pinnedEventsBannerState.selectedPinnedIndex == 2 } let deferredAction = deferFulfillment(viewModel.actions) { action in if case let .focusEvent(eventID) = action { @@ -158,9 +158,9 @@ class RoomScreenViewModelTests: XCTestCase { // check if the banner scrolls to the specific selected pin deferred = deferFulfillment(viewModel.context.$viewState) { viewState in - viewState.pinnedEventsBannerState.selectedPinIndex == 1 + viewState.pinnedEventsBannerState.selectedPinnedIndex == 1 } - viewModel.setSelectedPinEventID("test2") + viewModel.setSelectedPinnedEventID("test2") try await deferred.fulfill() } @@ -171,7 +171,7 @@ class RoomScreenViewModelTests: XCTestCase { roomProxyMock.canUserJoinCallUserIDReturnValue = .success(false) roomProxyMock.underlyingActionsPublisher = updateSubject.eraseToAnyPublisher() let viewModel = RoomScreenViewModel(roomProxy: roomProxyMock, - initialSelectedPinEventID: nil, + initialSelectedPinnedEventID: nil, mediaProvider: MockMediaProvider(), ongoingCallRoomIDPublisher: .init(.init(nil)), appMediator: AppMediatorMock.default, @@ -208,7 +208,7 @@ class RoomScreenViewModelTests: XCTestCase { let ongoingCallRoomIDSubject = CurrentValueSubject(nil) let roomProxyMock = JoinedRoomProxyMock(.init(id: "MyRoomID")) let viewModel = RoomScreenViewModel(roomProxy: roomProxyMock, - initialSelectedPinEventID: nil, + initialSelectedPinnedEventID: nil, mediaProvider: MockMediaProvider(), ongoingCallRoomIDPublisher: ongoingCallRoomIDSubject.asCurrentValuePublisher(), appMediator: AppMediatorMock.default,