diff --git a/addons/mail/static/src/core/common/message_model.js b/addons/mail/static/src/core/common/message_model.js index ac4f8273a78..db454e03221 100644 --- a/addons/mail/static/src/core/common/message_model.js +++ b/addons/mail/static/src/core/common/message_model.js @@ -110,7 +110,6 @@ export class Message extends Record { /** @type {number[]} */ needaction_partner_ids = []; /** - * @deprecated * Still necessary until custom insert()/update() rely on this. * Fields are computed only at end of update cycle, thus it is not * computed during custom update @@ -118,6 +117,7 @@ export class Message extends Record { get originThread() { return this._store.Thread.get({ model: this.model, id: this.res_id }); } + /** @deprecated */ originThread2 = Record.one("Thread", { inverse: "allMessages", compute() { diff --git a/addons/mail/static/src/core/common/record.js b/addons/mail/static/src/core/common/record.js index 53fdaad7675..1886ff1889e 100644 --- a/addons/mail/static/src/core/common/record.js +++ b/addons/mail/static/src/core/common/record.js @@ -1152,7 +1152,7 @@ export class Record { } static onChange(record, name, cb) { const selfRaw = toRaw(this); - this._onChange(record, name, (observe) => { + return this._onChange(record, name, (observe) => { const fn = () => { observe(); cb(); @@ -1174,6 +1174,7 @@ export class Record { * @param {Record} record * @param {string|string[]} key * @param {(observe: Function) => any} callback + * @returns {function} function to call to stop observing changes */ static _onChange(record, key, callback) { let proxy; @@ -1204,9 +1205,16 @@ export class Record { } return; } - proxy = reactive(record, () => callback(_observe)); + let ready = true; + proxy = reactive(record, () => { + if (ready) { + callback(_observe); + } + }); _observe(); - return proxy; + return () => { + ready = false; + }; } /** * Contains field definitions of the model: diff --git a/addons/mail/static/src/core/common/thread.js b/addons/mail/static/src/core/common/thread.js index 9d3f636cf9f..813c4f8a4f3 100644 --- a/addons/mail/static/src/core/common/thread.js +++ b/addons/mail/static/src/core/common/thread.js @@ -2,11 +2,13 @@ import { DateSection } from "@mail/core/common/date_section"; import { Message } from "@mail/core/common/message"; +import { Record } from "@mail/core/common/record"; import { useVisible } from "@mail/utils/common/hooks"; import { Component, onMounted, + onWillDestroy, onWillPatch, onWillUpdateProps, toRaw, @@ -84,7 +86,7 @@ export class Thread extends Component { this.loadOlderState = useVisible( "load-older", () => { - if (this.loadOlderState.isVisible && !this.isJumpingRecent) { + if (this.loadOlderState.isVisible) { this.threadService.fetchMoreMessages(this.props.thread); } }, @@ -93,7 +95,7 @@ export class Thread extends Component { this.loadNewerState = useVisible( "load-newer", () => { - if (this.loadNewerState.isVisible && !this.isJumpingRecent) { + if (this.loadNewerState.isVisible) { this.threadService.fetchMoreMessages(this.props.thread, "newer"); } }, @@ -112,6 +114,7 @@ export class Thread extends Component { useEffect( () => { if (this.props.jumpPresent !== this.lastJumpPresent) { + this.messageHighlight?.clearHighlight(); if (this.props.thread.loadNewer) { this.jumpToPresent("instant"); } else { @@ -152,12 +155,13 @@ export class Thread extends Component { useEffect( (isLoaded) => { this.state.mountedAndLoaded = isLoaded; - if (!isLoaded) { - this.loadOlderState.ready = false; - this.loadNewerState.ready = false; - } }, - () => [this.props.thread.isLoaded] + /** + * Observe `mountedAndLoaded` as well because it might change from + * other parts of the code without `useEffect` detecting any change + * for `isLoaded`, and it should still be reset when patching. + */ + () => [this.props.thread.isLoaded, this.state.mountedAndLoaded] ); useBus(this.env.bus, "MAIL:RELOAD-THREAD", ({ detail }) => { const { model, id } = this.props.thread; @@ -236,6 +240,40 @@ export class Thread extends Component { * in place (point 1). */ let loadNewer; + const reset = () => { + this.state.mountedAndLoaded = false; + this.loadOlderState.ready = false; + this.loadNewerState.ready = false; + lastSetValue = undefined; + snapshot = undefined; + newestPersistentMessage = undefined; + oldestPersistentMessage = undefined; + loadedAndPatched = false; + loadNewer = false; + }; + /** + * These states need to be immediately reset when the value changes on + * the record, because the transition is important, not only the final + * value. If resetting is depending on the update cycle, it can happen + * that the value quickly changes and then back again before there is + * any mounting/patching, and the change would therefore be undetected. + */ + let stopOnChange = Record.onChange(this.props.thread, "isLoaded", () => { + if (!this.props.thread.isLoaded || !this.state.mountedAndLoaded) { + reset(); + } + }); + onWillUpdateProps((nextProps) => { + if (nextProps.thread.notEq(this.props.thread)) { + stopOnChange(); + stopOnChange = Record.onChange(nextProps.thread, "isLoaded", () => { + if (!nextProps.thread.isLoaded || !this.state.mountedAndLoaded) { + reset(); + } + }); + } + }); + onWillDestroy(() => stopOnChange()); const saveScroll = () => { this.props.thread.scrollTop = ref.el.scrollHeight - ref.el.scrollTop - ref.el.clientHeight < 30 @@ -249,7 +287,7 @@ export class Thread extends Component { }; const applyScroll = () => { if (!this.props.thread.isLoaded || !this.state.mountedAndLoaded) { - loadedAndPatched = false; + reset(); return; } // Use toRaw() to prevent scroll check from triggering renders. @@ -264,9 +302,9 @@ export class Thread extends Component { (this.props.order === "asc" && newerMessages && (loadNewer || thread.scrollTop !== "bottom")); - if (snapshot && !this.isJumpingRecent && messagesAtTop) { + if (snapshot && messagesAtTop) { setScroll(snapshot.scrollTop + ref.el.scrollHeight - snapshot.scrollHeight); - } else if (snapshot && !this.isJumpingRecent && messagesAtBottom) { + } else if (snapshot && messagesAtBottom) { setScroll(snapshot.scrollTop); } else if ( !this.env.messageHighlight?.highlightedMessageId && @@ -331,20 +369,14 @@ export class Thread extends Component { } async jumpToPresent(behavior) { - this.isJumpingRecent = true; + this.messageHighlight?.clearHighlight(); await this.threadService.loadAround(this.props.thread); this.props.thread.loadNewer = false; this.present.el?.scrollIntoView({ behavior: behavior ?? (this.props.order === "asc" ? "smooth" : "instant"), // FIXME somehow smooth not working in desc mode block: "center", }); - // Let smooth scroll a bit so load more is not visible - // smooth scrolling starts after 1 animation frame, hence needs to wait 2 animation frames - // for load more becoming not visible - await new Promise((resolve) => setTimeout(() => requestAnimationFrame(resolve))); - await new Promise((resolve) => setTimeout(() => requestAnimationFrame(resolve))); this.state.showJumpPresent = false; - this.isJumpingRecent = false; } /** diff --git a/addons/mail/static/src/core/common/thread_service.js b/addons/mail/static/src/core/common/thread_service.js index b708d16e0ce..ee1dce06e4a 100644 --- a/addons/mail/static/src/core/common/thread_service.js +++ b/addons/mail/static/src/core/common/thread_service.js @@ -298,6 +298,7 @@ export class ThreadService { async loadAround(thread, messageId) { if (!thread.messages.some(({ id }) => id === messageId)) { thread.isLoaded = false; + thread.scrollTop = undefined; const { messages } = await this.rpc(this.getFetchRoute(thread), { ...this.getFetchParams(thread), around: messageId, @@ -315,8 +316,6 @@ export class ThreadService { } } this._enrichMessagesWithTransient(thread); - // Give some time to the UI to update. - await new Promise((resolve) => setTimeout(() => requestAnimationFrame(resolve))); } } diff --git a/addons/mail/static/src/utils/common/hooks.js b/addons/mail/static/src/utils/common/hooks.js index c854f44ec91..7bd933e3216 100644 --- a/addons/mail/static/src/utils/common/hooks.js +++ b/addons/mail/static/src/utils/common/hooks.js @@ -11,6 +11,7 @@ import { useState, } from "@odoo/owl"; +import { browser } from "@web/core/browser/browser"; import { useService } from "@web/core/utils/hooks"; export function useLazyExternalListener(target, eventName, handler, eventParams) { @@ -236,6 +237,7 @@ export function useScrollSnapshot(ref, { onWillPatch: p_onWillPatch, onPatched: /** * @typedef {Object} MessageHighlight + * @property {function} clearHighlight * @property {function} highlightMessage * @property {number|null} highlightedMessageId * @returns {MessageHighlight} @@ -244,6 +246,13 @@ export function useMessageHighlight(duration = 2000) { let timeout; const threadService = useService("mail.thread"); const state = useState({ + clearHighlight() { + if (this.highlightedMessageId) { + browser.clearTimeout(timeout); + timeout = null; + this.highlightedMessageId = null; + } + }, /** * @param {import("models").Message} message * @param {import("models").Thread} thread @@ -254,21 +263,17 @@ export function useMessageHighlight(duration = 2000) { } await threadService.loadAround(thread, message.id); const lastHighlightedMessageId = state.highlightedMessageId; - clearHighlight(); + this.clearHighlight(); if (lastHighlightedMessageId === message.id) { // Give some time for the state to update. await new Promise(setTimeout); } + thread.scrollTop = undefined; state.highlightedMessageId = message.id; - timeout = setTimeout(clearHighlight, duration); + timeout = browser.setTimeout(() => this.clearHighlight(), duration); }, highlightedMessageId: null, }); - function clearHighlight() { - clearTimeout(timeout); - timeout = null; - state.highlightedMessageId = null; - } return state; } diff --git a/addons/mail/static/tests/discuss/message_pin/pinned_messages_tests.js b/addons/mail/static/tests/discuss/message_pin/pinned_messages_tests.js index eba14e32a13..9c3b2ea2dee 100644 --- a/addons/mail/static/tests/discuss/message_pin/pinned_messages_tests.js +++ b/addons/mail/static/tests/discuss/message_pin/pinned_messages_tests.js @@ -134,6 +134,7 @@ QUnit.test("Jump to message from notification", async () => { } const { openDiscuss } = await start(); openDiscuss(channelId); + await contains(".o-mail-Message", { count: 21 }); await click(":nth-child(1 of .o-mail-Message) [title='Expand']"); await click(".dropdown-item", { text: "Pin" }); await click(".modal-footer button", { text: "Yeah, pin it!" });