From 9e703f399b78af1a198ba5d69353385b77dd701e Mon Sep 17 00:00:00 2001 From: Grace Guo Date: Tue, 30 Apr 2019 00:40:35 -0700 Subject: [PATCH] [dashboard] allow user re-order top-level tabs (#7390) --- .../dashboard/actions/dashboardLayout_spec.js | 67 ++++++++++++++++++- .../src/dashboard/actions/dashboardLayout.js | 29 +++++++- .../dashboard/components/DashboardBuilder.jsx | 7 +- .../components/gridComponents/Tab.jsx | 5 -- 4 files changed, 95 insertions(+), 13 deletions(-) diff --git a/superset/assets/spec/javascripts/dashboard/actions/dashboardLayout_spec.js b/superset/assets/spec/javascripts/dashboard/actions/dashboardLayout_spec.js index d57207d090..0dfca9cdbf 100644 --- 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 index b76966c934..b276e374bb 100644 --- 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,8 +154,10 @@ 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.`, + ), ), ); } @@ -162,11 +165,29 @@ export function handleComponentDrop(dropResult) { 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 index 12c8ff3368..4a39955d3f 100644 --- 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 index 49a0f187fb..d441c8e84d 100644 --- 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 }) => (