From c8dd31594eb82336497cc642546d30a1f9e9142c Mon Sep 17 00:00:00 2001 From: "Adrien Minne (adrm)" Date: Tue, 21 Feb 2023 11:28:59 +0000 Subject: [PATCH] [REF] figure: unify chart and image menus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this commit, the chart and image menus were handled separately in `figure_chart.ts` and `figure_image.ts`. This led to a lot of duplicated code since they both do 80% the same thing. Now the figure menu is handled in `figure.ts`, and a `menuRegistryBuilder` was added to `figureRegistry`. Odoo task 3199007 closes odoo/o-spreadsheet#2091 Signed-off-by: Lucas Lefèvre (lul) --- src/components/figures/figure/figure.ts | 44 +++++- src/components/figures/figure/figure.xml | 16 +++ .../figures/figure_chart/figure_chart.ts | 83 +---------- .../figures/figure_chart/figure_chart.xml | 21 +-- .../figures/figure_image/figure_image.ts | 91 +----------- .../figures/figure_image/figure_image.xml | 22 +-- src/registries/figure_registry.ts | 109 ++++++++++++++- .../__snapshots__/figure.test.ts.snap | 34 +++++ .../scorecard_chart.test.ts.snap | 132 +++++++++--------- tests/components/figure.test.ts | 6 +- 10 files changed, 276 insertions(+), 282 deletions(-) diff --git a/src/components/figures/figure/figure.ts b/src/components/figures/figure/figure.ts index e538062e87..349c18d330 100644 --- a/src/components/figures/figure/figure.ts +++ b/src/components/figures/figure/figure.ts @@ -1,12 +1,14 @@ -import { Component, useEffect, useRef } from "@odoo/owl"; +import { Component, useEffect, useRef, useState } from "@odoo/owl"; import { ComponentsImportance, FIGURE_BORDER_COLOR, + MENU_WIDTH, SELECTION_BORDER_COLOR, } from "../../../constants"; import { figureRegistry } from "../../../registries/index"; import { CSSProperties, + DOMCoordinates, Figure, Pixel, ResizeDirection, @@ -14,6 +16,8 @@ import { UID, } from "../../../types/index"; import { css, cssPropertiesToCss } from "../../helpers/css"; +import { useAbsoluteBoundingRect } from "../../helpers/position_hook"; +import { Menu, MenuState } from "../../menu/menu"; type ResizeAnchor = | "top left" @@ -90,9 +94,8 @@ css/*SCSS*/ ` .o-figure-menu { right: 0px; + top: 0px; display: none; - position: absolute; - padding: 5px; } .o-figure-menu-item { @@ -118,15 +121,20 @@ interface Props { export class FigureComponent extends Component { static template = "o-spreadsheet-FigureComponent"; - static components = {}; + static components = { Menu }; static defaultProps = { onFigureDeleted: () => {}, onMouseDown: () => {}, onClickAnchor: () => {}, }; - private borderWidth!: number; + private menuState: MenuState = useState({ isOpen: false, position: null, menuItems: [] }); + private figureRef = useRef("figure"); + private menuButtonRef = useRef("menuButton"); + private menuButtonRect = useAbsoluteBoundingRect(this.menuButtonRef); + + private borderWidth!: number; get isSelected(): boolean { return this.env.model.getters.getSelectedFigureId() === this.props.figure.id; @@ -243,6 +251,32 @@ export class FigureComponent extends Component { break; } } + + onContextMenu(ev: MouseEvent) { + if (this.env.isDashboard()) return; + const position = { + x: ev.clientX, + y: ev.clientY, + }; + this.openContextMenu(position); + } + + showMenu() { + const { x, y, width } = this.menuButtonRect; + const menuPosition = { + x: x >= MENU_WIDTH ? x - MENU_WIDTH : x + width, + y: y, + }; + this.openContextMenu(menuPosition); + } + + private openContextMenu(position: DOMCoordinates) { + this.menuState.isOpen = true; + this.menuState.position = position; + this.menuState.menuItems = figureRegistry + .get(this.props.figure.tag) + .menuBuilder(this.props.figure.id, this.props.onFigureDeleted, this.env); + } } FigureComponent.props = { diff --git a/src/components/figures/figure/figure.xml b/src/components/figures/figure/figure.xml index 119d92d42e..f609935cbd 100644 --- a/src/components/figures/figure/figure.xml +++ b/src/components/figures/figure/figure.xml @@ -4,6 +4,7 @@
+
+
+ +
+ +
diff --git a/src/components/figures/figure_chart/figure_chart.ts b/src/components/figures/figure_chart/figure_chart.ts index b1e5c482fc..cbced49ad9 100644 --- a/src/components/figures/figure_chart/figure_chart.ts +++ b/src/components/figures/figure_chart/figure_chart.ts @@ -1,12 +1,7 @@ -import { Component, useRef, useState } from "@odoo/owl"; -import { MENU_WIDTH } from "../../../constants"; +import { Component } from "@odoo/owl"; import { chartComponentRegistry } from "../../../registries/chart_types"; -import { MenuItemRegistry } from "../../../registries/menu_items_registry"; -import { _lt } from "../../../translation"; -import { ChartType, DOMCoordinates, Figure, SpreadsheetChildEnv } from "../../../types"; +import { ChartType, Figure, SpreadsheetChildEnv } from "../../../types"; import { css } from "../../helpers/css"; -import { useAbsoluteBoundingRect } from "../../helpers/position_hook"; -import { Menu, MenuState } from "../../menu/menu"; // ----------------------------------------------------------------------------- // STYLE @@ -28,84 +23,12 @@ interface Props { export class ChartFigure extends Component { static template = "o-spreadsheet-ChartFigure"; - static components = { Menu }; - private menuState: MenuState = useState({ isOpen: false, position: null, menuItems: [] }); - - private chartContainerRef = useRef("chartContainer"); - private menuButtonRef = useRef("menuButton"); - private menuButtonRect = useAbsoluteBoundingRect(this.menuButtonRef); - private position: DOMCoordinates = useAbsoluteBoundingRect(this.chartContainerRef); - - private getMenuItemRegistry(): MenuItemRegistry { - const registry = new MenuItemRegistry(); - registry.add("edit", { - name: _lt("Edit"), - sequence: 1, - action: () => { - this.env.model.dispatch("SELECT_FIGURE", { id: this.props.figure.id }); - this.env.openSidePanel("ChartPanel"); - }, - }); - registry.add("copy", { - name: _lt("Copy"), - sequence: 2, - action: async () => { - this.env.model.dispatch("SELECT_FIGURE", { id: this.props.figure.id }); - this.env.model.dispatch("COPY"); - await this.env.clipboard.write(this.env.model.getters.getClipboardContent()); - }, - }); - registry.add("cut", { - name: _lt("Cut"), - sequence: 3, - action: async () => { - this.env.model.dispatch("SELECT_FIGURE", { id: this.props.figure.id }); - this.env.model.dispatch("CUT"); - await this.env.clipboard.write(this.env.model.getters.getClipboardContent()); - }, - }); - registry.add("delete", { - name: _lt("Delete"), - sequence: 10, - action: () => { - this.env.model.dispatch("DELETE_FIGURE", { - sheetId: this.env.model.getters.getActiveSheetId(), - id: this.props.figure.id, - }); - this.props.onFigureDeleted(); - }, - }); - return registry; - } + static components = {}; get chartType(): ChartType { return this.env.model.getters.getChartType(this.props.figure.id); } - onContextMenu(ev: MouseEvent) { - const position = { - x: this.position.x + ev.offsetX, - y: this.position.y + ev.offsetY, - }; - this.openContextMenu(position); - } - - showMenu() { - const { x, y, width } = this.menuButtonRect; - const menuPosition = { - x: x >= MENU_WIDTH ? x - MENU_WIDTH : x + width, - y: y, - }; - this.openContextMenu(menuPosition); - } - - private openContextMenu(position: DOMCoordinates) { - const registry = this.getMenuItemRegistry(); - this.menuState.isOpen = true; - this.menuState.menuItems = registry.getMenuItems(); - this.menuState.position = position; - } - get chartComponent(): new (...args: any) => Component { const type = this.chartType; const component = chartComponentRegistry.get(type); diff --git a/src/components/figures/figure_chart/figure_chart.xml b/src/components/figures/figure_chart/figure_chart.xml index 0735943495..11e1d019ba 100644 --- a/src/components/figures/figure_chart/figure_chart.xml +++ b/src/components/figures/figure_chart/figure_chart.xml @@ -1,30 +1,11 @@ -
-
-
- -
-
+
- -
diff --git a/src/components/figures/figure_image/figure_image.ts b/src/components/figures/figure_image/figure_image.ts index 17a112472c..66dbc49e14 100644 --- a/src/components/figures/figure_image/figure_image.ts +++ b/src/components/figures/figure_image/figure_image.ts @@ -1,11 +1,5 @@ -import { Component, useRef, useState } from "@odoo/owl"; -import { MENU_WIDTH } from "../../../constants"; -import { getMaxFigureSize } from "../../../helpers/figures/figure/figure"; -import { createMenu } from "../../../registries/menu_items_registry"; -import { _lt } from "../../../translation"; -import { DOMCoordinates, Figure, SpreadsheetChildEnv, UID } from "../../../types"; -import { useAbsoluteBoundingRect } from "../../helpers/position_hook"; -import { Menu, MenuState } from "../../menu/menu"; +import { Component } from "@odoo/owl"; +import { Figure, SpreadsheetChildEnv, UID } from "../../../types"; interface Props { figure: Figure; @@ -14,86 +8,7 @@ interface Props { export class ImageFigure extends Component { static template = "o-spreadsheet-ImageFigure"; - static components = { Menu }; - private menuState: Pick = useState({ - isOpen: false, - position: null, - }); - - private imageContainerRef = useRef("o-image"); - private menuButtonRef = useRef("menuButton"); - private menuButtonRect = useAbsoluteBoundingRect(this.menuButtonRef); - private position: DOMCoordinates = useAbsoluteBoundingRect(this.imageContainerRef); - - readonly menuItems = createMenu([ - { - id: "copy", - name: _lt("Copy"), - description: "Ctrl+C", - action: async () => { - this.env.model.dispatch("SELECT_FIGURE", { id: this.figureId }); - this.env.model.dispatch("COPY"); - await this.env.clipboard.write(this.env.model.getters.getClipboardContent()); - }, - }, - { - id: "cut", - name: _lt("Cut"), - description: "Ctrl+X", - action: async () => { - this.env.model.dispatch("SELECT_FIGURE", { id: this.figureId }); - this.env.model.dispatch("CUT"); - await this.env.clipboard.write(this.env.model.getters.getClipboardContent()); - }, - }, - { - id: "reset_size", - name: _lt("Reset size"), - action: () => { - const size = this.env.model.getters.getImageSize(this.figureId); - const { height, width } = getMaxFigureSize(this.env.model.getters, size); - this.env.model.dispatch("UPDATE_FIGURE", { - sheetId: this.env.model.getters.getActiveSheetId(), - id: this.figureId, - height, - width, - }); - }, - }, - { - id: "delete", - name: _lt("Delete image"), - description: "delete", - action: () => { - this.env.model.dispatch("DELETE_FIGURE", { - sheetId: this.env.model.getters.getActiveSheetId(), - id: this.figureId, - }); - }, - }, - ]); - - onContextMenu(ev: MouseEvent) { - const position = { - x: this.position.x + ev.offsetX, - y: this.position.y + ev.offsetY, - }; - this.openContextMenu(position); - } - - showMenu() { - const { x, y, width } = this.menuButtonRect; - const menuPosition = { - x: x >= MENU_WIDTH ? x - MENU_WIDTH : x + width, - y: y, - }; - this.openContextMenu(menuPosition); - } - - private openContextMenu(position: DOMCoordinates) { - this.menuState.isOpen = true; - this.menuState.position = position; - } + static components = {}; // --------------------------------------------------------------------------- // Getters diff --git a/src/components/figures/figure_image/figure_image.xml b/src/components/figures/figure_image/figure_image.xml index 43b514ff78..6bfce224be 100644 --- a/src/components/figures/figure_image/figure_image.xml +++ b/src/components/figures/figure_image/figure_image.xml @@ -1,25 +1,5 @@ -
-
- -
-
- - + diff --git a/src/registries/figure_registry.ts b/src/registries/figure_registry.ts index 9e216ace7a..138d014bc9 100644 --- a/src/registries/figure_registry.ts +++ b/src/registries/figure_registry.ts @@ -1,5 +1,9 @@ import { ChartFigure } from "../components/figures/figure_chart/figure_chart"; import { ImageFigure } from "../components/figures/figure_image/figure_image"; +import { getMaxFigureSize } from "../helpers/figures/figure/figure"; +import { _lt } from "../translation"; +import { SpreadsheetChildEnv, UID } from "../types"; +import { createMenu, MenuItem, MenuItemSpec } from "./menu_items_registry"; import { Registry } from "./registry"; //------------------------------------------------------------------------------ @@ -15,6 +19,7 @@ import { Registry } from "./registry"; export interface FigureContent { Component: any; + menuBuilder: (figureId: UID, onFigureDeleted: () => void, env: SpreadsheetChildEnv) => MenuItem[]; SidePanelComponent?: string; keepRatio?: boolean; minFigSize?: number; @@ -22,10 +27,112 @@ export interface FigureContent { } export const figureRegistry = new Registry(); -figureRegistry.add("chart", { Component: ChartFigure, SidePanelComponent: "ChartPanel" }); +figureRegistry.add("chart", { + Component: ChartFigure, + SidePanelComponent: "ChartPanel", + menuBuilder: getChartMenu, +}); figureRegistry.add("image", { Component: ImageFigure, keepRatio: true, minFigSize: 20, borderWidth: 0, + menuBuilder: getImageMenuRegistry, }); + +function getChartMenu( + figureId: UID, + onFigureDeleted: () => void, + env: SpreadsheetChildEnv +): MenuItem[] { + const menuItemSpecs: MenuItemSpec[] = [ + { + id: "edit", + name: _lt("Edit"), + sequence: 1, + action: () => { + env.model.dispatch("SELECT_FIGURE", { id: figureId }); + env.openSidePanel("ChartPanel"); + }, + }, + getCopyMenuItem(figureId, env), + getCutMenuItem(figureId, env), + getDeleteMenuItem(figureId, onFigureDeleted, env), + ]; + return createMenu(menuItemSpecs); +} + +function getImageMenuRegistry( + figureId: UID, + onFigureDeleted: () => void, + env: SpreadsheetChildEnv +): MenuItem[] { + const menuItemSpecs: MenuItemSpec[] = [ + getCopyMenuItem(figureId, env), + getCutMenuItem(figureId, env), + { + id: "reset_size", + name: _lt("Reset size"), + sequence: 4, + action: () => { + const size = env.model.getters.getImageSize(figureId); + const { height, width } = getMaxFigureSize(env.model.getters, size); + env.model.dispatch("UPDATE_FIGURE", { + sheetId: env.model.getters.getActiveSheetId(), + id: figureId, + height, + width, + }); + }, + }, + getDeleteMenuItem(figureId, onFigureDeleted, env), + ]; + return createMenu(menuItemSpecs); +} + +function getCopyMenuItem(figureId: UID, env: SpreadsheetChildEnv): MenuItemSpec { + return { + id: "copy", + name: _lt("Copy"), + sequence: 2, + description: "Ctrl+C", + action: async () => { + env.model.dispatch("SELECT_FIGURE", { id: figureId }); + env.model.dispatch("COPY"); + await env.clipboard.write(env.model.getters.getClipboardContent()); + }, + }; +} + +function getCutMenuItem(figureId: UID, env: SpreadsheetChildEnv): MenuItemSpec { + return { + id: "cut", + name: _lt("Cut"), + sequence: 3, + description: "Ctrl+X", + action: async () => { + env.model.dispatch("SELECT_FIGURE", { id: figureId }); + env.model.dispatch("CUT"); + await env.clipboard.write(env.model.getters.getClipboardContent()); + }, + }; +} + +function getDeleteMenuItem( + figureId: UID, + onFigureDeleted: () => void, + env: SpreadsheetChildEnv +): MenuItemSpec { + return { + id: "delete", + name: _lt("Delete"), + sequence: 10, + action: () => { + env.model.dispatch("DELETE_FIGURE", { + sheetId: env.model.getters.getActiveSheetId(), + id: figureId, + }); + onFigureDeleted(); + }, + }; +} diff --git a/tests/components/__snapshots__/figure.test.ts.snap b/tests/components/__snapshots__/figure.test.ts.snap index 2fd5867fa7..80d5cf45c7 100644 --- a/tests/components/__snapshots__/figure.test.ts.snap +++ b/tests/components/__snapshots__/figure.test.ts.snap @@ -15,6 +15,40 @@ exports[`figures selected figure snapshot 1`] = ` > coucou
+ +
+
+ + + + + +
+ +
+
-
-
- - - - - -
-
-
- +
+ +
+
+ + + + + +
+ `; @@ -113,38 +113,6 @@ exports[`Scorecard charts scorecard text is resized while figure is resized 1`]
-
-
- - - - - -
-
-
- +
+ +
+
+ + + + + +
+ `; diff --git a/tests/components/figure.test.ts b/tests/components/figure.test.ts index d5e7dcf996..ca85e8d934 100644 --- a/tests/components/figure.test.ts +++ b/tests/components/figure.test.ts @@ -10,6 +10,7 @@ import { } from "../../src/constants"; import { figureRegistry } from "../../src/registries"; import { CreateFigureCommand, Figure, SpreadsheetChildEnv, UID } from "../../src/types"; + import { activateSheet, addColumns, @@ -104,7 +105,10 @@ mockGetBoundingClientRect({ }); beforeAll(() => { - figureRegistry.add("text", { Component: TextFigure }); + figureRegistry.add("text", { + Component: TextFigure, + menuBuilder: () => [], + }); }); afterAll(() => { figureRegistry.remove("text");