[dashboard] allow user re-order top-level tabs (apache#7390)

CybercentreCanada · Apr 30, 2019 · 103080a · 103080a
1 parent 19414b8
commit 103080a
Show file tree

Hide file tree

Showing 4 changed files with 95 additions and 13 deletions.
diff --git a/superset/assets/spec/javascripts/dashboard/actions/dashboardLayout_spec.js b/superset/assets/spec/javascripts/dashboard/actions/dashboardLayout_spec.js
@@ -39,7 +39,10 @@ import {
 } from '../../../../src/dashboard/actions/dashboardLayout';
 
 import { setUnsavedChanges } from '../../../../src/dashboard/actions/dashboardState';
-import { addInfoToast } from '../../../../src/messageToasts/actions';
+import {
+  addWarningToast,
+  ADD_TOAST,
+} from '../../../../src/messageToasts/actions';
 
 import {
   DASHBOARD_GRID_TYPE,
@@ -334,7 +337,9 @@ describe('dashboardLayout actions', () => {
 
       const thunk = handleComponentDrop(dropResult);
       thunk(dispatch, getState);
-      expect(dispatch.getCall(0).args[0].type).toEqual(addInfoToast('').type);
+      expect(dispatch.getCall(0).args[0].type).toEqual(
+        addWarningToast('').type,
+      );
 
       expect(dispatch.callCount).toBe(1);
     });
@@ -402,6 +407,64 @@ describe('dashboardLayout actions', () => {
 
       expect(dispatch.callCount).toBe(2);
     });
+
+    it('should dispatch a toast if drop top-level tab into nested tab', () => {
+      const { getState, dispatch } = setup({
+        dashboardLayout: {
+          present: {
+            [DASHBOARD_ROOT_ID]: {
+              children: ['TABS-ROOT_TABS'],
+              id: DASHBOARD_ROOT_ID,
+              type: 'ROOT',
+            },
+            'TABS-ROOT_TABS': {
+              children: ['TAB-iMppmTOQy', 'TAB-rt1y8cQ6K9', 'TAB-X_pnCIwPN'],
+              id: 'TABS-ROOT_TABS',
+              meta: {},
+              parents: ['ROOT_ID'],
+              type: TABS_TYPE,
+            },
+            'TABS-ROW_TABS': {
+              children: [
+                'TAB-dKIDBT03bQ',
+                'TAB-PtxY5bbTe',
+                'TAB-Wc2P-yGMz',
+                'TAB-U-xe_si7i',
+              ],
+              id: 'TABS-ROW_TABS',
+              meta: {},
+              parents: ['ROOT_ID', 'TABS-ROOT_TABS', 'TAB-X_pnCIwPN'],
+              type: TABS_TYPE,
+            },
+          },
+        },
+      });
+      const dropResult = {
+        source: {
+          id: 'TABS-ROOT_TABS',
+          index: 1,
+          type: TABS_TYPE,
+        },
+        destination: {
+          id: 'TABS-ROW_TABS',
+          index: 1,
+          type: TABS_TYPE,
+        },
+        dragging: {
+          id: 'TAB-rt1y8cQ6K9',
+          meta: { text: 'New Tab' },
+          type: 'TAB',
+        },
+      };
+
+      const thunk1 = handleComponentDrop(dropResult);
+      thunk1(dispatch, getState);
+
+      const thunk2 = dispatch.getCall(0).args[0];
+      thunk2(dispatch, getState);
+
+      expect(dispatch.getCall(1).args[0].type).toEqual(ADD_TOAST);
+    });
   });
 
   describe('undoLayoutAction', () => {

diff --git a/superset/assets/src/dashboard/actions/dashboardLayout.js b/superset/assets/src/dashboard/actions/dashboardLayout.js
@@ -17,8 +17,9 @@
  * under the License.
  */
 import { ActionCreators as UndoActionCreators } from 'redux-undo';
+import { t } from '@superset-ui/translation';
 
-import { addInfoToast } from '../../messageToasts/actions';
+import { addWarningToast } from '../../messageToasts/actions';
 import { setUnsavedChanges } from './dashboardState';
 import { TABS_TYPE, ROW_TYPE } from '../util/componentTypes';
 import {
@@ -153,20 +154,40 @@ export function handleComponentDrop(dropResult) {
 
     if (overflowsParent) {
       return dispatch(
-        addInfoToast(
-          `There is not enough space for this component. Try decreasing its width, or increasing the destination width.`,
+        addWarningToast(
+          t(
+            `There is not enough space for this component. Try decreasing its width, or increasing the destination width.`,
+          ),
         ),
       );
     }
 
     const { source, destination } = dropResult;
     const droppedOnRoot = destination && destination.id === DASHBOARD_ROOT_ID;
     const isNewComponent = source.id === NEW_COMPONENTS_SOURCE_ID;
+    const dashboardRoot = getState().dashboardLayout.present[DASHBOARD_ROOT_ID];
+    const rootChildId =
+      dashboardRoot && dashboardRoot.children ? dashboardRoot.children[0] : '';
 
     if (droppedOnRoot) {
       dispatch(createTopLevelTabs(dropResult));
     } else if (destination && isNewComponent) {
       dispatch(createComponent(dropResult));
+    } else if (
+      // Add additional allow-to-drop logic for tag/tags source.
+      // We only allow
+      // - top-level tab => top-level tab: rearrange top-level tab order
+      // - nested tab => top-level tab: allow row tab become top-level tab
+      // Dashboard does not allow top-level tab become nested tab, to avoid
+      // nested tab inside nested tab.
+      source.type === TABS_TYPE &&
+      destination.type === TABS_TYPE &&
+      source.id === rootChildId &&
+      destination.id !== rootChildId
+    ) {
+      return dispatch(
+        addWarningToast(t(`Can not move top level tab into nested tabs`)),
+      );
     } else if (
       destination &&
       source &&
@@ -176,6 +197,8 @@ export function handleComponentDrop(dropResult) {
       dispatch(moveComponent(dropResult));
     }
 
+    // call getState() again down here in case redux state is stale after
+    // previous dispatch(es)
     const { dashboardLayout: undoableLayout } = getState();
 
     // if we moved a child from a Tab or Row parent and it was the only child, delete the parent.

diff --git a/superset/assets/src/dashboard/components/DashboardBuilder.jsx b/superset/assets/src/dashboard/components/DashboardBuilder.jsx
@@ -84,10 +84,11 @@ class DashboardBuilder extends React.Component {
     const { dashboardLayout, directPathToChild } = props;
     const dashboardRoot = dashboardLayout[DASHBOARD_ROOT_ID];
     const rootChildId = dashboardRoot.children[0];
-    const topLevelTabs =
-      rootChildId !== DASHBOARD_GRID_ID && dashboardLayout[rootChildId];
     const tabIndex = findTabIndexByComponentId({
-      currentComponent: topLevelTabs || dashboardLayout[DASHBOARD_ROOT_ID],
+      currentComponent:
+        rootChildId === DASHBOARD_GRID_ID
+          ? dashboardLayout[DASHBOARD_ROOT_ID]
+          : dashboardLayout[rootChildId],
       directPathToChild,
     });
 

diff --git a/superset/assets/src/dashboard/components/gridComponents/Tab.jsx b/superset/assets/src/dashboard/components/gridComponents/Tab.jsx
@@ -26,7 +26,6 @@ import AnchorLink from '../../../components/AnchorLink';
 import DeleteComponentModal from '../DeleteComponentModal';
 import WithPopoverMenu from '../menu/WithPopoverMenu';
 import { componentShape } from '../../util/propShapes';
-import { DASHBOARD_ROOT_DEPTH } from '../../util/constants';
 
 export const RENDER_TAB = 'RENDER_TAB';
 export const RENDER_TAB_CONTENT = 'RENDER_TAB_CONTENT';
@@ -219,10 +218,6 @@ export default class Tab extends React.PureComponent {
         index={index}
         depth={depth}
         onDrop={this.handleDrop}
-        // disable drag drop of top-level Tab's to prevent invalid nesting of a child in
-        // itself, e.g. if a top-level Tab has a Tabs child, dragging the Tab into the Tabs would
-        // reusult in circular children
-        disableDragDrop={depth <= DASHBOARD_ROOT_DEPTH + 1}
         editMode={editMode}
       >
         {({ dropIndicatorProps, dragSourceRef }) => (