vector-im/element-x-ios/issues/53 - Various tweaks following code review

This commit is contained in:
Stefan Ceriu
2022-05-25 16:51:48 +03:00
committed by Stefan Ceriu
parent 73f507045b
commit 8fec97217f
23 changed files with 172 additions and 126 deletions
@@ -99,14 +99,15 @@ class StateStoreViewModel<State: BindableState, ViewAction> {
self.context = Context(initialViewState: initialViewState)
self.context.viewActions.sink { [weak self] action in
guard let self = self else { return }
self.process(viewAction: action)
Task { await self.process(viewAction: action) }
}
.store(in: &cancellables)
}
/// Override to handles incoming `ViewAction`s from the `ViewModel`.
/// - Parameter viewAction: The `ViewAction` to be processed in `ViewModel` implementation.
func process(viewAction: ViewAction) {
func process(viewAction: ViewAction) async {
// Default implementation, -no-op
}
}
@@ -17,8 +17,7 @@
import SwiftUI
import Combine
typealias HomeScreenViewModelType = StateStoreViewModel<HomeScreenViewState,
HomeScreenViewAction>
typealias HomeScreenViewModelType = StateStoreViewModel<HomeScreenViewState, HomeScreenViewAction>
class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol {
@@ -44,7 +43,7 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
// MARK: - Public
override func process(viewAction: HomeScreenViewAction) {
override func process(viewAction: HomeScreenViewAction) async {
switch viewAction {
case .logout:
completion?(.logout)
@@ -100,9 +99,7 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
return
}
Task {
await roomSummary.loadDetails()
}
Task { await roomSummary.loadDetails() }
}
private func buildOrUpdateRoomFromSummary(_ roomSummary: RoomSummaryProtocol) -> HomeScreenRoom {
@@ -16,8 +16,7 @@
import SwiftUI
typealias LoginScreenViewModelType = StateStoreViewModel<LoginScreenViewState,
LoginScreenViewAction>
typealias LoginScreenViewModelType = StateStoreViewModel<LoginScreenViewState, LoginScreenViewAction>
class LoginScreenViewModel: LoginScreenViewModelType, LoginScreenViewModelProtocol {
@@ -38,7 +37,7 @@ class LoginScreenViewModel: LoginScreenViewModelType, LoginScreenViewModelProtoc
// MARK: - Public
override func process(viewAction: LoginScreenViewAction) {
override func process(viewAction: LoginScreenViewAction) async {
switch viewAction {
case .login:
completion?(.login((username: context.username, password: context.password)))
@@ -16,8 +16,7 @@
import SwiftUI
typealias RoomScreenViewModelType = StateStoreViewModel<RoomScreenViewState,
RoomScreenViewAction>
typealias RoomScreenViewModelType = StateStoreViewModel<RoomScreenViewState, RoomScreenViewAction>
class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol {
@@ -26,12 +25,12 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
}
private let timelineController: RoomTimelineControllerProtocol
private let timelineViewFactory: RoomTimelineViewFactory
private let timelineViewFactory: RoomTimelineViewFactoryProtocol
// MARK: - Setup
init(timelineController: RoomTimelineControllerProtocol,
timelineViewFactory: RoomTimelineViewFactory,
timelineViewFactory: RoomTimelineViewFactoryProtocol,
roomName: String?) {
self.timelineController = timelineController
self.timelineViewFactory = timelineViewFactory
@@ -41,20 +40,20 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
timelineController.callbacks
.receive(on: DispatchQueue.main)
.sink { [weak self] callback in
guard let self = self else { return }
switch callback {
case .updatedTimelineItems:
self.buildTimelineViews()
case .updatedTimelineItem(let itemId):
guard let timelineItem = self.timelineController.timelineItems.first(where: { $0.id == itemId }),
let viewIndex = self.state.items.firstIndex(where: { $0.id == itemId }) else {
return
}
guard let self = self else { return }
self.state.items[viewIndex] = timelineViewFactory.buildTimelineViewFor(timelineItem)
}
}.store(in: &cancellables)
switch callback {
case .updatedTimelineItems:
self.buildTimelineViews()
case .updatedTimelineItem(let itemId):
guard let timelineItem = self.timelineController.timelineItems.first(where: { $0.id == itemId }),
let viewIndex = self.state.items.firstIndex(where: { $0.id == itemId }) else {
return
}
self.state.items[viewIndex] = timelineViewFactory.buildTimelineViewFor(timelineItem: timelineItem)
}
}.store(in: &cancellables)
state.contextMenuBuilder = buildContexMenuForItemId(_:)
@@ -63,31 +62,29 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
// MARK: - Public
override func process(viewAction: RoomScreenViewAction) {
Task {
switch viewAction {
case .loadPreviousPage:
state.isBackPaginating = true
switch await timelineController.paginateBackwards(Constants.backPaginationPageSize) {
default:
state.isBackPaginating = false
}
case .itemAppeared(let id):
await timelineController.processItemAppearance(id)
case .itemDisappeared(let id):
await timelineController.processItemDisappearance(id)
case .linkClicked(let url):
MXLog.warning("Link clicked: \(url)")
case .sendMessage:
guard state.bindings.composerText.count > 0 else {
return
}
await timelineController.sendMessage(state.bindings.composerText)
state.bindings.composerText = ""
override func process(viewAction: RoomScreenViewAction) async {
switch viewAction {
case .loadPreviousPage:
state.isBackPaginating = true
switch await timelineController.paginateBackwards(Constants.backPaginationPageSize) {
default:
state.isBackPaginating = false
}
case .itemAppeared(let id):
await timelineController.processItemAppearance(id)
case .itemDisappeared(let id):
await timelineController.processItemDisappearance(id)
case .linkClicked(let url):
MXLog.warning("Link clicked: \(url)")
case .sendMessage:
guard state.bindings.composerText.count > 0 else {
return
}
await timelineController.sendMessage(state.bindings.composerText)
state.bindings.composerText = ""
}
}
@@ -95,7 +92,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
private func buildTimelineViews() {
let stateItems = timelineController.timelineItems.map { item in
timelineViewFactory.buildTimelineViewFor(item)
timelineViewFactory.buildTimelineViewFor(timelineItem: item)
}
state.items = stateItems
@@ -60,9 +60,7 @@ class UserSession {
Benchmark.startTrackingForIdentifier("ClientSync", message: "Started sync.")
client.startSync()
Task {
await updateRooms()
}
Task { await updateRooms() }
}
var userIdentifier: String {
@@ -83,7 +81,8 @@ class UserSession {
return .failure(.failedRetrievingDisplayName)
}
}.value
}
.value
}
func loadUserAvatarURL() async -> Result<String, UserSessionError> {
@@ -94,7 +93,8 @@ class UserSession {
} catch {
return .failure(.failedRetrievingDisplayName)
}
}.value
}
.value
}
// MARK: ClientDelegate
@@ -116,7 +116,7 @@ class UserSession {
Benchmark.endTrackingForIdentifier("ClientRooms", message: "Retrieved \(sdkRooms.count) rooms")
Benchmark.startTrackingForIdentifier("ProcessingRooms", message: "Started processing \(sdkRooms.count) rooms")
let diff = sdkRooms.map({ $0.id()}).difference(from: currentRooms.map({ $0.id }))
let diff = sdkRooms.map({ $0.id() }).difference(from: currentRooms.map(\.id))
for change in diff {
switch change {
@@ -125,7 +125,7 @@ class UserSession {
MXLog.error("Failed retrieving sdk room with id: \(id)")
break
}
currentRooms.append(RoomProxy(room: sdkRoom, messageFactory: RoomMessageFactory()))
currentRooms.append(RoomProxy(room: sdkRoom, roomMessageFactory: RoomMessageFactory()))
case .remove(_, let id, _):
currentRooms.removeAll { $0.id == id }
}
@@ -73,6 +73,7 @@ struct MediaProvider: MediaProviderProtocol {
MXLog.error("Failed retrieving image with error: \(error)")
return .failure(.failedRetrievingImage)
}
}.value
}
.value
}
}
@@ -11,10 +11,6 @@ import UIKit
struct MockMediaProvider: MediaProviderProtocol {
func loadCurrentUserAvatar(_ completion: @escaping (Result<UIImage?, MediaProviderError>) -> Void) {
}
func imageFromSource(_ source: MediaSource?) -> UIImage? {
return nil
}
@@ -26,11 +22,7 @@ struct MockMediaProvider: MediaProviderProtocol {
func imageFromURL(_ url: String?) -> UIImage? {
return nil
}
func loadImageFromURL(_ url: String, _ completion: @escaping (Result<UIImage, MediaProviderError>) -> Void) {
}
func loadImageFromURL(_ url: String) async -> Result<UIImage, MediaProviderError> {
return .failure(.failedRetrievingImage)
}
@@ -9,7 +9,7 @@
import Foundation
import MatrixRustSDK
struct RoomMessageFactory {
struct RoomMessageFactory: RoomMessageFactoryProtocol {
func buildRoomMessageFrom(_ message: AnyMessage) -> RoomMessageProtocol {
if let textMessage = message.textMessage() {
return TextRoomMessage(message: textMessage)
@@ -0,0 +1,14 @@
//
// RoomMessageFactoryProtocol.swift
// ElementX
//
// Created by Stefan Ceriu on 26/05/2022.
// Copyright © 2022 element.io. All rights reserved.
//
import Foundation
import MatrixRustSDK
protocol RoomMessageFactoryProtocol {
func buildRoomMessageFrom(_ message: AnyMessage) -> RoomMessageProtocol
}
+15 -10
View File
@@ -27,7 +27,7 @@ private class WeakRoomProxyWrapper: RoomDelegate {
class RoomProxy: RoomProxyProtocol {
private let room: Room
private let messageFactory: RoomMessageFactory
private let roomMessageFactory: RoomMessageFactoryProtocol
private var backwardStream: BackwardsStreamProtocol?
@@ -37,9 +37,9 @@ class RoomProxy: RoomProxyProtocol {
private(set) var messages: [RoomMessageProtocol]
init(room: Room, messageFactory: RoomMessageFactory) {
init(room: Room, roomMessageFactory: RoomMessageFactoryProtocol) {
self.room = room
self.messageFactory = messageFactory
self.roomMessageFactory = roomMessageFactory
messages = []
room.setDelegate(delegate: WeakRoomProxyWrapper(roomProxy: self))
@@ -91,7 +91,8 @@ class RoomProxy: RoomProxyProtocol {
} catch {
return .failure(.failedRetrievingMemberAvatarURL)
}
}.value
}
.value
}
func loadDisplayNameForUserId(_ userId: String) async -> Result<String?, RoomProxyError> {
@@ -102,7 +103,8 @@ class RoomProxy: RoomProxyProtocol {
} catch {
return .failure(.failedRetrievingMemberDisplayName)
}
}.value
}
.value
}
func loadDisplayName() async -> Result<String, RoomProxyError> {
@@ -119,7 +121,8 @@ class RoomProxy: RoomProxyProtocol {
} catch {
return .failure(.failedRetrievingDisplayName)
}
}.value
}
.value
}
func paginateBackwards(count: UInt) async -> Result<Void, RoomProxyError> {
@@ -133,13 +136,14 @@ class RoomProxy: RoomProxyProtocol {
Benchmark.endTrackingForIdentifier("BackPagination \(self.id)", message: "Finished backpaginating \(count) message(s) in room \(self.id)")
let messages = sdkMessages.map { message in
self.messageFactory.buildRoomMessageFrom(message)
self.roomMessageFactory.buildRoomMessageFrom(message)
}.reversed()
self.messages.insert(contentsOf: messages, at: 0)
return .success(())
}.value
}
.value
}
func sendMessage(_ message: String) async -> Result<Void, RoomProxyError> {
@@ -153,13 +157,14 @@ class RoomProxy: RoomProxyProtocol {
} catch {
return .failure(.failedSendingMessage)
}
}.value
}
.value
}
// MARK: - Private
fileprivate func appendMessage(_ message: AnyMessage) {
let message = self.messageFactory.buildRoomMessageFrom(message)
let message = roomMessageFactory.buildRoomMessageFrom(message)
messages.append(message)
callbacks.send(.updatedMessages)
}
@@ -16,7 +16,7 @@ struct EventBriefFactory: EventBriefFactoryProtocol {
self.memberDetailProvider = memberDetailProvider
}
func eventBriefForMessage(_ message: RoomMessageProtocol?) async -> EventBrief? {
func buildEventBriefFor(message: RoomMessageProtocol?) async -> EventBrief? {
guard let message = message else {
return nil
}
@@ -37,7 +37,7 @@ struct EventBriefFactory: EventBriefFactoryProtocol {
// MARK: - Private
private func buildEventBrief(message: RoomMessageProtocol, htmlBody: String?) async -> EventBrief? {
private func buildEventBrief(message: RoomMessageProtocol, htmlBody: String?) async -> EventBrief? {
switch await memberDetailProvider.loadDisplayNameForUserId(message.sender) {
case .success(let displayName):
return EventBrief(eventId: message.id,
@@ -10,5 +10,5 @@ import Foundation
@MainActor
protocol EventBriefFactoryProtocol {
func eventBriefForMessage(_ message: RoomMessageProtocol?) async -> EventBrief?
func buildEventBriefFor(message: RoomMessageProtocol?) async -> EventBrief?
}
@@ -30,7 +30,7 @@ struct MockRoomSummary: RoomSummaryProtocol {
var avatar: UIImage?
func loadDetails() {
func loadDetails() async {
}
@@ -48,19 +48,19 @@ class RoomSummary: RoomSummaryProtocol {
private(set) var displayName: String? {
didSet {
self.callbacks.send(.updatedData)
callbacks.send(.updatedData)
}
}
private(set) var lastMessage: EventBrief? {
didSet {
self.callbacks.send(.updatedData)
callbacks.send(.updatedData)
}
}
private(set) var avatar: UIImage? {
didSet {
self.callbacks.send(.updatedData)
callbacks.send(.updatedData)
}
}
@@ -72,7 +72,7 @@ class RoomSummary: RoomSummaryProtocol {
self.eventBriefFactory = eventBriefFactory
Task {
lastMessage = await eventBriefFactory.eventBriefForMessage(roomProxy.messages.last)
lastMessage = await eventBriefFactory.buildEventBriefFor(message: roomProxy.messages.last)
}
roomProxy.callbacks
@@ -85,7 +85,7 @@ class RoomSummary: RoomSummaryProtocol {
switch callback {
case .updatedMessages:
Task {
self.lastMessage = await eventBriefFactory.eventBriefForMessage(roomProxy.messages.last)
self.lastMessage = await eventBriefFactory.buildEventBriefFor(message: roomProxy.messages.last)
}
}
}
@@ -98,13 +98,13 @@ class RoomSummary: RoomSummaryProtocol {
}
await withTaskGroup(of: Void.self) { group in
group.addTask(priority: .medium) {
group.addTask {
await self.loadDisplayName()
}
group.addTask(priority: .medium) {
group.addTask {
await self.loadAvatar()
}
group.addTask(priority: .medium) {
group.addTask {
await self.loadLastMessage()
}
}
@@ -143,7 +143,7 @@ class RoomSummary: RoomSummaryProtocol {
switch await roomProxy.paginateBackwards(count: 1) {
case .success:
lastMessage = await eventBriefFactory.eventBriefForMessage(roomProxy.messages.last)
lastMessage = await eventBriefFactory.buildEventBriefFor(message: roomProxy.messages.last)
case .failure(let error):
MXLog.error("Failed back paginating with error: \(error)")
}
@@ -12,7 +12,7 @@ import UIKit
class RoomTimelineController: RoomTimelineControllerProtocol {
private let timelineProvider: RoomTimelineProviderProtocol
private let timelineItemFactory: RoomTimelineItemFactory
private let timelineItemFactory: RoomTimelineItemFactoryProtocol
private let mediaProvider: MediaProviderProtocol
private let memberDetailProvider: MemberDetailProviderProtocol
@@ -23,7 +23,7 @@ class RoomTimelineController: RoomTimelineControllerProtocol {
private(set) var timelineItems = [RoomTimelineItemProtocol]()
init(timelineProvider: RoomTimelineProviderProtocol,
timelineItemFactory: RoomTimelineItemFactory,
timelineItemFactory: RoomTimelineItemFactoryProtocol,
mediaProvider: MediaProviderProtocol,
memberDetailProvider: MemberDetailProviderProtocol) {
self.timelineProvider = timelineProvider
@@ -35,13 +35,13 @@ class RoomTimelineController: RoomTimelineControllerProtocol {
.callbacks
.receive(on: DispatchQueue.main)
.sink { [weak self] callback in
guard let self = self else { return }
switch callback {
case .updatedMessages:
self.updateTimelineItems()
}
}.store(in: &cancellables)
guard let self = self else { return }
switch callback {
case .updatedMessages:
self.updateTimelineItems()
}
}.store(in: &cancellables)
updateTimelineItems()
@@ -59,7 +59,7 @@ class RoomTimelineController: RoomTimelineControllerProtocol {
}
func processItemAppearance(_ itemId: String) async {
guard let timelineItem = self.timelineItems.filter({ $0.id == itemId}).first else {
guard let timelineItem = timelineItems.first(where: { $0.id == itemId }) else {
return
}
@@ -98,8 +98,8 @@ class RoomTimelineController: RoomTimelineControllerProtocol {
var newTimelineItems = [RoomTimelineItemProtocol]()
var previousMessage: RoomMessageProtocol?
for message in self.timelineProvider.messages {
let areMessagesFromTheSameDay = self.haveSameDay(lhs: previousMessage, rhs: message)
for message in timelineProvider.messages {
let areMessagesFromTheSameDay = haveSameDay(lhs: previousMessage, rhs: message)
let shouldAddSectionHeader = !areMessagesFromTheSameDay
if shouldAddSectionHeader {
@@ -110,14 +110,14 @@ class RoomTimelineController: RoomTimelineControllerProtocol {
let areMessagesFromTheSameSender = (previousMessage?.sender == message.sender)
let shouldShowSenderDetails = !areMessagesFromTheSameSender || !areMessagesFromTheSameDay
newTimelineItems.append(timelineItemFactory.buildTimelineItemFor(message, showSenderDetails: shouldShowSenderDetails))
newTimelineItems.append(timelineItemFactory.buildTimelineItemFor(message: message, showSenderDetails: shouldShowSenderDetails))
previousMessage = message
}
self.timelineItems = newTimelineItems
timelineItems = newTimelineItems
self.callbacks.send(.updatedTimelineItems)
callbacks.send(.updatedTimelineItems)
}
private func haveSameDay(lhs: RoomMessageProtocol?, rhs: RoomMessageProtocol?) -> Bool {
@@ -1,5 +1,5 @@
//
// TimelineItemFactory.swift
// RoomTimelineItemFactory.swift
// ElementX
//
// Created by Stefan Ceriu on 16/03/2022.
@@ -9,8 +9,7 @@
import Foundation
import UIKit
@MainActor
struct RoomTimelineItemFactory {
struct RoomTimelineItemFactory: RoomTimelineItemFactoryProtocol {
private let mediaProvider: MediaProviderProtocol
private let memberDetailProvider: MemberDetailProviderProtocol
private let attributedStringBuilder: AttributedStringBuilderProtocol
@@ -23,12 +22,12 @@ struct RoomTimelineItemFactory {
self.attributedStringBuilder = attributedStringBuilder
}
func buildTimelineItemFor(_ roomMessage: RoomMessageProtocol, showSenderDetails: Bool) -> RoomTimelineItemProtocol {
let displayName = memberDetailProvider.displayNameForUserId(roomMessage.sender)
let avatarURL = memberDetailProvider.avatarURLForUserId(roomMessage.sender)
func buildTimelineItemFor(message: RoomMessageProtocol, showSenderDetails: Bool) -> RoomTimelineItemProtocol {
let displayName = memberDetailProvider.displayNameForUserId(message.sender)
let avatarURL = memberDetailProvider.avatarURLForUserId(message.sender)
let avatarImage = mediaProvider.imageFromURL(avatarURL)
switch roomMessage {
switch message {
case let message as TextRoomMessage:
return buildTextTimelineItemFromMessage(message, showSenderDetails, displayName, avatarImage)
case let message as ImageRoomMessage:
@@ -0,0 +1,14 @@
//
// RoomTimelineItemFactoryProtocol.swift
// ElementX
//
// Created by Stefan Ceriu on 26/05/2022.
// Copyright © 2022 element.io. All rights reserved.
//
import Foundation
@MainActor
protocol RoomTimelineItemFactoryProtocol {
func buildTimelineItemFor(message: RoomMessageProtocol, showSenderDetails: Bool) -> RoomTimelineItemProtocol
}
@@ -1,5 +1,5 @@
//
// TimelineViewFactory.swift
// RoomTimelineViewFactory.swift
// ElementX
//
// Created by Stefan Ceriu on 16/03/2022.
@@ -8,9 +8,8 @@
import Foundation
@MainActor
struct RoomTimelineViewFactory {
func buildTimelineViewFor(_ timelineItem: RoomTimelineItemProtocol) -> RoomTimelineViewProvider {
struct RoomTimelineViewFactory: RoomTimelineViewFactoryProtocol {
func buildTimelineViewFor(timelineItem: RoomTimelineItemProtocol) -> RoomTimelineViewProvider {
switch timelineItem {
case let item as TextRoomTimelineItem:
return .text(item)
@@ -0,0 +1,14 @@
//
// RoomTimelineViewFactoryProtocol.swift
// ElementX
//
// Created by Stefan Ceriu on 26/05/2022.
// Copyright © 2022 element.io. All rights reserved.
//
import Foundation
@MainActor
protocol RoomTimelineViewFactoryProtocol {
func buildTimelineViewFor(timelineItem: RoomTimelineItemProtocol) -> RoomTimelineViewProvider
}
+1 -1
View File
@@ -22,7 +22,7 @@ class UITestsAppCoordinator: Coordinator {
let screens = mockScreens()
let rootView = UITestsRootView(mockScreens: screens) { id in
guard let screen = screens.filter({ $0.id == id }).first else {
guard let screen = screens.first(where: { $0.id == id }) else {
fatalError()
}