Let a click on the separator wander a little before it counts as a drag (#34580)
The separator has to tell a click apart from a drag, because dragging it also ends in a click, and it did so by treating any pointer movement at all between press and release as a drag. A pointer rarely holds perfectly still, least of all on a trackpad, so clicking the separator to open the room list often did nothing and had to be tried again. Movement is now measured from where the pointer went down and only counts as a drag past a few pixels, which means the handlers need the pointer position and the separator passes its events through to get it. Movement with nothing held down is ignored too: those events fire on hover, and one of them used to spend the click that came after it. Tests: a click that wanders a couple of pixels still opens the panel, as does one that follows moving across the separator, while a real drag is still no click. Co-authored-by: R Midhun Suresh <hi@midhun.dev>
This commit is contained in:
co-authored by
R Midhun Suresh
parent
354aa7de2b
commit
ba5eb6b463
@@ -8,6 +8,7 @@
|
||||
// @vitest-environment happy-dom
|
||||
|
||||
import { vi, describe, it, expect, afterEach } from "vitest";
|
||||
import { type PointerEvent } from "react";
|
||||
|
||||
import { waitFor } from "test-utils-rtl";
|
||||
import { type PanelImperativeHandle } from "@element-hq/web-shared-components";
|
||||
@@ -17,6 +18,9 @@ import SettingsStore from "../../settings/SettingsStore";
|
||||
import { SettingLevel } from "../../settings/SettingLevel";
|
||||
import { CallStore } from "../../stores/CallStore";
|
||||
|
||||
/** The pointer handlers only read where the pointer is, so that is all a test has to give them. */
|
||||
const pointerAt = (x: number, y: number) => ({ clientX: x, clientY: y }) as PointerEvent;
|
||||
|
||||
describe("LeftPanelResizerViewModel", () => {
|
||||
afterEach(() => {
|
||||
localStorage.clear();
|
||||
@@ -67,7 +71,7 @@ describe("LeftPanelResizerViewModel", () => {
|
||||
const vm = new ResizerViewModel(CallStore.instance);
|
||||
expect(() => {
|
||||
// Click
|
||||
vm.onPointerDown();
|
||||
vm.onPointerDown(pointerAt(100, 100));
|
||||
vm.onPointerUp();
|
||||
}).not.toThrow();
|
||||
});
|
||||
@@ -86,13 +90,49 @@ describe("LeftPanelResizerViewModel", () => {
|
||||
vm.setPanelHandle(mockHandle);
|
||||
|
||||
// Simulate drag
|
||||
vm.onPointerDown();
|
||||
vm.onPointerMove();
|
||||
vm.onPointerDown(pointerAt(100, 100));
|
||||
vm.onPointerMove(pointerAt(160, 100));
|
||||
vm.onPointerUp();
|
||||
|
||||
expect(mockHandle.resize).not.toHaveBeenCalledWith("34%");
|
||||
});
|
||||
|
||||
it("should expand panel when a click wanders a little", () => {
|
||||
const vm = new ResizerViewModel(CallStore.instance);
|
||||
SettingsStore.setValue("RoomList.panelSize", null, SettingLevel.DEVICE, 34);
|
||||
const mockHandle = {
|
||||
resize: vi.fn(),
|
||||
isCollapsed: vi.fn().mockReturnValue(true),
|
||||
getSize: vi.fn().mockReturnValue(0),
|
||||
} as unknown as PanelImperativeHandle;
|
||||
vm.setPanelHandle(mockHandle);
|
||||
|
||||
// A trackpad rarely holds the pointer still between press and release.
|
||||
vm.onPointerDown(pointerAt(100, 100));
|
||||
vm.onPointerMove(pointerAt(101, 102));
|
||||
vm.onPointerUp();
|
||||
|
||||
expect(mockHandle.resize).toHaveBeenCalledWith("34%");
|
||||
});
|
||||
|
||||
it("should expand panel on a click that follows moving across the separator", () => {
|
||||
const vm = new ResizerViewModel(CallStore.instance);
|
||||
SettingsStore.setValue("RoomList.panelSize", null, SettingLevel.DEVICE, 34);
|
||||
const mockHandle = {
|
||||
resize: vi.fn(),
|
||||
isCollapsed: vi.fn().mockReturnValue(true),
|
||||
getSize: vi.fn().mockReturnValue(0),
|
||||
} as unknown as PanelImperativeHandle;
|
||||
vm.setPanelHandle(mockHandle);
|
||||
|
||||
// Pointer moves fire on hover too, with no button held.
|
||||
vm.onPointerMove(pointerAt(300, 300));
|
||||
vm.onPointerDown(pointerAt(100, 100));
|
||||
vm.onPointerUp();
|
||||
|
||||
expect(mockHandle.resize).toHaveBeenCalledWith("34%");
|
||||
});
|
||||
|
||||
describe("should expand panel on double click when panel is collapsed", () => {
|
||||
it("to last non-zero width that the user set", () => {
|
||||
const vm = new ResizerViewModel(CallStore.instance);
|
||||
@@ -104,7 +144,7 @@ describe("LeftPanelResizerViewModel", () => {
|
||||
} as unknown as PanelImperativeHandle;
|
||||
vm.setPanelHandle(mockHandle);
|
||||
// Simulate click
|
||||
vm.onPointerDown();
|
||||
vm.onPointerDown(pointerAt(100, 100));
|
||||
vm.onPointerUp();
|
||||
expect(mockHandle.resize).toHaveBeenCalledWith("34%");
|
||||
});
|
||||
@@ -118,7 +158,7 @@ describe("LeftPanelResizerViewModel", () => {
|
||||
} as unknown as PanelImperativeHandle;
|
||||
vm.setPanelHandle(mockHandle);
|
||||
// Simulate click
|
||||
vm.onPointerDown();
|
||||
vm.onPointerDown(pointerAt(100, 100));
|
||||
vm.onPointerUp();
|
||||
expect(mockHandle.resize).toHaveBeenCalledWith("100%");
|
||||
});
|
||||
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
type ResizerViewSnapshot,
|
||||
} from "@element-hq/web-shared-components";
|
||||
import { debounce } from "lodash";
|
||||
import { type PointerEvent } from "react";
|
||||
|
||||
import SettingsStore from "../../settings/SettingsStore";
|
||||
import { SettingLevel } from "../../settings/SettingLevel";
|
||||
@@ -133,15 +134,22 @@ export class ResizerViewModel
|
||||
this.mouseClickHandler.onPointerUp();
|
||||
};
|
||||
|
||||
public onPointerMove = (): void => {
|
||||
this.mouseClickHandler.onPointerMove();
|
||||
public onPointerMove = (event: PointerEvent): void => {
|
||||
this.mouseClickHandler.onPointerMove(event.clientX, event.clientY);
|
||||
};
|
||||
|
||||
public onPointerDown = (): void => {
|
||||
this.mouseClickHandler.onPointerDown();
|
||||
public onPointerDown = (event: PointerEvent): void => {
|
||||
this.mouseClickHandler.onPointerDown(event.clientX, event.clientY);
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* How far the pointer may travel between going down and coming up and still count as a click rather
|
||||
* than a drag. A trackpad rarely holds a pointer perfectly still, and a separator that only opens on
|
||||
* a pixel-perfect click reads as one that ignores clicks.
|
||||
*/
|
||||
const CLICK_TOLERANCE_PX = 5;
|
||||
|
||||
/**
|
||||
* Dragging the separator will emit a click event.
|
||||
* This class uses pointer event handlers to distinguish between a drag and a click
|
||||
@@ -150,17 +158,27 @@ export class ResizerViewModel
|
||||
class MouseClickHandler {
|
||||
public constructor(private readonly onClick: () => void) {}
|
||||
|
||||
/** Where the pointer went down, for as long as it is down. */
|
||||
private origin: { x: number; y: number } | null = null;
|
||||
private isResize = false;
|
||||
|
||||
public onPointerUp = (): void => {
|
||||
this.origin = null;
|
||||
if (!this.isResize) this.onClick();
|
||||
};
|
||||
|
||||
public onPointerDown = (): void => {
|
||||
public onPointerDown = (x: number, y: number): void => {
|
||||
this.origin = { x, y };
|
||||
this.isResize = false;
|
||||
};
|
||||
|
||||
public onPointerMove = (): void => {
|
||||
this.isResize = true;
|
||||
public onPointerMove = (x: number, y: number): void => {
|
||||
// Moving across the separator with no button held is not a drag, and must not be allowed to
|
||||
// spend the next click.
|
||||
if (!this.origin) return;
|
||||
|
||||
if (Math.abs(x - this.origin.x) > CLICK_TOLERANCE_PX || Math.abs(y - this.origin.y) > CLICK_TOLERANCE_PX) {
|
||||
this.isResize = true;
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
@@ -23,14 +23,16 @@ export interface SeparatorViewActions {
|
||||
onPointerUp: () => void;
|
||||
|
||||
/**
|
||||
* onPointerMove handler for separator.
|
||||
* onPointerMove handler for separator. Takes the event so that how far the pointer has travelled
|
||||
* since it went down can be measured.
|
||||
*/
|
||||
onPointerMove: () => void;
|
||||
onPointerMove: (event: React.PointerEvent) => void;
|
||||
|
||||
/**
|
||||
* onPointerDown handler for separator.
|
||||
* onPointerDown handler for separator. Takes the event so that where the pointer went down can be
|
||||
* measured from.
|
||||
*/
|
||||
onPointerDown: () => void;
|
||||
onPointerDown: (event: React.PointerEvent) => void;
|
||||
|
||||
/**
|
||||
* onDoubleClick handler for the separator.
|
||||
|
||||
Reference in New Issue
Block a user