fix(chat): better integrate new messages marker with loading state

- new messages marker is reevaluated only if chat has unviewed messages
- loading state is reevaluated only when chat is made active, this fixes
  case described here:
https://github.com/status-im/status-desktop/pull/10151#discussion_r1158702638

fixes: #10275

src/app/modules/main/chat_section/chat_content/messages/io_interface.nim

@@ -153,6 +153,9 @@ method resendChatMessage*(self: AccessInterface, messageId: string): string =
 method resetNewMessagesMarker*(self: AccessInterface) =
   raise newException(ValueError, "No implementation available")
 
+method removeNewMessagesMarker*(self: AccessInterface) =
+  raise newException(ValueError, "No implementation available")
+
 method resetAndScrollToNewMessagesMarker*(self: AccessInterface) =
   raise newException(ValueError, "No implementation available")
 
@@ -164,3 +167,9 @@ method updateCommunityDetails*(self: AccessInterface, community: CommunityDto) =
 
 method onFirstUnseenMessageLoaded*(self: AccessInterface, messageId: string) =
   raise newException(ValueError, "No implementation available")
+
+method isFirstUnseenMessageInitialized*(self: AccessInterface): bool =
+  raise newException(ValueError, "No implementation available")
+
+method reevaluateViewLoadingState*(self: AccessInterface) =
+  raise newException(ValueError, "No implementation available")

src/app/modules/main/chat_section/chat_content/messages/module.nim

@@ -25,6 +25,12 @@ const CHAT_IDENTIFIER_CLOCK = -2
 const FETCH_MORE_MESSAGES_MESSAGE_ID = "fetch-more_messages-message-id"
 const FETCH_MORE_MESSAGES_CLOCK = -1
 
+type
+  FirstUnseenMessageState = tuple
+    initialized: bool
+    fetching: bool
+    scrollToWhenFetched: bool
+
 type
   Module* = ref object of io_interface.AccessInterface
     delegate: delegate_interface.AccessInterface
@@ -32,7 +38,8 @@ type
     viewVariant: QVariant
     controller: Controller
     moduleLoaded: bool
-    scrollToFirstUnseenMessageWhenLoaded: bool
+    initialMessagesLoaded: bool
+    firstUnseenMessageState: FirstUnseenMessageState
 
 proc newModule*(delegate: delegate_interface.AccessInterface, events: EventEmitter, sectionId: string, chatId: string,
   belongsToCommunity: bool, contactService: contact_service.Service, communityService: community_service.Service,
@@ -45,7 +52,8 @@ proc newModule*(delegate: delegate_interface.AccessInterface, events: EventEmitt
   result.controller = controller.newController(result, events, sectionId, chatId, belongsToCommunity, contactService,
   communityService, chatService, messageService, mailserversService)
   result.moduleLoaded = false
-  result.scrollToFirstUnseenMessageWhenLoaded = true
+  result.initialMessagesLoaded = false
+  result.firstUnseenMessageState = (false, false, false)
 
 # Forward declaration
 proc createChatIdentifierItem(self: Module): Item
@@ -208,6 +216,11 @@ method currentUserWalletContainsAddress(self: Module, address: string): bool =
       return true
   return false
 
+method reevaluateViewLoadingState*(self: Module) =
+  self.view.setLoading(not self.initialMessagesLoaded or 
+                       not self.firstUnseenMessageState.initialized or
+                       self.firstUnseenMessageState.fetching)
+
 method newMessagesLoaded*(self: Module, messages: seq[MessageDto], reactions: seq[ReactionDto],
   pinnedMessages: seq[PinnedMessageDto]) =
   var viewItems: seq[Item]
@@ -341,7 +354,8 @@ method newMessagesLoaded*(self: Module, messages: seq[MessageDto], reactions: se
     # check if this loading was caused by the click on a messages from the app search result
     self.checkIfMessageLoadedAndScrollToItIfItIs()
 
-  self.view.initialMessagesAreLoaded()
+  self.initialMessagesLoaded = true
+  self.reevaluateViewLoadingState()
 
 method messagesAdded*(self: Module, messages: seq[MessageDto]) =
   var items: seq[Item]
@@ -684,13 +698,13 @@ method resendChatMessage*(self: Module, messageId: string): string =
   return self.controller.resendChatMessage(messageId)
 
 method resetNewMessagesMarker*(self: Module) =
-  self.scrollToFirstUnseenMessageWhenLoaded = false
-  self.view.setFirstUnseenMessageLoaded(false)
+  self.firstUnseenMessageState.fetching = true
+  self.firstUnseenMessageState.scrollToWhenFetched = false
   self.controller.getAsyncFirstUnseenMessageId()
 
 method resetAndScrollToNewMessagesMarker*(self: Module) =
-  self.scrollToFirstUnseenMessageWhenLoaded = true
-  self.view.setFirstUnseenMessageLoaded(false)
+  self.firstUnseenMessageState.fetching = true
+  self.firstUnseenMessageState.scrollToWhenFetched = true
   self.controller.getAsyncFirstUnseenMessageId()
 
 method removeNewMessagesMarker*(self: Module) =
@@ -728,9 +742,14 @@ proc updateItemsByAlbum(self: Module, items: var seq[Item], message: MessageDto)
         return true
   return false
 
+method isFirstUnseenMessageInitialized*(self: Module): bool =
+  return self.firstUnseenMessageState.initialized
+
 method onFirstUnseenMessageLoaded*(self: Module, messageId: string) =
   self.view.model().setFirstUnseenMessageId(messageId)
   self.view.model().resetNewMessagesMarker()
-  if self.scrollToFirstUnseenMessageWhenLoaded:
+  if self.firstUnseenMessageState.scrollToWhenFetched:
     self.scrollToMessage(messageId)
-  self.view.setFirstUnseenMessageLoaded(true)
+  self.firstUnseenMessageState.initialized = true
+  self.firstUnseenMessageState.fetching = false
+  self.reevaluateViewLoadingState()

src/app/modules/main/chat_section/chat_content/messages/view.nim

@@ -10,14 +10,13 @@ QtObject:
       delegate: io_interface.AccessInterface
       model: Model
       modelVariant: QVariant
-      initialMessagesLoaded: bool
       messageSearchOngoing: bool
       amIChatAdmin: bool
       isPinMessageAllowedForMembers: bool
       chatColor: string
       chatIcon: string
       chatType: int
-      firstUnseenMessageLoaded: bool
+      loading: bool
 
   proc delete*(self: View) =
     self.model.delete
@@ -30,14 +29,13 @@ QtObject:
     result.delegate = delegate
     result.model = newModel()
     result.modelVariant = newQVariant(result.model)
-    result.initialMessagesLoaded = false
     result.messageSearchOngoing = false
     result.amIChatAdmin = false
     result.isPinMessageAllowedForMembers = false
     result.chatColor = ""
     result.chatIcon = ""
     result.chatType = ChatType.Unknown.int
-
+    result.loading = false
 
   proc load*(self: View) =
     self.delegate.viewDidLoad()
@@ -80,21 +78,6 @@ QtObject:
   proc getNumberOfPinnedMessages*(self: View): int {.slot.} =
     return self.delegate.getNumberOfPinnedMessages()
 
-  proc initialMessagesLoadedChanged*(self: View) {.signal.}
-
-  proc getInitialMessagesLoaded(self: View): bool {.slot.} =
-    return self.initialMessagesLoaded
-
-  QtProperty[bool] initialMessagesLoaded:
-    read = getInitialMessagesLoaded
-    notify = initialMessagesLoadedChanged
-
-  proc initialMessagesAreLoaded*(self: View) = # this is not a slot
-    if (self.initialMessagesLoaded):
-      return
-    self.initialMessagesLoaded = true
-    self.initialMessagesLoadedChanged()
-
   proc loadMoreMessages*(self: View) {.slot.} =
     self.delegate.loadMoreMessages()
 
@@ -236,13 +219,13 @@ QtObject:
     self.chatType = value
     self.chatTypeChanged()
 
-  proc firstUnseenMessageLoadedChanged*(self: View) {.signal.}
-  proc getFirstUnseenMessageLoaded*(self: View): bool {.slot.} =
-    return self.firstUnseenMessageLoaded
-  proc setFirstUnseenMessageLoaded*(self: View, value: bool) =
-    self.firstUnseenMessageLoaded = value
-    self.firstUnseenMessageLoadedChanged()
+  proc loadingChanged*(self: View) {.signal.}
+  proc isLoading*(self: View): bool {.slot.} =
+    return self.loading
+  proc setLoading*(self: View, value: bool) =
+    self.loading = value
+    self.loadingChanged()
   
-  QtProperty[bool] firstUnseenMessageLoaded:
-    read = getFirstUnseenMessageLoaded
-    notify = firstUnseenMessageLoadedChanged
+  QtProperty[bool] loading:
+    read = isLoading
+    notify = loadingChanged

src/app/modules/main/chat_section/chat_content/module.nim

@@ -387,8 +387,16 @@ method contactTrustStatusChanged*(self: Module, publicKey: string, isUntrustwort
   self.view.updateTrustStatus(isUntrustworthy)
 
 method onMadeActive*(self: Module) =
-  self.messagesModule.resetAndScrollToNewMessagesMarker()
+  # The new messages marker is reset each time the chat is made active,
+  # as messages may arrive out of order and relying on the previous
+  # new messages marker could yield incorrect results.
+  if not self.messagesModule.isFirstUnseenMessageInitialized() or
+     self.controller.getChatDetails().unviewedMessagesCount > 0:
+    self.messagesModule.resetAndScrollToNewMessagesMarker()
+  self.messagesModule.reevaluateViewLoadingState()
   self.view.setActive()
 
 method onMadeInactive*(self: Module) =
+  if self.controller.getChatDetails().unviewedMessagesCount == 0:
+    self.messagesModule.removeNewMessagesMarker()
   self.view.setInactive()

ui/app/AppLayouts/Chat/stores/MessageStore.qml

@@ -11,8 +11,7 @@ QtObject {
     readonly property bool loadingHistoryMessagesInProgress: root.chatSectionModule? root.chatSectionModule.loadingHistoryMessagesInProgress : false
     readonly property int newMessagesCount: messagesModel ? messagesModel.newMessagesCount : 0
     readonly property bool messageSearchOngoing: messageModule ? messageModule.messageSearchOngoing : false
-    readonly property bool initialMessagesLoaded: messageModule ? messageModule.initialMessagesLoaded : false
-    readonly property bool firstUnseenMessageLoaded: messageModule ? messageModule.firstUnseenMessageLoaded : false
+    readonly property bool loading: messageModule ? messageModule.loading : false
 
     readonly property bool amIChatAdmin: messageModule ? messageModule.amIChatAdmin : false
     readonly property bool isPinMessageAllowedForMembers: messageModule ? messageModule.isPinMessageAllowedForMembers : false
@@ -31,7 +30,7 @@ QtObject {
         if(!messageModule)
             return
 
-        if(!messageModule.initialMessagesLoaded)
+        if(root.loading)
             return
 
         messageModule.loadMoreMessages()

ui/app/AppLayouts/Chat/views/ChatMessagesView.qml

@@ -59,8 +59,7 @@ Item {
                 return
             }
 
-            if (chatDetails && chatDetails.active && chatDetails.hasUnreadMessages &&
-                !messageStore.messageSearchOngoing && messageStore.firstUnseenMessageLoaded) {
+            if (chatDetails && chatDetails.active && chatDetails.hasUnreadMessages && !messageStore.loading) {
                 chatContentModule.markAllMessagesRead()
             }
         }
@@ -96,7 +95,7 @@ Item {
             d.markAllMessagesReadIfMostRecentMessageIsInViewport()
         }
 
-        function onFirstUnseenMessageLoadedChanged() {
+        function onLoadingChanged() {
             d.markAllMessagesReadIfMostRecentMessageIsInViewport()
         }
     }
@@ -116,11 +115,7 @@ Item {
 
             // HACK: we call `addNewMessagesMarker` later because messages model
             // may not be yet propagated with unread messages when this signal is emitted
-            if (chatLogView.visible) {
-                if (!d.isMostRecentMessageInViewport) {
-                    Qt.callLater(() => messageStore.addNewMessagesMarker())
-                }
-            } else {
+            if (chatLogView.visible && !d.isMostRecentMessageInViewport) {
                 Qt.callLater(() => messageStore.addNewMessagesMarker())
             }
         }
@@ -161,19 +156,17 @@ Item {
     Loader {
         id: loadingMessagesView
 
-        readonly property bool show: !messageStore.firstUnseenMessageLoaded ||
-                                     !messageStore.initialMessagesLoaded
-        active: show
-        visible: show
         anchors.top: loadingMessagesIndicator.bottom
         anchors.bottom: parent.bottom
         anchors.left: parent.left
         anchors.right: parent.right
-        sourceComponent:
-            MessagesLoadingView {
+
+        active: messageStore.loading
+        visible: active
+        sourceComponent: MessagesLoadingView {
             anchors.margins: 16
             anchors.fill: parent
-            }
+        }
     }
 
     StatusListView {
@@ -205,7 +198,7 @@ Item {
 
             // after inilial messages are loaded
             // load as much messages as the view requires
-            if (messageStore.initialMessagesLoaded) {
+            if (!messageStore.loading) {
                 d.loadMoreMessagesIfScrollBelowThreshold()
             }
         }