From b5e736504aeca3d751e711198a426cbfecf0211d Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Mon, 6 Mar 2023 16:42:11 +0100 Subject: [PATCH] PERF: applies optimisations on chat-live pane (#20532) - group writes when computing separators positions - shows skeleton only on initial load - forces date separator to be pinned when first message to prevent a pinned - not pinned - pinned sequence when loading more in past - relies on `message.visible` property instead of checking `isElementInViewport` - attempts to load next/prev messages earlier - do not scroll to on fetch more - hides `last visit` text while pinned --- .../chat/app/controllers/chat_controller.rb | 2 +- .../discourse/components/chat-live-pane.hbs | 68 ++-- .../discourse/components/chat-live-pane.js | 364 ++++++++---------- .../components/chat-message-collapser.hbs | 28 +- .../chat-message-separator-date.hbs | 10 +- .../components/chat-message-text.hbs | 6 +- .../discourse/components/chat-message.hbs | 1 + .../discourse/components/chat-message.js | 27 +- .../chat-scroll-to-bottom-arrow.hbs | 5 +- .../discourse/components/chat-skeleton.hbs | 6 +- .../discourse/components/collapser.hbs | 9 +- .../discourse/components/collapser.js | 3 + .../discourse/models/chat-message.js | 2 + .../modifiers/chat/did-mutate-childlist.js | 24 ++ .../modifiers/chat/on-throttled-scroll.js | 36 ++ .../discourse/services/chat-api.js | 2 +- .../services/chat-channels-manager.js | 4 + .../common/chat-message-separator.scss | 9 +- .../assets/stylesheets/common/common.scss | 8 +- .../chat/spec/system/bookmark_message_spec.rb | 1 - plugins/chat/spec/system/chat_channel_spec.rb | 1 - .../chat/spec/system/deleted_message_spec.rb | 1 - .../spec/system/hashtag_autocomplete_spec.rb | 2 - .../chat/spec/system/mention_warnings_spec.rb | 3 - .../spec/system/page_objects/chat/chat.rb | 1 + .../chat/spec/system/reply_indicator_spec.rb | 42 -- .../spec/system/replying_indicator_spec.rb | 35 -- plugins/chat/spec/system/transcript_spec.rb | 12 - .../components/chat-message-test.js | 3 + 29 files changed, 337 insertions(+), 378 deletions(-) create mode 100644 plugins/chat/assets/javascripts/discourse/modifiers/chat/did-mutate-childlist.js create mode 100644 plugins/chat/assets/javascripts/discourse/modifiers/chat/on-throttled-scroll.js delete mode 100644 plugins/chat/spec/system/reply_indicator_spec.rb delete mode 100644 plugins/chat/spec/system/replying_indicator_spec.rb diff --git a/plugins/chat/app/controllers/chat_controller.rb b/plugins/chat/app/controllers/chat_controller.rb index ffcad435e5..6b6f8f57a6 100644 --- a/plugins/chat/app/controllers/chat_controller.rb +++ b/plugins/chat/app/controllers/chat_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Chat::ChatController < Chat::ChatBaseController - PAST_MESSAGE_LIMIT = 20 + PAST_MESSAGE_LIMIT = 40 FUTURE_MESSAGE_LIMIT = 40 PAST = "past" FUTURE = "future" diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.hbs index 2be5212274..a9bd5db6a7 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.hbs @@ -3,6 +3,7 @@ "chat-live-pane" (if this.loading "loading") (if this.sendingLoading "sending-loading") + (unless this.loadedOnce "not-loaded-once") }} {{did-insert this.setupListeners}} {{will-destroy this.teardownListeners}} @@ -37,42 +38,41 @@ }} > -
+
-
+
- {{#if this.loadingMorePast}} - - {{/if}} + {{#if this.loadedOnce}} - {{#each @channel.messages key="id" as |message|}} - - {{/each}} - - {{#if (or this.loadingMoreFuture)}} - + {{#each @channel.messages key="id" as |message|}} + + {{/each}} + {{else}} + {{/if}}
@@ -86,7 +86,7 @@ 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 ab25fc88b6..8b5aa5dc1f 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js @@ -1,5 +1,4 @@ import { capitalize } from "@ember/string"; -import isElementInViewport from "discourse/lib/is-element-in-viewport"; import { cloneJSON } from "discourse-common/lib/object"; import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; import ChatMessageDraft from "discourse/plugins/chat/discourse/models/chat-message-draft"; @@ -9,7 +8,7 @@ import discourseDebounce from "discourse-common/lib/debounce"; import EmberObject, { action } from "@ember/object"; import { ajax } from "discourse/lib/ajax"; import { popupAjaxError } from "discourse/lib/ajax-error"; -import { cancel, next, schedule, throttle } from "@ember/runloop"; +import { cancel, schedule } from "@ember/runloop"; import discourseLater from "discourse-common/lib/later"; import { inject as service } from "@ember/service"; import { Promise } from "rsvp"; @@ -19,14 +18,10 @@ import { removeOnPresenceChange, } from "discourse/lib/user-presence"; import isZoomed from "discourse/plugins/chat/discourse/lib/zoom-check"; -import { isTesting } from "discourse-common/config/environment"; import { tracked } from "@glimmer/tracking"; import { getOwner } from "discourse-common/lib/get-owner"; -const STICKY_SCROLL_LENIENCE = 100; const PAGE_SIZE = 50; -const SCROLL_HANDLER_THROTTLE_MS = isTesting() ? 0 : 150; -const FETCH_MORE_MESSAGES_THROTTLE_MS = isTesting() ? 0 : 500; const PAST = "past"; const FUTURE = "future"; const READ_INTERVAL_MS = 1000; @@ -54,14 +49,18 @@ export default class ChatLivePane extends Component { @tracked includeHeader = true; @tracked editingMessage = null; @tracked replyToMsg = null; - @tracked hasNewMessages = null; - @tracked isDocked = true; - @tracked isAlmostDocked = true; + @tracked hasNewMessages = false; + @tracked needsArrow = false; @tracked loadedOnce = false; + isAtBottom = true; + isTowardsBottom = false; + isTowardsTop = false; + isAtTop = false; + _loadedChannelId = null; _scrollerEl = null; - _previousScrollTop = null; + _previousScrollTop = 0; _lastSelectedMessage = null; _mentionWarningsSeen = {}; _unreachableGroupMentions = []; @@ -70,14 +69,8 @@ export default class ChatLivePane extends Component { @action setupListeners(element) { this._scrollerEl = element.querySelector(".chat-messages-scroll"); - this._scrollerEl.addEventListener("scroll", this.onScrollHandler, { - passive: true, - }); - window.addEventListener("resize", this.onResizeHandler); - window.addEventListener("wheel", this.onScrollHandler, { - passive: true, - }); + window.addEventListener("resize", this.onResizeHandler); document.addEventListener("scroll", this._forceBodyScroll, { passive: true, }); @@ -88,12 +81,8 @@ export default class ChatLivePane extends Component { } @action - teardownListeners(element) { - element - .querySelector(".chat-messages-scroll") - ?.removeEventListener("scroll", this.onScrollHandler); + teardownListeners() { window.removeEventListener("resize", this.onResizeHandler); - window.removeEventListener("wheel", this.onScrollHandler); cancel(this.resizeHandler); document.removeEventListener("scroll", this._forceBodyScroll); removeOnPresenceChange(this.onPresenceChangeCallback); @@ -101,6 +90,11 @@ export default class ChatLivePane extends Component { this.requestedTargetMessageId = null; } + @action + resetIdle() { + resetIdle(); + } + @action updateChannel() { // Technically we could keep messages to avoid re-fetching them, but @@ -120,26 +114,21 @@ export default class ChatLivePane extends Component { @action loadMessages() { - this.loadedOnce = false; + if (!this.args.channel?.id) { + return; + } if (this.args.targetMessageId) { this.requestedTargetMessageId = parseInt(this.args.targetMessageId, 10); } - if (this.args.channel?.id) { - if (this.requestedTargetMessageId) { - this.highlightOrFetchMessage(this.requestedTargetMessageId); - } else { - this.fetchMessages({ fetchFromLastMessage: false }); - } + if (this.requestedTargetMessageId) { + this.highlightOrFetchMessage(this.requestedTargetMessageId); + } else { + this.fetchMessages({ fetchFromLastMessage: false }); } } - @bind - onScrollHandler(event) { - throttle(this, this.onScroll, event, SCROLL_HANDLER_THROTTLE_MS, false); - } - @bind onResizeHandler() { cancel(this.resizeHandler); @@ -168,7 +157,8 @@ export default class ChatLivePane extends Component { return; } - this.args.channel?.clearMessages(); + this.loadedOnce = false; + this._previousScrollTop = 0; this.loadingMorePast = true; const findArgs = { pageSize: PAGE_SIZE }; @@ -193,9 +183,9 @@ export default class ChatLivePane extends Component { this.args.channel, results ); - this.args.channel.addMessages(messages); + + this.args.channel.messages = messages; this.args.channel.details = meta; - this.loadedOnce = true; if (this.requestedTargetMessageId) { this.scrollToMessage(findArgs["targetMessageId"], { @@ -206,8 +196,6 @@ export default class ChatLivePane extends Component { } else if (messages.length) { this.scrollToMessage(messages[messages.length - 1].id); } - - this.fillPaneAttempt(); }) .catch(this._handleErrors) .finally(() => { @@ -215,24 +203,15 @@ export default class ChatLivePane extends Component { return; } + this.loadedOnce = true; this.requestedTargetMessageId = null; this.loadingMorePast = false; + this.fillPaneAttempt(); }); } - @action - onDestroySkeleton() { - this._iOSFix(); - this._throttleComputeSeparators(); - } - - @action - onDidInsertSkeleton() { - this._computeSeparators(); // this one is not throttled as we need instant feedback - } - @bind - _fetchMoreMessages({ direction, scrollTo = true }) { + fetchMoreMessages({ direction }) { const loadingPast = direction === PAST; const loadingMoreKey = `loadingMore${capitalize(direction)}`; @@ -272,99 +251,85 @@ export default class ChatLivePane extends Component { return; } + // prevents an edge case where user clicks bottom arrow + // just after scrolling to top + if (loadingPast && this.isAtBottom) { + return; + } + const [messages, meta] = this.afterFetchCallback( this.args.channel, results ); - this.args.channel.addMessages(messages); - this.args.channel.details = meta; - - if (!messages.length) { + if (!messages?.length) { return; } - if (scrollTo) { - if (!loadingPast) { - this.scrollToMessage(messageId, { position: "start" }); - } else { - if (this.site.desktopView) { - this.scrollToMessage(messages[messages.length - 1].id); - } - } - } + this.args.channel.addMessages(messages); + this.args.channel.details = meta; - this.fillPaneAttempt(); + // Edge case for IOS to avoid blank screens + // and/or scrolling to bottom losing track of scroll position + schedule("afterRender", () => { + if ( + !this._selfDeleted && + !loadingPast && + (this.capabilities.isIOS || !this.isScrolling) + ) { + this.scrollToMessage(messages[0].id, { position: "end" }); + } + }); }) .catch(() => { this._handleErrors(); }) .finally(() => { this[loadingMoreKey] = false; + this.fillPaneAttempt(); + this.computeDatesSeparators(); }); } + @debounce(500, false) fillPaneAttempt() { - next(() => { - if (this._selfDeleted) { - return; - } + if (this._selfDeleted) { + return; + } - // safeguard - if (this.args.channel.messages.length > 200) { - return; - } + // safeguard + if (this.args.channel.messages.length > 200) { + return; + } - if (!this.args.channel?.canLoadMorePast) { - return; - } + if (!this.args.channel?.canLoadMorePast) { + return; + } - schedule("afterRender", () => { - const firstMessageId = this.args.channel?.messages?.[0]?.id; - if (!firstMessageId) { - return; - } + const firstMessage = this.args.channel?.messages?.[0]; + if (!firstMessage?.visible) { + return; + } - const scroller = document.querySelector(".chat-messages-container"); - const messageContainer = scroller.querySelector( - `.chat-message-container[data-id="${firstMessageId}"]` - ); - - if ( - !scroller || - !messageContainer || - !isElementInViewport(messageContainer) - ) { - return; - } - - this._fetchMoreMessagesThrottled({ - direction: PAST, - scrollTo: false, - }); - }); + this.fetchMoreMessages({ + direction: PAST, }); } - _fetchMoreMessagesThrottled(params) { - throttle( - this, - this._fetchMoreMessages, - params, - FETCH_MORE_MESSAGES_THROTTLE_MS - ); - } - @bind afterFetchCallback(channel, results) { const messages = []; let foundFirstNew = false; - results.chat_messages.forEach((messageData) => { - // If a message has been hidden it is because the current user is ignoring - // the user who sent it, so we want to unconditionally hide it, even if - // we are going directly to the target + results.chat_messages.forEach((messageData, index) => { + if (index === 0) { + messageData.firstOfResults = true; + } + if (this.currentUser.ignored_users) { + // If a message has been hidden it is because the current user is ignoring + // the user who sent it, so we want to unconditionally hide it, even if + // we are going directly to the target messageData.hidden = this.currentUser.ignored_users.includes( messageData.user.username ); @@ -447,7 +412,7 @@ export default class ChatLivePane extends Component { }, 2000); } - this._iOSFix(() => { + this.forceRendering(() => { messageEl.scrollIntoView({ block: opts.position ?? "center", }); @@ -459,13 +424,11 @@ export default class ChatLivePane extends Component { didShowMessage(message) { message.visible = true; this.updateLastReadMessage(message); - this._throttleComputeSeparators(); } @action didHideMessage(message) { message.visible = false; - this._throttleComputeSeparators(); } @debounce(READ_INTERVAL_MS) @@ -490,65 +453,53 @@ export default class ChatLivePane extends Component { if (this.args.channel.canLoadMoreFuture) { this._fetchAndScrollToLatest(); } else { - if (this._scrollerEl) { - // Trigger a tiny scrollTop change so Safari scrollbar is placed at bottom. - // Setting to just 0 doesn't work (it's at 0 by default, so there is no change) - // Very hacky, but no way to get around this Safari bug - this._scrollerEl.scrollTop = -1; - - this._iOSFix(() => { - this._scrollerEl.scrollTop = 0; - this.hasNewMessages = false; - }); - } + this.scrollToMessage( + this.args.channel.messages[this.args.channel.messages.length - 1].id + ); } }); } - onScroll() { + @action + computeScrollState(event) { if (this._selfDeleted) { return; } - resetIdle(); + cancel(this.onScrollEndedHandler); - if (this.loading || this.loadingMorePast || this.loadingMoreFuture) { - return; - } + this.isScrolling = true; - const scrollPosition = Math.abs(this._scrollerEl.scrollTop); - const total = this._scrollerEl.scrollHeight - this._scrollerEl.clientHeight; + const scrollPosition = Math.abs(event.target.scrollTop); + const total = event.target.scrollHeight - event.target.clientHeight; + const ratio = (scrollPosition / total) * 100; + this.isTowardsTop = ratio < 99 && ratio >= 34; + this.isTowardsBottom = ratio > 1 && ratio <= 4; + this.isAtBottom = ratio <= 1; + this.isAtTop = ratio >= 99; + this.needsArrow = ratio >= 5; - this.isAlmostDocked = scrollPosition / this._scrollerEl.offsetHeight < 0.67; - this.isDocked = scrollPosition <= 1; - - if ( - this._previousScrollTop - this._scrollerEl.scrollTop > - this._previousScrollTop - ) { - const atTop = this._isBetween( - scrollPosition, - total - STICKY_SCROLL_LENIENCE, - total + STICKY_SCROLL_LENIENCE - ); - - if (atTop) { - this._fetchMoreMessagesThrottled({ direction: PAST }); + if (this._previousScrollTop - scrollPosition <= 0) { + if (this.isTowardsTop || this.isAtTop) { + this.fetchMoreMessages({ direction: PAST }); } } else { - const atBottom = this._isBetween( - scrollPosition, - 0 + STICKY_SCROLL_LENIENCE, - 0 - STICKY_SCROLL_LENIENCE - ); - - if (atBottom) { - this.hasNewMessages = false; - this._fetchMoreMessagesThrottled({ direction: FUTURE }); + if (this.isTowardsBottom || this.isAtBottom) { + this.fetchMoreMessages({ direction: FUTURE }); } } - this._previousScrollTop = this._scrollerEl.scrollTop; + this._previousScrollTop = scrollPosition; + this.onScrollEndedHandler = discourseLater(this, this.onScrollEnded, 25); + } + + @bind + onScrollEnded() { + this.isScrolling = false; + + if (this.isAtBottom) { + this.hasNewMessages = false; + } } _isBetween(target, a, b) { @@ -640,7 +591,7 @@ export default class ChatLivePane extends Component { if (this.args.channel.canLoadMoreFuture) { // If we can load more messages, we just notice the user of new messages this.hasNewMessages = true; - } else if (this._scrollerEl.scrollTop <= 1) { + } else if (this.isAtBottom || this.isTowardsBottom) { // If we are at the bottom, we append the message and scroll to it const message = ChatMessage.create(this.args.channel, data.chat_message); this.args.channel.addMessages([message]); @@ -1107,6 +1058,10 @@ export default class ChatLivePane extends Component { return; } + if (this.isScrolling) { + return; + } + if (message?.staged) { return; } @@ -1140,20 +1095,6 @@ export default class ChatLivePane extends Component { } } - this._onHoverMessageDebouncedHandler = discourseDebounce( - this, - this.debouncedOnHoverMessage, - message, - 250 - ); - } - - @bind - debouncedOnHoverMessage(message) { - if (this._selfDeleted) { - return; - } - this.hoveredMessageId = message?.id && message.id !== this.hoveredMessageId ? message.id : null; } @@ -1216,6 +1157,7 @@ export default class ChatLivePane extends Component { } _fetchAndScrollToLatest() { + this.loadedOnce = false; return this.fetchMessages({ fetchFromLastMessage: true, }); @@ -1235,26 +1177,31 @@ export default class ChatLivePane extends Component { // since -webkit-overflow-scrolling: touch can't be used anymore to disable momentum scrolling // we now use this hack to disable it @bind - _iOSFix(callback) { - if (!this._scrollerEl) { - return; - } + forceRendering(callback) { + schedule("afterRender", () => { + if (!this._scrollerEl) { + return; + } - if (this.capabilities.isIOS) { - this._scrollerEl.style.overflow = "hidden"; - } + if (this.capabilities.isIOS) { + this._scrollerEl.style.transform = "translateZ(0)"; + this._scrollerEl.style.overflow = "hidden"; + } - callback?.(); + callback?.(); - if (this.capabilities.isIOS) { - discourseLater(() => { - if (!this._scrollerEl) { - return; - } + if (this.capabilities.isIOS) { + discourseLater(() => { + if (!this._scrollerEl) { + return; + } - this._scrollerEl.style.overflow = "auto"; - }, 25); - } + this._scrollerEl.style.overflow = "auto"; + this._scrollerEl.style.transform = "unset"; + this.computeDatesSeparators(); + }, 50); + } + }); } @action @@ -1300,31 +1247,28 @@ export default class ChatLivePane extends Component { } } - _throttleComputeSeparators() { - throttle(this, this._computeSeparators, 32, false); - } + @action + computeDatesSeparators() { + schedule("afterRender", () => { + const dates = [ + ...this._scrollerEl.querySelectorAll(".chat-message-separator-date"), + ].reverse(); + const scrollHeight = this._scrollerEl.scrollHeight; - _computeSeparators() { - next(() => { - schedule("afterRender", () => { - const dates = this._scrollerEl.querySelectorAll( - ".chat-message-separator-date" - ); - const scrollHeight = document.querySelector( - ".chat-messages-scroll" - ).scrollHeight; - const reversedDates = [...dates].reverse(); - // TODO (joffrey): optimize this code to trigger less layout computation - reversedDates.forEach((date, index) => { + dates + .map((date, index) => { + const item = { bottom: "0px", date }; if (index > 0) { - date.style.bottom = - scrollHeight - reversedDates[index - 1].offsetTop + "px"; - } else { - date.style.bottom = 0; + item.bottom = scrollHeight - dates[index - 1].offsetTop + "px"; } - date.style.top = date.nextElementSibling.offsetTop + "px"; + item.top = date.nextElementSibling.offsetTop + "px"; + return item; + }) + // group all writes at the end + .forEach((item) => { + item.date.style.bottom = item.bottom; + item.date.style.top = item.top; }); - }); }); } } diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message-collapser.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-message-collapser.hbs index ec613e5902..206f343fc4 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message-collapser.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message-collapser.hbs @@ -2,18 +2,30 @@ {{#if this.hasUploads}} {{html-safe @cooked}} - -
- {{#each @uploads as |upload|}} - - {{/each}} -
+ + {{#unless collapsed}} +
+ {{#each @uploads as |upload|}} + + {{/each}} +
+ {{/unless}} {{else}} {{#each this.cookedBodies as |cooked|}} {{#if cooked.needsCollapser}} - - {{cooked.body}} + + {{#unless collapsed}} + {{cooked.body}} + {{/unless}} {{else}} {{cooked.body}} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message-separator-date.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-message-separator-date.hbs index a27b80a14d..bb8e70d91a 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message-separator-date.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message-separator-date.hbs @@ -2,7 +2,7 @@
- {{@message.firstMessageOfTheDayAt}} + {{@message.firstMessageOfTheDayAt}} {{#if @message.newest}} - - - {{i18n "chat.last_visit"}} + + - + {{i18n "chat.last_visit"}} + {{/if}}
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message-text.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-message-text.hbs index f6f1eeb80e..e6f8059e3f 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message-text.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message-text.hbs @@ -1,6 +1,10 @@
{{#if this.isCollapsible}} - + {{else}} {{html-safe @cooked}} {{/if}} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-message.hbs index 5adb036b4f..a384098942 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message.hbs @@ -117,6 +117,7 @@ @cooked={{@message.cooked}} @uploads={{@message.uploads}} @edited={{@message.edited}} + @onToggleCollapse={{fn @forceRendering (noop)}} > {{#if @message.reactions.length}}
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message.js b/plugins/chat/assets/javascripts/discourse/components/chat-message.js index 5d3ae25586..14534d88d8 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message.js @@ -270,16 +270,27 @@ export default class ChatMessage extends Component { } get hideUserInfo() { + const message = this.args.message; + const previousMessage = message?.previousMessage; + + if (!previousMessage) { + return false; + } + + // this is a micro optimization to avoid layout changes when we load more messages + if (message?.firstOfResults) { + return false; + } + return ( - !this.args.message?.chatWebhookEvent && - !this.args.message?.inReplyTo && - !this.args.message?.previousMessage?.deletedAt && + !message?.chatWebhookEvent && + (!message?.inReplyTo || + message?.inReplyTo?.user?.id !== message?.user?.id) && + !message?.previousMessage?.deletedAt && Math.abs( - new Date(this.args.message?.createdAt) - - new Date(this.args.message?.createdAt) + new Date(message?.createdAt) - new Date(previousMessage?.createdAt) ) < 300000 && // If the time between messages is over 5 minutes, break. - this.args.message?.user?.id === - this.args.message?.previousMessage?.user?.id + message?.user?.id === message?.previousMessage?.user?.id ); } @@ -506,6 +517,8 @@ export default class ChatMessage extends Component { this.currentUser.id ); + this.args.forceRendering?.(); + return ajax( `/chat/${this.args.message.channelId}/react/${this.args.message.id}`, { diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-scroll-to-bottom-arrow.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-scroll-to-bottom-arrow.hbs index 7e6a58dd23..564019772a 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-scroll-to-bottom-arrow.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-scroll-to-bottom-arrow.hbs @@ -3,10 +3,7 @@ class={{concat-class "btn-flat" "chat-scroll-to-bottom" - (if - (or (not @isAlmostDocked) @hasNewMessages @channel.canLoadMoreFuture) - "visible" - ) + (if (or @show @hasNewMessages) "visible") }} @action={{@scrollToBottom}} > diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-skeleton.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-skeleton.hbs index 0bb2edefab..719aa26077 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-skeleton.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-skeleton.hbs @@ -1,8 +1,4 @@ -
+
{{#each this.placeholders as |placeholder|}}
diff --git a/plugins/chat/assets/javascripts/discourse/components/collapser.hbs b/plugins/chat/assets/javascripts/discourse/components/collapser.hbs index 180186ee62..7bee61a37b 100644 --- a/plugins/chat/assets/javascripts/discourse/components/collapser.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/collapser.hbs @@ -16,6 +16,11 @@ {{/if}}
-
- {{yield}} +
+ {{yield this.collapsed}}
\ No newline at end of file diff --git a/plugins/chat/assets/javascripts/discourse/components/collapser.js b/plugins/chat/assets/javascripts/discourse/components/collapser.js index f7d05cf837..db6a355a6a 100644 --- a/plugins/chat/assets/javascripts/discourse/components/collapser.js +++ b/plugins/chat/assets/javascripts/discourse/components/collapser.js @@ -6,14 +6,17 @@ export default Component.extend({ collapsed: false, header: null, + onToggle: null, @action open() { this.set("collapsed", false); + this.onToggle?.(false); }, @action close() { this.set("collapsed", true); + this.onToggle?.(true); }, }); diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-message.js b/plugins/chat/assets/javascripts/discourse/models/chat-message.js index a7e5685bdc..ed8d519e81 100644 --- a/plugins/chat/assets/javascripts/discourse/models/chat-message.js +++ b/plugins/chat/assets/javascripts/discourse/models/chat-message.js @@ -47,11 +47,13 @@ export default class ChatMessage { @tracked availableFlags; @tracked newest = false; @tracked highlighted = false; + @tracked firstOfResults = false; constructor(channel, args = {}) { this.channel = channel; this.id = args.id; this.newest = args.newest; + this.firstOfResults = args.firstOfResults; this.staged = args.staged; this.edited = args.edited; this.availableFlags = args.available_flags; diff --git a/plugins/chat/assets/javascripts/discourse/modifiers/chat/did-mutate-childlist.js b/plugins/chat/assets/javascripts/discourse/modifiers/chat/did-mutate-childlist.js new file mode 100644 index 0000000000..5db323f49c --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/modifiers/chat/did-mutate-childlist.js @@ -0,0 +1,24 @@ +import Modifier from "ember-modifier"; +import { registerDestructor } from "@ember/destroyable"; + +export default class ChatDidMutateChildlist extends Modifier { + constructor(owner, args) { + super(owner, args); + registerDestructor(this, (instance) => instance.cleanup()); + } + + modify(element, [callback]) { + this.mutationObserver = new MutationObserver(() => { + callback(); + }); + + this.mutationObserver.observe(element, { + childList: true, + subtree: true, + }); + } + + cleanup() { + this.mutationObserver?.disconnect(); + } +} diff --git a/plugins/chat/assets/javascripts/discourse/modifiers/chat/on-throttled-scroll.js b/plugins/chat/assets/javascripts/discourse/modifiers/chat/on-throttled-scroll.js new file mode 100644 index 0000000000..212493e53f --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/modifiers/chat/on-throttled-scroll.js @@ -0,0 +1,36 @@ +import Modifier from "ember-modifier"; +import { registerDestructor } from "@ember/destroyable"; +import { cancel, throttle } from "@ember/runloop"; +import { bind } from "discourse-common/utils/decorators"; + +export default class ChatOnThrottledScroll extends Modifier { + constructor(owner, args) { + super(owner, args); + registerDestructor(this, (instance) => instance.cleanup()); + } + + modify(element, [callback, options]) { + this.element = element; + this.callback = callback; + this.options = options; + this.element.addEventListener("scroll", this.throttledCallback, { + passive: true, + }); + } + + @bind + throttledCallback(event) { + this.throttledHandler = throttle( + this, + this.callback, + event, + this.options.delay ?? 100, + this.options.immediate ?? false + ); + } + + cleanup() { + cancel(this.throttledHandler); + this.element.removeEventListener("scroll", this.throttledCallback); + } +} diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-api.js b/plugins/chat/assets/javascripts/discourse/services/chat-api.js index 4b51143681..44ad75dc5c 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-api.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-api.js @@ -243,7 +243,7 @@ export default class ChatApi extends Service { * @param {integer} data.pageSize - Max number of messages to fetch. * @returns {Promise} */ - async messages(channelId, data = {}) { + messages(channelId, data = {}) { let path; const args = {}; diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js index 47a4fb88d8..0905f46802 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js @@ -134,6 +134,10 @@ export default class ChatChannelsManager extends Service { } #cache(channel) { + if (!channel) { + return; + } + this._cached[channel.id] = channel; } diff --git a/plugins/chat/assets/stylesheets/common/chat-message-separator.scss b/plugins/chat/assets/stylesheets/common/chat-message-separator.scss index 6b43783391..94c5aca2ff 100644 --- a/plugins/chat/assets/stylesheets/common/chat-message-separator.scss +++ b/plugins/chat/assets/stylesheets/common/chat-message-separator.scss @@ -44,7 +44,7 @@ justify-content: center; pointer-events: none; - &.last-visit { + &.with-last-visit { & + .chat-message-separator__line-container { .chat-message-separator__line { border-color: var(--danger-medium); @@ -57,11 +57,16 @@ position: sticky; top: -1px; - &.is-pinned { + &.is-pinned, + &.is-force-pinned { .chat-message-separator__text { border: 1px solid var(--primary-medium); border-radius: 3px; } + + .chat-message-separator__last-visit { + display: none; + } } } diff --git a/plugins/chat/assets/stylesheets/common/common.scss b/plugins/chat/assets/stylesheets/common/common.scss index 4ef4c7cf17..e6f8e3fbf5 100644 --- a/plugins/chat/assets/stylesheets/common/common.scss +++ b/plugins/chat/assets/stylesheets/common/common.scss @@ -145,9 +145,13 @@ $float-height: 530px; word-wrap: break-word; white-space: normal; position: relative; + will-change: transform; + transform: translateZ(0); .chat-message-container { display: grid; + will-change: transform; + transform: translateZ(0); &.selecting-messages { grid-template-columns: 1.5em 1fr; @@ -332,7 +336,7 @@ $float-height: 530px; position: absolute; z-index: 1; flex-direction: column; - bottom: -75px; + bottom: -25px; background: none; opacity: 0; transition: opacity 0.25s ease, transform 0.5s ease; @@ -350,7 +354,7 @@ $float-height: 530px; } &.visible { - transform: translateY(-75px) scale(1); + transform: translateY(-32px) scale(1); opacity: 0.8; } diff --git a/plugins/chat/spec/system/bookmark_message_spec.rb b/plugins/chat/spec/system/bookmark_message_spec.rb index e8afe05c91..1f62d0ebf6 100644 --- a/plugins/chat/spec/system/bookmark_message_spec.rb +++ b/plugins/chat/spec/system/bookmark_message_spec.rb @@ -52,7 +52,6 @@ RSpec.describe "Bookmark message", type: :system, js: true do context "when mobile", mobile: true do it "allows to bookmark a message" do chat.visit_channel(category_channel_1) - expect(channel).to have_no_loading_skeleton i = 0.5 try_until_success(timeout: 20) do diff --git a/plugins/chat/spec/system/chat_channel_spec.rb b/plugins/chat/spec/system/chat_channel_spec.rb index 34d6a933f5..b2b8369397 100644 --- a/plugins/chat/spec/system/chat_channel_spec.rb +++ b/plugins/chat/spec/system/chat_channel_spec.rb @@ -35,7 +35,6 @@ RSpec.describe "Chat channel", type: :system, js: true do it "allows to edit this message once persisted" do chat.visit_channel(channel_1) - expect(channel).to have_no_loading_skeleton channel.send_message("aaaaaaaaaaaaaaaaaaaa") expect(page).to have_no_css(".chat-message-staged") last_message = find(".chat-message-container:last-child") diff --git a/plugins/chat/spec/system/deleted_message_spec.rb b/plugins/chat/spec/system/deleted_message_spec.rb index 8bea5d4a17..aa6d4f9237 100644 --- a/plugins/chat/spec/system/deleted_message_spec.rb +++ b/plugins/chat/spec/system/deleted_message_spec.rb @@ -16,7 +16,6 @@ RSpec.describe "Deleted message", type: :system, js: true do context "when deleting a message" do it "shows as deleted" do chat_page.visit_channel(channel_1) - expect(channel_page).to have_no_loading_skeleton channel_page.send_message("aaaaaaaaaaaaaaaaaaaa") expect(page).to have_no_css(".chat-message-staged") last_message = find(".chat-message-container:last-child") diff --git a/plugins/chat/spec/system/hashtag_autocomplete_spec.rb b/plugins/chat/spec/system/hashtag_autocomplete_spec.rb index 47d4401c7a..a8514631e4 100644 --- a/plugins/chat/spec/system/hashtag_autocomplete_spec.rb +++ b/plugins/chat/spec/system/hashtag_autocomplete_spec.rb @@ -25,7 +25,6 @@ describe "Using #hashtag autocompletion to search for and lookup channels", it "searches for channels, categories, and tags with # and prioritises channels in the results" do chat_page.visit_channel(channel1) - expect(chat_channel_page).to have_no_loading_skeleton chat_channel_page.type_in_composer("this is #ra") expect(page).to have_css( ".hashtag-autocomplete .hashtag-autocomplete__option .hashtag-autocomplete__link", @@ -53,7 +52,6 @@ describe "Using #hashtag autocompletion to search for and lookup channels", it "cooks the hashtags for channels, categories, and tags serverside when the chat message is saved to the database" do chat_page.visit_channel(channel1) - expect(chat_channel_page).to have_no_loading_skeleton chat_channel_page.type_in_composer( "this is #random and this is #raspberry-beret and this is #razed which is cool", ) diff --git a/plugins/chat/spec/system/mention_warnings_spec.rb b/plugins/chat/spec/system/mention_warnings_spec.rb index e217b9bb29..6ad80f2a75 100644 --- a/plugins/chat/spec/system/mention_warnings_spec.rb +++ b/plugins/chat/spec/system/mention_warnings_spec.rb @@ -21,7 +21,6 @@ RSpec.describe "Mentions warnings", type: :system, js: true do it "displays a warning" do chat_page.visit_channel(channel_1) - expect(chat_channel_page).to have_no_loading_skeleton chat_channel_page.type_in_composer("@#{admin_mentionable_group.name} ") expect(page).to have_css(".chat-mention-warnings") @@ -46,7 +45,6 @@ RSpec.describe "Mentions warnings", type: :system, js: true do it "displays a warning" do chat_page.visit_channel(channel_1) - expect(chat_channel_page).to have_no_loading_skeleton chat_channel_page.type_in_composer("@#{publicly_mentionable_group.name} ") expect(page).to have_css(".chat-mention-warnings") @@ -61,7 +59,6 @@ RSpec.describe "Mentions warnings", type: :system, js: true do it "displays a warning" do chat_page.visit_channel(channel_1) - expect(chat_channel_page).to have_no_loading_skeleton chat_channel_page.type_in_composer( "@#{user_2.username} @#{publicly_mentionable_group.name} ", ) diff --git a/plugins/chat/spec/system/page_objects/chat/chat.rb b/plugins/chat/spec/system/page_objects/chat/chat.rb index 0ceceb7373..4ed41f580a 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat.rb @@ -19,6 +19,7 @@ module PageObjects def visit_channel(channel, mobile: false) visit(channel.url + (mobile ? "?mobile_view=1" : "")) + has_no_css?(".not-loaded-once") has_no_css?(".chat-skeleton") end diff --git a/plugins/chat/spec/system/reply_indicator_spec.rb b/plugins/chat/spec/system/reply_indicator_spec.rb deleted file mode 100644 index bae2eac310..0000000000 --- a/plugins/chat/spec/system/reply_indicator_spec.rb +++ /dev/null @@ -1,42 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe "Reply indicator", type: :system, js: true do - let(:chat_page) { PageObjects::Pages::Chat.new } - let(:channel_page) { PageObjects::Pages::ChatChannel.new } - - fab!(:channel_1) { Fabricate(:category_channel) } - fab!(:current_user) { Fabricate(:admin) } - - before do - chat_system_bootstrap - channel_1.add(current_user) - sign_in(current_user) - end - - context "when clicking on a reply indicator of a loaded message" do - fab!(:replied_to_message) do - Fabricate(:chat_message, chat_channel: channel_1, created_at: 2.hours.ago) - end - fab!(:reply) do - Fabricate( - :chat_message, - chat_channel: channel_1, - in_reply_to: replied_to_message, - created_at: 1.minute.ago, - ) - end - - before do - 10.times { Fabricate(:chat_message, chat_channel: channel_1, created_at: 1.hour.ago) } - end - - it "highlights the message without refreshing the pane" do - chat_page.visit_channel(channel_1) - - find("[data-id='#{reply.id}'] .chat-reply").click - - expect(page).to have_no_selector(".chat-skeleton") - expect(page).to have_selector("[data-id='#{replied_to_message.id}'].highlighted", wait: 0.1) - end - end -end diff --git a/plugins/chat/spec/system/replying_indicator_spec.rb b/plugins/chat/spec/system/replying_indicator_spec.rb deleted file mode 100644 index d2c1dd1a25..0000000000 --- a/plugins/chat/spec/system/replying_indicator_spec.rb +++ /dev/null @@ -1,35 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe "Replying indicator", type: :system, js: true do - fab!(:channel_1) { Fabricate(:chat_channel) } - fab!(:current_user) { Fabricate(:user) } - fab!(:other_user) { Fabricate(:user) } - - let(:chat) { PageObjects::Pages::Chat.new } - - before do - channel_1.add(current_user) - channel_1.add(other_user) - chat_system_bootstrap - sign_in(current_user) - end - - context "when on a channel" do - context "when another user is replying" do - it "shows the replying indicator" do - using_session(:user_1) do - sign_in(other_user) - chat.visit_channel(channel_1) - find(".chat-composer-input").fill_in(with: "hello there") - end - - chat.visit_channel(channel_1) - - expect(page).to have_selector( - ".chat-replying-indicator", - text: I18n.t("js.chat.replying_indicator.single_user", username: other_user.username), - ) - end - end - end -end diff --git a/plugins/chat/spec/system/transcript_spec.rb b/plugins/chat/spec/system/transcript_spec.rb index 886219096a..4f6297b181 100644 --- a/plugins/chat/spec/system/transcript_spec.rb +++ b/plugins/chat/spec/system/transcript_spec.rb @@ -93,8 +93,6 @@ RSpec.describe "Quoting chat message transcripts", type: :system, js: true do it "quotes the message" do chat_page.visit_channel(chat_channel_1) - expect(chat_channel_page).to have_no_loading_skeleton - clip_text = copy_messages_to_clipboard(message_1) topic_page.visit_topic_and_open_composer(post_1.topic) topic_page.fill_in_composer("This is a new post!\n\n" + clip_text) @@ -117,8 +115,6 @@ RSpec.describe "Quoting chat message transcripts", type: :system, js: true do it "quotes the messages" do chat_page.visit_channel(chat_channel_1) - expect(chat_channel_page).to have_no_loading_skeleton - clip_text = copy_messages_to_clipboard([message_1, message_2]) topic_page.visit_topic_and_open_composer(post_1.topic) topic_page.fill_in_composer("This is a new post!\n\n" + clip_text) @@ -149,8 +145,6 @@ RSpec.describe "Quoting chat message transcripts", type: :system, js: true do it "works" do chat_page.visit_channel(chat_channel_1) - expect(chat_channel_page).to have_no_loading_skeleton - clip_text = copy_messages_to_clipboard(message_1) topic_page.visit_topic_and_open_composer(post_1.topic) topic_page.fill_in_composer(clip_text) @@ -167,8 +161,6 @@ RSpec.describe "Quoting chat message transcripts", type: :system, js: true do it "quotes the message" do chat_page.visit_channel(chat_channel_1) - expect(chat_channel_page).to have_no_loading_skeleton - clip_text = copy_messages_to_clipboard(message_1) click_selection_button("cancel") chat_channel_page.send_message(clip_text) @@ -191,8 +183,6 @@ RSpec.describe "Quoting chat message transcripts", type: :system, js: true do it "opens the topic composer with correct state" do chat_page.visit_channel(chat_channel_1) - expect(chat_channel_page).to have_no_loading_skeleton - select_message_desktop(message_1) click_selection_button("quote") @@ -219,8 +209,6 @@ RSpec.describe "Quoting chat message transcripts", type: :system, js: true do mobile: true do chat_page.visit_channel(chat_channel_1) - expect(chat_channel_page).to have_no_loading_skeleton - chat_channel_page.click_message_action_mobile(message_1, "selectMessage") click_selection_button("quote") diff --git a/plugins/chat/test/javascripts/components/chat-message-test.js b/plugins/chat/test/javascripts/components/chat-message-test.js index 7ffa9da1d0..d75f7fd528 100644 --- a/plugins/chat/test/javascripts/components/chat-message-test.js +++ b/plugins/chat/test/javascripts/components/chat-message-test.js @@ -57,6 +57,7 @@ module("Discourse Chat | Component | chat-message", function (hooks) { onHoverMessage: () => {}, didShowMessage: () => {}, didHideMessage: () => {}, + forceRendering: () => {}, }; } @@ -75,6 +76,8 @@ module("Discourse Chat | Component | chat-message", function (hooks) { @onHoverMessage={{this.onHoverMessage}} @didShowMessage={{this.didShowMessage}} @didHideMessage={{this.didHideMessage}} + @didHideMessage={{this.didHideMessage}} + @forceRendering={{this.forceRendering}} /> `;