From 931eedeb668226a842f38471015ad3469b81679d Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Thu, 9 Mar 2023 11:03:33 +1000 Subject: [PATCH] Revert "FIX: more precise unread message detection (#20588)" (#20604) This reverts commit d78fed7dc63565b2864381bf7877ba6673dc642c. Causing some issues with clearing unreads. --- .../discourse/components/chat-live-pane.js | 36 ++------------ .../discourse/modifiers/chat/track-message.js | 2 +- .../chat/spec/system/mark_message_as_read.rb | 48 ------------------- 3 files changed, 5 insertions(+), 81 deletions(-) delete mode 100644 plugins/chat/spec/system/mark_message_as_read.rb diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js b/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js index 0bd2317cfb4..817a1c4cc2f 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js @@ -425,6 +425,7 @@ export default class ChatLivePane extends Component { @action didShowMessage(message) { message.visible = true; + this.updateLastReadMessage(message); } @action @@ -440,33 +441,12 @@ export default class ChatLivePane extends Component { const lastReadId = this.args.channel.currentUserMembership?.last_read_message_id; - let lastUnreadVisibleMessage = this.args.channel.visibleMessages.findLast( + const lastUnreadVisibleMessage = this.args.channel.visibleMessages.findLast( (message) => !lastReadId || message.id > lastReadId ); - - // all intersecting messages are read - if (!lastUnreadVisibleMessage) { - return; + if (lastUnreadVisibleMessage) { + this.args.channel.updateLastReadMessage(lastUnreadVisibleMessage.id); } - - 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 @@ -522,8 +502,6 @@ export default class ChatLivePane extends Component { if (this.isAtBottom) { this.hasNewMessages = false; } - - this.updateLastReadMessage(); } _isBetween(target, a, b) { @@ -1289,10 +1267,4 @@ export default class ChatLivePane extends Component { }); }); } - - #isBottomOfMessageVisible(element, container) { - const rect = element.getBoundingClientRect(); - const containerRect = container.getBoundingClientRect(); - return rect.bottom <= containerRect.bottom; - } } diff --git a/plugins/chat/assets/javascripts/discourse/modifiers/chat/track-message.js b/plugins/chat/assets/javascripts/discourse/modifiers/chat/track-message.js index 856fb4fcaa6..469188eaa32 100644 --- a/plugins/chat/assets/javascripts/discourse/modifiers/chat/track-message.js +++ b/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, + threshold: 0.9, } ); diff --git a/plugins/chat/spec/system/mark_message_as_read.rb b/plugins/chat/spec/system/mark_message_as_read.rb deleted file mode 100644 index 70c16631b01..00000000000 --- a/plugins/chat/spec/system/mark_message_as_read.rb +++ /dev/null @@ -1,48 +0,0 @@ -# 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