fix: dashboard top level tabs edit (#19722)

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<DashboardBuilderProps> = () => {
     [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 }) => (
       <div>

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<DashboardContainerProps> = ({ topLevelTabs }) => {
+  const nativeFilterScopes = useNativeFilterScopes();
+  const dispatch = useDispatch();
+
   const dashboardLayout = useSelector<RootState, DashboardLayout>(
     state => state.dashboardLayout.present,
   );
-  const nativeFilterScopes = useNativeFilterScopes();
   const directPathToChild = useSelector<RootState, string[]>(
     state => state.dashboardState.directPathToChild,
   );
   const charts = useSelector<RootState, ChartsState>(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 (

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) {