[FIX] spreadsheet: avoid parasitic renders

Clicking on a button on the topbar would trigger 3 renders:
- one on model update (expected)
- one because we received a collaborative event for our own revision
(useless)
- one because the onClick of the topbar calls
`DOMFocusableElementStore.focus`, which triggers a render because a
store method was called (useless)

closes #5450

Task: 4373054
X-original-commit: d926ab9
Signed-off-by: Rémi Rahir (rar) <[email protected]>

src/collaborative/session.ts

@@ -325,6 +325,9 @@ export class Session extends EventBus<CollaborativeEvent> {
       }
     }
     this.acknowledge(message);
+    if (message.type === "REMOTE_REVISION" && message.clientId === this.clientId) {
+      return;
+    }
     this.trigger("collaborative-event-received");
   }
 

src/components/bottom_bar/bottom_bar_sheet/bottom_bar_sheet.ts

@@ -169,11 +169,11 @@ export class BottomBarSheet extends Component<Props, SpreadsheetChildEnv> {
     if (ev.key === "Enter") {
       ev.preventDefault();
       this.stopEdition();
-      this.DOMFocusableElementStore.focus();
+      this.DOMFocusableElementStore.focusableElement?.focus();
     }
     if (ev.key === "Escape") {
       this.cancelEdition();
-      this.DOMFocusableElementStore.focus();
+      this.DOMFocusableElementStore.focusableElement?.focus();
     }
   }
 

src/components/composer/composer/composer.ts

@@ -264,7 +264,7 @@ export class Composer extends Component<CellComposerProps, SpreadsheetChildEnv>
         this.props.composerStore.editionMode === "inactive" &&
         !this.props.isDefaultFocus
       ) {
-        this.DOMFocusableElementStore.focus();
+        this.DOMFocusableElementStore.focusableElement?.focus();
       }
     });
 

src/components/grid/grid.ts

@@ -182,7 +182,7 @@ export class Grid extends Component<Props, SpreadsheetChildEnv> {
     useEffect(
       () => {
         if (!this.sidePanel.isOpen) {
-          this.DOMFocusableElementStore.focus();
+          this.DOMFocusableElementStore.focusableElement?.focus();
         }
       },
       () => [this.sidePanel.isOpen]
@@ -408,7 +408,7 @@ export class Grid extends Component<Props, SpreadsheetChildEnv> {
       !this.env.model.getters.getSelectedFigureId() &&
       this.composerFocusStore.activeComposer.editionMode === "inactive"
     ) {
-      this.DOMFocusableElementStore.focus();
+      this.DOMFocusableElementStore.focusableElement?.focus();
     }
   }
 

src/stores/DOM_focus_store.ts

@@ -1,12 +1,8 @@
 export class DOMFocusableElementStore {
-  mutators = ["setFocusableElement", "focus"] as const;
-  private focusableElement: HTMLElement | undefined = undefined;
+  mutators = ["setFocusableElement"] as const;
+  focusableElement: HTMLElement | undefined = undefined;
 
   setFocusableElement(element: HTMLElement) {
     this.focusableElement = element;
   }
-
-  focus() {
-    this.focusableElement?.focus();
-  }
 }

tests/bottom_bar/bottom_bar_component.test.ts

@@ -355,14 +355,20 @@ describe("BottomBar component", () => {
     test.each(["Enter", "Escape"])(
       "Pressing %s ends the edition and yields back the DOM focus",
       async (key) => {
+        const focusElement = jest.fn();
+        const focusableElementStore = env.getStore(DOMFocusableElementStore);
+        const defaultFocusElement = document.createElement("div");
+        defaultFocusElement.focus = focusElement;
+        focusableElementStore.setFocusableElement(defaultFocusElement);
+
         const sheetName = fixture.querySelector<HTMLElement>(".o-sheet-name")!;
         // will give focus back to the component main node
         triggerMouseEvent(sheetName, "dblclick");
         await nextTick();
         sheetName.textContent = "New name";
+        expect(focusElement).not.toHaveBeenCalled();
         await keyDown({ key });
-        const focusableElementStore = env.getStore(DOMFocusableElementStore);
-        expect(focusableElementStore.focus).toHaveBeenCalled();
+        expect(focusElement).toHaveBeenCalled();
       }
     );
   });

tests/top_bar_component.test.ts

@@ -3,11 +3,12 @@ import { Model } from "../src";
 import { CellComposerStore } from "../src/components/composer/composer/cell_composer_store";
 import { PaintFormatStore } from "../src/components/paint_format_button/paint_format_store";
 import { TopBar } from "../src/components/top_bar/top_bar";
-import { DEFAULT_FONT_SIZE } from "../src/constants";
+import { DEBOUNCE_TIME, DEFAULT_FONT_SIZE } from "../src/constants";
 import { toZone, zoneToXc } from "../src/helpers";
 import { topbarComponentRegistry, topbarMenuRegistry } from "../src/registries";
 import { ConditionalFormat, Currency, Pixel, SpreadsheetChildEnv, Style } from "../src/types";
 import { FileStore } from "./__mocks__/mock_file_store";
+import { MockTransportService } from "./__mocks__/transport_service";
 import {
   addCellToSelection,
   createTableWithFilter,
@@ -913,3 +914,21 @@ describe("Topbar svg icon", () => {
     expect(icon?.classList.contains(iconClass)).toBeTruthy();
   });
 });
+
+test("Clicking on a topbar button only trigger a single render", async () => {
+  jest.useFakeTimers();
+  const transportService = new MockTransportService();
+
+  const model = new Model({}, { transportService });
+  const { fixture, env } = await mountSpreadsheet({ model });
+  jest.advanceTimersByTime(DEBOUNCE_TIME + 10); // wait for the debounce of session.move
+  jest.useRealTimers();
+
+  const triggerRender = jest.fn();
+  model.on("update", {}, triggerRender);
+  env["__spreadsheet_stores__"].on("store-updated", null, triggerRender);
+
+  await click(fixture, ".o-spreadsheet-topbar [title='Bold (Ctrl+B)']");
+
+  expect(triggerRender).toHaveBeenCalledTimes(1);
+});