[FIX] Figure: only focus when the selected figure changes

src/components/figures/figure/figure.ts

@@ -114,22 +114,25 @@ export class FigureComponent extends Component<SpreadsheetChildEnv> {
   setup() {
     const borderWidth = figureRegistry.get(this.props.figureUI.tag).borderWidth;
     this.borderWidth = borderWidth !== undefined ? borderWidth : BORDER_WIDTH;
-    useLayoutEffect(() => {
-      const selectedFigureIds = this.env.model.getters.getSelectedFigureIds();
-      const thisFigureId = this.props.figureUI.id;
-      const el = this.figureRef();
-      if (selectedFigureIds.includes(thisFigureId)) {
-        /** Scrolling on a newly inserted figure that overflows outside the viewport
-         * will break the whole layout.
-         * NOTE: `preventScroll`does not work on mobile but then again,
-         * mobile is not really supported ATM.
-         *
-         * TODO: When implementing proper mobile, we will need to scroll the viewport
-         * correctly (and render?) before focusing the element.
-         */
-        el?.focus({ preventScroll: true });
-      }
-    });
+    useLayoutEffect(
+      () => {
+        const selectedFigureIds = this.env.model.getters.getSelectedFigureIds();
+        const thisFigureId = this.props.figureUI.id;
+        const el = this.figureRef();
+        if (selectedFigureIds.includes(thisFigureId)) {
+          /** Scrolling on a newly inserted figure that overflows outside the viewport
+           * will break the whole layout.
+           * NOTE: `preventScroll`does not work on mobile but then again,
+           * mobile is not really supported ATM.
+           *
+           * TODO: When implementing proper mobile, we will need to scroll the viewport
+           * correctly (and render?) before focusing the element.
+           */
+          el?.focus({ preventScroll: true });
+        }
+      },
+      () => [this.env.model.getters.getSelectedFigureIds(), this.props.figureUI.id]
+    );
   }
 
   clickAnchor(dirX: ResizeDirection, dirY: ResizeDirection, ev: MouseEvent) {

tests/figures/chart/charts_component.test.ts

@@ -61,6 +61,7 @@ import { TEST_CHART_DATA } from "../../test_helpers/constants";
 import {
   click,
   clickAndDrag,
+  clickCell,
   doubleClick,
   editSelectComponent,
   focusAndKeyDown,
@@ -3404,3 +3405,37 @@ test("Can update the chart data source from the side panel", async () => {
     })
   );
 });
+
+test("Can edit a chart range", async () => {
+  model = new Model();
+  createChart(
+    model,
+    {
+      type: "bar",
+      ...toChartDataSource({
+        dataSets: [{ dataRange: "A1:A3" }],
+        dataSetsHaveTitle: false,
+      }),
+    },
+    chartId
+  );
+  await mountSpreadsheet();
+  selectFigure(model, model.getters.getFigureIdFromChartId(chartId)!);
+  await simulateClick(".o-figure");
+  env.openSidePanel("ChartPanel");
+  await nextTick();
+  expect(document.activeElement).toBe(fixture.querySelector(".o-figure")!);
+  await simulateClick(".o-selection-input input.o-input");
+  await clickCell(model, "B1");
+  await keyDown({ key: "ArrowDown" });
+
+  await simulateClick(".o-data-series .o-selection-ok");
+  const definition = model.getters.getChartDefinition(chartId) as BarChartDefinition<string>;
+
+  expect(definition).toMatchObject(
+    toChartDataSource({
+      dataSets: [{ dataRange: "B2", dataSetId: expect.any(String) }],
+      dataSetsHaveTitle: false,
+    })
+  );
+});

tests/figures/figure_component.test.ts

@@ -1222,6 +1222,19 @@ describe("figures", () => {
     expect(document.activeElement).toBe(panelInput);
   });
 
+  test("Can navigate the figure menu with the keyboard", async () => {
+    createChart(model, { type: "bar" });
+    await nextTick();
+    await simulateClick(".o-figure-menu-item");
+    // NOTE: need to "force" a rendering such that the menu takes the focus back
+    // See task https://www.odoo.com/odoo/2328/tasks/6272762
+    await keyDown({ key: "ArrowDown" });
+    expect(document.activeElement).toHaveClass("o-menu-wrapper");
+    await keyDown({ key: "ArrowDown" });
+    expect(document.activeElement).toHaveClass("o-menu-item");
+    expect(document.activeElement).toHaveAttribute("data-name", "edit");
+  });
+
   describe("Figure drag & drop snap", () => {
     describe("Move figure", () => {
       test.each([