[IMP] font_size_editor: use popover and improve reusability

- Replaced the dropdown with a popover to avoid issues with overflow in side panels. - Made the font_size_editor component more generic for use in other contexts. Task: 4316051 Part-of: #5249 Signed-off-by: Lucas Lefèvre (lul) <[email protected]>
odoo · Dec 3, 2024 · 22c8483 · 22c8483
1 parent 873498c
commit 22c8483
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 36 deletions.
diff --git a/src/components/font_size_editor/font_size_editor.ts b/src/components/font_size_editor/font_size_editor.ts
@@ -1,19 +1,20 @@
 import { Component, useExternalListener, useRef, useState } from "@odoo/owl";
-import { setStyle } from "../../actions/menu_items_actions";
-import { DEFAULT_FONT_SIZE, FONT_SIZES, SELECTION_BORDER_COLOR } from "../../constants";
+import { FONT_SIZES, SELECTION_BORDER_COLOR } from "../../constants";
 import { clip } from "../../helpers/index";
 import { SpreadsheetChildEnv } from "../../types/index";
 import { css } from "../helpers/css";
 import { isChildEvent } from "../helpers/dom_helpers";
+import { Popover, PopoverProps } from "../popover";
 
 interface State {
   isOpen: boolean;
 }
 
 interface Props {
-  onToggle: () => void;
-  dropdownStyle: string;
+  currentFontSize: number;
   class: string;
+  onFontSizeChanged: (fontSize: number) => void;
+  onToggle?: () => void;
 }
 
 css/* scss */ `
@@ -38,36 +39,43 @@ css/* scss */ `
 export class FontSizeEditor extends Component<Props, SpreadsheetChildEnv> {
   static template = "o-spreadsheet-FontSizeEditor";
   static props = {
-    onToggle: Function,
-    dropdownStyle: String,
+    currentFontSize: Number,
+    onFontSizeChanged: Function,
+    onToggle: { type: Function, optional: true },
     class: String,
   };
-  static components = {};
+  static components = { Popover };
   fontSizes = FONT_SIZES;
 
   dropdown: State = useState({ isOpen: false });
 
   private inputRef = useRef("inputFontSize");
   private rootEditorRef = useRef("FontSizeEditor");
+  private fontSizeListRef = useRef("fontSizeList");
 
   setup() {
     useExternalListener(window, "click", this.onExternalClick, { capture: true });
   }
 
+  get popoverProps(): PopoverProps {
+    const { x, y, width, height } = this.rootEditorRef.el!.getBoundingClientRect();
+    return {
+      anchorRect: { x, y, width, height },
+      positioning: "BottomLeft",
+      verticalOffset: 0,
+    };
+  }
+
   onExternalClick(ev: MouseEvent) {
-    if (!isChildEvent(this.rootEditorRef.el!, ev)) {
+    if (!isChildEvent(this.fontSizeListRef.el!, ev) && !isChildEvent(this.rootEditorRef.el!, ev)) {
       this.closeFontList();
     }
   }
 
-  get currentFontSize(): number {
-    return this.env.model.getters.getCurrentStyle().fontSize || DEFAULT_FONT_SIZE;
-  }
-
   toggleFontList() {
     const isOpen = this.dropdown.isOpen;
     if (!isOpen) {
-      this.props.onToggle();
+      this.props.onToggle?.();
       this.inputRef.el!.focus();
     } else {
       this.closeFontList();
@@ -80,7 +88,7 @@ export class FontSizeEditor extends Component<Props, SpreadsheetChildEnv> {
 
   private setSize(fontSizeStr: string) {
     const fontSize = clip(Math.floor(parseFloat(fontSizeStr)), 1, 400);
-    setStyle(this.env, { fontSize });
+    this.props.onFontSizeChanged(fontSize);
     this.closeFontList();
   }
 
@@ -103,9 +111,9 @@ export class FontSizeEditor extends Component<Props, SpreadsheetChildEnv> {
       const target = ev.target as HTMLInputElement;
       // In the case of a ESCAPE key, we get the previous font size back
       if (ev.key === "Escape") {
-        target.value = `${this.currentFontSize}`;
+        target.value = `${this.props.currentFontSize}`;
       }
-      this.props.onToggle();
+      this.props.onToggle?.();
     }
   }
 }
diff --git a/src/components/font_size_editor/font_size_editor.xml b/src/components/font_size_editor/font_size_editor.xml
@@ -14,27 +14,25 @@
           t-on-keydown="onInputKeydown"
           t-on-click.stop=""
           t-on-focus.stop="onInputFocused"
-          t-att-value="currentFontSize"
+          t-att-value="props.currentFontSize"
           t-on-change="setSizeFromInput"
           t-ref="inputFontSize"
         />
         <span>
           <t t-call="o-spreadsheet-Icon.CARET_DOWN"/>
         </span>
       </div>
-      <div
-        class="o-dropdown-content o-text-options"
-        t-if="dropdown.isOpen"
-        t-on-click.stop=""
-        t-att-style="props.dropdownStyle">
-        <t t-foreach="fontSizes" t-as="fontSize" t-key="fontSize">
-          <div
-            t-esc="fontSize"
-            t-att-data-size="fontSize"
-            t-on-click="() => this.setSizeFromList(fontSize)"
-          />
-        </t>
-      </div>
+      <Popover t-if="dropdown.isOpen" t-props="popoverProps">
+        <div class="o-text-options bg-white" t-on-click.stop="" t-ref="fontSizeList">
+          <t t-foreach="fontSizes" t-as="fontSize" t-key="fontSize">
+            <div
+              t-esc="fontSize"
+              t-att-data-size="fontSize"
+              t-on-click="() => this.setSizeFromList(fontSize)"
+            />
+          </t>
+        </div>
+      </Popover>
     </div>
   </t>
 </templates>
diff --git a/src/components/top_bar/top_bar.ts b/src/components/top_bar/top_bar.ts
@@ -15,6 +15,7 @@ import {
   BUTTON_ACTIVE_BG,
   BUTTON_ACTIVE_TEXT_COLOR,
   ComponentsImportance,
+  DEFAULT_FONT_SIZE,
   SEPARATOR_COLOR,
   TOPBAR_TOOLBAR_HEIGHT,
 } from "../../constants";
@@ -142,9 +143,6 @@ export class TopBar extends Component<Props, SpreadsheetChildEnv> {
     onClick: Function,
     dropdownMaxHeight: Number,
   };
-  get dropdownStyle() {
-    return `max-height:${this.props.dropdownMaxHeight}px`;
-  }
   static components = {
     ColorPickerWidget,
     ColorPicker,
@@ -186,6 +184,10 @@ export class TopBar extends Component<Props, SpreadsheetChildEnv> {
       .filter((item) => !item.isVisible || item.isVisible(this.env));
   }
 
+  get currentFontSize(): number {
+    return this.env.model.getters.getCurrentStyle().fontSize || DEFAULT_FONT_SIZE;
+  }
+
   onExternalClick(ev: MouseEvent) {
     // TODO : manage click events better. We need this piece of code
     // otherwise the event opening the menu would close it on the same frame.
@@ -272,4 +274,8 @@ export class TopBar extends Component<Props, SpreadsheetChildEnv> {
     setStyle(this.env, { [target]: color });
     this.onClick();
   }
+
+  setFontSize(fontSize: number) {
+    setStyle(this.env, { fontSize });
+  }
 }
diff --git a/src/components/top_bar/top_bar.xml b/src/components/top_bar/top_bar.xml
@@ -58,9 +58,10 @@
 
             <div class="o-divider"/>
             <FontSizeEditor
+              currentFontSize="currentFontSize"
+              onFontSizeChanged.bind="this.setFontSize"
               class="'o-hoverable-button'"
               onToggle.bind="this.onClick"
-              dropdownStyle="this.dropdownStyle"
             />
             <div class="o-divider"/>
 

diff --git a/tests/top_bar_component.test.ts b/tests/top_bar_component.test.ts
@@ -328,7 +328,7 @@ describe("TopBar component", () => {
     const fontSizeText = fixture.querySelector("input.o-font-size")! as HTMLInputElement;
     expect(fontSizeText.value.trim()).toBe(DEFAULT_FONT_SIZE.toString());
     await click(fixture, ".o-font-size-editor");
-    await click(fixture, '.o-dropdown-content [data-size="8"]');
+    await click(fixture, '.o-text-options [data-size="8"]');
     expect(fontSizeText.value.trim()).toBe("8");
     expect(getStyle(model, "A1").fontSize).toBe(8);
   });
@@ -585,7 +585,7 @@ describe("TopBar component", () => {
     ["Horizontal align", ".o-dropdown-content"],
     ["Vertical align", ".o-dropdown-content"],
     ["Wrapping", ".o-dropdown-content"],
-    ["Font Size", ".o-dropdown-content"],
+    ["Font Size", ".o-text-options"],
     ["More formats", ".o-menu"],
   ])(
     "Clicking a static element inside a dropdown '%s' don't close the dropdown",