Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Column resizing: handle coalesced pointer move events #3594

Merged
merged 1 commit into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 20 additions & 10 deletions src/HeaderCell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -122,22 +122,37 @@ export default function HeaderCell<R, SR>({
const headerCell = currentTarget.parentElement!;
const { right, left } = headerCell.getBoundingClientRect();
const offset = isRtl ? event.clientX - left : right - event.clientX;
let hasDoubleClicked = false;

function onPointerMove(event: PointerEvent) {
const { right, left } = headerCell.getBoundingClientRect();
const width = isRtl ? right + offset - event.clientX : event.clientX + offset - left;
if (width > 0) {
onColumnResize(column, clampColumnWidth(width, column));
const { width, right, left } = headerCell.getBoundingClientRect();
let newWidth = isRtl ? right + offset - event.clientX : event.clientX + offset - left;
newWidth = clampColumnWidth(newWidth, column);
if (width > 0 && newWidth !== width) {
onColumnResize(column, newWidth);
}
}

function onLostPointerCapture() {
function onDoubleClick() {
hasDoubleClicked = true;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could have kept the onDoubleClick prop, and used a ref, but I'm not sure that'd be 100% safe, what if we fail to reset the ref in time in the onLostPointerCapture handler?

onColumnResize(column, 'max-content');
}

function onLostPointerCapture(event: PointerEvent) {
// Handle final pointer position that may have been skipped by coalesced pointer move events.
// Skip move pointer handling if the user double-clicked.
if (!hasDoubleClicked) {
onPointerMove(event);
}

currentTarget.removeEventListener('pointermove', onPointerMove);
currentTarget.removeEventListener('dblclick', onDoubleClick);
currentTarget.removeEventListener('lostpointercapture', onLostPointerCapture);
}

currentTarget.setPointerCapture(pointerId);
currentTarget.addEventListener('pointermove', onPointerMove);
currentTarget.addEventListener('dblclick', onDoubleClick);
currentTarget.addEventListener('lostpointercapture', onLostPointerCapture);
}

Expand Down Expand Up @@ -186,10 +201,6 @@ export default function HeaderCell<R, SR>({
}
}

function onDoubleClick() {
onColumnResize(column, 'max-content');
}

function handleFocus(event: React.FocusEvent<HTMLDivElement>) {
onFocus?.(event);
if (shouldFocusGrid) {
Expand Down Expand Up @@ -295,7 +306,6 @@ export default function HeaderCell<R, SR>({
<div
className={resizeHandleClassname}
onClick={stopPropagation}
onDoubleClick={onDoubleClick}
onPointerDown={onPointerDown}
/>
)}
Expand Down
44 changes: 18 additions & 26 deletions test/browser/column/resizable.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,27 @@ interface Row {
readonly col2: string;
}

function queryResizeHandle(column: HTMLElement) {
return column.querySelector(`.${resizeHandleClassname}`);
}

function getResizeHandle(column: HTMLElement) {
const resizeHandle = column.querySelector(`.${resizeHandleClassname}`);

if (resizeHandle === null) {
throw new Error('Resize handle not found');
}

return resizeHandle;
}

interface ResizeArgs {
readonly column: HTMLElement;
readonly resizeBy: number;
}

async function resize({ column, resizeBy }: ResizeArgs) {
const resizeHandle = column.querySelector(`.${resizeHandleClassname}`);
if (resizeHandle === null) return;
expect(getResizeHandle(column)).toBeInTheDocument();

await act(async () => {
// @ts-expect-error
Expand All @@ -26,8 +39,7 @@ async function resize({ column, resizeBy }: ResizeArgs) {
}

async function autoResize(column: HTMLElement) {
const resizeHandle = column.querySelector(`.${resizeHandleClassname}`);
if (resizeHandle === null) return;
const resizeHandle = getResizeHandle(column);

// eslint-disable-next-line testing-library/no-unnecessary-act
await act(async () => {
Expand All @@ -51,14 +63,10 @@ const columns: readonly Column<Row>[] = [
}
];

test('should not resize column if resizable is not specified', async () => {
test('cannot not resize or auto resize column when resizable is not specified', () => {
setup<Row, unknown>({ columns, rows: [] });
const [col1] = getHeaderCells();
expect(getGrid()).toHaveStyle({ gridTemplateColumns: '100px 200px' });
await resize({ column: col1, resizeBy: 50 });
expect(getGrid()).toHaveStyle({ gridTemplateColumns: '100px 200px' });
await resize({ column: col1, resizeBy: -50 });
expect(getGrid()).toHaveStyle({ gridTemplateColumns: '100px 200px' });
expect(queryResizeHandle(col1)).not.toBeInTheDocument();
});

test('should resize column when dragging the handle', async () => {
Expand Down Expand Up @@ -86,22 +94,6 @@ test('should use the minWidth if specified', async () => {
expect(getGrid()).toHaveStyle({ gridTemplateColumns: '100px 100px' });
});

test('should not auto resize column if resizable is not specified', async () => {
setup<Row, unknown>({
columns,
rows: [
{
col1: 1,
col2: 'a'.repeat(50)
}
]
});
const [col1] = getHeaderCells();
expect(getGrid()).toHaveStyle({ gridTemplateColumns: '100px 200px' });
await autoResize(col1);
expect(getGrid()).toHaveStyle({ gridTemplateColumns: '100px 200px' });
});

test('should auto resize column when resize handle is double clicked', async () => {
setup<Row, unknown>({
columns,
Expand Down
2 changes: 1 addition & 1 deletion vitest.workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { BrowserCommand } from 'vitest/node';
const resizeColumn: BrowserCommand<[resizeBy: number]> = async (context, resizeBy) => {
const page = context.page;
const frame = await context.frame();
const resizeHandle = frame.locator('[role="columnheader"][aria-colindex="2"] div');
const resizeHandle = frame.locator('[role="columnheader"][aria-colindex="2"] div');
const { x, y } = (await resizeHandle.boundingBox())!;
await resizeHandle.hover({
position: { x: 5, y: 5 }
Expand Down