You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2024/05/15 11:48:20 UTC
(superset) branch master updated: fix(dashboard): invalid drop item on a tab (#28507)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 65e0d54fa5 fix(dashboard): invalid drop item on a tab (#28507)
65e0d54fa5 is described below
commit 65e0d54fa52e308e5a9e3c6680a4db46fece3956
Author: JUST.in DO IT <ju...@airbnb.com>
AuthorDate: Wed May 15 04:48:14 2024 -0700
fix(dashboard): invalid drop item on a tab (#28507)
---
.../src/dashboard/components/dnd/handleDrop.js | 9 ++-
.../dashboard/components/gridComponents/Tab.jsx | 6 ++
.../components/gridComponents/Tab.test.tsx | 84 +++++++++++++++++++++-
.../src/dashboard/util/getDropPosition.js | 2 +-
4 files changed, 97 insertions(+), 4 deletions(-)
diff --git a/superset-frontend/src/dashboard/components/dnd/handleDrop.js b/superset-frontend/src/dashboard/components/dnd/handleDrop.js
index 6d50d39c45..f4b847cd6d 100644
--- a/superset-frontend/src/dashboard/components/dnd/handleDrop.js
+++ b/superset-frontend/src/dashboard/components/dnd/handleDrop.js
@@ -55,8 +55,11 @@ export default function handleDrop(props, monitor, Component) {
},
};
+ const shouldAppendToChildren =
+ typeof dropToChild === 'function' ? dropToChild(draggingItem) : dropToChild;
+
// simplest case, append as child
- if (dropToChild) {
+ if (shouldAppendToChildren) {
dropResult.destination = {
id: component.id,
type: component.type,
@@ -74,7 +77,9 @@ export default function handleDrop(props, monitor, Component) {
const sameParent =
parentComponent && draggingItem.parentId === parentComponent.id;
const sameParentLowerIndex =
- sameParent && draggingItem.index < componentIndex;
+ sameParent &&
+ draggingItem.index < componentIndex &&
+ draggingItem.type !== component.type;
const nextIndex = sameParentLowerIndex
? componentIndex - 1
diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx b/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx
index b546947e31..bd71afa081 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx
@@ -32,6 +32,7 @@ import DragDroppable, {
Droppable,
} from 'src/dashboard/components/dnd/DragDroppable';
import { componentShape } from 'src/dashboard/util/propShapes';
+import { TAB_TYPE } from 'src/dashboard/util/componentTypes';
export const RENDER_TAB = 'RENDER_TAB';
export const RENDER_TAB_CONTENT = 'RENDER_TAB_CONTENT';
@@ -139,6 +140,10 @@ class Tab extends React.PureComponent {
}
}
+ shouldDropToChild(item) {
+ return item.type !== TAB_TYPE;
+ }
+
renderTabContent() {
const {
component: tabComponent,
@@ -275,6 +280,7 @@ class Tab extends React.PureComponent {
onDrop={this.handleDrop}
onHover={this.handleOnHover}
editMode={editMode}
+ dropToChild={this.shouldDropToChild}
>
{({ dropIndicatorProps, dragSourceRef }) => (
<TabTitleContainer
diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tab.test.tsx b/superset-frontend/src/dashboard/components/gridComponents/Tab.test.tsx
index e19a086d81..6534a64f68 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/Tab.test.tsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/Tab.test.tsx
@@ -19,12 +19,18 @@
import userEvent from '@testing-library/user-event';
import React from 'react';
-import { render, screen } from 'spec/helpers/testing-library';
+import {
+ fireEvent,
+ render,
+ screen,
+ waitFor,
+} from 'spec/helpers/testing-library';
import DashboardComponent from 'src/dashboard/containers/DashboardComponent';
import EditableTitle from 'src/components/EditableTitle';
import { setEditMode } from 'src/dashboard/actions/dashboardState';
import Tab from './Tab';
+import Markdown from './Markdown';
jest.mock('src/dashboard/containers/DashboardComponent', () =>
jest.fn(() => <div data-test="DashboardComponent" />),
@@ -128,6 +134,82 @@ test('Render tab (no content) editMode:true', () => {
expect(getByTestId('dragdroppable-object')).toBeInTheDocument();
});
+test('Drop on a tab', async () => {
+ const props = createProps();
+ const mockOnDropOnTab = jest.fn();
+ render(
+ <>
+ <Tab {...props} renderType="RENDER_TAB" editMode />
+ <Tab
+ {...props}
+ renderType="RENDER_TAB"
+ index={2}
+ component={{
+ ...props.component,
+ id: 'TAB-Next-',
+ meta: { text: 'Next Tab' } as any,
+ }}
+ handleComponentDrop={mockOnDropOnTab}
+ editMode
+ />
+ <Markdown
+ id="MARKDOWN-1"
+ parentId="GRID_ID"
+ parentComponent={{
+ id: 'GRID_ID',
+ type: 'GRID',
+ parents: ['ROOT_ID'],
+ }}
+ depth={0}
+ editMode
+ index={1}
+ availableColumnCount={12}
+ columnWidth={120}
+ component={{
+ ...props.component,
+ type: 'MARKDOWN',
+ id: 'MARKDOWN-1',
+ meta: { code: 'Dashboard Component' } as any,
+ }}
+ logEvent={jest.fn()}
+ deleteComponent={jest.fn()}
+ handleComponentDrop={jest.fn()}
+ onResizeStart={jest.fn()}
+ onResize={jest.fn()}
+ onResizeStop={jest.fn()}
+ updateComponents={jest.fn()}
+ addDangerToast={jest.fn()}
+ />
+ </>,
+ {
+ useRedux: true,
+ useDnd: true,
+ },
+ );
+
+ fireEvent.dragStart(screen.getByText('🚀 Aspiring Developers'));
+ fireEvent.drop(screen.getByText('Next Tab'));
+ await waitFor(() => expect(mockOnDropOnTab).toHaveBeenCalled());
+ expect(mockOnDropOnTab).toHaveBeenCalledWith(
+ expect.objectContaining({
+ destination: { id: props.parentComponent.id, index: 2, type: 'TABS' },
+ }),
+ );
+
+ fireEvent.dragStart(screen.getByText('Dashboard Component'));
+ fireEvent.drop(screen.getByText('Next Tab'));
+ await waitFor(() => expect(mockOnDropOnTab).toHaveBeenCalledTimes(2));
+ expect(mockOnDropOnTab).toHaveBeenLastCalledWith(
+ expect.objectContaining({
+ destination: {
+ id: 'TAB-Next-',
+ index: props.component.children.length,
+ type: 'TAB',
+ },
+ }),
+ );
+});
+
test('Edit table title', () => {
const props = createProps();
props.editMode = true;
diff --git a/superset-frontend/src/dashboard/util/getDropPosition.js b/superset-frontend/src/dashboard/util/getDropPosition.js
index c4a21ad113..9a68808b05 100644
--- a/superset-frontend/src/dashboard/util/getDropPosition.js
+++ b/superset-frontend/src/dashboard/util/getDropPosition.js
@@ -82,7 +82,7 @@ export default function getDropPosition(monitor, Component) {
const siblingDropOrientation =
orientation === 'row' ? 'horizontal' : 'vertical';
- if (isDraggingOverShallow && validChild && !validSibling) {
+ if (validChild && !validSibling) {
// easiest case, insert as child
if (childDropOrientation === 'vertical') {
return hasChildren ? DROP_RIGHT : DROP_LEFT;