From 9fcea20a54694aed40fffc2fc1a208b5d512d895 Mon Sep 17 00:00:00 2001 From: Diego Medina Date: Tue, 19 Apr 2022 17:43:54 -0300 Subject: [PATCH] fix: dashboard top level tabs edit --- .../DashboardBuilder/DashboardBuilder.tsx | 30 ++++++++++++++- .../DashboardBuilder/DashboardContainer.tsx | 23 +++++------ .../components/gridComponents/Tabs.jsx | 38 +++++++++---------- 3 files changed, 57 insertions(+), 34 deletions(-) diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx index 11a5b28e561b2..8352482ed8ab8 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx @@ -18,7 +18,14 @@ */ /* eslint-env browser */ import cx from 'classnames'; -import React, { FC, useCallback, useEffect, useState, useMemo } from 'react'; +import React, { + FC, + useCallback, + useEffect, + useState, + useMemo, + useRef, +} from 'react'; import { JsonObject, styled, css, t } from '@superset-ui/core'; import { Global } from '@emotion/react'; import { useDispatch, useSelector } from 'react-redux'; @@ -319,6 +326,27 @@ const DashboardBuilder: FC = () => { [dashboardFiltersOpen, editMode, nativeFiltersEnabled], ); + // If a new tab was added, update the directPathToChild to reflect it + const currentTopLevelTabs = useRef(topLevelTabs); + useEffect(() => { + const currentTabsLength = currentTopLevelTabs.current?.children?.length; + const newTabsLength = topLevelTabs?.children?.length; + + if ( + currentTabsLength !== undefined && + newTabsLength !== undefined && + newTabsLength > currentTabsLength + ) { + const lastTab = getDirectPathToTabIndex( + getRootLevelTabsComponent(dashboardLayout), + newTabsLength - 1, + ); + dispatch(setDirectPathToChild(lastTab)); + } + + currentTopLevelTabs.current = topLevelTabs; + }, [topLevelTabs]); + const renderDraggableContent = useCallback( ({ dropIndicatorProps }: { dropIndicatorProps: JsonObject }) => (
diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx index b08a7cd6339f5..c763f07267f55 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx @@ -18,7 +18,7 @@ */ // ParentSize uses resize observer so the dashboard will update size // when its container size changes, due to e.g., builder side panel opening -import React, { FC, useEffect, useMemo, useState } from 'react'; +import React, { FC, useEffect, useMemo } from 'react'; import { useDispatch, useSelector } from 'react-redux'; import { FeatureFlag, @@ -36,7 +36,6 @@ import { LayoutItem, RootState, } from 'src/dashboard/types'; -import getLeafComponentIdFromPath from 'src/dashboard/util/getLeafComponentIdFromPath'; import { DASHBOARD_GRID_ID, DASHBOARD_ROOT_DEPTH, @@ -68,29 +67,27 @@ const useNativeFilterScopes = () => { }; const DashboardContainer: FC = ({ topLevelTabs }) => { + const nativeFilterScopes = useNativeFilterScopes(); + const dispatch = useDispatch(); + const dashboardLayout = useSelector( state => state.dashboardLayout.present, ); - const nativeFilterScopes = useNativeFilterScopes(); const directPathToChild = useSelector( state => state.dashboardState.directPathToChild, ); const charts = useSelector(state => state.charts); - const [tabIndex, setTabIndex] = useState( - getRootLevelTabIndex(dashboardLayout, directPathToChild), - ); - const dispatch = useDispatch(); - - useEffect(() => { + const tabIndex = useMemo(() => { const nextTabIndex = findTabIndexByComponentId({ currentComponent: getRootLevelTabsComponent(dashboardLayout), directPathToChild, }); - if (nextTabIndex > -1) { - setTabIndex(nextTabIndex); - } - }, [getLeafComponentIdFromPath(directPathToChild)]); + + return nextTabIndex > -1 + ? nextTabIndex + : getRootLevelTabIndex(dashboardLayout, directPathToChild); + }, [dashboardLayout, directPathToChild]); useEffect(() => { if ( diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx b/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx index 8f2643533f3e4..c579abb911b15 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx @@ -155,22 +155,6 @@ export class Tabs extends React.PureComponent { this.setState(() => ({ tabIndex: maxIndex })); } - if (nextTabsIds.length) { - const lastTabId = nextTabsIds[nextTabsIds.length - 1]; - // if a new tab is added focus on it immediately - if (nextTabsIds.length > currTabsIds.length) { - // a new tab's path may be empty, here also need to set tabIndex - this.setState(() => ({ - activeKey: lastTabId, - tabIndex: maxIndex, - })); - } - // if a tab is removed focus on the first - if (nextTabsIds.length < currTabsIds.length) { - this.setState(() => ({ activeKey: nextTabsIds[0] })); - } - } - if (nextProps.isComponentVisible) { const nextFocusComponent = getLeafComponentIdFromPath( nextProps.directPathToChild, @@ -179,7 +163,14 @@ export class Tabs extends React.PureComponent { this.props.directPathToChild, ); - if (nextFocusComponent !== currentFocusComponent) { + // If the currently selected component is different than the new one, + // or the tab length/order changed, calculate the new tab index and + // replace it if it's different than the current one + if ( + nextFocusComponent !== currentFocusComponent || + (nextFocusComponent === currentFocusComponent && + currTabsIds !== nextTabsIds) + ) { const nextTabIndex = findTabIndexByComponentId({ currentComponent: nextProps.component, directPathToChild: nextProps.directPathToChild, @@ -219,9 +210,12 @@ export class Tabs extends React.PureComponent { }); }; - handleEdit = (key, action) => { + handleEdit = (event, action) => { const { component, createComponent } = this.props; if (action === 'add') { + // Prevent the tab container to be selected + event?.stopPropagation?.(); + createComponent({ destination: { id: component.id, @@ -234,7 +228,7 @@ export class Tabs extends React.PureComponent { }, }); } else if (action === 'remove') { - this.showDeleteConfirmModal(key); + this.showDeleteConfirmModal(event); } }; @@ -261,7 +255,11 @@ export class Tabs extends React.PureComponent { } handleDeleteTab(tabIndex) { - this.handleClickTab(Math.max(0, tabIndex - 1)); + // If we're removing the currently selected tab, + // select the previous one (if any) + if (this.state.tabIndex === tabIndex) { + this.handleClickTab(Math.max(0, tabIndex - 1)); + } } handleDropOnTab(dropResult) {