Master carousel tabs adrm

src/components/figures/figure_carousel/figure_carousel.scss

@@ -26,6 +26,27 @@
       box-shadow: 0 2px 4px rgba(0, 0, 0, 0.1);
     }
 
+    .o-carousel-title {
+      max-width: 60%;
+    }
+
+    .o-carousel-tabs {
+      width: 0; /* To make flex-fill work */
+    }
+
+    .o-carousel-tabs-dropdown {
+      cursor: pointer;
+      padding: 2px;
+      font-size: 16px;
+      line-height: 16px;
+
+      &.active,
+      &:hover {
+        background-color: $os-button-hover-bg;
+        color: $os-button-hover-text-color;
+      }
+    }
+
     .o-carousel-tab {
       cursor: pointer;
       max-width: 150px;

src/components/figures/figure_carousel/figure_carousel.ts

@@ -1,4 +1,5 @@
-import { Component, useEffect } from "@odoo/owl";
+import { Component, useEffect, useRef, useState } from "@odoo/owl";
+import { ActionSpec, createActions } from "../../../actions/action";
 import { DEFAULT_CAROUSEL_TITLE_STYLE } from "../../../constants";
 import { chartStyleToCellStyle, deepEquals } from "../../../helpers";
 import { getCarouselItemTitle } from "../../../helpers/carousel_helpers";
@@ -9,9 +10,12 @@ import {
   CarouselItem,
   CSSProperties,
   FigureUI,
+  MenuMouseEvent,
   SpreadsheetChildEnv,
 } from "../../../types";
 import { cellTextStyleToCss, cssPropertiesToCss } from "../../helpers";
+import { getBoundingRectAsPOJO, getRefBoundingRect } from "../../helpers/dom_helpers";
+import { MenuPopover, MenuState } from "../../menu_popover/menu_popover";
 import { ChartDashboardMenu } from "../chart/chart_dashboard_menu/chart_dashboard_menu";
 import { ChartAnimationStore } from "../chart/chartJs/chartjs_animation_store";
 
@@ -28,7 +32,13 @@ export class CarouselFigure extends Component<Props, SpreadsheetChildEnv> {
     onFigureDeleted: Function,
     editFigureStyle: { type: Function, optional: true },
   };
-  static components = { ChartDashboardMenu };
+  static components = { ChartDashboardMenu, MenuPopover };
+
+  private carouselTabsRef = useRef("carouselTabs");
+  private carouselTabsDropdownRef = useRef("carouselTabsDropdown");
+
+  private menuState = useState<MenuState>({ isOpen: false, anchorRect: null, menuItems: [] });
+  private hiddenItems: CarouselItem[] = [];
 
   protected animationStore: Store<ChartAnimationStore> | undefined;
 
@@ -41,6 +51,7 @@ export class CarouselFigure extends Component<Props, SpreadsheetChildEnv> {
       } else {
         this.props.editFigureStyle?.({ "pointer-events": "auto" });
       }
+      this.updateTabsVisibility();
     });
   }
 
@@ -109,4 +120,56 @@ export class CarouselFigure extends Component<Props, SpreadsheetChildEnv> {
     const style = { ...DEFAULT_CAROUSEL_TITLE_STYLE, ...this.carousel.title };
     return cssPropertiesToCss(cellTextStyleToCss(chartStyleToCellStyle(style)));
   }
+
+  private updateTabsVisibility(): void {
+    const tabsContainerEl = this.carouselTabsRef.el;
+    const dropDownEl = this.carouselTabsDropdownRef.el;
+    if (!tabsContainerEl || !dropDownEl) {
+      return;
+    }
+
+    this.hiddenItems = [];
+
+    const containerRect = getBoundingRectAsPOJO(tabsContainerEl);
+    const tabs = Array.from(tabsContainerEl.children) as HTMLElement[];
+
+    for (const tab of tabs) {
+      tab.style.display = "block";
+    }
+
+    const tabWidths = tabs.map((tab) => getBoundingRectAsPOJO(tab).width);
+
+    let currentWidth = 0;
+    for (let i = 0; i < tabs.length; i++) {
+      const shouldBeHidden = currentWidth + tabWidths[i] > containerRect.width;
+      currentWidth += tabWidths[i];
+      if (shouldBeHidden) {
+        tabs[i].style.display = "none";
+        this.hiddenItems.push(this.carousel.items[i]);
+      }
+    }
+
+    dropDownEl.style.display = this.hiddenItems.length ? "block" : "none";
+  }
+
+  get menuId() {
+    return "carousel-tabs-menu-";
+  }
+
+  toggleMenu(ev: MenuMouseEvent) {
+    if (ev.closedMenuId === this.menuId) {
+      this.menuState.isOpen = false;
+      return;
+    }
+    const rect = getRefBoundingRect(this.carouselTabsDropdownRef);
+    const menuItems: ActionSpec[] = this.hiddenItems.map((item) => ({
+      name: this.getItemTitle(item),
+      execute: () => this.onCarouselTabClick(item),
+      isActive: () => this.isItemSelected(item),
+      isReadonlyAllowed: true,
+    }));
+    this.menuState.isOpen = true;
+    this.menuState.anchorRect = rect;
+    this.menuState.menuItems = createActions(menuItems);
+  }
 }

src/components/figures/figure_carousel/figure_carousel.xml

@@ -9,8 +9,13 @@
           'o-carousel-header-floating': !env.isDashboard() and selectedItem?.type === 'carouselDataView',
          }"
         t-att-style="headerStyle">
-        <div class="o-carousel-title text-truncate" t-esc="title" t-att-style="titleStyle"/>
-        <div class="ms-auto d-flex">
+        <div
+          class="o-carousel-title text-truncate flex-shrink-0 pe-2"
+          t-esc="title"
+          t-att-title="title"
+          t-att-style="titleStyle"
+        />
+        <div class="o-carousel-tabs d-flex flex-fill justify-content-end" t-ref="carouselTabs">
           <t t-foreach="carousel.items" t-as="item" t-key="item_index">
             <div
               class="o-carousel-tab text-truncate px-2 mt-1 flex-shrink-0"
@@ -20,6 +25,21 @@
             />
           </t>
         </div>
+        <div
+          class="o-carousel-tabs-dropdown flex-shrink-0 rounded"
+          t-att-class="{'active': menuState.isOpen}"
+          t-ref="carouselTabsDropdown"
+          t-on-click="toggleMenu">
+          <div class="fa fa-angle-down"/>
+          <MenuPopover
+            t-if="menuState.isOpen"
+            menuId="menuId"
+            anchorRect="menuState.anchorRect"
+            menuItems="menuState.menuItems"
+            onClose="() => this.menuState.isOpen=false"
+            popoverPositioning="'bottom-left'"
+          />
+        </div>
       </div>
       <div
         t-if="!selectedItem"

src/components/figures/figure_container/figure_container.ts

@@ -1,5 +1,5 @@
 import { Component, onMounted, onWillUpdateProps, useState } from "@odoo/owl";
-import { ComponentsImportance, MIN_FIG_SIZE } from "../../../constants";
+import { ComponentsImportance, DRAG_THRESHOLD, MIN_FIG_SIZE } from "../../../constants";
 import { isDefined } from "../../../helpers";
 import { rectUnion } from "../../../helpers/rectangle";
 import { figureRegistry } from "../../../registries/figures_registry";
@@ -331,9 +331,18 @@ export class FiguresContainer extends Component<Props, SpreadsheetChildEnv> {
       ).end,
     };
 
+    let hasStartedDnd = false;
     const onMouseMove = (ev: MouseEvent) => {
       const getters = this.env.model.getters;
       const currentMousePosition = { x: ev.clientX, y: ev.clientY };
+
+      const offsetX = Math.abs(currentMousePosition.x - initialMousePosition.x);
+      const offsetY = Math.abs(currentMousePosition.y - initialMousePosition.y);
+      if (!hasStartedDnd && offsetX < DRAG_THRESHOLD && offsetY < DRAG_THRESHOLD) {
+        return; // add a small threshold to avoid dnd when just clicking
+      }
+      hasStartedDnd = true;
+
       const draggedFigure = dragFigureForMove(
         currentMousePosition,
         initialMousePosition,

src/constants.ts

@@ -346,3 +346,5 @@ export const tokenColors = {
   ARG_SEPARATOR: functionColor,
   ORPHAN_RIGHT_PAREN: "#ff0000",
 } as const;
+
+export const DRAG_THRESHOLD = 5; // in pixels, to avoid unwanted drag when clicking

src/variables.scss

@@ -6,6 +6,8 @@ $os-gray-900: #111827;
 
 $os-text-body: #374151;
 $os-button-primary-bg: #714b67;
+$os-button-hover-bg: $os-gray-300;
+$os-button-hover-text-color: $os-gray-900;
 $os-button-active-bg: #e6f2f3;
 $os-button-active-text-color: $os-gray-900;
 

tests/figures/carousel/carousel_figure_component.test.ts

@@ -13,6 +13,7 @@ import {
 } from "../../test_helpers/commands_helpers";
 import { click, clickAndDrag, getElStyle, triggerMouseEvent } from "../../test_helpers/dom_helper";
 import { makeTestEnv, mockChart, mountSpreadsheet, nextTick } from "../../test_helpers/helpers";
+import { extendMockGetBoundingClientRect } from "../../test_helpers/mock_helpers";
 
 jest.mock("../../../src/components/helpers/dom_helpers", () => {
   return {
@@ -162,6 +163,51 @@ describe("Carousel figure component", () => {
     expect(enableAnimationSpy).toHaveBeenLastCalledWith(barId);
   });
 
+  test("Having too many tabs will put some of them inside a dropdown", async () => {
+    extendMockGetBoundingClientRect({
+      "o-carousel-tabs": () => ({ width: 200 }),
+      "o-carousel-tab": () => ({ width: 100 }),
+    });
+
+    const getCarouselTabs = () =>
+      Array.from(document.querySelectorAll<HTMLElement>(".o-carousel-tab")).map((tab) => ({
+        label: tab.textContent,
+        isHidden: tab.style.display === "none",
+      }));
+
+    createCarousel(model, { items: [{ type: "carouselDataView" }] }, "carouselId");
+    addNewChartToCarousel(model, "carouselId", { type: "pie" });
+    const { fixture } = await mountSpreadsheet({ model });
+
+    expect(getCarouselTabs()).toEqual([
+      { label: "Data", isHidden: false },
+      { label: "Pie", isHidden: false },
+    ]);
+    expect(".o-carousel-tabs-dropdown").toHaveStyle({ display: "none" });
+
+    const radarChartId = addNewChartToCarousel(model, "carouselId", { type: "radar" });
+    addNewChartToCarousel(model, "carouselId", { type: "line" });
+    await nextTick();
+
+    expect(getCarouselTabs()).toEqual([
+      { label: "Data", isHidden: false },
+      { label: "Pie", isHidden: false },
+      { label: "Radar", isHidden: true },
+      { label: "Line", isHidden: true },
+    ]);
+    expect(".o-carousel-tabs-dropdown").toHaveStyle({ display: "block" });
+
+    await click(fixture, ".o-carousel-tabs-dropdown");
+    expect(".o-menu-item").toHaveCount(2);
+    expect(".o-menu-item:nth-child(1)").toHaveText("Radar");
+    expect(".o-menu-item:nth-child(2)").toHaveText("Line");
+
+    await click(fixture, ".o-menu-item:nth-child(1)");
+    expect(model.getters.getSelectedCarouselItem("carouselId")).toMatchObject({
+      chartId: radarChartId,
+    });
+  });
+
   describe("Carousel menu items", () => {
     let env: SpreadsheetChildEnv;
     let openSidePanel: jest.Mock;

tests/figures/figure_component.test.ts

@@ -58,6 +58,13 @@ jest.mock("../../src/components/helpers/dom_helpers", () => {
   };
 });
 
+const constantsMocks = jest.requireMock("../../src/constants");
+jest.mock("../../src/constants", () => ({ ...jest.requireActual("../../src/constants") }));
+
+beforeEach(() => {
+  constantsMocks.DRAG_THRESHOLD = 0; // mock drag threshold to 0 for easier testing of snap
+});
+
 const cellHeight = DEFAULT_CELL_HEIGHT;
 const cellWidth = DEFAULT_CELL_WIDTH;
 
@@ -669,14 +676,14 @@ describe("figures", () => {
     createFigure(model, { id: figureId, offset: { x: 0, y: 200 } });
     model.updateMode("readonly");
     await nextTick();
-    const figure = fixture.querySelector(".o-figure")!;
+    const figure = fixture.querySelector<HTMLElement>(".o-figure")!;
     await simulateClick(".o-figure");
     expect(document.activeElement).not.toBe(figure);
     expect(fixture.querySelector(".o-fig-anchor")).toBeNull();
 
     triggerMouseEvent(figure, "pointerdown", 300, 200);
     await nextTick();
-    expect(figure.classList).not.toContain("o-dragging");
+    expect(figure).not.toHaveStyle({ cursor: "grabbing" });
   });
 
   describe("Figure border", () => {
@@ -926,7 +933,20 @@ describe("figures", () => {
     await nextTick();
     triggerMouseEvent(".o-figure", "pointerdown", 0, 0);
     await nextTick();
-    expect(fixture.querySelector(".o-figure")?.classList.contains("o-dragging")).toBeFalsy();
+    expect(".o-figure").not.toHaveStyle({ cursor: "grabbing" });
+  });
+
+  test("There is a small threshold before the figure is marked as dragging", async () => {
+    constantsMocks.DRAG_THRESHOLD = 5;
+    createFigure(model);
+    await nextTick();
+
+    await clickAndDrag(".o-figure", { x: 3, y: 1 }, undefined, false);
+    expect(".o-figure").not.toHaveStyle({ cursor: "grabbing" });
+    expect(fixture.querySelector<HTMLElement>(".o-figure")?.style.cursor).not.toBe("grabbing");
+
+    await clickAndDrag(".o-figure", { x: 6, y: 1 }, undefined, false);
+    expect(".o-figure").toHaveStyle({ cursor: "grabbing" });
   });
 
   test("Figure container is properly computed based on the sheetView size", async () => {

tests/setup/jest_extend.ts

@@ -39,6 +39,7 @@ declare global {
       toHaveCount(count: number): R;
       toHaveClass(className: string): R;
       toHaveAttribute(attribute: string, value: string): R;
+      toHaveStyle(style: Record<string, string>): R;
     }
   }
 }
@@ -298,6 +299,29 @@ CancelledReasons: ${this.utils.printReceived(dispatchResult.reasons)}
           )}`;
     return { pass, message };
   },
+  toHaveStyle(target: DOMTarget, expectedStyle: Record<string, string>) {
+    const element = getTarget(target);
+    if (!(element instanceof HTMLElement)) {
+      const message = element ? "Target is not an HTML element" : "Target not found";
+      return { pass: false, message: () => message };
+    }
+    const receivedStyle: Record<string, string> = {};
+    for (const key of Object.keys(expectedStyle)) {
+      receivedStyle[key] = element.style.getPropertyValue(key);
+    }
+    const pass = this.equals(receivedStyle, expectedStyle, [this.utils.iterableEquality]);
+    const message = () =>
+      pass
+        ? ""
+        : `expect(target).toHaveStyle(expected);\n\n${this.utils.printDiffOrStringify(
+            expectedStyle,
+            receivedStyle,
+            "Expected style",
+            "Received style",
+            false
+          )}`;
+    return { pass, message };
+  },
 });
 
 function getTarget(target: DOMTarget): Element | Document | Window {