Skip to content

Commit 3920b14

Browse files
authored
fix: Cherry-pick DH-22093: Fix web UI freezing bug from grid-block-events (#2647)
Cherry-pick #2646
1 parent 9b60dc3 commit 3920b14

File tree

2 files changed

+209
-5
lines changed

2 files changed

+209
-5
lines changed

packages/grid/src/Grid.test.tsx

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1217,3 +1217,191 @@ describe('column separators', () => {
12171217
});
12181218
});
12191219
});
1220+
1221+
describe('grid-block-events cleanup', () => {
1222+
const resizableTheme = { ...defaultTheme, allowColumnResize: true };
1223+
1224+
function getColumnSeparatorX(columnIndex: VisibleIndex): number {
1225+
const { rowHeaderWidth, columnWidth } = resizableTheme;
1226+
return rowHeaderWidth + columnWidth * (columnIndex + 1) - 2;
1227+
}
1228+
1229+
function getColumnHeaderY(): number {
1230+
return Math.floor(resizableTheme.columnHeaderHeight / 2);
1231+
}
1232+
1233+
function hasBlockEventsClass(): boolean {
1234+
return document.documentElement.classList.contains('grid-block-events');
1235+
}
1236+
1237+
beforeEach(() => {
1238+
// Ensure clean state before each test
1239+
document.documentElement.classList.remove('grid-block-events');
1240+
});
1241+
1242+
afterEach(() => {
1243+
// Clean up after each test
1244+
document.documentElement.classList.remove('grid-block-events');
1245+
});
1246+
1247+
it('should remove grid-block-events after normal drag and mouse up', () => {
1248+
const component = makeGridComponent(new MockGridModel(), resizableTheme);
1249+
const separatorX = getColumnSeparatorX(0);
1250+
const headerY = getColumnHeaderY();
1251+
1252+
expect(hasBlockEventsClass()).toBe(false);
1253+
1254+
// Start drag on column separator
1255+
mouseDown(0, 0, component, {}, separatorX, headerY);
1256+
1257+
// Drag to trigger handleMouseDrag which adds the class
1258+
mouseMove(0, 0, component, {}, separatorX + 50, headerY);
1259+
1260+
expect(hasBlockEventsClass()).toBe(true);
1261+
1262+
// Normal mouse up should clean up
1263+
mouseUp(0, 0, component, { button: 0 }, separatorX + 50, headerY);
1264+
1265+
expect(hasBlockEventsClass()).toBe(false);
1266+
});
1267+
1268+
it('should remove grid-block-events when right-clicking during drag (case 1)', () => {
1269+
const component = makeGridComponent(new MockGridModel(), resizableTheme);
1270+
const separatorX = getColumnSeparatorX(0);
1271+
const headerY = getColumnHeaderY();
1272+
1273+
// Start drag
1274+
mouseDown(0, 0, component, {}, separatorX, headerY);
1275+
mouseMove(0, 0, component, {}, separatorX + 50, headerY);
1276+
1277+
expect(hasBlockEventsClass()).toBe(true);
1278+
1279+
// Right-click during drag (button=2)
1280+
mouseUp(0, 0, component, { button: 2 }, separatorX + 50, headerY);
1281+
1282+
// Right-click during drag returns early, class should still be present
1283+
// Then complete with left mouse up
1284+
mouseUp(0, 0, component, { button: 0 }, separatorX + 50, headerY);
1285+
1286+
expect(hasBlockEventsClass()).toBe(false);
1287+
});
1288+
1289+
it('should ignore middle-click during drag and continue dragging (case 2)', () => {
1290+
const component = makeGridComponent(new MockGridModel(), resizableTheme);
1291+
const separatorX = getColumnSeparatorX(0);
1292+
const headerY = getColumnHeaderY();
1293+
1294+
// Start drag
1295+
mouseDown(0, 0, component, {}, separatorX, headerY);
1296+
mouseMove(0, 0, component, {}, separatorX + 50, headerY);
1297+
1298+
expect(hasBlockEventsClass()).toBe(true);
1299+
1300+
// Middle mouse button up during drag (button=1) should be ignored
1301+
// Drag should continue
1302+
mouseUp(0, 0, component, { button: 1 }, separatorX + 50, headerY);
1303+
1304+
// Class should still be present since drag is ongoing
1305+
expect(hasBlockEventsClass()).toBe(true);
1306+
1307+
// Left mouse up should properly end the drag and clean up
1308+
mouseUp(0, 0, component, { button: 0 }, separatorX + 50, headerY);
1309+
1310+
expect(hasBlockEventsClass()).toBe(false);
1311+
});
1312+
1313+
it('should handle addDocumentCursor with null cursor (case 3)', () => {
1314+
const component = makeGridComponent(new MockGridModel(), resizableTheme);
1315+
1316+
expect(hasBlockEventsClass()).toBe(false);
1317+
1318+
// First add a real cursor - this sets documentCursor and adds grid-block-events
1319+
component.addDocumentCursor('move');
1320+
expect(hasBlockEventsClass()).toBe(true);
1321+
1322+
// Now simulate cursor being cleared to null (e.g., mouse handler sets cursor = null)
1323+
// This sets documentCursor to null but keeps grid-block-events
1324+
component.addDocumentCursor(null);
1325+
1326+
// grid-block-events should still be present since it was added
1327+
expect(hasBlockEventsClass()).toBe(true);
1328+
1329+
// removeDocumentCursor should still clean up
1330+
// BUG: Currently it doesn't because documentCursor is null and
1331+
// the entire function body is guarded by `if (this.documentCursor != null)`
1332+
component.removeDocumentCursor();
1333+
1334+
// After fix, this should be false
1335+
expect(hasBlockEventsClass()).toBe(false);
1336+
});
1337+
1338+
it('should handle multiple Grid instances without interference (case 5)', () => {
1339+
// Create two grid instances
1340+
const component1 = makeGridComponent(new MockGridModel(), resizableTheme);
1341+
const component2 = makeGridComponent(new MockGridModel(), resizableTheme);
1342+
1343+
const separatorX = getColumnSeparatorX(0);
1344+
const headerY = getColumnHeaderY();
1345+
1346+
expect(hasBlockEventsClass()).toBe(false);
1347+
1348+
// Start drag on Grid 1
1349+
mouseDown(0, 0, component1, {}, separatorX, headerY);
1350+
mouseMove(0, 0, component1, {}, separatorX + 50, headerY);
1351+
1352+
expect(hasBlockEventsClass()).toBe(true);
1353+
1354+
// Grid 2 unmounts while Grid 1 is dragging
1355+
// This simulates closing a panel or tab
1356+
component2.componentWillUnmount();
1357+
1358+
// Grid 2's unmount should NOT affect Grid 1's drag state
1359+
// The class should still be present for Grid 1
1360+
expect(hasBlockEventsClass()).toBe(true);
1361+
1362+
// Complete Grid 1's drag
1363+
mouseUp(0, 0, component1, { button: 0 }, separatorX + 50, headerY);
1364+
1365+
expect(hasBlockEventsClass()).toBe(false);
1366+
});
1367+
1368+
it('should clean up grid-block-events when component unmounts during drag (case 6)', () => {
1369+
const component = makeGridComponent(new MockGridModel(), resizableTheme);
1370+
const separatorX = getColumnSeparatorX(0);
1371+
const headerY = getColumnHeaderY();
1372+
1373+
// Start drag
1374+
mouseDown(0, 0, component, {}, separatorX, headerY);
1375+
mouseMove(0, 0, component, {}, separatorX + 50, headerY);
1376+
1377+
expect(hasBlockEventsClass()).toBe(true);
1378+
1379+
// Component unmounts (simulates user navigating away or closing panel)
1380+
// This could happen if user drags outside window and releases there
1381+
component.componentWillUnmount();
1382+
1383+
// componentWillUnmount should clean up the class
1384+
expect(hasBlockEventsClass()).toBe(false);
1385+
});
1386+
1387+
it('should not leave grid-block-events when drag ends outside grid boundaries', () => {
1388+
const component = makeGridComponent(new MockGridModel(), resizableTheme);
1389+
const separatorX = getColumnSeparatorX(0);
1390+
const headerY = getColumnHeaderY();
1391+
1392+
// Start drag
1393+
mouseDown(0, 0, component, {}, separatorX, headerY);
1394+
mouseMove(0, 0, component, {}, separatorX + 50, headerY);
1395+
1396+
expect(hasBlockEventsClass()).toBe(true);
1397+
1398+
// Move mouse far outside grid (simulating dragging outside window)
1399+
mouseMove(0, 0, component, {}, -1000, -1000);
1400+
1401+
// Mouse up outside grid boundaries
1402+
mouseUp(0, 0, component, { button: 0 }, -1000, -1000);
1403+
1404+
// Should still clean up properly
1405+
expect(hasBlockEventsClass()).toBe(false);
1406+
});
1407+
});

packages/grid/src/Grid.tsx

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,10 @@ class Grid extends PureComponent<GridProps, GridState> {
328328
// blocked pointer events that would otherwise prevent cursor styling from showing
329329
documentCursor: string | null;
330330

331+
// Track if this Grid instance added the grid-block-events class
332+
// Used to ensure only the Grid that added the class removes it
333+
hasAddedBlockEvents: boolean;
334+
331335
dragTimer: ReturnType<typeof setTimeout> | null;
332336

333337
keyHandlers: readonly KeyHandler[];
@@ -385,6 +389,7 @@ class Grid extends PureComponent<GridProps, GridState> {
385389
// Note: on document, not body so that cursor styling can be combined with
386390
// blocked pointer events that would otherwise prevent cursor styling from showing
387391
this.documentCursor = null;
392+
this.hasAddedBlockEvents = false;
388393

389394
this.dragTimer = null;
390395

@@ -1652,14 +1657,20 @@ class Grid extends PureComponent<GridProps, GridState> {
16521657
document.documentElement.classList.add(this.documentCursor);
16531658
}
16541659
document.documentElement.classList.add('grid-block-events');
1660+
this.hasAddedBlockEvents = true;
16551661
}
16561662

16571663
removeDocumentCursor(): void {
16581664
if (this.documentCursor != null) {
16591665
document.documentElement.classList.remove(this.documentCursor);
1660-
document.documentElement.classList.remove('grid-block-events');
16611666
this.documentCursor = null;
16621667
}
1668+
// Only remove grid-block-events if this Grid instance added it
1669+
// This prevents one Grid from removing the class on unmount while another Grid is still dragging
1670+
if (this.hasAddedBlockEvents) {
1671+
document.documentElement.classList.remove('grid-block-events');
1672+
this.hasAddedBlockEvents = false;
1673+
}
16631674
}
16641675

16651676
startDragTimer(event: React.MouseEvent): void {
@@ -1868,18 +1879,23 @@ class Grid extends PureComponent<GridProps, GridState> {
18681879
}
18691880

18701881
handleMouseUp(event: MouseEvent): void {
1882+
// Ignore non-left click while dragging to allow drag to continue
1883+
const { isDragging } = this.state;
1884+
if (isDragging && event.button !== 0) {
1885+
return;
1886+
}
1887+
18711888
window.removeEventListener('mousemove', this.handleMouseDrag, true);
18721889
window.removeEventListener('mouseup', this.handleMouseUp, true);
18731890

1891+
this.stopDragTimer();
1892+
this.removeDocumentCursor();
1893+
18741894
if (event.button != null && event.button !== 0) {
18751895
return;
18761896
}
18771897

18781898
this.notifyMouseHandlers('onUp', event, false);
1879-
1880-
this.stopDragTimer();
1881-
1882-
this.removeDocumentCursor();
18831899
}
18841900

18851901
handleResize(): void {

0 commit comments

Comments
 (0)