ehealthsolutions/element-x-ios
3
0
Fork 0
mirror of https://github.com/Novaloop-AG/element-x-ios.git synced 2026-11-04 00:49:44 +00:00
Code Issues Packages Projects Releases Wiki Activity

Room details: improve members count UX (#1050)

Browse Source 
* Inject user id into room’s details

* Refactor member fetch logic in RoomDetailsScreenViewModel

* Refactor DI

* Fetch members in RoomMembersListScreenViewModel

* Fetch members in InviteUsersScreen

* Use joinedMembersCount in room members list

* Cleanup

* Limit members refresh

* Cleanup binding

* Fix UT build errors

* Fix RoomDetailsScreenViewModelTests

* Fix RoomMembersListScreenViewModelTests

* Fix UTs

* Fix RoomDetailsScreenUITests

* Refine RoomDetailsScreenViewModelTests

* Add roomProxy extension

* PR comments

* Refactor room’s counter to Int

* Improve updateMembers() performances

* Fix memory leaks
This commit is contained in:
Alfonso Grillo
2023-06-09 16:21:01 +02:00
committed by GitHub
parent 42e083dcaf
commit 63df68fd29
27 changed files with 368 additions and 292 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenCoordinator.swift
+25 -21
View File
@@ -18,7 +18,8 @@ import Combine
import SwiftUI
struct RoomDetailsScreenCoordinatorParameters {
let navigationStackCoordinator: NavigationStackCoordinator
let accountUserID: String
weak var navigationStackCoordinator: NavigationStackCoordinator?
let roomProxy: RoomProxyProtocol
let mediaProvider: MediaProviderProtocol
let userDiscoveryService: UserDiscoveryServiceProtocol
@@ -33,7 +34,7 @@ final class RoomDetailsScreenCoordinator: CoordinatorProtocol {
private var viewModel: RoomDetailsScreenViewModelProtocol
private var cancellables: Set<AnyCancellable> = .init()
private let selectedUsers: CurrentValueSubject<[UserProfileProxy], Never> = .init([])
private var navigationStackCoordinator: NavigationStackCoordinator {
private var navigationStackCoordinator: NavigationStackCoordinator? {
parameters.navigationStackCoordinator
}
@@ -42,7 +43,8 @@ final class RoomDetailsScreenCoordinator: CoordinatorProtocol {
init(parameters: RoomDetailsScreenCoordinatorParameters) {
self.parameters = parameters
viewModel = RoomDetailsScreenViewModel(roomProxy: parameters.roomProxy,
viewModel = RoomDetailsScreenViewModel(accountUserID: parameters.accountUserID,
roomProxy: parameters.roomProxy,
mediaProvider: parameters.mediaProvider)
}
@@ -53,10 +55,10 @@ final class RoomDetailsScreenCoordinator: CoordinatorProtocol {
guard let self else { return }
switch action {
case .requestMemberDetailsPresentation(let members):
self.presentRoomMembersList(members)
case .requestInvitePeoplePresentation(let members):
self.presentInviteUsersScreen(members: members)
case .requestMemberDetailsPresentation:
self.presentRoomMembersList()
case .requestInvitePeoplePresentation:
self.presentInviteUsersScreen()
case .leftRoom:
self.callback?(.leftRoom)
case .requestEditDetailsPresentation(let accountOwner):
@@ -71,32 +73,32 @@ final class RoomDetailsScreenCoordinator: CoordinatorProtocol {
// MARK: - Private
private func presentRoomMembersList(_ members: [RoomMemberProxyProtocol]) {
private func presentRoomMembersList() {
let params = RoomMembersListScreenCoordinatorParameters(navigationStackCoordinator: navigationStackCoordinator,
mediaProvider: parameters.mediaProvider,
members: members)
roomProxy: parameters.roomProxy)
let coordinator = RoomMembersListScreenCoordinator(parameters: params)
coordinator.callback = { [weak self] action in
switch action {
case .invite:
self?.presentInviteUsersScreen(members: members)
self?.presentInviteUsersScreen()
}
}
navigationStackCoordinator.push(coordinator)
navigationStackCoordinator?.push(coordinator)
}
private func presentInviteUsersScreen(members: [RoomMemberProxyProtocol]) {
let navigationStackCoordinator = NavigationStackCoordinator()
let userIndicatorController = UserIndicatorController(rootCoordinator: navigationStackCoordinator)
private func presentInviteUsersScreen() {
let inviteUsersStackCoordinator = NavigationStackCoordinator()
let userIndicatorController = UserIndicatorController(rootCoordinator: inviteUsersStackCoordinator)
let inviteParameters = InviteUsersScreenCoordinatorParameters(selectedUsers: .init(selectedUsers),
roomType: .room(members: members, userIndicatorController: userIndicatorController),
roomType: .room(roomProxy: parameters.roomProxy),
mediaProvider: parameters.mediaProvider,
userDiscoveryService: parameters.userDiscoveryService)
userDiscoveryService: parameters.userDiscoveryService, userIndicatorController: userIndicatorController)
let coordinator = InviteUsersScreenCoordinator(parameters: inviteParameters)
navigationStackCoordinator.setRootCoordinator(coordinator)
inviteUsersStackCoordinator.setRootCoordinator(coordinator)
coordinator.actions.sink { [weak self] result in
guard let self else { return }
@@ -112,7 +114,9 @@ final class RoomDetailsScreenCoordinator: CoordinatorProtocol {
}
.store(in: &cancellables)
parameters.navigationStackCoordinator.setSheetCoordinator(userIndicatorController)
parameters.navigationStackCoordinator?.setSheetCoordinator(userIndicatorController) { [weak self] in
self?.selectedUsers.value = []
}
}
private func presentRoomDetailsEditScreen(accountOwner: RoomMemberProxyProtocol) {
@@ -129,14 +133,14 @@ final class RoomDetailsScreenCoordinator: CoordinatorProtocol {
roomDetailsEditCoordinator.actions.sink { [weak self] action in
switch action {
case .dismiss:
self?.parameters.navigationStackCoordinator.setSheetCoordinator(nil)
self?.navigationStackCoordinator?.setSheetCoordinator(nil)
}
}
.store(in: &cancellables)
navigationStackCoordinator.setRootCoordinator(roomDetailsEditCoordinator)
parameters.navigationStackCoordinator.setSheetCoordinator(userIndicatorController)
self.navigationStackCoordinator?.setSheetCoordinator(userIndicatorController)
}
private func toggleUser(_ user: UserProfileProxy) {
@@ -150,7 +154,7 @@ final class RoomDetailsScreenCoordinator: CoordinatorProtocol {
}
private func inviteUsers(_ users: [String], in room: RoomProxyProtocol) {
navigationStackCoordinator.setSheetCoordinator(nil)
navigationStackCoordinator?.setSheetCoordinator(nil)
Task {
let result: Result<Void, RoomProxyError> = await withTaskGroup(of: Result<Void, RoomProxyError>.self) { group in
ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenModels.swift
+5 -13
View File
@@ -22,8 +22,8 @@ import UIKit
// MARK: View model
enum RoomDetailsScreenViewModelAction {
case requestMemberDetailsPresentation([RoomMemberProxyProtocol])
case requestInvitePeoplePresentation([RoomMemberProxyProtocol])
case requestMemberDetailsPresentation
case requestInvitePeoplePresentation
case leftRoom
case requestEditDetailsPresentation(RoomMemberProxyProtocol)
}
@@ -35,22 +35,18 @@ struct RoomDetailsScreenViewState: BindableState {
let canonicalAlias: String?
let isEncrypted: Bool
let isDirect: Bool
let permalink: URL?
var title = ""
var topic: String?
var avatarURL: URL?
let permalink: URL?
var members: [RoomMemberDetails] = []
var joinedMembersCount = 0
var joinedMembersCount: Int
var isProcessingIgnoreRequest = false
var canInviteUsers = false
var canEditRoomName = false
var canEditRoomTopic = false
var canEditRoomAvatar = false
var isLoadingMembers: Bool {
members.isEmpty
}
var canEdit: Bool {
!isDirect && (canEditRoomName || canEditRoomTopic || canEditRoomAvatar)
}
@@ -62,10 +58,6 @@ struct RoomDetailsScreenViewState: BindableState {
var bindings: RoomDetailsScreenViewStateBindings
var dmRecipient: RoomMemberDetails?
private var isDMRoom: Bool {
isEncrypted && isDirect && members.count == 2
}
}
struct RoomDetailsScreenViewStateBindings {
ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModel.swift
+57 -72
View File
@@ -20,35 +20,36 @@ import SwiftUI
typealias RoomDetailsScreenViewModelType = StateStoreViewModel<RoomDetailsScreenViewState, RoomDetailsScreenViewAction>
class RoomDetailsScreenViewModel: RoomDetailsScreenViewModelType, RoomDetailsScreenViewModelProtocol {
private let accountUserID: String
private let roomProxy: RoomProxyProtocol
private var members: [RoomMemberProxyProtocol] = []
private var accountOwner: RoomMemberProxyProtocol? {
didSet { updatePowerLevelPermissions() }
}
private var dmRecipient: RoomMemberProxyProtocol?
private var accountOwner: RoomMemberProxyProtocol?
@CancellableTask
private var buildMembersDetailsTask: Task<Void, Never>?
var callback: ((RoomDetailsScreenViewModelAction) -> Void)?
init(roomProxy: RoomProxyProtocol,
init(accountUserID: String,
roomProxy: RoomProxyProtocol,
mediaProvider: MediaProviderProtocol) {
self.accountUserID = accountUserID
self.roomProxy = roomProxy
super.init(initialViewState: .init(roomId: roomProxy.id,
canonicalAlias: roomProxy.canonicalAlias,
isEncrypted: roomProxy.isEncrypted,
isDirect: roomProxy.isDirect,
permalink: roomProxy.permalink,
title: roomProxy.roomTitle,
topic: roomProxy.topic,
avatarURL: roomProxy.avatarURL,
permalink: roomProxy.permalink,
joinedMembersCount: roomProxy.joinedMembersCount,
bindings: .init()),
imageProvider: mediaProvider)
setupSubscriptions()
Task {
await roomProxy.updateMembers()
}
setupRoomSubscription()
fetchMembers()
}
// MARK: - Public
@@ -57,12 +58,11 @@ class RoomDetailsScreenViewModel: RoomDetailsScreenViewModelType, RoomDetailsScr
override func process(viewAction: RoomDetailsScreenViewAction) {
switch viewAction {
case .processTapPeople:
callback?(.requestMemberDetailsPresentation(members))
callback?(.requestMemberDetailsPresentation)
case .processTapInvite:
callback?(.requestInvitePeoplePresentation(members))
callback?(.requestInvitePeoplePresentation)
case .processTapLeave:
let joinedMembers = members.filter { $0.membership == .join }
guard joinedMembers.count > 1 else {
guard state.joinedMembersCount > 1 else {
state.bindings.leaveRoomAlertItem = LeaveRoomAlertItem(roomId: roomProxy.id, state: .empty)
return
}
@@ -88,7 +88,7 @@ class RoomDetailsScreenViewModel: RoomDetailsScreenViewModelType, RoomDetailsScr
// MARK: - Private
private func setupSubscriptions() {
private func setupRoomSubscription() {
switch roomProxy.registerTimelineListenerIfNeeded() {
case .success, .failure(.roomListenerAlreadyRegistered):
break
@@ -96,32 +96,6 @@ class RoomDetailsScreenViewModel: RoomDetailsScreenViewModelType, RoomDetailsScr
MXLog.error("Failed to register a room listener in room's details for the room \(roomProxy.id)")
}
roomProxy.membersPublisher
.sink { [weak self] members in
guard let self else { return }
buildMembersDetailsTask = Task {
let roomMembersDetails = await self.buildMembersDetails(members: members)
guard !Task.isCancelled else { return }
if self.roomProxy.isDirect, self.roomProxy.isEncrypted, members.count == 2 {
self.dmRecipient = members.first(where: { !$0.isAccountOwner })
}
self.state.members = roomMembersDetails.members
self.state.joinedMembersCount = roomMembersDetails.joinedMembersCount
self.state.dmRecipient = self.dmRecipient.map(RoomMemberDetails.init(withProxy:))
self.state.canInviteUsers = roomMembersDetails.accountOwner?.canInviteUsers ?? false
self.state.canEditRoomName = roomMembersDetails.accountOwner?.canSendStateEvent(type: .roomName) ?? false
self.state.canEditRoomTopic = roomMembersDetails.accountOwner?.canSendStateEvent(type: .roomTopic) ?? false
self.state.canEditRoomAvatar = roomMembersDetails.accountOwner?.canSendStateEvent(type: .roomAvatar) ?? false
self.members = members
self.accountOwner = roomMembersDetails.accountOwner
}
}
.store(in: &cancellables)
roomProxy.updatesPublisher
.throttle(for: .milliseconds(200), scheduler: DispatchQueue.main, latest: true)
.sink { [weak self] _ in
@@ -129,33 +103,50 @@ class RoomDetailsScreenViewModel: RoomDetailsScreenViewModelType, RoomDetailsScr
self.state.title = self.roomProxy.roomTitle
self.state.topic = self.roomProxy.topic
self.state.avatarURL = self.roomProxy.avatarURL
self.state.joinedMembersCount = self.roomProxy.joinedMembersCount
}
.store(in: &cancellables)
}
private func buildMembersDetails(members: [RoomMemberProxyProtocol]) async -> RoomMembersDetails {
await Task.detached {
// accessing RoomMember's properties is very slow. We need to do it in a background thread.
var roomMembersDetails: [RoomMemberDetails] = []
var accountOwner: RoomMemberProxyProtocol?
var joinedMembersCount = 0
roomMembersDetails.reserveCapacity(members.count)
for member in members {
roomMembersDetails.append(RoomMemberDetails(withProxy: member))
if member.membership == .join {
joinedMembersCount += 1
}
if accountOwner == nil, member.isAccountOwner {
accountOwner = member
}
}
return .init(members: roomMembersDetails, joinedMembersCount: joinedMembersCount, accountOwner: accountOwner)
private func fetchMembers() {
Task {
await fetchMembersIfNeeded()
await fetchAccountOwner()
}
.value
}
private func fetchMembersIfNeeded() async {
// We need to fetch members just in 1-to-1 chat to get the member object for the other person
guard roomProxy.isEncryptedOneToOneRoom else {
return
}
roomProxy.membersPublisher
.sink { [weak self] members in
guard let self else { return }
let dmRecipient = members.first(where: { !$0.isAccountOwner })
self.dmRecipient = dmRecipient
self.state.dmRecipient = dmRecipient.map(RoomMemberDetails.init(withProxy:))
}
.store(in: &cancellables)
await roomProxy.updateMembers()
}
private func fetchAccountOwner() async {
switch await roomProxy.getMember(userID: accountUserID) {
case .success(let member):
accountOwner = member
case .failure(let error):
MXLog.error("Failed (error: \(error) to get account owner member with id: \(accountUserID), in room: \(roomProxy.id)")
}
}
private func updatePowerLevelPermissions() {
state.canInviteUsers = accountOwner?.canInviteUsers ?? false
state.canEditRoomName = accountOwner?.canSendStateEvent(type: .roomName) ?? false
state.canEditRoomTopic = accountOwner?.canSendStateEvent(type: .roomTopic) ?? false
state.canEditRoomAvatar = accountOwner?.canSendStateEvent(type: .roomAvatar) ?? false
}
private static let leaveRoomLoadingID = "LeaveRoomLoading"
@@ -196,9 +187,3 @@ class RoomDetailsScreenViewModel: RoomDetailsScreenViewModelType, RoomDetailsScr
}
}
}
private struct RoomMembersDetails {
let members: [RoomMemberDetails]
let joinedMembersCount: Int
let accountOwner: RoomMemberProxyProtocol?
}
ElementX/Sources/Screens/RoomDetailsScreen/View/RoomDetailsScreen.swift
+8 -11
View File
@@ -137,13 +137,9 @@ struct RoomDetailsScreen: View {
context.send(viewAction: .processTapPeople)
} label: {
LabeledContent {
if context.viewState.isLoadingMembers {
ProgressView()
} else {
Text(String(context.viewState.joinedMembersCount))
.foregroundColor(.element.tertiaryContent)
.font(.compound.bodyLG)
}
Text(String(context.viewState.joinedMembersCount))
.foregroundColor(.element.tertiaryContent)
.font(.compound.bodyLG)
} label: {
Label(L10n.commonPeople, systemImage: "person")
}
@@ -160,9 +156,8 @@ struct RoomDetailsScreen: View {
}
}
.listRowSeparatorTint(.element.quinaryContent)
.buttonStyle(FormButtonStyle(accessory: context.viewState.isLoadingMembers ? nil : .navigationLink))
.buttonStyle(FormButtonStyle(accessory: .navigationLink))
.foregroundColor(.element.primaryContent)
.disabled(context.viewState.isLoadingMembers)
}
@ViewBuilder
@@ -257,7 +252,8 @@ struct RoomDetailsScreen_Previews: PreviewProvider {
canonicalAlias: "#alias:domain.com",
members: members))
return RoomDetailsScreenViewModel(roomProxy: roomProxy,
return RoomDetailsScreenViewModel(accountUserID: "@owner:somewhere.com",
roomProxy: roomProxy,
mediaProvider: MockMediaProvider())
}()
@@ -274,7 +270,8 @@ struct RoomDetailsScreen_Previews: PreviewProvider {
canonicalAlias: "#alias:domain.com",
members: members))
return RoomDetailsScreenViewModel(roomProxy: roomProxy,
return RoomDetailsScreenViewModel(accountUserID: "@owner:somewhere.com",
roomProxy: roomProxy,
mediaProvider: MockMediaProvider())
}()