Skip to content

Commit 2e401cc

Browse files
committed
[IMP] figures: add drag threshold
When clicking on a figure, a very small mouse movement between the mousedown and mouseup events could trigger a drag and drop operation, canceling the click event. This is particularly annoying when trying to interact with carousels, as they have a lot of clickable areas. Most applications use a drag threshold, allowing small mouse movements to be ignored. This commit adds a drag threshold for figures. Also fixed some test that checked for the `o-dragging` class on figure. That class does not exist for figures. Task: 5059476 Part-of: #7029 Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
1 parent a96ba1b commit 2e401cc

File tree

3 files changed

+35
-4
lines changed

3 files changed

+35
-4
lines changed

src/components/figures/figure_container/figure_container.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Component, onMounted, onWillUpdateProps, useState } from "@odoo/owl";
2-
import { ComponentsImportance, MIN_FIG_SIZE } from "../../../constants";
2+
import { ComponentsImportance, DRAG_THRESHOLD, MIN_FIG_SIZE } from "../../../constants";
33
import { isDefined } from "../../../helpers";
44
import { rectUnion } from "../../../helpers/rectangle";
55
import { figureRegistry } from "../../../registries/figures_registry";
@@ -331,9 +331,18 @@ export class FiguresContainer extends Component<Props, SpreadsheetChildEnv> {
331331
).end,
332332
};
333333

334+
let hasStartedDnd = false;
334335
const onMouseMove = (ev: MouseEvent) => {
335336
const getters = this.env.model.getters;
336337
const currentMousePosition = { x: ev.clientX, y: ev.clientY };
338+
339+
const offsetX = Math.abs(currentMousePosition.x - initialMousePosition.x);
340+
const offsetY = Math.abs(currentMousePosition.y - initialMousePosition.y);
341+
if (!hasStartedDnd && offsetX < DRAG_THRESHOLD && offsetY < DRAG_THRESHOLD) {
342+
return; // add a small threshold to avoid dnd when just clicking
343+
}
344+
hasStartedDnd = true;
345+
337346
const draggedFigure = dragFigureForMove(
338347
currentMousePosition,
339348
initialMousePosition,

src/constants.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,3 +346,5 @@ export const tokenColors = {
346346
ARG_SEPARATOR: functionColor,
347347
ORPHAN_RIGHT_PAREN: "#ff0000",
348348
} as const;
349+
350+
export const DRAG_THRESHOLD = 5; // in pixels, to avoid unwanted drag when clicking

tests/figures/figure_component.test.ts

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,13 @@ jest.mock("../../src/components/helpers/dom_helpers", () => {
5858
};
5959
});
6060

61+
const constantsMocks = jest.requireMock("../../src/constants");
62+
jest.mock("../../src/constants", () => ({ ...jest.requireActual("../../src/constants") }));
63+
64+
beforeEach(() => {
65+
constantsMocks.DRAG_THRESHOLD = 0; // mock drag threshold to 0 for easier testing of snap
66+
});
67+
6168
const cellHeight = DEFAULT_CELL_HEIGHT;
6269
const cellWidth = DEFAULT_CELL_WIDTH;
6370

@@ -669,14 +676,14 @@ describe("figures", () => {
669676
createFigure(model, { id: figureId, offset: { x: 0, y: 200 } });
670677
model.updateMode("readonly");
671678
await nextTick();
672-
const figure = fixture.querySelector(".o-figure")!;
679+
const figure = fixture.querySelector<HTMLElement>(".o-figure")!;
673680
await simulateClick(".o-figure");
674681
expect(document.activeElement).not.toBe(figure);
675682
expect(fixture.querySelector(".o-fig-anchor")).toBeNull();
676683

677684
triggerMouseEvent(figure, "pointerdown", 300, 200);
678685
await nextTick();
679-
expect(figure.classList).not.toContain("o-dragging");
686+
expect(figure).not.toHaveStyle({ cursor: "grabbing" });
680687
});
681688

682689
describe("Figure border", () => {
@@ -926,7 +933,20 @@ describe("figures", () => {
926933
await nextTick();
927934
triggerMouseEvent(".o-figure", "pointerdown", 0, 0);
928935
await nextTick();
929-
expect(fixture.querySelector(".o-figure")?.classList.contains("o-dragging")).toBeFalsy();
936+
expect(".o-figure").not.toHaveStyle({ cursor: "grabbing" });
937+
});
938+
939+
test("There is a small threshold before the figure is marked as dragging", async () => {
940+
constantsMocks.DRAG_THRESHOLD = 5;
941+
createFigure(model);
942+
await nextTick();
943+
944+
await clickAndDrag(".o-figure", { x: 3, y: 1 }, undefined, false);
945+
expect(".o-figure").not.toHaveStyle({ cursor: "grabbing" });
946+
expect(fixture.querySelector<HTMLElement>(".o-figure")?.style.cursor).not.toBe("grabbing");
947+
948+
await clickAndDrag(".o-figure", { x: 6, y: 1 }, undefined, false);
949+
expect(".o-figure").toHaveStyle({ cursor: "grabbing" });
930950
});
931951

932952
test("Figure container is properly computed based on the sheetView size", async () => {

0 commit comments

Comments
 (0)