diff --git a/packages/grid/src/Grid.test.tsx b/packages/grid/src/Grid.test.tsx index ae1452fa0a..db84b493ec 100644 --- a/packages/grid/src/Grid.test.tsx +++ b/packages/grid/src/Grid.test.tsx @@ -1181,3 +1181,243 @@ describe('column separators', () => { }); }); }); + +describe('grid-block-events cleanup', () => { + const resizableTheme = { ...defaultTheme, allowColumnResize: true }; + + function getColumnSeparatorX(columnIndex: VisibleIndex): number { + const { rowHeaderWidth, columnWidth } = resizableTheme; + return rowHeaderWidth + columnWidth * (columnIndex + 1) - 2; + } + + function getColumnHeaderY(): number { + return Math.floor(resizableTheme.columnHeaderHeight / 2); + } + + function hasBlockEventsClass(): boolean { + return document.documentElement.classList.contains('grid-block-events'); + } + + beforeEach(() => { + // Ensure clean state before each test + document.documentElement.classList.remove('grid-block-events'); + }); + + afterEach(() => { + // Clean up after each test + document.documentElement.classList.remove('grid-block-events'); + }); + + it('should remove grid-block-events after normal drag and mouse up', () => { + const component = makeGridComponent(new MockGridModel(), resizableTheme); + const separatorX = getColumnSeparatorX(0); + const headerY = getColumnHeaderY(); + + expect(hasBlockEventsClass()).toBe(false); + + // Start drag on column separator + mouseDown(0, 0, component, {}, separatorX, headerY); + + // Drag to trigger handleMouseDrag which adds the class + fireEvent.mouseMove(window, { + clientX: separatorX + 50, + clientY: headerY, + }); + + expect(hasBlockEventsClass()).toBe(true); + + // Normal mouse up should clean up + fireEvent.mouseUp(window, { + clientX: separatorX + 50, + clientY: headerY, + button: 0, + }); + + expect(hasBlockEventsClass()).toBe(false); + }); + + it('should remove grid-block-events when right-clicking during drag (case 1)', () => { + const component = makeGridComponent(new MockGridModel(), resizableTheme); + const separatorX = getColumnSeparatorX(0); + const headerY = getColumnHeaderY(); + + // Start drag + mouseDown(0, 0, component, {}, separatorX, headerY); + fireEvent.mouseMove(window, { + clientX: separatorX + 50, + clientY: headerY, + }); + + expect(hasBlockEventsClass()).toBe(true); + + // Right-click during drag (button=2) + fireEvent.mouseUp(window, { + clientX: separatorX + 50, + clientY: headerY, + button: 2, + }); + + // Right-click during drag returns early, class should still be present + // Then complete with left mouse up + fireEvent.mouseUp(window, { + clientX: separatorX + 50, + clientY: headerY, + button: 0, + }); + + expect(hasBlockEventsClass()).toBe(false); + }); + + it('should ignore middle-click during drag and continue dragging (case 2)', () => { + const component = makeGridComponent(new MockGridModel(), resizableTheme); + const separatorX = getColumnSeparatorX(0); + const headerY = getColumnHeaderY(); + + // Start drag + mouseDown(0, 0, component, {}, separatorX, headerY); + fireEvent.mouseMove(window, { + clientX: separatorX + 50, + clientY: headerY, + }); + + expect(hasBlockEventsClass()).toBe(true); + + // Middle mouse button up during drag (button=1) should be ignored + // Drag should continue + fireEvent.mouseUp(window, { + clientX: separatorX + 50, + clientY: headerY, + button: 1, + }); + + // Class should still be present since drag is ongoing + expect(hasBlockEventsClass()).toBe(true); + + // Left mouse up should properly end the drag and clean up + fireEvent.mouseUp(window, { + clientX: separatorX + 50, + clientY: headerY, + button: 0, + }); + + expect(hasBlockEventsClass()).toBe(false); + }); + + it('should handle addDocumentCursor with null cursor (case 3)', () => { + const component = makeGridComponent(new MockGridModel(), resizableTheme); + + expect(hasBlockEventsClass()).toBe(false); + + // First add a real cursor - this sets documentCursor and adds grid-block-events + component.addDocumentCursor('move'); + expect(hasBlockEventsClass()).toBe(true); + + // Now simulate cursor being cleared to null (e.g., mouse handler sets cursor = null) + // This sets documentCursor to null but keeps grid-block-events + component.addDocumentCursor(null); + + // grid-block-events should still be present since it was added + expect(hasBlockEventsClass()).toBe(true); + + // removeDocumentCursor should still clean up + // BUG: Currently it doesn't because documentCursor is null and + // the entire function body is guarded by `if (this.documentCursor != null)` + component.removeDocumentCursor(); + + // After fix, this should be false + expect(hasBlockEventsClass()).toBe(false); + }); + + it('should handle multiple Grid instances without interference (case 5)', () => { + // Create two grid instances + const component1 = makeGridComponent(new MockGridModel(), resizableTheme); + const component2 = makeGridComponent(new MockGridModel(), resizableTheme); + + const separatorX = getColumnSeparatorX(0); + const headerY = getColumnHeaderY(); + + expect(hasBlockEventsClass()).toBe(false); + + // Start drag on Grid 1 + fireEvent.mouseDown(component1.canvas!, { + clientX: separatorX, + clientY: headerY, + }); + fireEvent.mouseMove(window, { + clientX: separatorX + 50, + clientY: headerY, + }); + + expect(hasBlockEventsClass()).toBe(true); + + // Grid 2 unmounts while Grid 1 is dragging + // This simulates closing a panel or tab + component2.componentWillUnmount(); + + // Grid 2's unmount should NOT affect Grid 1's drag state + // The class should still be present for Grid 1 + expect(hasBlockEventsClass()).toBe(true); + + // Complete Grid 1's drag + fireEvent.mouseUp(window, { + clientX: separatorX + 50, + clientY: headerY, + button: 0, + }); + + expect(hasBlockEventsClass()).toBe(false); + }); + + it('should clean up grid-block-events when component unmounts during drag (case 6)', () => { + const component = makeGridComponent(new MockGridModel(), resizableTheme); + const separatorX = getColumnSeparatorX(0); + const headerY = getColumnHeaderY(); + + // Start drag + mouseDown(0, 0, component, {}, separatorX, headerY); + fireEvent.mouseMove(window, { + clientX: separatorX + 50, + clientY: headerY, + }); + + expect(hasBlockEventsClass()).toBe(true); + + // Component unmounts (simulates user navigating away or closing panel) + // This could happen if user drags outside window and releases there + component.componentWillUnmount(); + + // componentWillUnmount should clean up the class + expect(hasBlockEventsClass()).toBe(false); + }); + + it('should not leave grid-block-events when drag ends outside grid boundaries', () => { + const component = makeGridComponent(new MockGridModel(), resizableTheme); + const separatorX = getColumnSeparatorX(0); + const headerY = getColumnHeaderY(); + + // Start drag + mouseDown(0, 0, component, {}, separatorX, headerY); + fireEvent.mouseMove(window, { + clientX: separatorX + 50, + clientY: headerY, + }); + + expect(hasBlockEventsClass()).toBe(true); + + // Move mouse far outside grid (simulating dragging outside window) + fireEvent.mouseMove(window, { + clientX: -1000, + clientY: -1000, + }); + + // Mouse up outside grid boundaries + fireEvent.mouseUp(window, { + clientX: -1000, + clientY: -1000, + button: 0, + }); + + // Should still clean up properly + expect(hasBlockEventsClass()).toBe(false); + }); +}); diff --git a/packages/grid/src/Grid.tsx b/packages/grid/src/Grid.tsx index 92e741b9aa..1ef28b8d04 100644 --- a/packages/grid/src/Grid.tsx +++ b/packages/grid/src/Grid.tsx @@ -353,6 +353,10 @@ class Grid extends PureComponent { // blocked pointer events that would otherwise prevent cursor styling from showing documentCursor: string | null; + // Track if this Grid instance added the grid-block-events class + // Used to ensure only the Grid that added the class removes it + hasAddedBlockEvents: boolean; + dragTimer: ReturnType | null; keyHandlers: readonly KeyHandler[]; @@ -410,6 +414,7 @@ class Grid extends PureComponent { // Note: on document, not body so that cursor styling can be combined with // blocked pointer events that would otherwise prevent cursor styling from showing this.documentCursor = null; + this.hasAddedBlockEvents = false; this.dragTimer = null; @@ -1703,14 +1708,20 @@ class Grid extends PureComponent { document.documentElement.classList.add(this.documentCursor); } document.documentElement.classList.add('grid-block-events'); + this.hasAddedBlockEvents = true; } removeDocumentCursor(): void { if (this.documentCursor != null) { document.documentElement.classList.remove(this.documentCursor); - document.documentElement.classList.remove('grid-block-events'); this.documentCursor = null; } + // Only remove grid-block-events if this Grid instance added it + // This prevents one Grid from removing the class on unmount while another Grid is still dragging + if (this.hasAddedBlockEvents) { + document.documentElement.classList.remove('grid-block-events'); + this.hasAddedBlockEvents = false; + } } startDragTimer(event: React.MouseEvent): void { @@ -1918,24 +1929,23 @@ class Grid extends PureComponent { } handleMouseUp(event: MouseEvent): void { - // Ignore right click while dragging + // Ignore non-left click while dragging to allow drag to continue const { isDragging } = this.state; - if (isDragging && event.button === 2) { + if (isDragging && event.button !== 0) { return; } window.removeEventListener('mousemove', this.handleMouseDrag, true); window.removeEventListener('mouseup', this.handleMouseUp, true); + this.stopDragTimer(); + this.removeDocumentCursor(); + if (event.button != null && event.button !== 0) { return; } this.notifyMouseHandlers('onUp', event, false); - - this.stopDragTimer(); - - this.removeDocumentCursor(); } handleResize(): void {