diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-channel.gjs b/plugins/chat/assets/javascripts/discourse/components/chat-channel.gjs index a39ab3b4010..a2ca6d48619 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-channel.gjs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-channel.gjs @@ -20,6 +20,7 @@ import i18n from "discourse-common/helpers/i18n"; import discourseDebounce from "discourse-common/lib/debounce"; import { bind } from "discourse-common/utils/decorators"; import ChatChannelStatus from "discourse/plugins/chat/discourse/components/chat-channel-status"; +import firstVisibleMessageId from "discourse/plugins/chat/discourse/helpers/first-visible-message-id"; import ChatChannelSubscriptionManager from "discourse/plugins/chat/discourse/lib/chat-channel-subscription-manager"; import { FUTURE, @@ -412,27 +413,27 @@ export default class ChatChannel extends Component { return; } - if (!this.lastFullyVisibleMessageId) { + const firstFullyVisibleMessageId = firstVisibleMessageId(this.scroller); + if (!firstFullyVisibleMessageId) { return; } - let lastUnreadVisibleMessage = this.messagesManager.findMessage( - this.lastFullyVisibleMessageId + let firstMessage = this.messagesManager.findMessage( + firstFullyVisibleMessageId ); - - if (!lastUnreadVisibleMessage) { + if (!firstMessage) { return; } const lastReadId = this.args.channel.currentUserMembership?.lastReadMessageId; - if (lastReadId >= lastUnreadVisibleMessage.id) { + if (lastReadId >= firstMessage.id) { return; } return this.chatApi.markChannelAsRead( this.args.channel.id, - lastUnreadVisibleMessage.id + firstMessage.id ); } @@ -460,7 +461,6 @@ export default class ChatChannel extends Component { (state.distanceToBottom.pixels > 250 && !state.atBottom); this.isScrolling = true; this.debouncedUpdateLastReadMessage(); - this.lastFullyVisibleMessageId = state.lastVisibleId; if ( state.atTop || @@ -484,7 +484,6 @@ export default class ChatChannel extends Component { (state.distanceToBottom.pixels > 250 && !state.atBottom); this.isScrolling = false; this.atBottom = state.atBottom; - this.lastFullyVisibleMessageId = state.lastVisibleId; if (state.atBottom) { this.fetchMoreMessages({ direction: FUTURE }); @@ -492,7 +491,7 @@ export default class ChatChannel extends Component { } else { this.chatChannelScrollPositions.set( this.args.channel.id, - state.lastVisibleId + state.firstVisibleId ); } } diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-thread.gjs b/plugins/chat/assets/javascripts/discourse/components/chat-thread.gjs index cce9cb6dabc..b9127aba779 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-thread.gjs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-thread.gjs @@ -12,6 +12,7 @@ import { resetIdle } from "discourse/lib/desktop-notifications"; import { NotificationLevels } from "discourse/lib/notification-levels"; import discourseDebounce from "discourse-common/lib/debounce"; import { bind } from "discourse-common/utils/decorators"; +import firstVisibleMessageId from "discourse/plugins/chat/discourse/helpers/first-visible-message-id"; import ChatChannelThreadSubscriptionManager from "discourse/plugins/chat/discourse/lib/chat-channel-thread-subscription-manager"; import { FUTURE, @@ -123,7 +124,6 @@ export default class ChatThread extends Component { (state.distanceToBottom.pixels > 250 && !state.atBottom); this.isScrolling = true; this.debounceUpdateLastReadMessage(); - this.lastFullyVisibleMessageId = state.lastVisibleId; if ( state.atTop || @@ -148,7 +148,6 @@ export default class ChatThread extends Component { this.resetIdle(); this.atBottom = state.atBottom; this.args.setFullTitle?.(state.atTop); - this.lastFullyVisibleMessageId = state.lastVisibleId; if (state.atBottom) { this.fetchMoreMessages({ direction: FUTURE }); @@ -169,27 +168,27 @@ export default class ChatThread extends Component { return; } - if (!this.lastFullyVisibleMessageId) { + const firstFullyVisibleMessageId = firstVisibleMessageId(this.scroller); + if (!firstFullyVisibleMessageId) { return; } - const lastUnreadVisibleMessage = this.messagesManager.findMessage( - this.lastFullyVisibleMessageId + const firstMessage = this.messagesManager.findMessage( + firstFullyVisibleMessageId ); - - if (!lastUnreadVisibleMessage) { + if (!firstMessage) { return; } const lastReadId = this.args.thread.currentUserMembership.lastReadMessageId; - if (lastReadId >= lastUnreadVisibleMessage.id) { + if (lastReadId >= firstMessage.id) { return; } return this.chatApi.markThreadAsRead( this.args.thread.channel.id, this.args.thread.id, - lastUnreadVisibleMessage.id + firstMessage.id ); } diff --git a/plugins/chat/assets/javascripts/discourse/helpers/first-visible-message-id.js b/plugins/chat/assets/javascripts/discourse/helpers/first-visible-message-id.js new file mode 100644 index 00000000000..3734600fc24 --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/helpers/first-visible-message-id.js @@ -0,0 +1,20 @@ +import { checkMessageBottomVisibility } from "discourse/plugins/chat/discourse/lib/check-message-visibility"; + +export default function firstVisibleMessageId(container) { + let _found; + const messages = container.querySelectorAll( + ":scope .chat-messages-container > [data-id]" + ); + + for (let i = messages.length - 1; i >= 0; i--) { + const message = messages[i]; + + if (checkMessageBottomVisibility(container, message)) { + _found = message; + break; + } + } + + const id = _found?.dataset?.id; + return id ? parseInt(id, 10) : null; +} diff --git a/plugins/chat/assets/javascripts/discourse/modifiers/chat/scrollable-list.js b/plugins/chat/assets/javascripts/discourse/modifiers/chat/scrollable-list.js index c020dccd179..dd0111d6f84 100644 --- a/plugins/chat/assets/javascripts/discourse/modifiers/chat/scrollable-list.js +++ b/plugins/chat/assets/javascripts/discourse/modifiers/chat/scrollable-list.js @@ -3,7 +3,7 @@ import { cancel, throttle } from "@ember/runloop"; import Modifier from "ember-modifier"; import discourseLater from "discourse-common/lib/later"; import { bind } from "discourse-common/utils/decorators"; -import { checkMessageBottomVisibility } from "discourse/plugins/chat/discourse/lib/check-message-visibility"; +import firstVisibleMessageId from "discourse/plugins/chat/discourse/helpers/first-visible-message-id"; const UP = "up"; const DOWN = "down"; @@ -54,7 +54,7 @@ export default class ChatScrollableList extends Modifier { this.scrollTimer = discourseLater(() => { this.options.onScrollEnd?.( Object.assign(this.computeState(), { - lastVisibleId: this.computeFirstVisibleMessageId(), + firstVisibleId: firstVisibleMessageId(this.element), }) ); }, this.options.delay || 250); @@ -134,23 +134,4 @@ export default class ChatScrollableList extends Modifier { return this.element.scrollTop < this.lastScrollTop ? UP : DOWN; } - - computeFirstVisibleMessageId() { - let firstVisibleMessage; - const messages = this.element.querySelectorAll( - ":scope .chat-messages-container > [data-id]" - ); - - for (let i = messages.length - 1; i >= 0; i--) { - const message = messages[i]; - - if (checkMessageBottomVisibility(this.element, message)) { - firstVisibleMessage = message; - break; - } - } - - const id = firstVisibleMessage?.dataset?.id; - return id ? parseInt(id, 10) : null; - } } diff --git a/plugins/chat/spec/system/update_last_read_spec.rb b/plugins/chat/spec/system/update_last_read_spec.rb index 8ef66f20612..bd9de788a6c 100644 --- a/plugins/chat/spec/system/update_last_read_spec.rb +++ b/plugins/chat/spec/system/update_last_read_spec.rb @@ -3,7 +3,7 @@ RSpec.describe "Update last read", type: :system do fab!(:current_user) { Fabricate(:user) } fab!(:channel_1) { Fabricate(:chat_channel) } - fab!(:first_unread) { Fabricate(:chat_message, chat_channel: channel_1) } + fab!(:messages) { Fabricate.times(15, :chat_message, chat_channel: channel_1) } let(:chat_page) { PageObjects::Pages::Chat.new } let(:channel_page) { PageObjects::Pages::ChatChannel.new } @@ -12,21 +12,27 @@ RSpec.describe "Update last read", type: :system do before do chat_system_bootstrap channel_1.add(current_user) - membership.update!(last_read_message_id: first_unread.id) - Fabricate.times(25, :chat_message, chat_channel: channel_1) + + membership.update!(last_read_message_id: messages.last.id) sign_in(current_user) end context "when the full message is visible" do - xit "marks it as read" do + it "marks it as read" do last_message = Fabricate(:chat_message, chat_channel: channel_1) chat_page.visit_channel(channel_1) - try_until_success do - page.execute_script("document.querySelector('.chat-messages-scroller').scrollTo(0, 1)") - page.execute_script("document.querySelector('.chat-messages-scroller').scrollTo(0, 0)") - expect(membership.reload.last_read_message_id).to eq(last_message.id) - end + try_until_success { expect(membership.reload.last_read_message_id).to eq(last_message.id) } + end + end + + context "when receiving a messages" do + it "marks it as read" do + chat_page.visit_channel(channel_1) + + last_message = Fabricate(:chat_message, chat_channel: channel_1, use_service: true) + + try_until_success { expect(membership.reload.last_read_message_id).to eq(last_message.id) } end end