From 92797109bab952746e4a5a0a846450bbc6133480 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Tue, 21 Mar 2023 11:30:32 +0100 Subject: [PATCH] DEV: uses container resize event instead of mutation (#20757) This commit takes advantage of the `ResizeObserver` to know when dates should be re-computed, it works like this: ``` scrollable-div -- child-enclosing-div with resize observer ---- message 1 ---- message 2 ---- message x ``` It also switches to bottom/height for date separators sizing, instead of bottom/top, it prevents a bug where setting the top of the first item (at the top) would cause scrollbar to move to top. --- .../discourse/components/chat-live-pane.hbs | 11 ++-- .../discourse/components/chat-live-pane.js | 63 +++++++++++-------- .../discourse/components/chat-message.hbs | 1 - .../discourse/components/chat-message.js | 2 - .../modifiers/chat/did-mutate-childlist.js | 24 ------- .../discourse/modifiers/chat/on-resize.js | 29 +++++++++ .../{on-throttled-scroll.js => on-scroll.js} | 2 +- .../components/chat-message-test.js | 2 - 8 files changed, 71 insertions(+), 63 deletions(-) delete mode 100644 plugins/chat/assets/javascripts/discourse/modifiers/chat/did-mutate-childlist.js create mode 100644 plugins/chat/assets/javascripts/discourse/modifiers/chat/on-resize.js rename plugins/chat/assets/javascripts/discourse/modifiers/chat/{on-throttled-scroll.js => on-scroll.js} (93%) 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 15cd7bdf434..3b64a467f62 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.hbs @@ -41,14 +41,11 @@
-
+
{{#if this.loadedOnce}} {{#each @channel.messages key="id" as |message|}} {{/each}} {{else}} @@ -75,6 +71,7 @@ {{/if}}
+ {{! at bottom even if shown at top due to column-reverse }} {{#if (and this.loadedOnce (not @channel.messagesManager.canLoadMorePast))}}
{{i18n "chat.all_loaded"}} 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 67c3c09757a..f9a8109b4b7 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js @@ -4,11 +4,10 @@ import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; import ChatMessageDraft from "discourse/plugins/chat/discourse/models/chat-message-draft"; import Component from "@glimmer/component"; import { bind, debounce } from "discourse-common/utils/decorators"; -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, schedule } from "@ember/runloop"; +import { cancel, schedule, throttle } from "@ember/runloop"; import discourseLater from "discourse-common/lib/later"; import { inject as service } from "@ember/service"; import { Promise } from "rsvp"; @@ -64,7 +63,6 @@ export default class ChatLivePane extends Component { setupListeners(element) { this._scrollerEl = element.querySelector(".chat-messages-scroll"); - window.addEventListener("resize", this.onResizeHandler); document.addEventListener("scroll", this._forceBodyScroll, { passive: true, }); @@ -76,14 +74,19 @@ export default class ChatLivePane extends Component { @action teardownListeners() { - window.removeEventListener("resize", this.onResizeHandler); - cancel(this.resizeHandler); document.removeEventListener("scroll", this._forceBodyScroll); removeOnPresenceChange(this.onPresenceChangeCallback); this._unsubscribeToUpdates(this._loadedChannelId); this.requestedTargetMessageId = null; } + @action + didResizePane() { + this.fillPaneAttempt(); + this.computeDatesSeparators(); + this.forceRendering(); + } + @action resetIdle() { resetIdle(); @@ -126,17 +129,6 @@ export default class ChatLivePane extends Component { } } - @bind - onResizeHandler() { - cancel(this.resizeHandler); - this.resizeHandler = discourseDebounce( - this, - this.fillPaneAttempt, - this.details, - 250 - ); - } - @bind onPresenceChangeCallback(present) { if (present) { @@ -286,7 +278,6 @@ export default class ChatLivePane extends Component { .finally(() => { this[loadingMoreKey] = false; this.fillPaneAttempt(); - this.computeDatesSeparators(); }); } @@ -1238,7 +1229,6 @@ export default class ChatLivePane extends Component { } if (this.capabilities.isIOS) { - this._scrollerEl.style.transform = "translateZ(0)"; this._scrollerEl.style.overflow = "hidden"; } @@ -1251,8 +1241,6 @@ export default class ChatLivePane extends Component { } this._scrollerEl.style.overflow = "auto"; - this._scrollerEl.style.transform = "unset"; - this.computeDatesSeparators(); }, 50); } }); @@ -1307,25 +1295,48 @@ export default class ChatLivePane extends Component { @action computeDatesSeparators() { + throttle(this, this._computeDatesSeparators, 50, false); + } + + _computeDatesSeparators() { schedule("afterRender", () => { const dates = [ ...this._scrollerEl.querySelectorAll(".chat-message-separator-date"), ].reverse(); - const scrollHeight = this._scrollerEl.scrollHeight; + const height = this._scrollerEl.querySelector( + ".chat-messages-container" + ).clientHeight; dates .map((date, index) => { - const item = { bottom: "0px", date }; + const item = { bottom: 0, date }; + const line = date.nextElementSibling; + if (index > 0) { - item.bottom = scrollHeight - dates[index - 1].offsetTop + "px"; + const prevDate = dates[index - 1]; + const prevLine = prevDate.nextElementSibling; + item.bottom = height - prevLine.offsetTop; } - item.top = date.nextElementSibling.offsetTop + "px"; + + if (dates.length === 1) { + item.height = height; + } else { + if (index === 0) { + item.height = height - line.offsetTop; + } else { + const prevDate = dates[index - 1]; + const prevLine = prevDate.nextElementSibling; + item.height = + height - line.offsetTop - (height - prevLine.offsetTop); + } + } + return item; }) // group all writes at the end .forEach((item) => { - item.date.style.bottom = item.bottom; - item.date.style.top = item.top; + item.date.style.bottom = item.bottom + "px"; + item.date.style.height = item.height + "px"; }); }); } diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-message.hbs index da06ec0699d..916eebaf3f5 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message.hbs @@ -120,7 +120,6 @@ @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 eb8f13ef4c2..2eed450fec6 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message.js @@ -517,8 +517,6 @@ 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/modifiers/chat/did-mutate-childlist.js b/plugins/chat/assets/javascripts/discourse/modifiers/chat/did-mutate-childlist.js deleted file mode 100644 index 5db323f49c4..00000000000 --- a/plugins/chat/assets/javascripts/discourse/modifiers/chat/did-mutate-childlist.js +++ /dev/null @@ -1,24 +0,0 @@ -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-resize.js b/plugins/chat/assets/javascripts/discourse/modifiers/chat/on-resize.js new file mode 100644 index 00000000000..a836d690fde --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/modifiers/chat/on-resize.js @@ -0,0 +1,29 @@ +import Modifier from "ember-modifier"; +import { registerDestructor } from "@ember/destroyable"; +import { cancel, throttle } from "@ember/runloop"; + +export default class ChatOnResize extends Modifier { + constructor(owner, args) { + super(owner, args); + registerDestructor(this, (instance) => instance.cleanup()); + } + + modify(element, [fn, options = {}]) { + this.resizeObserver = new ResizeObserver((entries) => { + this.throttleHandler = throttle( + this, + fn, + entries, + options.delay ?? 0, + options.immediate ?? false + ); + }); + + this.resizeObserver.observe(element); + } + + cleanup() { + cancel(this.throttleHandler); + this.resizeObserver?.disconnect(); + } +} diff --git a/plugins/chat/assets/javascripts/discourse/modifiers/chat/on-throttled-scroll.js b/plugins/chat/assets/javascripts/discourse/modifiers/chat/on-scroll.js similarity index 93% rename from plugins/chat/assets/javascripts/discourse/modifiers/chat/on-throttled-scroll.js rename to plugins/chat/assets/javascripts/discourse/modifiers/chat/on-scroll.js index 212493e53f7..f8368b4513c 100644 --- a/plugins/chat/assets/javascripts/discourse/modifiers/chat/on-throttled-scroll.js +++ b/plugins/chat/assets/javascripts/discourse/modifiers/chat/on-scroll.js @@ -3,7 +3,7 @@ import { registerDestructor } from "@ember/destroyable"; import { cancel, throttle } from "@ember/runloop"; import { bind } from "discourse-common/utils/decorators"; -export default class ChatOnThrottledScroll extends Modifier { +export default class ChatOnScroll extends Modifier { constructor(owner, args) { super(owner, args); registerDestructor(this, (instance) => instance.cleanup()); diff --git a/plugins/chat/test/javascripts/components/chat-message-test.js b/plugins/chat/test/javascripts/components/chat-message-test.js index e220825339f..1e85fbc289f 100644 --- a/plugins/chat/test/javascripts/components/chat-message-test.js +++ b/plugins/chat/test/javascripts/components/chat-message-test.js @@ -57,7 +57,6 @@ module("Discourse Chat | Component | chat-message", function (hooks) { onHoverMessage: () => {}, messageDidEnterViewport: () => {}, messageDidLeaveViewport: () => {}, - forceRendering: () => {}, }; } @@ -76,7 +75,6 @@ module("Discourse Chat | Component | chat-message", function (hooks) { @onHoverMessage={{this.onHoverMessage}} @messageDidEnterViewport={{this.messageDidEnterViewport}} @messageDidLeaveViewport={{this.messageDidLeaveViewport}} - @forceRendering={{this.forceRendering}} /> `;