Display members of a space #4629
Conversation
…dular and reusable this is required since we will need to reuse this module also in the space settings, and later we could also replace it in the RoomFlowCoordinator.
Generated by 🚫 Danger Swift against e962de3 |
|
…ne a follow UP should do the refactor.
|
| case presentingChild(childSpaceID: String, previousState: State) | ||
| /// A room flow is in progress | ||
| case roomFlow(previousState: State) | ||
| /// A members flow in progress |
There was a problem hiding this comment.
| /// A members flow in progress | |
| /// A members flow is in progress |
There was a problem hiding this comment.
I don't think this one is used right? Maybe we could remove it from this PR for simplicity?
There was a problem hiding this comment.
Looks like this should be reverted 🙂
|
|
||
| private func startMembersFlow() async { | ||
| guard case let .space(spaceRoomListProxy) = entryPoint, | ||
| case let .joined(roomProxy) = await flowParameters.userSession.clientProxy.roomForIdentifier(spaceRoomListProxy.id) else { |
There was a problem hiding this comment.
It would be much nicer for the SpaceScreen to include the JoinedRoomProxy in its displayMembers action. That's what is happening when you present a subspace and means that if you fail to get the proxy, you can show a nice error on the screen itself.
| if context.viewState.isSpaceJoined { | ||
| Button { context.send(viewAction: .displayMembers) } label: { | ||
| Label(L10n.screenSpaceMenuActionMembers, icon: \.user) | ||
| } | ||
| } |
There was a problem hiding this comment.
Then this thing could become
| if context.viewState.isSpaceJoined { | |
| Button { context.send(viewAction: .displayMembers) } label: { | |
| Label(L10n.screenSpaceMenuActionMembers, icon: \.user) | |
| } | |
| } | |
| if let roomProxy = context.viewState.roomProxy { | |
| Button { context.send(viewAction: .displayMembers(roomProxy)) } label: { | |
| Label(L10n.screenSpaceMenuActionMembers, icon: \.user) | |
| } | |
| } |
| // TODO: Implement | ||
| } | ||
|
|
||
| private func configureStateMachine() { |
There was a problem hiding this comment.
I would love for use to re-write this using the same Machine.addRoutes(event:transitions:handler:) and Machine.addRouteMapping(_:handler:) that we use in the SpaceFlowCoordinator. It's so much cleaner to have the route and the handler in the same place.
| case .proceed: | ||
| break | ||
| case .invite(let users): | ||
| inviteUsers(users, in: roomProxy) | ||
| case .toggleUser(let user): | ||
| var selectedUsers = selectedUsersSubject.value | ||
|
|
||
| if let index = selectedUsers.firstIndex(where: { $0.userID == user.userID }) { | ||
| selectedUsers.remove(at: index) | ||
| } else { | ||
| selectedUsers.append(user) | ||
| } | ||
|
|
||
| selectedUsersSubject.send(selectedUsers) |
There was a problem hiding this comment.
Would you mind moving these cases into the view model itself where they belong? Then we can remove inviteUsers showLoadingIndicator and hideLoadingIndicator from this flow.
| case presentRoomMemberDetails(userID: String) | ||
| case dismissRoomMemberDetails | ||
|
|
||
| case presentInviteUsersScreen | ||
| case dismissInviteUsersScreen | ||
|
|
||
| case presentUserProfile(userID: String) | ||
| case dismissUserProfile |
There was a problem hiding this comment.
| case presentRoomMemberDetails(userID: String) | |
| case dismissRoomMemberDetails | |
| case presentInviteUsersScreen | |
| case dismissInviteUsersScreen | |
| case presentUserProfile(userID: String) | |
| case dismissUserProfile | |
| case presentRoomMemberDetails(userID: String) | |
| case dismissedRoomMemberDetails | |
| case presentInviteUsersScreen | |
| case dismissedInviteUsersScreen | |
| case presentUserProfile(userID: String) | |
| case dismissedUserProfile |
| // TODO: Implement | ||
| // Not sure wha to do here since this can be presented also internally by the space flow | ||
| // stateMachine.tryEvent(.startChildFlow, userInfo: SpaceFlowCoordinatorEntryPoint.space(spaceRoomListProxy)) |
There was a problem hiding this comment.
Ok this is not possible since only DMs can be opened up from this flow, so no need to handle it.
| func clearRoute(animated: Bool) { | ||
| // TODO: Implement | ||
| } |
There was a problem hiding this comment.
Switch state machine and clear the route accordingly (check the SpaceFlowCoordinator)
2 participants
fixes #4632
This PR allows spaces to present a their members.
Most of the PR is about extracting the existing logic of navigating through members into its own RoomMembersFlowCoordinator so that it can be reused multiple times (since it's used in the standard, room in the space, and in the future will also be used in the Space settings flow).
As of right now the flow coordinator implementation is only used in the space, but a follow up PR will soon be opened when I'll also replace the existing code present in the
RoomFlowCoordinatorto use this flow coordinator too.Things left to do in this PR:
continueWithSpaceaction if we are already inside a space.