From 9694264fdf36d07b98c9be0e83dbd6268a272274 Mon Sep 17 00:00:00 2001 From: "Adrien Minne (adrm)" Date: Tue, 14 Oct 2025 16:23:33 +0200 Subject: [PATCH] [FIX] figure: wrong focus change on figure unmount When a figure is unmounted, the focus is moved to the default focusable element (the grid composer). But this change is done even if the figure wasn't focused. For example, the focus would be removed from the find & replace search input when a collaborative user would delete a figure. Task: 5154025 --- src/components/figures/figure/figure.ts | 15 ++++++++------ src/components/figures/figure/figure.xml | 1 - .../figures/figure_chart/figure_chart.ts | 2 -- .../figure_container/figure_container.ts | 8 ++------ .../figure_container/figure_container.xml | 1 - .../figures/figure_image/figure_image.ts | 2 -- src/components/grid/grid.xml | 1 - .../grid_add_rows_footer.ts | 20 ++++++++++--------- src/components/grid_overlay/grid_overlay.ts | 3 --- src/components/grid_overlay/grid_overlay.xml | 3 +-- tests/figures/figure_component.test.ts | 17 ++++++++++++++++ 11 files changed, 40 insertions(+), 33 deletions(-) diff --git a/src/components/figures/figure/figure.ts b/src/components/figures/figure/figure.ts index 337ea6e776..33563a2d6e 100644 --- a/src/components/figures/figure/figure.ts +++ b/src/components/figures/figure/figure.ts @@ -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; } @@ -124,7 +123,6 @@ export class FigureComponent extends Component { static template = "o-spreadsheet-FigureComponent"; static components = { Menu }; static defaultProps = { - onFigureDeleted: () => {}, onMouseDown: () => {}, onClickAnchor: () => {}, }; @@ -209,7 +207,7 @@ export class FigureComponent extends Component { ); onWillUnmount(() => { - this.props.onFigureDeleted(); + this.onFigureDeleted(); }); } @@ -231,7 +229,7 @@ export class FigureComponent extends Component { sheetId: this.env.model.getters.getActiveSheetId(), id: figure.id, }); - this.props.onFigureDeleted(); + this.onFigureDeleted(); ev.preventDefault(); ev.stopPropagation(); break; @@ -296,14 +294,19 @@ 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.onFigureDeleted.bind(this), this.env); + } + + private onFigureDeleted() { + if (document.activeElement === this.figureRef.el) { + this.env.focusableElement.focus(); + } } } FigureComponent.props = { figure: Object, style: { type: String, optional: true }, - onFigureDeleted: { type: Function, optional: true }, onMouseDown: { type: Function, optional: true }, onClickAnchor: { type: Function, optional: true }, }; diff --git a/src/components/figures/figure/figure.xml b/src/components/figures/figure/figure.xml index d5fe16f7fd..3fc4299af3 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 d7176dbbdb..66993db765 100644 --- a/src/components/figures/figure_chart/figure_chart.ts +++ b/src/components/figures/figure_chart/figure_chart.ts @@ -18,7 +18,6 @@ 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 { @@ -46,5 +45,4 @@ export class ChartFigure extends Component { ChartFigure.props = { figure: Object, - onFigureDeleted: Function, }; diff --git a/src/components/figures/figure_container/figure_container.ts b/src/components/figures/figure_container/figure_container.ts index 0665e6398e..18ca029d7a 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; @@ -464,6 +462,4 @@ export class FiguresContainer extends Component { } } -FiguresContainer.props = { - onFigureDeleted: Function, -}; +FiguresContainer.props = {}; 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 { @@ -25,5 +24,4 @@ export class ImageFigure extends Component { ImageFigure.props = { figure: Object, - onFigureDeleted: Function, }; diff --git a/src/components/grid/grid.xml b/src/components/grid/grid.xml index 107b6d5067..64f3d2358f 100644 --- a/src/components/grid/grid.xml +++ b/src/components/grid/grid.xml @@ -15,7 +15,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"; @@ -55,7 +53,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(); } @@ -82,7 +80,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(); @@ -100,10 +98,14 @@ 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.env.focusableElement.focus(); + } } } -GridAddRowsFooter.props = { - focusGrid: Function, -}; +GridAddRowsFooter.props = {}; diff --git a/src/components/grid_overlay/grid_overlay.ts b/src/components/grid_overlay/grid_overlay.ts index 581a541ade..ef7ccfb1ad 100644 --- a/src/components/grid_overlay/grid_overlay.ts +++ b/src/components/grid_overlay/grid_overlay.ts @@ -168,7 +168,6 @@ interface Props { onGridResized: (dimension: Rect) => void; onGridMoved: (deltaX: Pixel, deltaY: Pixel) => void; gridOverlayDimensions: string; - onFigureDeleted: () => void; } export class GridOverlay extends Component { @@ -180,7 +179,6 @@ export class GridOverlay extends Component { onCellClicked: () => {}, onCellRightClicked: () => {}, onGridResized: () => {}, - onFigureDeleted: () => {}, }; private gridOverlay: Ref = useRef("gridOverlay"); private gridOverlayRect = useAbsoluteBoundingRect(this.gridOverlay); @@ -261,7 +259,6 @@ GridOverlay.props = { onCellClicked: { type: Function, optional: true }, onCellRightClicked: { type: Function, optional: true }, onGridResized: { type: Function, optional: true }, - onFigureDeleted: { type: Function, optional: true }, onGridMoved: Function, gridOverlayDimensions: String, }; diff --git a/src/components/grid_overlay/grid_overlay.xml b/src/components/grid_overlay/grid_overlay.xml index 1d7f43116f..047a4e977c 100644 --- a/src/components/grid_overlay/grid_overlay.xml +++ b/src/components/grid_overlay/grid_overlay.xml @@ -8,12 +8,11 @@ t-on-mousedown="onMouseDown" t-on-dblclick.self="onDoubleClick" t-on-contextmenu.stop.prevent="onContextMenu"> - +
diff --git a/tests/figures/figure_component.test.ts b/tests/figures/figure_component.test.ts index 59573d4749..f4fa29a6ae 100644 --- a/tests/figures/figure_component.test.ts +++ b/tests/figures/figure_component.test.ts @@ -703,6 +703,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([