From d97047070298441196d34f396cd94d607468df7c Mon Sep 17 00:00:00 2001 From: Evgeny Date: Sat, 10 Aug 2024 14:04:37 +0100 Subject: [PATCH] ui: fix throttled chat ordering (#4645) * ios: fix throttled chat ordering * optimize * account for added chats * revert kotlin change * dont pop chat that is already on top, unify with addChat * android, desktop: fix chat ordering * update * clear * fix ios * refactor sorting --- apps/ios/Shared/Model/ChatModel.swift | 79 ++++++++++++++----- apps/ios/Shared/Model/SimpleXAPI.swift | 3 + .../Shared/Views/ChatList/ChatListView.swift | 4 +- .../Views/LocalAuth/LocalAuthView.swift | 1 + .../chat/simplex/common/model/ChatModel.kt | 69 +++++++++++----- .../chat/simplex/common/model/SimpleXAPI.kt | 1 + .../common/views/database/DatabaseView.kt | 1 + 7 files changed, 118 insertions(+), 40 deletions(-) diff --git a/apps/ios/Shared/Model/ChatModel.swift b/apps/ios/Shared/Model/ChatModel.swift index 2e79c7b8ed..0539269ac7 100644 --- a/apps/ios/Shared/Model/ChatModel.swift +++ b/apps/ios/Shared/Model/ChatModel.swift @@ -228,10 +228,17 @@ final class ChatModel: ObservableObject { chats.firstIndex(where: { $0.id == id }) } - func addChat(_ chat: Chat, at position: Int = 0) { - withAnimation { - chats.insert(chat, at: position) + func addChat(_ chat: Chat) { + if chatId == nil { + withAnimation { addChat_(chat, at: 0) } + } else { + addChat_(chat, at: 0) } + popChatCollector.throttlePopChat(chat.chatInfo.id, currentPosition: 0) + } + + func addChat_(_ chat: Chat, at position: Int = 0) { + chats.insert(chat, at: position) } func updateChatInfo(_ cInfo: ChatInfo) { @@ -305,10 +312,11 @@ final class ChatModel: ObservableObject { } } } else { - addChat(Chat(c), at: i) + addChat_(Chat(c), at: i) } } NtfManager.shared.setNtfBadgeCount(totalUnreadCountForAllUsers()) + popChatCollector.clear() } // func addGroup(_ group: SimpleXChat.Group) { @@ -335,7 +343,7 @@ final class ChatModel: ObservableObject { if case .rcvNew = cItem.meta.itemStatus { unreadCollector.changeUnreadCounter(cInfo.id, by: 1) } - popChatCollector.addChat(cInfo.id) + popChatCollector.throttlePopChat(cInfo.id, currentPosition: i) } else { addChat(Chat(chatInfo: cInfo, chatItems: [cItem])) } @@ -610,29 +618,61 @@ final class ChatModel: ObservableObject { class PopChatCollector { private let subject = PassthroughSubject() private var bag = Set() + private var chatsToPop: [ChatId: Date] = [:] + private let popTsComparator = KeyPathComparator(\.popTs, order: .reverse) init() { subject .throttle(for: 2, scheduler: DispatchQueue.main, latest: true) - .sink { - let m = ChatModel.shared - if m.chatId == nil { - withAnimation { - m.chats = m.chats.sorted(using: KeyPathComparator(\.popTs, order: .reverse)) - } - } else { - m.chats = m.chats.sorted(using: KeyPathComparator(\.popTs, order: .reverse)) - } - } + .sink { self.popCollectedChats() } .store(in: &bag) } - func addChat(_ chatId: ChatId) { - if let index = ChatModel.shared.getChatIndex(chatId) { - ChatModel.shared.chats[index].popTs = CFAbsoluteTimeGetCurrent() + func throttlePopChat(_ chatId: ChatId, currentPosition: Int) { + let m = ChatModel.shared + if currentPosition > 0 && m.chatId == chatId { + m.chatToTop = chatId + } + if currentPosition > 0 || !chatsToPop.isEmpty { + chatsToPop[chatId] = Date.now subject.send() } } + + func clear() { + chatsToPop = [:] + } + + func popCollectedChats() { + let m = ChatModel.shared + var ixs: IndexSet = [] + var chs: [Chat] = [] + // collect chats that received updates + for (chatId, popTs) in self.chatsToPop { + // Currently opened chat is excluded, removing it from the list would navigate out of it + // It will be popped to top later when user exits from the list. + if m.chatId != chatId, let i = m.getChatIndex(chatId) { + ixs.insert(i) + let ch = m.chats[i] + ch.popTs = popTs + chs.append(ch) + } + } + + let removeInsert = { + m.chats.remove(atOffsets: ixs) + // sort chats by pop timestamp in descending order + m.chats.insert(contentsOf: chs.sorted(using: self.popTsComparator), at: 0) + } + + if m.chatId == nil { + withAnimation { removeInsert() } + } else { + removeInsert() + } + + self.chatsToPop = [:] + } } private func markChatItemRead_(_ i: Int) { @@ -729,6 +769,7 @@ final class ChatModel: ObservableObject { func popChat(_ id: String) { if let i = getChatIndex(id) { + // no animation here, for it not to look like it just moved when leaving the chat popChat_(i) } } @@ -848,7 +889,7 @@ final class Chat: ObservableObject, Identifiable, ChatLike { @Published var chatItems: [ChatItem] @Published var chatStats: ChatStats var created = Date.now - var popTs: CFAbsoluteTime? + fileprivate var popTs: Date? init(_ cData: ChatData) { self.chatInfo = cData.chatInfo diff --git a/apps/ios/Shared/Model/SimpleXAPI.swift b/apps/ios/Shared/Model/SimpleXAPI.swift index 247e83c419..5ebdc74788 100644 --- a/apps/ios/Shared/Model/SimpleXAPI.swift +++ b/apps/ios/Shared/Model/SimpleXAPI.swift @@ -1594,6 +1594,7 @@ func getUserChatData() throws { m.chatItemTTL = try getChatItemTTL() let chats = try apiGetChats() m.chats = chats.map { Chat.init($0) } + m.popChatCollector.clear() } private func getUserChatDataAsync() async throws { @@ -1606,11 +1607,13 @@ private func getUserChatDataAsync() async throws { m.userAddress = userAddress m.chatItemTTL = chatItemTTL m.chats = chats.map { Chat.init($0) } + m.popChatCollector.clear() } } else { await MainActor.run { m.userAddress = nil m.chats = [] + m.popChatCollector.clear() } } } diff --git a/apps/ios/Shared/Views/ChatList/ChatListView.swift b/apps/ios/Shared/Views/ChatList/ChatListView.swift index 8a27ac2a9c..059a287620 100644 --- a/apps/ios/Shared/Views/ChatList/ChatListView.swift +++ b/apps/ios/Shared/Views/ChatList/ChatListView.swift @@ -225,8 +225,8 @@ struct ChatListView: View { .offset(x: -8) } .listStyle(.plain) - .onChange(of: chatModel.chatId) { chId in - if chId == nil, let chatId = chatModel.chatToTop { + .onChange(of: chatModel.chatId) { currentChatId in + if let chatId = chatModel.chatToTop, currentChatId != chatId { chatModel.chatToTop = nil chatModel.popChat(chatId) } diff --git a/apps/ios/Shared/Views/LocalAuth/LocalAuthView.swift b/apps/ios/Shared/Views/LocalAuth/LocalAuthView.swift index b75cbf85b4..5e64f7fb0d 100644 --- a/apps/ios/Shared/Views/LocalAuth/LocalAuthView.swift +++ b/apps/ios/Shared/Views/LocalAuth/LocalAuthView.swift @@ -66,6 +66,7 @@ struct LocalAuthView: View { m.chatId = nil ItemsModel.shared.reversedChatItems = [] m.chats = [] + m.popChatCollector.clear() m.users = [] _ = kcAppPassword.set(password) _ = kcSelfDestructPassword.remove() diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt index 19c8b11ca5..e5fb314cb6 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt @@ -206,7 +206,10 @@ object ChatModel { class ChatsContext { val chats = _chats - fun addChat(chat: Chat) = chats.add(index = 0, chat) + suspend fun addChat(chat: Chat) { + chats.add(index = 0, chat) + popChatCollector.throttlePopChat(chat.remoteHostId, chat.id, currentPosition = 0) + } fun updateChatInfo(rhId: Long?, cInfo: ChatInfo) { val i = getChatIndex(rhId, cInfo.id) @@ -231,19 +234,19 @@ object ChatModel { } } - fun updateContactConnection(rhId: Long?, contactConnection: PendingContactConnection) = updateChat(rhId, ChatInfo.ContactConnection(contactConnection)) + suspend fun updateContactConnection(rhId: Long?, contactConnection: PendingContactConnection) = updateChat(rhId, ChatInfo.ContactConnection(contactConnection)) - fun updateContact(rhId: Long?, contact: Contact) = updateChat(rhId, ChatInfo.Direct(contact), addMissing = contact.directOrUsed) + suspend fun updateContact(rhId: Long?, contact: Contact) = updateChat(rhId, ChatInfo.Direct(contact), addMissing = contact.directOrUsed) - fun updateContactConnectionStats(rhId: Long?, contact: Contact, connectionStats: ConnectionStats) { + suspend fun updateContactConnectionStats(rhId: Long?, contact: Contact, connectionStats: ConnectionStats) { val updatedConn = contact.activeConn?.copy(connectionStats = connectionStats) val updatedContact = contact.copy(activeConn = updatedConn) updateContact(rhId, updatedContact) } - fun updateGroup(rhId: Long?, groupInfo: GroupInfo) = updateChat(rhId, ChatInfo.Group(groupInfo)) + suspend fun updateGroup(rhId: Long?, groupInfo: GroupInfo) = updateChat(rhId, ChatInfo.Group(groupInfo)) - private fun updateChat(rhId: Long?, cInfo: ChatInfo, addMissing: Boolean = true) { + private suspend fun updateChat(rhId: Long?, cInfo: ChatInfo, addMissing: Boolean = true) { if (hasChat(rhId, cInfo.id)) { updateChatInfo(rhId, cInfo) } else if (addMissing) { @@ -252,8 +255,8 @@ object ChatModel { } fun updateChats(newChats: List) { - chats.clear() - chats.addAll(newChats) + chats.replaceAll(newChats) + popChatCollector.clear() val cId = chatId.value // If chat is null, it was deleted in background after apiGetChats call @@ -262,13 +265,13 @@ object ChatModel { } } - fun replaceChat(rhId: Long?, id: String, chat: Chat) { + suspend fun replaceChat(rhId: Long?, id: String, chat: Chat) { val i = getChatIndex(rhId, id) if (i >= 0) { chats[i] = chat } else { // invalid state, correcting - chats.add(index = 0, chat) + addChat(chat) } } suspend fun addChatItem(rhId: Long?, cInfo: ChatInfo, cItem: ChatItem) { @@ -303,7 +306,11 @@ object ChatModel { else chat.chatStats ) - popChatCollector.addChat(chat.remoteHostId, chat.id) + if (appPlatform.isDesktop && cItem.chatDir.sent) { + addChat(chats.removeAt(i)) + } else { + popChatCollector.throttlePopChat(chat.remoteHostId, chat.id, currentPosition = i) + } } else { addChat(Chat(remoteHostId = rhId, chatInfo = cInfo, chatItems = arrayListOf(cItem))) } @@ -421,10 +428,12 @@ object ChatModel { } } - private val popChatCollector = PopChatCollector() + val popChatCollector = PopChatCollector() class PopChatCollector { private val subject = MutableSharedFlow() + private var remoteHostId: Long? = null + private val chatsToPop = mutableMapOf() init { withLongRunningApi { @@ -432,19 +441,41 @@ object ChatModel { .throttleLatest(2000) .collect { withChats { - chats.replaceAll(chats.value.sortedByDescending { it.popTs }) + chats.replaceAll(popCollectedChats()) } } } } - suspend fun addChat(remoteHostId: Long?, chatId: ChatId) { - val index = getChatIndex(remoteHostId, chatId) - if (index >= 0) { - chats.value[index].popTs = Clock.System.now() + suspend fun throttlePopChat(rhId: Long?, chatId: ChatId, currentPosition: Int) { + if (rhId != remoteHostId) { + chatsToPop.clear() + remoteHostId = rhId + } + if (currentPosition > 0 || chatsToPop.isNotEmpty()) { + chatsToPop[chatId] = Clock.System.now() subject.emit(Unit) } } + + fun clear() = chatsToPop.clear() + + private fun popCollectedChats(): List { + val chs = mutableListOf() + // collect chats that received updates + for ((chatId, popTs) in chatsToPop.entries) { + val ch = getChat(chatId) + if (ch != null) { + ch.popTs = popTs + chs.add(ch) + } + } + // sort chats by pop timestamp in descending order + val newChats = ArrayList(chs.sortedByDescending { it.popTs }) + newChats.addAll(chats.value.filter { !chatsToPop.containsKey(it.chatInfo.id) } ) + chatsToPop.clear() + return newChats + } } fun markChatItemsRead(remoteHostId: Long?, chatInfo: ChatInfo, range: CC.ItemRange? = null, unreadCountAfter: Int? = null) { @@ -487,7 +518,7 @@ object ChatModel { chats.removeAll { it.id == id && it.remoteHostId == rhId } } - fun upsertGroupMember(rhId: Long?, groupInfo: GroupInfo, member: GroupMember): Boolean { + suspend fun upsertGroupMember(rhId: Long?, groupInfo: GroupInfo, member: GroupMember): Boolean { // user member was updated if (groupInfo.membership.groupMemberId == member.groupMemberId) { updateGroup(rhId, groupInfo) @@ -524,7 +555,7 @@ object ChatModel { } } - fun updateGroupMemberConnectionStats(rhId: Long?, groupInfo: GroupInfo, member: GroupMember, connectionStats: ConnectionStats) { + suspend fun updateGroupMemberConnectionStats(rhId: Long?, groupInfo: GroupInfo, member: GroupMember, connectionStats: ConnectionStats) { val memberConn = member.activeConn if (memberConn != null) { val updatedConn = memberConn.copy(connectionStats = connectionStats) diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt index 967427162b..8e7543d12c 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt @@ -2718,6 +2718,7 @@ object ChatController { chatModel.chatItems.clear() withChats { chats.clear() + popChatCollector.clear() } } val statuses = apiGetNetworkStatuses(rhId) diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/database/DatabaseView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/database/DatabaseView.kt index ecf2aaf371..87ee0723f8 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/database/DatabaseView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/database/DatabaseView.kt @@ -505,6 +505,7 @@ fun deleteChatDatabaseFilesAndState() { withLongRunningApi { withChats { chats.clear() + popChatCollector.clear() } } chatModel.users.clear()