diff --git a/src/components/figures/figure/figure.ts b/src/components/figures/figure/figure.ts index 997f46e8ef..8ec4f7dbab 100644 --- a/src/components/figures/figure/figure.ts +++ b/src/components/figures/figure/figure.ts @@ -1,4 +1,4 @@ -import { Component, onWillUnmount, useEffect, useRef, useState } from "@odoo/owl"; +import { Component, useEffect, useRef, useState } from "@odoo/owl"; import { ComponentsImportance, FIGURE_BORDER_COLOR, @@ -115,7 +115,6 @@ css/*SCSS*/ ` interface Props { figure: Figure; style: string; - onFigureDeleted: () => void; onMouseDown: (ev: MouseEvent) => void; onClickAnchor(dirX: ResizeDirection, dirY: ResizeDirection, ev: MouseEvent): void; } @@ -125,13 +124,11 @@ export class FigureComponent extends Component { static props = { figure: Object, style: { type: String, optional: true }, - onFigureDeleted: { type: Function, optional: true }, onMouseDown: { type: Function, optional: true }, onClickAnchor: { type: Function, optional: true }, }; static components = { Menu }; static defaultProps = { - onFigureDeleted: () => {}, onMouseDown: () => {}, onClickAnchor: () => {}, }; @@ -214,10 +211,6 @@ export class FigureComponent extends Component { }, () => [this.env.model.getters.getSelectedFigureId(), this.props.figure.id, this.figureRef.el] ); - - onWillUnmount(() => { - this.props.onFigureDeleted(); - }); } clickAnchor(dirX: ResizeDirection, dirY: ResizeDirection, ev: MouseEvent) { @@ -239,7 +232,6 @@ export class FigureComponent extends Component { sheetId: this.env.model.getters.getActiveSheetId(), id: figure.id, }); - this.props.onFigureDeleted(); ev.preventDefault(); ev.stopPropagation(); break; @@ -304,6 +296,6 @@ export class FigureComponent extends Component { this.menuState.position = position; this.menuState.menuItems = figureRegistry .get(this.props.figure.tag) - .menuBuilder(this.props.figure.id, this.props.onFigureDeleted, this.env); + .menuBuilder(this.props.figure.id, this.env); } } diff --git a/src/components/figures/figure/figure.xml b/src/components/figures/figure/figure.xml index 009379e9cb..cfccd58828 100644 --- a/src/components/figures/figure/figure.xml +++ b/src/components/figures/figure/figure.xml @@ -14,7 +14,6 @@
diff --git a/src/components/figures/figure_chart/figure_chart.ts b/src/components/figures/figure_chart/figure_chart.ts index 7b629f393d..f8aaadbfbd 100644 --- a/src/components/figures/figure_chart/figure_chart.ts +++ b/src/components/figures/figure_chart/figure_chart.ts @@ -18,14 +18,12 @@ interface Props { // props figure is currently necessary scorecards, we need the chart dimension at render to avoid having to force the // style by hand in the useEffect() figure: Figure; - onFigureDeleted: () => void; } export class ChartFigure extends Component { static template = "o-spreadsheet-ChartFigure"; static props = { figure: Object, - onFigureDeleted: Function, }; static components = {}; diff --git a/src/components/figures/figure_container/figure_container.ts b/src/components/figures/figure_container/figure_container.ts index 8c97e8bee4..d501580250 100644 --- a/src/components/figures/figure_container/figure_container.ts +++ b/src/components/figures/figure_container/figure_container.ts @@ -22,9 +22,7 @@ import { FigureComponent } from "../figure/figure"; type ContainerType = "topLeft" | "topRight" | "bottomLeft" | "bottomRight" | "dnd"; -interface Props { - onFigureDeleted: () => void; -} +interface Props {} interface Container { type: ContainerType; @@ -127,9 +125,7 @@ css/*SCSS*/ ` */ export class FiguresContainer extends Component { static template = "o-spreadsheet-FiguresContainer"; - static props = { - onFigureDeleted: Function, - }; + static props = {}; static components = { FigureComponent }; dnd = useState({ diff --git a/src/components/figures/figure_container/figure_container.xml b/src/components/figures/figure_container/figure_container.xml index 2cff25b0c1..6e2a56d086 100644 --- a/src/components/figures/figure_container/figure_container.xml +++ b/src/components/figures/figure_container/figure_container.xml @@ -11,7 +11,6 @@ t-att-style="container.inverseViewportStyle"> void; } export class ImageFigure extends Component { static template = "o-spreadsheet-ImageFigure"; static props = { figure: Object, - onFigureDeleted: Function, }; static components = {}; diff --git a/src/components/grid/grid.xml b/src/components/grid/grid.xml index 577124b601..2db6484a2e 100644 --- a/src/components/grid/grid.xml +++ b/src/components/grid/grid.xml @@ -16,7 +16,6 @@ onGridResized.bind="onGridResized" onGridMoved.bind="moveCanvas" gridOverlayDimensions="gridOverlayDimensions" - onFigureDeleted.bind="focusDefaultElement" /> void; -} +interface Props {} export class GridAddRowsFooter extends Component { static template = "o-spreadsheet-GridAddRowsFooter"; - static props = { - focusGrid: Function, - }; + static props = {}; static components = { ValidationMessages }; + + private DOMFocusableElementStore!: Store; + inputRef = useRef("inputRef"); state = useState({ inputValue: "100", @@ -37,6 +38,7 @@ export class GridAddRowsFooter extends Component { }); setup() { + this.DOMFocusableElementStore = useStore(DOMFocusableElementStore); useExternalListener(window, "click", this.onExternalClick, { capture: true }); } @@ -58,7 +60,7 @@ export class GridAddRowsFooter extends Component { onKeydown(ev: KeyboardEvent) { if (ev.key.toUpperCase() === "ESCAPE") { - this.props.focusGrid(); + this.focusDefaultElement(); } else if (ev.key.toUpperCase() === "ENTER") { this.onConfirm(); } @@ -85,7 +87,7 @@ export class GridAddRowsFooter extends Component { quantity, dimension: "ROW", }); - this.props.focusGrid(); + this.focusDefaultElement(); // After adding new rows, scroll down to the new last row const { scrollX } = this.env.model.getters.getActiveSheetDOMScrollInfo(); @@ -103,6 +105,12 @@ export class GridAddRowsFooter extends Component { if (this.inputRef.el !== document.activeElement || ev.target === this.inputRef.el) { return; } - this.props.focusGrid(); + this.focusDefaultElement(); + } + + private focusDefaultElement() { + if (document.activeElement === this.inputRef.el) { + this.DOMFocusableElementStore.focus(); + } } } diff --git a/src/components/grid_overlay/grid_overlay.ts b/src/components/grid_overlay/grid_overlay.ts index 63426dfa13..30cf63cf0e 100644 --- a/src/components/grid_overlay/grid_overlay.ts +++ b/src/components/grid_overlay/grid_overlay.ts @@ -129,7 +129,6 @@ interface Props { onGridResized: (dimension: Rect) => void; onGridMoved: (deltaX: Pixel, deltaY: Pixel) => void; gridOverlayDimensions: string; - onFigureDeleted: () => void; } export class GridOverlay extends Component { @@ -140,7 +139,6 @@ export class GridOverlay extends Component { onCellClicked: { type: Function, optional: true }, onCellRightClicked: { type: Function, optional: true }, onGridResized: { type: Function, optional: true }, - onFigureDeleted: { type: Function, optional: true }, onGridMoved: Function, gridOverlayDimensions: String, }; @@ -156,7 +154,6 @@ export class GridOverlay extends Component { onCellClicked: () => {}, onCellRightClicked: () => {}, onGridResized: () => {}, - onFigureDeleted: () => {}, }; private gridOverlay: Ref = useRef("gridOverlay"); private gridOverlayRect = useAbsoluteBoundingRect(this.gridOverlay); diff --git a/src/components/grid_overlay/grid_overlay.xml b/src/components/grid_overlay/grid_overlay.xml index 16a364037c..308fb4c71a 100644 --- a/src/components/grid_overlay/grid_overlay.xml +++ b/src/components/grid_overlay/grid_overlay.xml @@ -8,13 +8,12 @@ t-on-pointerdown="onMouseDown" t-on-dblclick.self="onDoubleClick" t-on-contextmenu.stop.prevent="onContextMenu"> - +
diff --git a/src/components/spreadsheet/spreadsheet.ts b/src/components/spreadsheet/spreadsheet.ts index 5a0e847d4b..80be1808c6 100644 --- a/src/components/spreadsheet/spreadsheet.ts +++ b/src/components/spreadsheet/spreadsheet.ts @@ -405,23 +405,20 @@ export class Spreadsheet extends Component { - /** - * Only refocus the grid if the active element is not a child of the spreadsheet - * (i.e. activeElement is outside of the spreadsheetRef component) - * and spreadsheet is a child of that element. Anything else means that the focus - * is on an element that needs to keep it. - */ - if ( - !this.spreadsheetRef.el!.contains(document.activeElement) && - document.activeElement?.contains(this.spreadsheetRef.el!) - ) { - this.focusGrid(); - } - }, - () => [this.env.model.getters.getActiveSheetId()] - ); + useEffect(() => { + /** + * Only refocus the grid if the active element is not a child of the spreadsheet + * (i.e. activeElement is outside of the spreadsheetRef component) + * and spreadsheet is a child of that element. Anything else means that the focus + * is on an element that needs to keep it. + */ + if ( + !this.spreadsheetRef.el!.contains(document.activeElement) && + document.activeElement?.contains(this.spreadsheetRef.el!) + ) { + this.focusGrid(); + } + }); useExternalListener(window as any, "resize", () => this.render(true)); diff --git a/src/registries/figure_registry.ts b/src/registries/figure_registry.ts index e6ccd1212d..a7982e5951 100644 --- a/src/registries/figure_registry.ts +++ b/src/registries/figure_registry.ts @@ -19,7 +19,7 @@ import { Registry } from "./registry"; export interface FigureContent { Component: any; - menuBuilder: (figureId: UID, onFigureDeleted: () => void, env: SpreadsheetChildEnv) => Action[]; + menuBuilder: (figureId: UID, env: SpreadsheetChildEnv) => Action[]; SidePanelComponent?: string; keepRatio?: boolean; minFigSize?: number; @@ -42,7 +42,7 @@ figureRegistry.add("image", { function getChartMenu( figureId: UID, - onFigureDeleted: () => void, + env: SpreadsheetChildEnv ): Action[] { const menuItemSpecs: ActionSpec[] = [ @@ -58,14 +58,14 @@ function getChartMenu( }, getCopyMenuItem(figureId, env), getCutMenuItem(figureId, env), - getDeleteMenuItem(figureId, onFigureDeleted, env), + getDeleteMenuItem(figureId, env), ]; return createActions(menuItemSpecs); } function getImageMenuRegistry( figureId: UID, - onFigureDeleted: () => void, + env: SpreadsheetChildEnv ): Action[] { const menuItemSpecs: ActionSpec[] = [ @@ -94,7 +94,7 @@ function getImageMenuRegistry( }, icon: "o-spreadsheet-Icon.REFRESH", }, - getDeleteMenuItem(figureId, onFigureDeleted, env), + getDeleteMenuItem(figureId, env), ]; return createActions(menuItemSpecs); } @@ -131,7 +131,7 @@ function getCutMenuItem(figureId: UID, env: SpreadsheetChildEnv): ActionSpec { function getDeleteMenuItem( figureId: UID, - onFigureDeleted: () => void, + env: SpreadsheetChildEnv ): ActionSpec { return { @@ -143,7 +143,6 @@ function getDeleteMenuItem( sheetId: env.model.getters.getActiveSheetId(), id: figureId, }); - onFigureDeleted(); }, icon: "o-spreadsheet-Icon.TRASH", }; diff --git a/tests/figures/figure_component.test.ts b/tests/figures/figure_component.test.ts index aa96276482..343d36f630 100644 --- a/tests/figures/figure_component.test.ts +++ b/tests/figures/figure_component.test.ts @@ -714,6 +714,23 @@ describe("figures", () => { expect(bottomRightContainerStyle.height).toEqual(`${height - 2 * DEFAULT_CELL_HEIGHT}px`); }); + test("Deleting a figure does not change the DOM focus if the figure was not focused", async () => { + createFigure(model); + await nextTick(); + env.openSidePanel("FindAndReplace"); + await nextTick(); + + const panelInput = fixture.querySelector(".o-sidePanel input"); + panelInput?.focus(); + expect(document.activeElement).toBe(panelInput); + + const figureId = model.getters.getFigures(sheetId)[0].id; + model.dispatch("DELETE_FIGURE", { sheetId, id: figureId }); + await nextTick(); + + expect(document.activeElement).toBe(panelInput); + }); + describe("Figure drag & drop snap", () => { describe("Move figure", () => { test.each([