From bdd50c7553c5d7c701491f22f6f27e32672e45d6 Mon Sep 17 00:00:00 2001
From: Ross Mabbett <92495987+rtexelm@users.noreply.github.com>
Date: Mon, 30 Sep 2024 05:15:39 -0400
Subject: [PATCH] feat(dashboard): update tab drag and drop reordering with
positional placement and indicators for UI (#29395)
---
.../components/dnd/DragDroppable.jsx | 45 +++++++
.../components/dnd/DragDroppable.test.jsx | 16 ++-
.../components/dnd/dragDroppableConfig.js | 2 +
.../dashboard/components/dnd/handleDrop.js | 1 +
.../components/gridComponents/Tab.jsx | 26 +++-
.../components/gridComponents/Tab.test.tsx | 4 +
.../components/gridComponents/Tabs.jsx | 119 +++++++++++++++---
.../components/gridComponents/Tabs.test.tsx | 56 +++++----
.../src/dashboard/reducers/dashboardLayout.js | 3 +-
.../src/dashboard/util/dnd-reorder.js | 56 ++++++++-
.../src/dashboard/util/dnd-reorder.test.js | 119 +++++++++++++++++-
11 files changed, 396 insertions(+), 51 deletions(-)
diff --git a/superset-frontend/src/dashboard/components/dnd/DragDroppable.jsx b/superset-frontend/src/dashboard/components/dnd/DragDroppable.jsx
index a68d42c63..ce47656d1 100644
--- a/superset-frontend/src/dashboard/components/dnd/DragDroppable.jsx
+++ b/superset-frontend/src/dashboard/components/dnd/DragDroppable.jsx
@@ -19,6 +19,7 @@
import { getEmptyImage } from 'react-dnd-html5-backend';
import { PureComponent } from 'react';
import PropTypes from 'prop-types';
+import { TAB_TYPE } from 'src/dashboard/util/componentTypes';
import { DragSource, DropTarget } from 'react-dnd';
import cx from 'classnames';
import { css, styled } from '@superset-ui/core';
@@ -40,6 +41,8 @@ const propTypes = {
style: PropTypes.object,
onDrop: PropTypes.func,
onHover: PropTypes.func,
+ onDropIndicatorChange: PropTypes.func,
+ onDragTab: PropTypes.func,
editMode: PropTypes.bool,
useEmptyDragPreview: PropTypes.bool,
@@ -47,6 +50,8 @@ const propTypes = {
isDragging: PropTypes.bool,
isDraggingOver: PropTypes.bool,
isDraggingOverShallow: PropTypes.bool,
+ dragComponentType: PropTypes.string,
+ dragComponentId: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
droppableRef: PropTypes.func,
dragSourceRef: PropTypes.func,
dragPreviewRef: PropTypes.func,
@@ -61,6 +66,8 @@ const defaultProps = {
children() {},
onDrop() {},
onHover() {},
+ onDropIndicatorChange() {},
+ onDragTab() {},
orientation: 'row',
useEmptyDragPreview: false,
isDragging: false,
@@ -129,6 +136,38 @@ export class UnwrappedDragDroppable extends PureComponent {
this.mounted = false;
}
+ componentDidUpdate(prevProps, prevState) {
+ const {
+ onDropIndicatorChange,
+ isDraggingOver,
+ component,
+ index,
+ dragComponentId,
+ onDragTab,
+ } = this.props;
+ const { dropIndicator } = this.state;
+ const isTabsType = component.type === TAB_TYPE;
+ const validStateChange =
+ dropIndicator !== prevState.dropIndicator ||
+ isDraggingOver !== prevProps.isDraggingOver ||
+ index !== prevProps.index;
+
+ if (onDropIndicatorChange && isTabsType && validStateChange) {
+ onDropIndicatorChange({ dropIndicator, isDraggingOver, index });
+ }
+
+ if (dragComponentId !== prevProps.dragComponentId) {
+ setTimeout(() => {
+ /**
+ * This timeout ensures the dargSourceRef and dragPreviewRef are set
+ * before the component is removed in Tabs.jsx. Otherwise react-dnd
+ * will not render the drag preview.
+ */
+ onDragTab(dragComponentId);
+ });
+ }
+ }
+
setRef(ref) {
this.ref = ref;
// this is needed for a custom drag preview
@@ -155,6 +194,8 @@ export class UnwrappedDragDroppable extends PureComponent {
isDraggingOver,
style,
editMode,
+ component,
+ dragComponentType,
} = this.props;
const { dropIndicator } = this.state;
@@ -168,10 +209,14 @@ export class UnwrappedDragDroppable extends PureComponent {
}
: null;
+ const draggingTabOnTab =
+ component.type === TAB_TYPE && dragComponentType === TAB_TYPE;
+
const childProps = editMode
? {
dragSourceRef,
dropIndicatorProps,
+ draggingTabOnTab,
}
: {};
diff --git a/superset-frontend/src/dashboard/components/dnd/DragDroppable.test.jsx b/superset-frontend/src/dashboard/components/dnd/DragDroppable.test.jsx
index 095af6407..09cf62d3f 100644
--- a/superset-frontend/src/dashboard/components/dnd/DragDroppable.test.jsx
+++ b/superset-frontend/src/dashboard/components/dnd/DragDroppable.test.jsx
@@ -23,7 +23,11 @@ import {
import sinon from 'sinon';
import newComponentFactory from 'src/dashboard/util/newComponentFactory';
-import { CHART_TYPE, ROW_TYPE } from 'src/dashboard/util/componentTypes';
+import {
+ CHART_TYPE,
+ ROW_TYPE,
+ TAB_TYPE,
+} from 'src/dashboard/util/componentTypes';
import { UnwrappedDragDroppable as DragDroppable } from 'src/dashboard/components/dnd/DragDroppable';
describe('DragDroppable', () => {
@@ -63,6 +67,16 @@ describe('DragDroppable', () => {
expect(childrenSpy.callCount).toBe(1);
});
+ it('should call onDropIndicatorChange when isDraggingOver changes', () => {
+ const onDropIndicatorChange = sinon.spy();
+ const wrapper = setup({
+ onDropIndicatorChange,
+ component: newComponentFactory(TAB_TYPE),
+ });
+ wrapper.setProps({ isDraggingOver: true });
+ expect(onDropIndicatorChange.callCount).toBe(1);
+ });
+
it('should call its child function with "dragSourceRef" if editMode=true', () => {
const children = sinon.spy();
const dragSourceRef = () => {};
diff --git a/superset-frontend/src/dashboard/components/dnd/dragDroppableConfig.js b/superset-frontend/src/dashboard/components/dnd/dragDroppableConfig.js
index 4d7185148..bd8ab744b 100644
--- a/superset-frontend/src/dashboard/components/dnd/dragDroppableConfig.js
+++ b/superset-frontend/src/dashboard/components/dnd/dragDroppableConfig.js
@@ -47,6 +47,8 @@ export const dragConfig = [
dragSourceRef: connect.dragSource(),
dragPreviewRef: connect.dragPreview(),
isDragging: monitor.isDragging(),
+ dragComponentType: monitor.getItem()?.type,
+ dragComponentId: monitor.getItem()?.id,
};
},
];
diff --git a/superset-frontend/src/dashboard/components/dnd/handleDrop.js b/superset-frontend/src/dashboard/components/dnd/handleDrop.js
index f4b847cd6..89bda18c4 100644
--- a/superset-frontend/src/dashboard/components/dnd/handleDrop.js
+++ b/superset-frontend/src/dashboard/components/dnd/handleDrop.js
@@ -53,6 +53,7 @@ export default function handleDrop(props, monitor, Component) {
type: draggingItem.type,
meta: draggingItem.meta,
},
+ position: dropPosition,
};
const shouldAppendToChildren =
diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx b/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx
index eaa8ca00a..7b9bc8d48 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx
@@ -47,6 +47,8 @@ const propTypes = {
depth: PropTypes.number.isRequired,
renderType: PropTypes.oneOf([RENDER_TAB, RENDER_TAB_CONTENT]).isRequired,
onDropOnTab: PropTypes.func,
+ onDropPositionChange: PropTypes.func,
+ onDragTab: PropTypes.func,
onHoverTab: PropTypes.func,
editMode: PropTypes.bool.isRequired,
canEdit: PropTypes.bool.isRequired,
@@ -69,6 +71,8 @@ const defaultProps = {
availableColumnCount: 0,
columnWidth: 0,
onDropOnTab() {},
+ onDropPositionChange() {},
+ onDragTab() {},
onHoverTab() {},
onResizeStart() {},
onResize() {},
@@ -86,6 +90,14 @@ const TabTitleContainer = styled.div`
`}
`;
+const TitleDropIndicator = styled.div`
+ &.drop-indicator {
+ position: absolute;
+ top: 0;
+ border-radius: 4px;
+ }
+`;
+
const renderDraggableContent = dropProps =>
dropProps.dropIndicatorProps &&
;
@@ -268,6 +280,8 @@ class Tab extends PureComponent {
editMode,
isFocused,
isHighlighted,
+ onDropPositionChange,
+ onDragTab,
} = this.props;
return (
@@ -279,10 +293,12 @@ class Tab extends PureComponent {
depth={depth}
onDrop={this.handleDrop}
onHover={this.handleOnHover}
+ onDropIndicatorChange={onDropPositionChange}
+ onDragTab={onDragTab}
editMode={editMode}
dropToChild={this.shouldDropToChild}
>
- {({ dropIndicatorProps, dragSourceRef }) => (
+ {({ dropIndicatorProps, dragSourceRef, draggingTabOnTab }) => (
= 5 ? 'left' : 'right'}
/>
)}
-
- {dropIndicatorProps && }
+ {dropIndicatorProps && !draggingTabOnTab && (
+
+ )}
)}
diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tab.test.tsx b/superset-frontend/src/dashboard/components/gridComponents/Tab.test.tsx
index 35f47de04..28802cc60 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/Tab.test.tsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/Tab.test.tsx
@@ -196,6 +196,10 @@ test('Drop on a tab', async () => {
);
fireEvent.dragStart(screen.getByText('Dashboard Component'));
+ fireEvent.dragOver(screen.getByText('Next Tab'));
+ await waitFor(() =>
+ expect(screen.getByTestId('title-drop-indicator')).toBeVisible(),
+ );
fireEvent.drop(screen.getByText('Next Tab'));
await waitFor(() => expect(mockOnDropOnTab).toHaveBeenCalledTimes(2));
expect(mockOnDropOnTab).toHaveBeenLastCalledWith(
diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx b/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx
index 929d25915..901d8729b 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx
@@ -21,8 +21,10 @@ import PropTypes from 'prop-types';
import { styled, t } from '@superset-ui/core';
import { connect } from 'react-redux';
import { LineEditableTabs } from 'src/components/Tabs';
+import Icons from 'src/components/Icons';
import { LOG_ACTIONS_SELECT_DASHBOARD_TAB } from 'src/logger/LogUtils';
import { AntdModal } from 'src/components';
+import { DROP_LEFT, DROP_RIGHT } from 'src/dashboard/util/getDropPosition';
import { Draggable } from '../dnd/DragDroppable';
import DragHandle from '../dnd/DragHandle';
import DashboardComponent from '../../containers/DashboardComponent';
@@ -108,6 +110,29 @@ const StyledTabsContainer = styled.div`
}
`;
+const StyledCancelXIcon = styled(Icons.CancelX)`
+ color: ${({ theme }) => theme.colors.grayscale.base};
+`;
+
+const DropIndicator = styled.div`
+ border: 2px solid ${({ theme }) => theme.colors.primary.base};
+ width: 5px;
+ height: 100%;
+ position: absolute;
+ top: 0;
+ ${({ pos }) => (pos === 'left' ? 'left: -4px' : 'right: -4px')};
+ border-radius: 2px;
+`;
+
+const CloseIconWithDropIndicator = props => (
+ <>
+
+ {props.showDropIndicators.right && (
+
+ )}
+ >
+);
+
export class Tabs extends PureComponent {
constructor(props) {
super(props);
@@ -122,6 +147,8 @@ export class Tabs extends PureComponent {
this.handleDeleteTab = this.handleDeleteTab.bind(this);
this.handleDropOnTab = this.handleDropOnTab.bind(this);
this.handleDrop = this.handleDrop.bind(this);
+ this.handleGetDropPosition = this.handleGetDropPosition.bind(this);
+ this.handleDragggingTab = this.handleDragggingTab.bind(this);
}
componentDidMount() {
@@ -286,6 +313,19 @@ export class Tabs extends PureComponent {
}
}
+ handleGetDropPosition(dragObject) {
+ const { dropIndicator, isDraggingOver, index } = dragObject;
+
+ if (isDraggingOver) {
+ this.setState(() => ({
+ dropPosition: dropIndicator,
+ dragOverTabIndex: index,
+ }));
+ } else {
+ this.setState(() => ({ dropPosition: null }));
+ }
+ }
+
handleDropOnTab(dropResult) {
const { component } = this.props;
@@ -311,6 +351,14 @@ export class Tabs extends PureComponent {
}
}
+ handleDragggingTab(tabId) {
+ if (tabId) {
+ this.setState(() => ({ draggingTabId: tabId }));
+ } else {
+ this.setState(() => ({ draggingTabId: null }));
+ }
+ }
+
render() {
const {
depth,
@@ -330,7 +378,20 @@ export class Tabs extends PureComponent {
} = this.props;
const { children: tabIds } = tabsComponent;
- const { tabIndex: selectedTabIndex, activeKey } = this.state;
+ const {
+ tabIndex: selectedTabIndex,
+ activeKey,
+ dropPosition,
+ dragOverTabIndex,
+ } = this.state;
+
+ const showDropIndicators = currentDropTabIndex =>
+ currentDropTabIndex === dragOverTabIndex && {
+ left: editMode && dropPosition === DROP_LEFT,
+ right: editMode && dropPosition === DROP_RIGHT,
+ };
+
+ const removeDraggedTab = tabID => this.state.draggingTabId === tabID;
let tabsToHighlight;
const highlightedFilterId =
@@ -374,21 +435,47 @@ export class Tabs extends PureComponent {
this.handleClickTab(tabIndex)}
- isFocused={activeKey === tabId}
- isHighlighted={
- activeKey !== tabId && tabsToHighlight?.includes(tabId)
- }
- />
+ removeDraggedTab(tabId) ? (
+ <>>
+ ) : (
+ <>
+ {showDropIndicators(tabIndex).left && (
+
+ )}
+ this.handleClickTab(tabIndex)}
+ isFocused={activeKey === tabId}
+ isHighlighted={
+ activeKey !== tabId &&
+ tabsToHighlight?.includes(tabId)
+ }
+ />
+ >
+ )
+ }
+ closeIcon={
+ removeDraggedTab(tabId) ? (
+ <>>
+ ) : (
+
+ )
}
>
{renderTabContent && (
diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tabs.test.tsx b/superset-frontend/src/dashboard/components/gridComponents/Tabs.test.tsx
index a5c588db9..98ae968fd 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/Tabs.test.tsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/Tabs.test.tsx
@@ -122,21 +122,24 @@ test('Should render editMode:true', () => {
const props = createProps();
render(, { useRedux: true, useDnd: true });
expect(screen.getAllByRole('tab')).toHaveLength(3);
- expect(Draggable).toBeCalledTimes(1);
- expect(DashboardComponent).toBeCalledTimes(4);
- expect(DeleteComponentButton).toBeCalledTimes(1);
expect(screen.getAllByRole('button', { name: 'remove' })).toHaveLength(3);
expect(screen.getAllByRole('button', { name: 'Add tab' })).toHaveLength(2);
+ expect(Draggable).toHaveBeenCalledTimes(1);
+ expect(DashboardComponent).toHaveBeenCalledTimes(4);
+ expect(DeleteComponentButton).toHaveBeenCalledTimes(1);
});
test('Should render editMode:false', () => {
const props = createProps();
props.editMode = false;
- render(, { useRedux: true, useDnd: true });
+ render(, {
+ useRedux: true,
+ useDnd: true,
+ });
expect(screen.getAllByRole('tab')).toHaveLength(3);
- expect(Draggable).toBeCalledTimes(1);
- expect(DashboardComponent).toBeCalledTimes(4);
- expect(DeleteComponentButton).not.toBeCalled();
+ expect(Draggable).toHaveBeenCalledTimes(1);
+ expect(DashboardComponent).toHaveBeenCalledTimes(4);
+ expect(DeleteComponentButton).not.toHaveBeenCalled();
expect(
screen.queryByRole('button', { name: 'remove' }),
).not.toBeInTheDocument();
@@ -153,11 +156,11 @@ test('Update component props', () => {
useRedux: true,
useDnd: true,
});
- expect(DeleteComponentButton).not.toBeCalled();
+ expect(DeleteComponentButton).not.toHaveBeenCalled();
props.editMode = true;
rerender();
- expect(DeleteComponentButton).toBeCalledTimes(1);
+ expect(DeleteComponentButton).toHaveBeenCalledTimes(1);
});
test('Clicking on "DeleteComponentButton"', () => {
@@ -167,9 +170,12 @@ test('Clicking on "DeleteComponentButton"', () => {
useDnd: true,
});
- expect(props.deleteComponent).not.toBeCalled();
+ expect(props.deleteComponent).not.toHaveBeenCalled();
userEvent.click(screen.getByTestId('DeleteComponentButton'));
- expect(props.deleteComponent).toBeCalledWith('TABS-L-d9eyOE-b', 'GRID_ID');
+ expect(props.deleteComponent).toHaveBeenCalledWith(
+ 'TABS-L-d9eyOE-b',
+ 'GRID_ID',
+ );
});
test('Add new tab', () => {
@@ -179,9 +185,9 @@ test('Add new tab', () => {
useDnd: true,
});
- expect(props.createComponent).not.toBeCalled();
+ expect(props.createComponent).not.toHaveBeenCalled();
userEvent.click(screen.getAllByRole('button', { name: 'Add tab' })[0]);
- expect(props.createComponent).toBeCalled();
+ expect(props.createComponent).toHaveBeenCalled();
});
test('Removing a tab', async () => {
@@ -191,16 +197,16 @@ test('Removing a tab', async () => {
useDnd: true,
});
- expect(props.deleteComponent).not.toBeCalled();
+ expect(props.deleteComponent).not.toHaveBeenCalled();
expect(screen.queryByText('Delete dashboard tab?')).not.toBeInTheDocument();
userEvent.click(screen.getAllByRole('button', { name: 'remove' })[0]);
- expect(props.deleteComponent).not.toBeCalled();
+ expect(props.deleteComponent).not.toHaveBeenCalled();
expect(await screen.findByText('Delete dashboard tab?')).toBeInTheDocument();
- expect(props.deleteComponent).not.toBeCalled();
+ expect(props.deleteComponent).not.toHaveBeenCalled();
userEvent.click(screen.getByRole('button', { name: 'DELETE' }));
- expect(props.deleteComponent).toBeCalled();
+ expect(props.deleteComponent).toHaveBeenCalled();
});
test('Switching tabs', () => {
@@ -210,11 +216,11 @@ test('Switching tabs', () => {
useDnd: true,
});
- expect(props.logEvent).not.toBeCalled();
- expect(props.onChangeTab).not.toBeCalled();
+ expect(props.logEvent).not.toHaveBeenCalled();
+ expect(props.onChangeTab).not.toHaveBeenCalled();
userEvent.click(screen.getAllByRole('tab')[2]);
- expect(props.logEvent).toBeCalled();
- expect(props.onChangeTab).toBeCalled();
+ expect(props.logEvent).toHaveBeenCalled();
+ expect(props.onChangeTab).toHaveBeenCalled();
});
test('Call "DashboardComponent.onDropOnTab"', async () => {
@@ -224,12 +230,12 @@ test('Call "DashboardComponent.onDropOnTab"', async () => {
useDnd: true,
});
- expect(props.logEvent).not.toBeCalled();
- expect(props.onChangeTab).not.toBeCalled();
+ expect(props.logEvent).not.toHaveBeenCalled();
+ expect(props.onChangeTab).not.toHaveBeenCalled();
userEvent.click(screen.getAllByText('DashboardComponent')[0]);
await waitFor(() => {
- expect(props.logEvent).toBeCalled();
- expect(props.onChangeTab).toBeCalled();
+ expect(props.logEvent).toHaveBeenCalled();
+ expect(props.onChangeTab).toHaveBeenCalled();
});
});
diff --git a/superset-frontend/src/dashboard/reducers/dashboardLayout.js b/superset-frontend/src/dashboard/reducers/dashboardLayout.js
index 6ba297be9..34edf0cee 100644
--- a/superset-frontend/src/dashboard/reducers/dashboardLayout.js
+++ b/superset-frontend/src/dashboard/reducers/dashboardLayout.js
@@ -134,7 +134,7 @@ const actionHandlers = {
const {
payload: { dropResult },
} = action;
- const { source, destination, dragging } = dropResult;
+ const { source, destination, dragging, position } = dropResult;
if (!source || !destination || !dragging) return state;
@@ -142,6 +142,7 @@ const actionHandlers = {
entitiesMap: state,
source,
destination,
+ position,
});
if (componentIsResizable(nextEntities[dragging.id])) {
diff --git a/superset-frontend/src/dashboard/util/dnd-reorder.js b/superset-frontend/src/dashboard/util/dnd-reorder.js
index f92a247ef..94d1a74c5 100644
--- a/superset-frontend/src/dashboard/util/dnd-reorder.js
+++ b/superset-frontend/src/dashboard/util/dnd-reorder.js
@@ -16,6 +16,10 @@
* specific language governing permissions and limitations
* under the License.
*/
+
+import { TABS_TYPE } from './componentTypes';
+import { DROP_LEFT, DROP_RIGHT } from './getDropPosition';
+
export function reorder(list, startIndex, endIndex) {
const result = [...list];
const [removed] = result.splice(startIndex, 1);
@@ -24,14 +28,49 @@ export function reorder(list, startIndex, endIndex) {
return result;
}
-export default function reorderItem({ entitiesMap, source, destination }) {
+export default function reorderItem({
+ entitiesMap,
+ source,
+ destination,
+ position,
+}) {
const current = [...entitiesMap[source.id].children];
const next = [...entitiesMap[destination.id].children];
const target = current[source.index];
+ const isSameSource = source.id === destination.id;
+ const isTabsType = source.type && destination.type === TABS_TYPE;
+
// moving to same list
- if (source.id === destination.id) {
- const reordered = reorder(current, source.index, destination.index);
+ let dropIndex = destination.index;
+
+ if (isSameSource) {
+ if (isTabsType) {
+ if (position === DROP_LEFT) {
+ dropIndex = Math.max(dropIndex, 0);
+ } else if (position === DROP_RIGHT) {
+ dropIndex += 1;
+ }
+
+ const isRightPosition =
+ position === DROP_RIGHT && source.index === destination.index + 1;
+ const isLeftPosition =
+ position === DROP_LEFT && source.index === destination.index - 1;
+
+ const sameTabSourceIndex = isRightPosition || isLeftPosition;
+
+ if (sameTabSourceIndex) {
+ // If the source tab is dropped to be the same index as the source
+ // tab, no change is needed in entitiesMap
+ return entitiesMap;
+ }
+
+ // Adjust dropIndex to account for the source tab being removed
+ if (dropIndex > source.index) {
+ dropIndex -= 1;
+ }
+ }
+ const reordered = reorder(current, source.index, dropIndex);
const result = {
...entitiesMap,
@@ -44,9 +83,18 @@ export default function reorderItem({ entitiesMap, source, destination }) {
return result;
}
+ if (isTabsType) {
+ // Ensure the dropIndex is within the bounds of the destination children
+ if (position === DROP_LEFT) {
+ dropIndex = Math.max(dropIndex, 0);
+ } else if (position === DROP_RIGHT) {
+ dropIndex = Math.min(dropIndex + 1, current.length - 1);
+ }
+ }
+
// moving to different list
current.splice(source.index, 1); // remove from original
- next.splice(destination.index, 0, target); // insert into next
+ next.splice(dropIndex, 0, target); // insert into next
const result = {
...entitiesMap,
diff --git a/superset-frontend/src/dashboard/util/dnd-reorder.test.js b/superset-frontend/src/dashboard/util/dnd-reorder.test.js
index 056a0b45c..c78cd7a37 100644
--- a/superset-frontend/src/dashboard/util/dnd-reorder.test.js
+++ b/superset-frontend/src/dashboard/util/dnd-reorder.test.js
@@ -17,6 +17,8 @@
* under the License.
*/
import reorderItem from 'src/dashboard/util/dnd-reorder';
+import { TABS_TYPE } from './componentTypes';
+import { DROP_LEFT, DROP_RIGHT } from './getDropPosition';
describe('dnd-reorderItem', () => {
it('should remove the item from its source entity and add it to its destination entity', () => {
@@ -72,6 +74,121 @@ describe('dnd-reorderItem', () => {
destination: { id: 'b', index: 1 },
});
- expect(result.iAmExtra === extraEntity).toBe(true);
+ expect(result.iAmExtra).toBe(extraEntity);
+ });
+
+ it('should handle out of bounds destination index gracefully', () => {
+ const result = reorderItem({
+ entitiesMap: {
+ a: {
+ id: 'a',
+ children: ['x', 'y', 'z'],
+ },
+ b: {
+ id: 'b',
+ children: ['banana'],
+ },
+ },
+ source: { id: 'a', index: 1 },
+ destination: { id: 'b', index: 5 },
+ });
+
+ expect(result.a.children).toEqual(['x', 'z']);
+ expect(result.b.children).toEqual(['banana', 'y']);
+ });
+
+ it('should do nothing if source and destination are the same and indices are the same', () => {
+ const result = reorderItem({
+ entitiesMap: {
+ a: {
+ id: 'a',
+ children: ['x', 'y', 'z'],
+ },
+ },
+ source: { id: 'a', index: 1 },
+ destination: { id: 'a', index: 1 },
+ });
+
+ expect(result.a.children).toEqual(['x', 'y', 'z']);
+ });
+
+ it('should handle DROP_LEFT in the same TABS_TYPE list correctly', () => {
+ const result = reorderItem({
+ entitiesMap: {
+ a: {
+ id: 'a',
+ type: TABS_TYPE,
+ children: ['x', 'y', 'z'],
+ },
+ },
+ source: { id: 'a', type: TABS_TYPE, index: 2 },
+ destination: { id: 'a', type: TABS_TYPE, index: 1 },
+ position: DROP_LEFT,
+ });
+
+ expect(result.a.children).toEqual(['x', 'z', 'y']);
+ });
+
+ it('should handle DROP_RIGHT in the same TABS_TYPE list correctly', () => {
+ const result = reorderItem({
+ entitiesMap: {
+ a: {
+ id: 'a',
+ type: TABS_TYPE,
+ children: ['x', 'y', 'z'],
+ },
+ },
+ source: { id: 'a', type: TABS_TYPE, index: 0 },
+ destination: { id: 'a', type: TABS_TYPE, index: 1 },
+ position: DROP_RIGHT,
+ });
+
+ expect(result.a.children).toEqual(['y', 'x', 'z']);
+ });
+
+ it('should handle DROP_LEFT when moving between different TABS_TYPE lists', () => {
+ const result = reorderItem({
+ entitiesMap: {
+ a: {
+ id: 'a',
+ type: TABS_TYPE,
+ children: ['x', 'y'],
+ },
+ b: {
+ id: 'b',
+ type: TABS_TYPE,
+ children: ['banana'],
+ },
+ },
+ source: { id: 'a', type: TABS_TYPE, index: 1 },
+ destination: { id: 'b', type: TABS_TYPE, index: 0 },
+ position: DROP_LEFT,
+ });
+
+ expect(result.a.children).toEqual(['x']);
+ expect(result.b.children).toEqual(['y', 'banana']);
+ });
+
+ it('should handle DROP_RIGHT when moving between different TABS_TYPE lists', () => {
+ const result = reorderItem({
+ entitiesMap: {
+ a: {
+ id: 'a',
+ type: TABS_TYPE,
+ children: ['x', 'y'],
+ },
+ b: {
+ id: 'b',
+ type: TABS_TYPE,
+ children: ['banana'],
+ },
+ },
+ source: { id: 'a', type: TABS_TYPE, index: 0 },
+ destination: { id: 'b', type: TABS_TYPE, index: 0 },
+ position: DROP_RIGHT,
+ });
+
+ expect(result.a.children).toEqual(['y']);
+ expect(result.b.children).toEqual(['banana', 'x']);
});
});