From 9365d8b5444692304e93c436824a4f093f886eee Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Fri, 19 Jan 2024 16:34:11 +0100 Subject: [PATCH] FEATURE: save/retrieve scroll position in chat channel (#25336) Note this is only saved on each tab session. --- .../discourse/components/chat-channel.gjs | 23 ++++++++++++++-- .../modifiers/chat/scrollable-list.js | 26 ++++++++++++++++++- .../services/chat-channel-scroll-positions.js | 17 ++++++++++++ plugins/chat/spec/system/chat_channel_spec.rb | 20 ++++++++++++++ 4 files changed, 83 insertions(+), 3 deletions(-) create mode 100644 plugins/chat/assets/javascripts/discourse/services/chat-channel-scroll-positions.js diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-channel.gjs b/plugins/chat/assets/javascripts/discourse/components/chat-channel.gjs index 44e7274c2fe..c1f94cc1ef5 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-channel.gjs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-channel.gjs @@ -63,6 +63,7 @@ export default class ChatChannel extends Component { @service chatDraftsManager; @service chatEmojiPickerManager; @service chatStateManager; + @service chatChannelScrollPositions; @service("chat-channel-composer") composer; @service("chat-channel-pane") pane; @service currentUser; @@ -96,6 +97,10 @@ export default class ChatChannel extends Component { return this.args.channel.currentUserMembership; } + get hasSavedScrollPosition() { + return !!this.chatChannelScrollPositions.get(this.args.channel.id); + } + @action setScrollable(element) { this.scrollable = element; @@ -158,6 +163,10 @@ export default class ChatChannel extends Component { if (this.args.targetMessageId) { this.debounceHighlightOrFetchMessage(this.args.targetMessageId); + } else if (this.chatChannelScrollPositions.get(this.args.channel.id)) { + this.debounceHighlightOrFetchMessage( + this.chatChannelScrollPositions.get(this.args.channel.id) + ); } else { this.fetchMessages({ fetch_from_last_read: true }); } @@ -192,7 +201,10 @@ export default class ChatChannel extends Component { ); if (findArgs.target_message_id) { - this.scrollToMessageId(findArgs.target_message_id, { highlight: true }); + this.scrollToMessageId(findArgs.target_message_id, { + highlight: true, + position: findArgs.position, + }); } else if (findArgs.fetch_from_last_read) { const lastReadMessageId = this.currentUserMembership?.lastReadMessageId; this.scrollToMessageId(lastReadMessageId); @@ -379,7 +391,7 @@ export default class ChatChannel extends Component { ) ); } else { - this.fetchMessages({ target_message_id: messageId }); + this.fetchMessages({ target_message_id: messageId, position: "end" }); } } @@ -484,6 +496,12 @@ export default class ChatChannel extends Component { if (state.atBottom) { this.fetchMoreMessages({ direction: FUTURE }); + this.chatChannelScrollPositions.remove(this.args.channel.id); + } else { + this.chatChannelScrollPositions.set( + this.args.channel.id, + state.lastVisibleId + ); } } @@ -670,6 +688,7 @@ export default class ChatChannel extends Component { "chat-channel" (if this.messagesLoader.loading "loading") (if this.pane.sending "chat-channel--sending") + (if this.hasSavedScrollPosition "chat-channel--saved-scroll-position") (unless this.messagesLoader.fetchedOnce "chat-channel--not-loaded-once") }} {{willDestroy this.teardown}} 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 be96bd9f0ae..0c24cecc3eb 100644 --- a/plugins/chat/assets/javascripts/discourse/modifiers/chat/scrollable-list.js +++ b/plugins/chat/assets/javascripts/discourse/modifiers/chat/scrollable-list.js @@ -3,6 +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"; const UP = "up"; const DOWN = "down"; @@ -49,7 +50,11 @@ export default class ChatScrollableList extends Modifier { cancel(this.scrollTimer); this.throttleTimer = throttle(this, this.computeScroll, 50, true); this.scrollTimer = discourseLater(() => { - this.options.onScrollEnd?.(this.computeState()); + this.options.onScrollEnd?.( + Object.assign(this.computeState(), { + lastVisibleId: this.computeFirstVisibleMessageId(), + }) + ); }, this.options.delay || 250); } @@ -127,4 +132,23 @@ 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/assets/javascripts/discourse/services/chat-channel-scroll-positions.js b/plugins/chat/assets/javascripts/discourse/services/chat-channel-scroll-positions.js new file mode 100644 index 00000000000..9a4888c7608 --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/services/chat-channel-scroll-positions.js @@ -0,0 +1,17 @@ +import { tracked } from "@glimmer/tracking"; +import Service from "@ember/service"; +import { TrackedMap } from "@ember-compat/tracked-built-ins"; + +export default class ChatChannelScrollPositions extends Service { + @tracked positions = new TrackedMap(); + + add(channelId, position) { + this.positions.set(channelId, position); + } + + remove(channelId) { + if (this.positions.has(channelId)) { + this.positions.delete(channelId); + } + } +} diff --git a/plugins/chat/spec/system/chat_channel_spec.rb b/plugins/chat/spec/system/chat_channel_spec.rb index 3983a90c1ff..1acbb05eb9b 100644 --- a/plugins/chat/spec/system/chat_channel_spec.rb +++ b/plugins/chat/spec/system/chat_channel_spec.rb @@ -358,4 +358,24 @@ RSpec.describe "Chat channel", type: :system do ) end end + + context "when navigating from one channel to another" do + fab!(:channel_2) { Fabricate(:chat_channel) } + + before do + channel_2.add(current_user) + Fabricate.times(50, :chat_message, chat_channel: channel_1) + end + + it "remembers the scroll position" do + chat_page.visit_channel(channel_1, message_id: channel_1.chat_messages[2].id) + + expect(channel_page).to have_css(".chat-channel--saved-scroll-position") + + sidebar_page.open_channel(channel_2) + sidebar_page.open_channel(channel_1) + + expect(channel_page.messages).to have_message(id: channel_1.chat_messages[2].id) + end + end end