Skip to content

Commit

Permalink
PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@Velin92
Velin92 committed Aug 22, 2024
1 parent 970719b commit 115c0af
Show file tree
Hide file tree
Showing 12 changed files with 118 additions and 114 deletions.
3 changes: 2 additions & 1 deletion ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"originHash" : "1b1fb14b1ac1438c44ffd2f0151af65c83018d71b8674cd7f13ec21abeb31bcd",
"pins" : [
{
"identity" : "compound-design-tokens",
Expand Down Expand Up @@ -288,5 +289,5 @@
}
}
],
"version" : 2
"version" : 3
}
4 changes: 2 additions & 2 deletions ElementX/Sources/FlowCoordinators/PinnedEventsTimelineFlowCoordinator.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
Expand Down
40 changes: 21 additions & 19 deletions ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<State, Event> = .init(state: .initial)

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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

Expand All @@ -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(),
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion ...tX/Sources/Screens/PinnedEventsTimelineScreen/PinnedEventsTimelineScreenCoordinator.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
25 changes: 12 additions & 13 deletions ElementX/Sources/Screens/RoomScreen/RoomScreenCoordinator.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -63,21 +63,21 @@ 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,
appSettings: ServiceLocator.shared.settings,
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,
Expand Down Expand Up @@ -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)
Expand All @@ -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:
Expand All @@ -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) }
}
Expand Down
52 changes: 27 additions & 25 deletions ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
Expand All @@ -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))
Expand All @@ -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
Expand All @@ -158,31 +160,31 @@ enum PinnedEventsBannerState: Equatable {
struct PinnedEventsState: Equatable {
var pinnedEventContents: OrderedDictionary<String, AttributedString> = [:] {
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
Expand All @@ -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]
}
}
}
Loading

0 comments on commit 115c0af

Please sign in to comment.