FIX: more precise unread message detection (#20588)

plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js

@@ -425,7 +425,6 @@ export default class ChatLivePane extends Component {
   @action
   didShowMessage(message) {
     message.visible = true;
-    this.updateLastReadMessage(message);
   }
 
   @action
@@ -441,12 +440,33 @@ export default class ChatLivePane extends Component {
 
     const lastReadId =
       this.args.channel.currentUserMembership?.last_read_message_id;
-    const lastUnreadVisibleMessage = this.args.channel.visibleMessages.findLast(
+    let lastUnreadVisibleMessage = this.args.channel.visibleMessages.findLast(
       (message) => !lastReadId || message.id > lastReadId
     );
-    if (lastUnreadVisibleMessage) {
-      this.args.channel.updateLastReadMessage(lastUnreadVisibleMessage.id);
+
+    // all intersecting messages are read
+    if (!lastUnreadVisibleMessage) {
+      return;
     }
+
+    const element = this._scrollerEl.querySelector(
+      `[data-id='${lastUnreadVisibleMessage.id}']`
+    );
+
+    // if the last visible message is not fully visible, we don't want to mark it as read
+    // attempt to mark previous one as read
+    if (!this.#isBottomOfMessageVisible(element, this._scrollerEl)) {
+      lastUnreadVisibleMessage = lastUnreadVisibleMessage.previousMessage;
+
+      if (
+        !lastUnreadVisibleMessage &&
+        lastReadId > lastUnreadVisibleMessage.id
+      ) {
+        return;
+      }
+    }
+
+    this.args.channel.updateLastReadMessage(lastUnreadVisibleMessage.id);
   }
 
   @action
@@ -502,6 +522,8 @@ export default class ChatLivePane extends Component {
     if (this.isAtBottom) {
       this.hasNewMessages = false;
     }
+
+    this.updateLastReadMessage();
   }
 
   _isBetween(target, a, b) {
@@ -1267,4 +1289,10 @@ export default class ChatLivePane extends Component {
         });
     });
   }
+
+  #isBottomOfMessageVisible(element, container) {
+    const rect = element.getBoundingClientRect();
+    const containerRect = container.getBoundingClientRect();
+    return rect.bottom <= containerRect.bottom;
+  }
 }

plugins/chat/assets/javascripts/discourse/modifiers/chat/track-message.js

@@ -19,7 +19,7 @@ export default class ChatTrackMessage extends Modifier {
       this._intersectionObserverCallback,
       {
         root: document,
-        threshold: 0.9,
+        threshold: 0,
       }
     );
 

plugins/chat/spec/system/mark_message_as_read.rb

@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+RSpec.describe "Mark message as read", type: :system, js: true do
+  fab!(:current_user) { Fabricate(:user) }
+  fab!(:channel_1) { Fabricate(:chat_channel) }
+
+  let(:chat_page) { PageObjects::Pages::Chat.new }
+  let(:channel_page) { PageObjects::Pages::ChatChannel.new }
+  let(:membership) { Chat::ChatChannelMembershipManager.new(channel_1).find_for_user(current_user) }
+
+  before do
+    chat_system_bootstrap
+    channel_1.add(current_user)
+    membership.update!(last_read_message_id: first_unread.id)
+    25.times { |i| Fabricate(:chat_message, chat_channel: channel_1) }
+  end
+
+  context "when the full message is not visible" do
+    fab!(:first_unread) { Fabricate(:chat_message, chat_channel: channel_1) }
+
+    it "doesn’t mark it as read" do
+      sign_in(current_user)
+      before_last_message = Fabricate(:chat_message, chat_channel: channel_1)
+      last_message = Fabricate(:chat_message, chat_channel: channel_1)
+      chat_page.visit_channel(channel_1)
+
+      page.execute_script("document.querySelector('.chat-messages-scroll').scrollTo(0, -5)")
+
+      try_until_success(timeout: 5) do
+        membership.reload.last_read_message_id = before_last_message.id
+      end
+    end
+  end
+
+  context "when the full message is visible" do
+    fab!(:first_unread) { Fabricate(:chat_message, chat_channel: channel_1) }
+
+    it "marks it as read" do
+      sign_in(current_user)
+      last_message = Fabricate(:chat_message, chat_channel: channel_1)
+      chat_page.visit_channel(channel_1)
+
+      page.execute_script("document.querySelector('.chat-messages-scroll').scrollTo(0, 0)")
+
+      try_until_success(timeout: 5) { membership.reload.last_read_message_id = last_message.id }
+    end
+  end
+end