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

Room list fixes (#431)

Browse Source 
* Fixes #396 - Avoid duplicates in room list, cleanup logic throughout

* Re-enabled the allRoomsView with a hard 500 room limit and reduced the diffing collection time.

* Fixes #385, #411 - Filter out spaces, invites and tombstoned rooms from the room list

* Various tweaks following code review
This commit is contained in:
Stefan Ceriu
2023-01-10 17:34:42 +02:00
committed by GitHub
parent edc06bf662
commit 3d1266ca25
9 changed files with 151 additions and 107 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift
+12 -4
View File
@@ -70,8 +70,8 @@ struct HomeScreenViewState: BindableState {
var bindings = HomeScreenViewStateBindings()
var placeholderRooms: [HomeScreenRoom] {
(1...10).map { index in
HomeScreenRoom.placeholder(id: "\(index)")
(1...10).map { _ in
HomeScreenRoom.placeholder()
}
}
}
@@ -83,8 +83,13 @@ struct HomeScreenViewStateBindings {
}
struct HomeScreenRoom: Identifiable, Equatable {
/// The list item identifier can be a real room identifier, a custom one for invalidated entries
/// or a completely unique one for empty items and skeletons
let id: String
/// The real room identifier this item points to
let roomId: String?
let name: String
let hasUnreads: Bool
@@ -93,12 +98,15 @@ struct HomeScreenRoom: Identifiable, Equatable {
var lastMessage: AttributedString?
var avatarURLString: String?
var avatar: UIImage?
var isPlaceholder = false
static func placeholder(id: String) -> HomeScreenRoom {
HomeScreenRoom(id: id,
static func placeholder() -> HomeScreenRoom {
HomeScreenRoom(id: UUID().uuidString,
roomId: nil,
name: "Placeholder room name",
hasUnreads: false,
timestamp: "Now",
ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift
+35 -38
View File
@@ -24,7 +24,6 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
private let visibleRoomsSummaryProvider: RoomSummaryProviderProtocol?
private let allRoomsSummaryProvider: RoomSummaryProviderProtocol?
private let attributedStringBuilder: AttributedStringBuilderProtocol
private var roomsForIdentifiers = [String: HomeScreenRoom]()
var callback: ((HomeScreenViewModelAction) -> Void)?
@@ -139,28 +138,21 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
// MARK: - Private
private func loadDataForRoomIdentifier(_ identifier: String) {
guard let visibleRoomsSummaryProvider else {
MXLog.error("Room summary provider unavailable")
return
}
guard let roomSummary = visibleRoomsSummaryProvider.roomListPublisher.value.first(where: { $0.asFilled?.id == identifier })?.asFilled,
let roomIndex = state.rooms.firstIndex(where: { $0.id == identifier }) else {
return
}
var room = state.rooms[roomIndex]
guard room.avatar == nil,
let avatarURLString = roomSummary.avatarURLString else {
guard let room = state.rooms.first(where: { $0.roomId == identifier }),
room.avatar == nil,
let avatarURLString = room.avatarURLString else {
return
}
Task {
if case let .success(image) = await userSession.mediaProvider.loadImageFromURLString(avatarURLString, avatarSize: .room(on: .home)) {
guard let roomIndex = state.rooms.firstIndex(where: { $0.roomId == identifier }) else {
return
}
var room = state.rooms[roomIndex]
room.avatar = image
state.rooms[roomIndex] = room
roomsForIdentifiers[roomSummary.id] = room
}
}
}
@@ -175,46 +167,51 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
}
var rooms = [HomeScreenRoom]()
var newRoomsForIdentifiers = [String: HomeScreenRoom]()
for (index, summary) in visibleRoomsSummaryProvider.roomListPublisher.value.enumerated() {
switch summary {
case .empty(let id):
guard case let .filled(summary) = allRoomsSummaryProvider?.roomListPublisher.value[safe: index] else {
rooms.append(HomeScreenRoom.placeholder(id: id))
case .empty:
guard let allRoomsRoomSummary = allRoomsSummaryProvider?.roomListPublisher.value[safe: index] else {
rooms.append(HomeScreenRoom.placeholder())
continue
}
let room = buildRoomForSummary(summary)
switch allRoomsRoomSummary {
case .empty:
rooms.append(HomeScreenRoom.placeholder())
case .filled(let details), .invalidated(let details):
let room = buildRoom(with: details, invalidated: true)
rooms.append(room)
}
case .filled(let details):
let room = buildRoom(with: details, invalidated: false)
rooms.append(room)
newRoomsForIdentifiers[summary.id] = room
case .filled(let summary):
let room = buildRoomForSummary(summary)
case .invalidated(let details):
let room = buildRoom(with: details, invalidated: true)
rooms.append(room)
newRoomsForIdentifiers[summary.id] = room
}
}
state.rooms = rooms
roomsForIdentifiers = newRoomsForIdentifiers
}
private func buildRoomForSummary(_ summary: RoomSummaryDetails) -> HomeScreenRoom {
let oldRoom = roomsForIdentifiers[summary.id]
let avatarImage = userSession.mediaProvider.imageFromURLString(summary.avatarURLString, avatarSize: .room(on: .home))
private func buildRoom(with details: RoomSummaryDetails, invalidated: Bool) -> HomeScreenRoom {
let avatarImage = userSession.mediaProvider.imageFromURLString(details.avatarURLString, avatarSize: .room(on: .home))
var timestamp: String?
if let lastMessageTimestamp = summary.lastMessageTimestamp {
if let lastMessageTimestamp = details.lastMessageTimestamp {
timestamp = lastMessageTimestamp.formatted(date: .omitted, time: .shortened)
}
return HomeScreenRoom(id: summary.id,
name: summary.name,
hasUnreads: summary.unreadNotificationCount > 0,
timestamp: timestamp ?? oldRoom?.timestamp,
lastMessage: summary.lastMessage ?? oldRoom?.lastMessage,
avatar: avatarImage ?? oldRoom?.avatar)
let identifier = invalidated ? "invalidated-" + details.id : details.id
return HomeScreenRoom(id: identifier,
roomId: details.id,
name: details.name,
hasUnreads: details.unreadNotificationCount > 0,
timestamp: timestamp,
lastMessage: details.lastMessage,
avatarURLString: details.avatarURLString,
avatar: avatarImage)
}
private func updateVisibleRange(visibleItemIdentifiers items: Set<String>) {
ElementX/Sources/Screens/HomeScreen/View/HomeScreenRoomCell.swift
+20 -14
View File
@@ -24,7 +24,9 @@ struct HomeScreenRoomCell: View {
var body: some View {
Button {
context.send(viewAction: .selectRoom(roomIdentifier: room.id))
if let roomId = room.roomId {
context.send(viewAction: .selectRoom(roomIdentifier: roomId))
}
} label: {
HStack(spacing: 16.0) {
if let avatar = room.avatar {
@@ -35,7 +37,7 @@ struct HomeScreenRoomCell: View {
.clipShape(Circle())
.accessibilityHidden(true)
} else {
PlaceholderAvatarImage(text: room.name, contentId: room.id)
PlaceholderAvatarImage(text: room.name, contentId: room.roomId)
.clipShape(Circle())
.frame(width: avatarSize, height: avatarSize)
.accessibilityHidden(true)
@@ -86,7 +88,9 @@ struct HomeScreenRoomCell: View {
.frame(minHeight: 64.0)
.accessibilityElement(children: .combine)
.task {
context.send(viewAction: .loadRoomData(roomIdentifier: room.id))
if let roomId = room.roomId {
context.send(viewAction: .loadRoomData(roomIdentifier: roomId))
}
}
}
.accessibilityIdentifier("roomName:\(room.name)")
@@ -113,27 +117,29 @@ struct HomeScreenRoomCell_Previews: PreviewProvider {
static var previews: some View {
body.tint(.element.accent)
}
static var body: some View {
let summaryProvider = MockRoomSummaryProvider(state: .loaded)
let userSession = MockUserSession(clientProxy: MockClientProxy(userIdentifier: "John Doe", roomSummaryProvider: summaryProvider),
mediaProvider: MockMediaProvider())
let viewModel = HomeScreenViewModel(userSession: userSession,
attributedStringBuilder: AttributedStringBuilder())
let rooms: [HomeScreenRoom] = summaryProvider.roomListPublisher.value.compactMap { summary in
guard let summary = summary.asFilled else {
let rooms: [HomeScreenRoom] = summaryProvider.roomListPublisher.value.compactMap { summary -> HomeScreenRoom? in
switch summary {
case .empty:
return nil
case .filled(let details), .invalidated(let details):
return HomeScreenRoom(id: UUID().uuidString,
roomId: details.id,
name: details.name,
hasUnreads: details.unreadNotificationCount > 0,
timestamp: Date.now.formatted(date: .omitted, time: .shortened))
}
return HomeScreenRoom(id: summary.id,
name: summary.name,
hasUnreads: summary.unreadNotificationCount > 0,
timestamp: Date.now.formatted(date: .omitted, time: .shortened))
}
return VStack {
ForEach(rooms) { room in
HomeScreenRoomCell(room: room, context: viewModel.context)
ElementX/Sources/Services/BugReport/BugReportService.swift
+1 -1
View File
@@ -141,7 +141,7 @@ class BugReportService: BugReportServiceProtocol {
}
return result
} // swiftlint: enable function_body_length
}
// MARK: - Private
ElementX/Sources/Services/Client/ClientProxy.swift
+25 -9
View File
@@ -72,6 +72,7 @@ class ClientProxy: ClientProxyProtocol {
let callbacks = PassthroughSubject<ClientProxyCallback, Never>()
// swiftlint:disable:next function_body_length
init(client: ClientProtocol,
backgroundTaskService: BackgroundTaskServiceProtocol) async {
self.client = client
@@ -80,15 +81,29 @@ class ClientProxy: ClientProxyProtocol {
attributes: .concurrent)
mediaProxy = MediaProxy(client: client,
clientQueue: clientQueue)
await Task.dispatch(on: clientQueue) {
do {
let slidingSyncBuilder = try client.slidingSync().homeserver(url: ServiceLocator.shared.settings.slidingSyncProxyBaseURLString)
let requiredState = [RequiredState(key: "m.room.avatar", value: ""),
RequiredState(key: "m.room.encryption", value: "")]
let filters = SlidingSyncRequestListFilters(isDm: nil,
spaces: [],
isEncrypted: nil,
isInvite: false,
isTombstoned: false,
roomTypes: [],
notRoomTypes: ["m.space"],
roomNameLike: nil,
tags: [],
notTags: [])
let visibleRoomsView = try SlidingSyncViewBuilder()
.timelineLimit(limit: 30)
.requiredState(requiredState: [RequiredState(key: "m.room.avatar", value: ""),
RequiredState(key: "m.room.encryption", value: "")])
.timelineLimit(limit: 20)
.requiredState(requiredState: requiredState)
.filters(filters: filters)
.name(name: "CurrentlyVisibleRooms")
.syncMode(mode: .selective)
.addRange(from: 0, to: 20)
@@ -96,16 +111,17 @@ class ClientProxy: ClientProxyProtocol {
let allRoomsView = try SlidingSyncViewBuilder()
.noTimelineLimit()
.requiredState(requiredState: [RequiredState(key: "m.room.avatar", value: ""),
RequiredState(key: "m.room.encryption", value: "")])
.requiredState(requiredState: requiredState)
.filters(filters: filters)
.name(name: "AllRooms")
.syncMode(mode: .growingFullSync)
.batchSize(batchSize: 20)
.batchSize(batchSize: 100)
.roomLimit(limit: 500)
.build()
let slidingSync = try slidingSyncBuilder
.addView(v: visibleRoomsView)
// .addView(v: allRoomsView) // FIXME: Intentionally disabled as it doesn't work properly
.addView(v: allRoomsView)
.withCommonExtensions()
.coldCache(name: "ElementX")
.build()
ElementX/Sources/Services/Client/MockClientProxy.swift
+8 -4
View File
@@ -43,12 +43,16 @@ class MockClientProxy: ClientProxyProtocol {
func restartSync() { }
func roomForIdentifier(_ identifier: String) async -> RoomProxyProtocol? {
guard let room = visibleRoomsSummaryProvider?.roomListPublisher.value.first(where: { $0.id == identifier }),
let displayName = room.asFilled?.name else {
guard let room = visibleRoomsSummaryProvider?.roomListPublisher.value.first(where: { $0.id == identifier }) else {
return nil
}
return MockRoomProxy(displayName: displayName)
switch room {
case .empty:
return MockRoomProxy(displayName: "Empty room")
case .filled(let details), .invalidated(let details):
return MockRoomProxy(displayName: details.name)
}
}
func loadUserDisplayName() async -> Result<String, ClientProxyError> {
ElementX/Sources/Services/Room/RoomSummary/MockRoomSummaryProvider.swift
+1 -1
View File
@@ -71,6 +71,6 @@ class MockRoomSummaryProvider: RoomSummaryProviderProtocol {
lastMessage: try? AttributedString(markdown: "**@mock:client.com**: T-bone beef ribs bacon"),
lastMessageTimestamp: .distantPast,
unreadNotificationCount: 0)),
.empty(id: "3")
.empty
]
}
ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift
+41 -22
View File
@@ -56,7 +56,7 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {
.store(in: &cancellables)
slidingSyncViewProxy.diffPublisher
.collect(.byTime(serialDispatchQueue, 0.25))
.collect(.byTime(serialDispatchQueue, 0.1))
.sink { [weak self] in self?.updateRoomsWithDiffs($0) }
.store(in: &cancellables)
}
@@ -65,15 +65,16 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {
guard statePublisher.value == .live else {
return
}
var changes = [CollectionDifference<RoomSummary>.Change]()
for identifier in identifiers {
guard let index = rooms.firstIndex(where: { $0.id == identifier }) else {
guard let index = rooms.firstIndex(where: { $0.id == identifier }),
let roomListEntry = slidingSyncViewProxy.currentRoomsList().first(where: { $0.id == identifier }) else {
continue
}
let oldRoom = rooms[index]
let newRoom = buildRoomSummaryForIdentifier(identifier)
let newRoom = buildRoomSummaryForIdentifier(identifier, invalidated: roomListEntry.isInvalidated)
changes.append(.remove(offset: index, element: oldRoom, associatedWith: nil))
changes.append(.insert(offset: index, element: newRoom, associatedWith: nil))
@@ -120,15 +121,11 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {
MXLog.info("Finished applying diffs")
}
private func buildEmptyRoomSummary(forIdentifier identifier: String = UUID().uuidString) -> RoomSummary {
.empty(id: identifier)
}
private func buildRoomSummaryForIdentifier(_ identifier: String) -> RoomSummary {
private func buildRoomSummaryForIdentifier(_ identifier: String, invalidated: Bool) -> RoomSummary {
guard let room = try? slidingSyncViewProxy.roomForIdentifier(identifier) else {
MXLog.error("Failed finding room with id: \(identifier)")
return buildEmptyRoomSummary(forIdentifier: identifier)
return .empty
}
var attributedLastMessage: AttributedString?
@@ -145,23 +142,25 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {
lastMessageTimestamp = lastMessage.timestamp
}
return .filled(details: RoomSummaryDetails(id: room.roomId(),
name: room.name() ?? room.roomId(),
isDirect: room.isDm() ?? false,
avatarURLString: room.fullRoom()?.avatarUrl(),
lastMessage: attributedLastMessage,
lastMessageTimestamp: lastMessageTimestamp,
unreadNotificationCount: UInt(room.unreadNotifications().notificationCount())))
let details = RoomSummaryDetails(id: room.roomId(),
name: room.name() ?? room.roomId(),
isDirect: room.isDm() ?? false,
avatarURLString: room.fullRoom()?.avatarUrl(),
lastMessage: attributedLastMessage,
lastMessageTimestamp: lastMessageTimestamp,
unreadNotificationCount: UInt(room.unreadNotifications().notificationCount()))
return invalidated ? .invalidated(details: details) : .filled(details: details)
}
private func buildSummaryForRoomListEntry(_ entry: RoomListEntry) -> RoomSummary {
switch entry {
case .empty:
return buildEmptyRoomSummary()
return .empty
case .filled(let roomId):
return buildRoomSummaryForIdentifier(roomId)
return buildRoomSummaryForIdentifier(roomId, invalidated: false)
case .invalidated(let roomId):
return buildRoomSummaryForIdentifier(roomId)
return buildRoomSummaryForIdentifier(roomId, invalidated: true)
}
}
@@ -236,3 +235,23 @@ extension RoomSummaryProviderState {
}
}
}
extension MatrixRustSDK.RoomListEntry {
var id: String? {
switch self {
case .empty:
return nil
case .invalidated(let roomId), .filled(let roomId):
return roomId
}
}
var isInvalidated: Bool {
switch self {
case .invalidated:
return true
default:
return false
}
}
}
ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProviderProtocol.swift
+8 -14
View File
@@ -16,6 +16,7 @@
import Combine
import Foundation
import MatrixRustSDK
enum RoomSummaryProviderState {
case cold
@@ -24,25 +25,18 @@ enum RoomSummaryProviderState {
case live
}
enum RoomSummary: Identifiable {
case empty(id: String)
enum RoomSummary {
case empty
case filled(details: RoomSummaryDetails)
case invalidated(details: RoomSummaryDetails)
var id: String {
var id: String? {
switch self {
case .empty(let id):
return id
case .filled(let summary):
return summary.id
}
}
var asFilled: RoomSummaryDetails? {
guard case let .filled(details) = self else {
case .empty:
return nil
case .invalidated(let details), .filled(let details):
return details.id
}
return details
}
}