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

@@ -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<Void, Never>()
         private var bag = Set<AnyCancellable>()
+        private var chatsToPop: [ChatId: Date] = [:]
+        private let popTsComparator = KeyPathComparator<Chat>(\.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

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()
         }
     }
 }

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)
                     }

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()

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<Chat>) {
-      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<Unit>()
+      private var remoteHostId: Long? = null
+      private val chatsToPop = mutableMapOf<ChatId, Instant>()
 
       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<Chat> {
+        val chs = mutableListOf<Chat>()
+        // 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)

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)

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()