[FIX] mail: show chatter attachment loading after 1 second

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) <aku@odoo.com>

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]
+        );
     }
 
     /**

addons/mail/static/src/core/web/chatter.xml

@@ -138,7 +138,7 @@
     <button class="btn btn-link text-action px-1 d-flex align-items-center" aria-label="Attach files" t-att-class="{ 'my-2': !props.compactHeight }" t-on-click="onClickAddAttachments">
         <i class="fa fa-paperclip fa-lg me-1"/>
         <span t-if="attachments.length > 0" t-esc="attachments.length"/>
-        <i t-if="!state.thread.areAttachmentsLoaded and state.thread.isLoadingAttachments" class="fa fa-circle-o-notch fa-spin" aria-label="Attachment counter loading..."/>
+        <i t-if="!state.thread.areAttachmentsLoaded and state.thread.isLoadingAttachments and state.showAttachmentLoading" class="fa fa-circle-o-notch fa-spin" aria-label="Attachment counter loading..."/>
     </button>
 </t>
 

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");
     }

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");