Dashboards: Fix drag-to-tabs edge cases and improve drop behavior
- Use capture phase for pointerup listeners to handle tab header stopPropagation - Drop items into current tab when released on tab header after cross-tab drag - Handle RowsLayoutManager in TabItem.draggedGridItemInside (add to first row) - Clear tab activation timer on row drag pointerup to prevent late tab switches - Add tests for cross-tab drag scenarios
This commit is contained in:
oscarkilhed
@@ -10,6 +10,130 @@ import { TabItem } from './layout-tabs/TabItem';
|
||||
import { TabsLayoutManager } from './layout-tabs/TabsLayoutManager';
|
||||
|
||||
describe('DashboardLayoutOrchestrator', () => {
|
||||
describe('cross-tab drag cancel', () => {
|
||||
it('should drop item into current tab when dropped on tab header after detach', () => {
|
||||
const { orchestrator, tab1Manager, tab2Manager, gridItem, tabsManager, tab1 } = setupWithTwoTabs();
|
||||
|
||||
// Simulate state after cross-tab drag started:
|
||||
// - Item was detached from source
|
||||
// - We're on Tab 2 now
|
||||
// - User releases mouse over tab header (no valid drop target under mouse)
|
||||
// Expected: Item drops into Tab 2's layout
|
||||
|
||||
orchestrator.setState({
|
||||
draggingGridItem: gridItem.getRef(),
|
||||
sourceTabKey: tab1.state.key,
|
||||
});
|
||||
|
||||
const tab2 = tabsManager.state.tabs[1];
|
||||
|
||||
// @ts-expect-error - accessing private property for testing
|
||||
orchestrator._sourceDropTarget = tab1Manager;
|
||||
// @ts-expect-error - accessing private property for testing
|
||||
// lastDropTarget is the TabItem (set when tab switches)
|
||||
orchestrator._lastDropTarget = tab2;
|
||||
// @ts-expect-error - accessing private property for testing
|
||||
orchestrator._itemDetachedFromSource = true;
|
||||
|
||||
// Simulate the item being removed from source (as happens during tab switch)
|
||||
tab1Manager.draggedGridItemOutside(gridItem);
|
||||
|
||||
// Switch to tab 2 (simulating what happens after 600ms hover)
|
||||
tabsManager.switchToTab(tab2);
|
||||
|
||||
// Verify item was removed from tab1
|
||||
expect(tab1Manager.state.layout.state.children).toHaveLength(0);
|
||||
// Verify tab2 is empty before drop
|
||||
expect(tab2Manager.state.layout.state.children).toHaveLength(0);
|
||||
|
||||
// Mock _getDropTargetUnderMouse to return null (simulating cursor over tab header)
|
||||
// @ts-expect-error - accessing private method for testing
|
||||
const originalGetDropTargetUnderMouse = orchestrator._getDropTargetUnderMouse;
|
||||
// @ts-expect-error - accessing private method for testing
|
||||
orchestrator._getDropTargetUnderMouse = jest.fn().mockReturnValue(null);
|
||||
|
||||
// Create a mock pointer event
|
||||
const mockEvent = {
|
||||
clientX: 100,
|
||||
clientY: 100,
|
||||
} as PointerEvent;
|
||||
|
||||
// Call _stopDraggingSync (this is what happens on mouse release)
|
||||
// @ts-expect-error - accessing private method for testing
|
||||
orchestrator._stopDraggingSync(mockEvent);
|
||||
|
||||
// Restore original methods
|
||||
// @ts-expect-error - accessing private method for testing
|
||||
orchestrator._getDropTargetUnderMouse = originalGetDropTargetUnderMouse;
|
||||
|
||||
// Wait for setTimeout to execute
|
||||
return new Promise<void>((resolve) => {
|
||||
setTimeout(() => {
|
||||
// Verify item was dropped into tab2
|
||||
expect(tab2Manager.state.layout.state.children).toHaveLength(1);
|
||||
expect(tab2Manager.state.layout.state.children[0]).toBe(gridItem);
|
||||
|
||||
// Tab1 should still be empty
|
||||
expect(tab1Manager.state.layout.state.children).toHaveLength(0);
|
||||
|
||||
// We should still be on tab2
|
||||
expect(tabsManager.getCurrentTab()).toBe(tab2);
|
||||
|
||||
resolve();
|
||||
}, 0);
|
||||
});
|
||||
});
|
||||
|
||||
it('should complete normal drop when valid drop target exists', () => {
|
||||
const { orchestrator, tab1Manager, tab2Manager, gridItem, tab1 } = setupWithTwoTabs();
|
||||
|
||||
// Simulate state after cross-tab drag started
|
||||
orchestrator.setState({
|
||||
draggingGridItem: gridItem.getRef(),
|
||||
sourceTabKey: tab1.state.key,
|
||||
});
|
||||
|
||||
// @ts-expect-error - accessing private property for testing
|
||||
orchestrator._sourceDropTarget = tab1Manager;
|
||||
// @ts-expect-error - accessing private property for testing
|
||||
orchestrator._lastDropTarget = tab2Manager;
|
||||
// @ts-expect-error - accessing private property for testing
|
||||
orchestrator._itemDetachedFromSource = true;
|
||||
|
||||
// Simulate the item being removed from source
|
||||
tab1Manager.draggedGridItemOutside(gridItem);
|
||||
expect(tab1Manager.state.layout.state.children).toHaveLength(0);
|
||||
|
||||
// Mock _getDropTargetUnderMouse to return the tab2Manager (valid drop target)
|
||||
// @ts-expect-error - accessing private method for testing
|
||||
const originalGetDropTargetUnderMouse = orchestrator._getDropTargetUnderMouse;
|
||||
// @ts-expect-error - accessing private method for testing
|
||||
orchestrator._getDropTargetUnderMouse = jest.fn().mockReturnValue(tab2Manager);
|
||||
|
||||
const mockEvent = {
|
||||
clientX: 100,
|
||||
clientY: 100,
|
||||
} as PointerEvent;
|
||||
|
||||
// @ts-expect-error - accessing private method for testing
|
||||
orchestrator._stopDraggingSync(mockEvent);
|
||||
|
||||
// @ts-expect-error - accessing private method for testing
|
||||
orchestrator._getDropTargetUnderMouse = originalGetDropTargetUnderMouse;
|
||||
|
||||
return new Promise<void>((resolve) => {
|
||||
setTimeout(() => {
|
||||
// Verify item was NOT returned to source (it should go to tab2)
|
||||
expect(tab1Manager.state.layout.state.children).toHaveLength(0);
|
||||
expect(tab2Manager.state.layout.state.children).toHaveLength(1);
|
||||
expect(tab2Manager.state.layout.state.children[0]).toBe(gridItem);
|
||||
|
||||
resolve();
|
||||
}, 0);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('isDragging', () => {
|
||||
it('should return false when nothing is being dragged', () => {
|
||||
const { orchestrator } = setup();
|
||||
@@ -319,3 +443,66 @@ function setupAutoGrid() {
|
||||
|
||||
return { manager, gridItem1, gridItem2, panel1, panel2 };
|
||||
}
|
||||
|
||||
function setupWithTwoTabs() {
|
||||
// Create panel for Tab 1
|
||||
const panel1 = new VizPanel({
|
||||
title: 'Panel in Tab 1',
|
||||
key: 'panel-tab1',
|
||||
pluginId: 'table',
|
||||
});
|
||||
|
||||
const gridItem = new AutoGridItem({
|
||||
key: 'grid-item-tab1',
|
||||
body: panel1,
|
||||
});
|
||||
|
||||
const tab1Manager = new AutoGridLayoutManager({
|
||||
key: 'tab1-manager',
|
||||
layout: new AutoGridLayout({ children: [gridItem] }),
|
||||
});
|
||||
|
||||
const tab1 = new TabItem({
|
||||
key: 'tab-1',
|
||||
title: 'Tab 1',
|
||||
layout: tab1Manager,
|
||||
});
|
||||
|
||||
// Create empty Tab 2
|
||||
const tab2Manager = new AutoGridLayoutManager({
|
||||
key: 'tab2-manager',
|
||||
layout: new AutoGridLayout({ children: [] }),
|
||||
});
|
||||
|
||||
const tab2 = new TabItem({
|
||||
key: 'tab-2',
|
||||
title: 'Tab 2',
|
||||
layout: tab2Manager,
|
||||
});
|
||||
|
||||
const tabsManager = new TabsLayoutManager({
|
||||
tabs: [tab1, tab2],
|
||||
});
|
||||
|
||||
const orchestrator = new DashboardLayoutOrchestrator();
|
||||
|
||||
const dashboard = new DashboardScene({
|
||||
body: tabsManager,
|
||||
layoutOrchestrator: orchestrator,
|
||||
});
|
||||
|
||||
// Activate the scene hierarchy to set up parent relationships
|
||||
dashboard.activate();
|
||||
|
||||
return {
|
||||
orchestrator,
|
||||
tabsManager,
|
||||
tab1,
|
||||
tab2,
|
||||
tab1Manager,
|
||||
tab2Manager,
|
||||
gridItem,
|
||||
panel1,
|
||||
dashboard,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -97,8 +97,8 @@ export class DashboardLayoutOrchestrator extends SceneObjectBase<DashboardLayout
|
||||
return () => {
|
||||
document.body.removeEventListener('pointermove', this._onPointerMove);
|
||||
document.body.removeEventListener('pointermove', this._onRowDragPointerMove);
|
||||
document.body.removeEventListener('pointerup', this._stopDraggingSync);
|
||||
document.body.removeEventListener('pointerup', this._onRowDragPointerUp);
|
||||
document.body.removeEventListener('pointerup', this._stopDraggingSync, true);
|
||||
document.body.removeEventListener('pointerup', this._onRowDragPointerUp, true);
|
||||
this._clearTabActivationTimer();
|
||||
this._clearDragPreview();
|
||||
};
|
||||
@@ -133,65 +133,83 @@ export class DashboardLayoutOrchestrator extends SceneObjectBase<DashboardLayout
|
||||
this._captureDragOffset(evt.clientX, evt.clientY, gridItem);
|
||||
|
||||
document.body.addEventListener('pointermove', this._onPointerMove);
|
||||
document.body.addEventListener('pointerup', this._stopDraggingSync);
|
||||
// Use capture phase to ensure we receive the event even if something calls stopPropagation
|
||||
// (e.g., tab headers call stopPropagation on pointerup)
|
||||
document.body.addEventListener('pointerup', this._stopDraggingSync, true);
|
||||
|
||||
const sourceTabKey = this._findParentTabKey(gridItem);
|
||||
this.setState({ draggingGridItem: gridItem.getRef(), sourceTabKey });
|
||||
}
|
||||
|
||||
private _stopDraggingSync(_evt: PointerEvent) {
|
||||
private _stopDraggingSync(evt: PointerEvent) {
|
||||
const gridItem = this.state.draggingGridItem?.resolve();
|
||||
const wasDetached = this._itemDetachedFromSource;
|
||||
// Capture these before cleanup since setTimeout runs after cleanup
|
||||
const sourceDropTarget = this._sourceDropTarget;
|
||||
const lastDropTarget = this._lastDropTarget;
|
||||
const dropPosition = this._currentDropPosition;
|
||||
const isCrossLayoutDrop = sourceDropTarget !== lastDropTarget || wasDetached;
|
||||
|
||||
// Handle cross-layout or cross-tab drop
|
||||
if (isCrossLayoutDrop) {
|
||||
// Wrapped in setTimeout to ensure that any event handlers are called
|
||||
// Useful for allowing react-grid-layout to remove placeholders, etc.
|
||||
// Check if there's a valid drop target under the mouse
|
||||
// (tab headers and other non-drop areas return null)
|
||||
const validDropTargetUnderMouse = this._getDropTargetUnderMouse(evt);
|
||||
|
||||
// If item was detached (cross-tab drag started) but there's no valid drop target under mouse,
|
||||
// drop into the current tab if lastDropTarget is a TabItem (e.g., dropped on tab header)
|
||||
const noTargetUnderMouse = wasDetached && !validDropTargetUnderMouse && gridItem;
|
||||
const canDropIntoCurrentTab = noTargetUnderMouse && lastDropTarget instanceof TabItem;
|
||||
|
||||
if (canDropIntoCurrentTab) {
|
||||
// Drop into the current tab's layout
|
||||
setTimeout(() => {
|
||||
if (gridItem) {
|
||||
// Only remove from source if not already detached during tab switch
|
||||
if (!wasDetached) {
|
||||
sourceDropTarget?.draggedGridItemOutside?.(gridItem);
|
||||
}
|
||||
// Pass drop position for precise placement (AutoGrid uses this)
|
||||
// Note: draggedGridItemInside also clears isDropTarget and dropPosition
|
||||
lastDropTarget?.draggedGridItemInside?.(gridItem, dropPosition ?? undefined);
|
||||
|
||||
// Clean up source grid's drag state (CSS variables and draggingKey) after item is moved.
|
||||
// This is done here (after movement) to prevent flickering where the item
|
||||
// would momentarily appear at wrong position (CSS vars cleared but draggingKey set
|
||||
// = absolute positioning with no valid position values).
|
||||
if (sourceDropTarget instanceof AutoGridLayoutManager) {
|
||||
sourceDropTarget.state.layout.endExternalDrag();
|
||||
}
|
||||
} else {
|
||||
const warningMessage = 'No grid item to drag';
|
||||
console.warn(warningMessage);
|
||||
logWarning(warningMessage);
|
||||
lastDropTarget.draggedGridItemInside?.(gridItem);
|
||||
// Clean up source grid state
|
||||
if (sourceDropTarget instanceof AutoGridLayoutManager) {
|
||||
sourceDropTarget.state.layout.endExternalDrag();
|
||||
}
|
||||
});
|
||||
} else {
|
||||
const isCrossLayoutDrop = sourceDropTarget !== lastDropTarget || wasDetached;
|
||||
|
||||
// Handle cross-layout or cross-tab drop
|
||||
if (isCrossLayoutDrop) {
|
||||
// Wrapped in setTimeout to ensure that any event handlers are called
|
||||
// Useful for allowing react-grid-layout to remove placeholders, etc.
|
||||
setTimeout(() => {
|
||||
if (gridItem) {
|
||||
// Only remove from source if not already detached during tab switch
|
||||
if (!wasDetached) {
|
||||
sourceDropTarget?.draggedGridItemOutside?.(gridItem);
|
||||
}
|
||||
// Pass drop position for precise placement (AutoGrid uses this)
|
||||
// Note: draggedGridItemInside also clears isDropTarget and dropPosition
|
||||
lastDropTarget?.draggedGridItemInside?.(gridItem, dropPosition ?? undefined);
|
||||
|
||||
// Clean up source grid's drag state (CSS variables and draggingKey) after item is moved.
|
||||
// This is done here (after movement) to prevent flickering where the item
|
||||
// would momentarily appear at wrong position (CSS vars cleared but draggingKey set
|
||||
// = absolute positioning with no valid position values).
|
||||
if (sourceDropTarget instanceof AutoGridLayoutManager) {
|
||||
sourceDropTarget.state.layout.endExternalDrag();
|
||||
}
|
||||
} else {
|
||||
const warningMessage = 'No grid item to drag';
|
||||
console.warn(warningMessage);
|
||||
logWarning(warningMessage);
|
||||
}
|
||||
});
|
||||
} else {
|
||||
// For same-layout drops, clear drop position state synchronously
|
||||
this._clearDropPosition();
|
||||
this._lastDropTarget?.setIsDropTarget?.(false);
|
||||
}
|
||||
}
|
||||
|
||||
document.body.removeEventListener('pointermove', this._onPointerMove);
|
||||
document.body.removeEventListener('pointerup', this._stopDraggingSync);
|
||||
document.body.removeEventListener('pointerup', this._stopDraggingSync, true);
|
||||
|
||||
this._clearTabActivationTimer();
|
||||
this._clearDragPreview();
|
||||
|
||||
// For cross-layout drops, don't clear drop position/target state synchronously.
|
||||
// The placeholder should remain visible until the item is added by draggedGridItemInside,
|
||||
// which also clears isDropTarget and dropPosition. This prevents flickering where the
|
||||
// grid would momentarily shrink (placeholder removed) before expanding again (item added).
|
||||
if (!isCrossLayoutDrop) {
|
||||
this._clearDropPosition();
|
||||
this._lastDropTarget?.setIsDropTarget?.(false);
|
||||
}
|
||||
|
||||
// Clear internal tracking state (but not the visual state on the target for cross-layout drops)
|
||||
this._currentDropPosition = null;
|
||||
this._lastHoveredAutoGridItemKey = null;
|
||||
@@ -227,7 +245,8 @@ export class DashboardLayoutOrchestrator extends SceneObjectBase<DashboardLayout
|
||||
// Add pointer move listener for tab hover detection during row drag
|
||||
document.body.addEventListener('pointermove', this._onRowDragPointerMove);
|
||||
// Add pointerup listener to handle drop after cross-tab switch
|
||||
document.body.addEventListener('pointerup', this._onRowDragPointerUp);
|
||||
// Use capture phase to ensure we receive the event even if something calls stopPropagation
|
||||
document.body.addEventListener('pointerup', this._onRowDragPointerUp, true);
|
||||
}
|
||||
|
||||
private _onRowDragPointerMove = (evt: PointerEvent): void => {
|
||||
@@ -249,6 +268,10 @@ export class DashboardLayoutOrchestrator extends SceneObjectBase<DashboardLayout
|
||||
};
|
||||
|
||||
private _onRowDragPointerUp = (_evt: PointerEvent): void => {
|
||||
// Always clear the tab activation timer on pointerup to prevent
|
||||
// the tab from switching after the user has released the mouse
|
||||
this._clearTabActivationTimer();
|
||||
|
||||
// Handle drop after cross-tab row drag
|
||||
if (this._itemDetachedFromSource) {
|
||||
const row = this.state.draggingRow?.resolve();
|
||||
@@ -259,13 +282,9 @@ export class DashboardLayoutOrchestrator extends SceneObjectBase<DashboardLayout
|
||||
dropTarget.acceptDroppedRow?.(row);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Clean up - this will be called in addition to stopRowDrag from hello-pangea/dnd
|
||||
// but only if the drag was detached
|
||||
if (this._itemDetachedFromSource) {
|
||||
this._finalizeRowDrag();
|
||||
}
|
||||
// If not detached, stopRowDrag from hello-pangea/dnd will handle cleanup
|
||||
};
|
||||
|
||||
/**
|
||||
@@ -285,7 +304,7 @@ export class DashboardLayoutOrchestrator extends SceneObjectBase<DashboardLayout
|
||||
|
||||
private _finalizeRowDrag(): void {
|
||||
document.body.removeEventListener('pointermove', this._onRowDragPointerMove);
|
||||
document.body.removeEventListener('pointerup', this._onRowDragPointerUp);
|
||||
document.body.removeEventListener('pointerup', this._onRowDragPointerUp, true);
|
||||
this._clearTabActivationTimer();
|
||||
this._clearDragPreview();
|
||||
this._lastDropTarget?.setIsDropTarget?.(false);
|
||||
|
||||
@@ -232,6 +232,19 @@ export class TabItem
|
||||
|
||||
if (isDashboardLayoutGrid(layout)) {
|
||||
layout.addGridItem(gridItem);
|
||||
} else if (layout instanceof RowsLayoutManager) {
|
||||
// For RowsLayoutManager, add to the first row's layout
|
||||
const firstRow = layout.state.rows[0];
|
||||
if (firstRow) {
|
||||
const rowLayout = firstRow.getLayout();
|
||||
if (isDashboardLayoutGrid(rowLayout)) {
|
||||
rowLayout.addGridItem(gridItem);
|
||||
} else {
|
||||
const warningMessage = 'First row layout does not support addGridItem';
|
||||
console.warn(warningMessage);
|
||||
logWarning(warningMessage);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
const warningMessage = 'Layout manager does not support addGridItem';
|
||||
console.warn(warningMessage);
|
||||
|
||||
Reference in New Issue
Block a user