From 92d9b6d7796f855a0f7d37b79a781513b0cf95f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20K=C3=BChn?= Date: Thu, 17 Aug 2023 14:43:57 +0000 Subject: [PATCH] [FIX] mail: show chatter attachment loading after 1 second MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this commit, when chatter data were fetched, it shows spinner icon on attachments immediately until data are fetched. This RPC is quite fast, so it's annoying to see spinner and then no spinner whenever chatter is reloaded. This is even more bothersome when most chatters have no attachments and some followers, which gives the illusion loading affects followers rather than attachments, as the spinner takes place of attachment counter that is empty when no attachment. This commit fixes the issue by having a 1 second delay for showing the loading icon next to attachment. This prevents most flickers when RPC is fast, while keeping it relevant when it's slow so it's clear data are still loading. Task-3356922 closes odoo/odoo#132463 X-original-commit: 96b00d25becdf2253975d1b427c30ee287cda98f Signed-off-by: Alexandre Kühn (aku) --- addons/mail/static/src/core/web/chatter.js | 21 +++++++++++++++++++ addons/mail/static/src/core/web/chatter.xml | 2 +- addons/mail/static/tests/web/chatter_tests.js | 7 +++++-- .../static/tests/web/chatter_topbar_tests.js | 14 +++++++++---- 4 files changed, 37 insertions(+), 7 deletions(-) diff --git a/addons/mail/static/src/core/web/chatter.js b/addons/mail/static/src/core/web/chatter.js index 903ff7b68f9..a5b04544195 100644 --- a/addons/mail/static/src/core/web/chatter.js +++ b/addons/mail/static/src/core/web/chatter.js @@ -26,6 +26,7 @@ import { useState, } from "@odoo/owl"; +import { browser } from "@web/core/browser/browser"; import { Dropdown } from "@web/core/dropdown/dropdown"; import { _t } from "@web/core/l10n/translation"; import { usePopover } from "@web/core/popover/popover_hook"; @@ -34,6 +35,8 @@ import { escape } from "@web/core/utils/strings"; import { useThrottleForAnimation } from "@web/core/utils/timing"; import { FileUploader } from "@web/views/fields/file_handler"; +export const DELAY_FOR_SPINNER = 1000; + /** * @typedef {Object} Props * @property {function} [close] @@ -82,6 +85,8 @@ export class Chatter extends Component { isInFormSheetBg: true, threadId: false, }; + /** @type {number|null} */ + loadingAttachmentTimeout = null; setup() { this.action = useService("action"); @@ -97,6 +102,7 @@ export class Chatter extends Component { isAttachmentBoxOpened: this.props.isAttachmentBoxVisibleInitially, jumpThreadPresent: 0, showActivities: true, + showAttachmentLoading: false, /** @type {import("@mail/core/common/thread_model").Thread} */ thread: undefined, }); @@ -169,6 +175,21 @@ export class Chatter extends Component { }, () => [this.state.isAttachmentBoxOpened] ); + useEffect( + () => { + browser.clearTimeout(this.loadingAttachmentTimeout); + if (this.state.thread?.isLoadingAttachments) { + this.loadingAttachmentTimeout = browser.setTimeout( + () => (this.state.showAttachmentLoading = true), + DELAY_FOR_SPINNER + ); + } else { + this.state.showAttachmentLoading = false; + } + return () => browser.clearTimeout(this.loadingAttachmentTimeout); + }, + () => [this.state.thread?.isLoadingAttachments] + ); } /** diff --git a/addons/mail/static/src/core/web/chatter.xml b/addons/mail/static/src/core/web/chatter.xml index bdbffbbb364..e2e760c3179 100644 --- a/addons/mail/static/src/core/web/chatter.xml +++ b/addons/mail/static/src/core/web/chatter.xml @@ -138,7 +138,7 @@ diff --git a/addons/mail/static/tests/web/chatter_tests.js b/addons/mail/static/tests/web/chatter_tests.js index 065ccacfd89..604ea9162ba 100644 --- a/addons/mail/static/tests/web/chatter_tests.js +++ b/addons/mail/static/tests/web/chatter_tests.js @@ -12,6 +12,7 @@ import { startServer, waitUntil, } from "@mail/../tests/helpers/test_utils"; +import { DELAY_FOR_SPINNER } from "@mail/core/web/chatter"; import { editInput, triggerHotkey } from "@web/../tests/helpers/utils"; import { file } from "@web/../tests/legacy/helpers/test_utils"; @@ -130,7 +131,8 @@ QUnit.test("No attachment loading spinner when creating records", async (assert) QUnit.test( "No attachment loading spinner when switching from loading record to creation of record", async (assert) => { - const { openFormView, pyEnv } = await start({ + const { advanceTime, openFormView, pyEnv } = await start({ + hasTimeControl: true, async mockRPC(route) { if (route === "/mail/thread/data") { await new Promise(() => {}); @@ -139,7 +141,8 @@ QUnit.test( }); const partnerId = pyEnv["res.partner"].create({ name: "John" }); await openFormView("res.partner", partnerId, { waitUntilDataLoaded: false }); - assert.containsOnce($, "button[aria-label='Attach files'] .fa-spin"); + await advanceTime(DELAY_FOR_SPINNER); + await waitUntil("button[aria-label='Attach files'] .fa-spin"); await click(".o_form_button_create"); assert.containsNone($, "button[aria-label='Attach files'] .fa-spin"); } diff --git a/addons/mail/static/tests/web/chatter_topbar_tests.js b/addons/mail/static/tests/web/chatter_topbar_tests.js index f066ad49de4..b2d1c33d3ac 100644 --- a/addons/mail/static/tests/web/chatter_topbar_tests.js +++ b/addons/mail/static/tests/web/chatter_topbar_tests.js @@ -6,7 +6,9 @@ import { nextAnimationFrame, start, startServer, + waitUntil, } from "@mail/../tests/helpers/test_utils"; +import { DELAY_FOR_SPINNER } from "@mail/core/web/chatter"; import { makeDeferred } from "@web/../tests/helpers/utils"; @@ -181,7 +183,8 @@ QUnit.test("attachment counter with attachments", async (assert) => { QUnit.test("attachment counter while loading attachments", async (assert) => { const pyEnv = await startServer(); const partnerId = pyEnv["res.partner"].create({}); - const { openView } = await start({ + const { advanceTime, openView } = await start({ + hasTimeControl: true, async mockRPC(route) { if (route.includes("/mail/thread/data")) { await makeDeferred(); // simulate long loading @@ -193,7 +196,8 @@ QUnit.test("attachment counter while loading attachments", async (assert) => { res_model: "res.partner", views: [[false, "form"]], }); - assert.containsOnce($, "button[aria-label='Attach files'] .fa-spin"); + await advanceTime(DELAY_FOR_SPINNER); + await waitUntil("button[aria-label='Attach files'] .fa-spin"); assert.containsNone($, "button[aria-label='Attach files']:contains(0)"); }); @@ -201,7 +205,8 @@ QUnit.test("attachment counter transition when attachments become loaded", async const pyEnv = await startServer(); const partnerId = pyEnv["res.partner"].create({}); const deferred = makeDeferred(); - const { openView } = await start({ + const { advanceTime, openView } = await start({ + hasTimeControl: true, async mockRPC(route) { if (route.includes("/mail/thread/data")) { await deferred; @@ -213,7 +218,8 @@ QUnit.test("attachment counter transition when attachments become loaded", async res_model: "res.partner", views: [[false, "form"]], }); - assert.containsOnce($, "button[aria-label='Attach files'] .fa-spin"); + await advanceTime(DELAY_FOR_SPINNER); + await waitUntil("button[aria-label='Attach files'] .fa-spin"); await afterNextRender(() => deferred.resolve()); assert.containsNone($, "button[aria-label='Attach files'] .fa-spin");