(superset) 01/05: fix(dashboard): reconcile stale results tab in Chart Data modal

[en...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

EnxDev pushed a commit to branch enxdev/fix/explore-stale-results-tab
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 33e252cd8ee9968e2c33bff2b3480f2c32d6d444
Author: Enzo Martellucci <en...@gmail.com>
AuthorDate: Tue Aug 25 12:11:02 2026 +0200

    fix(dashboard): reconcile stale results tab in Chart Data modal
    
    Address PR #43454 review feedback: reuse DataTablesPane's stale-tab
    fallback so ResultsPaneOnDashboard doesn't render blank when a mixed
    chart's active results tab disappears, and drop CSS rules now
    redundant with the fullHeight prop.
    
    Co-Authored-By: Claude Sonnet 5 <no...@anthropic.com>
---
 .../components/DataTablesPane/DataTablesPane.tsx   | 17 +------
 .../components/ResultsPaneOnDashboard.tsx          | 22 +++++----
 .../test/ResultsPaneOnDashboard.test.tsx           | 56 ++++++++++++++++++++++
 .../test/getStaleResultsTabFallback.test.ts        |  2 +-
 .../src/explore/components/DataTablesPane/utils.ts | 35 ++++++++++++++
 5 files changed, 106 insertions(+), 26 deletions(-)

diff --git a/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.tsx b/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.tsx
index cb8878e3bf9..7a6e3d649cd 100644
--- a/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.tsx
+++ b/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.tsx
@@ -29,22 +29,7 @@ import {
 } from 'src/utils/localStorageHelpers';
 import { SamplesPane, useResultsPane } from './components';
 import { DataTablesPaneProps, ResultTypes } from './types';
-
-/**
- * A mixed chart can be reconfigured to return fewer result panes than before
- * (e.g. dropping a query), which removes the corresponding results tab. If the
- * selected tab was one of those, the active key goes stale and the data panel
- * renders blank until the user reselects a valid tab. Returns the first
- * results tab to fall back to in that case, otherwise undefined.
- */
-export const getStaleResultsTabFallback = (
-  activeTabKey: string,
-  resultsTabKeys: string[],
-): string | undefined =>
-  activeTabKey.startsWith(ResultTypes.Results) &&
-  !resultsTabKeys.includes(activeTabKey)
-    ? ResultTypes.Results
-    : undefined;
+import { getStaleResultsTabFallback } from './utils';
 
 const StyledDiv = styled.div`
   ${() => `
diff --git a/superset-frontend/src/explore/components/DataTablesPane/components/ResultsPaneOnDashboard.tsx b/superset-frontend/src/explore/components/DataTablesPane/components/ResultsPaneOnDashboard.tsx
index 4485ca79208..2cada3a2429 100644
--- a/superset-frontend/src/explore/components/DataTablesPane/components/ResultsPaneOnDashboard.tsx
+++ b/superset-frontend/src/explore/components/DataTablesPane/components/ResultsPaneOnDashboard.tsx
@@ -20,22 +20,15 @@ import { t } from '@apache-superset/core/translation';
 import { styled } from '@apache-superset/core/theme';
 import Tabs from '@superset-ui/core/components/Tabs';
 import { ResultTypes, ResultsPaneProps } from '../types';
+import { getStaleResultsTabFallback } from '../utils';
 import { useResultsPane } from './useResultsPane';
-import { useState } from 'react';
+import { useEffect, useState } from 'react';
 
 const Wrapper = styled.div`
   display: flex;
   flex-direction: column;
   height: 100%;
 
-  .ant-tabs {
-    height: 100%;
-  }
-
-  .ant-tabs-body {
-    height: 100%;
-  }
-
   .ant-tabs-content {
     display: flex;
     flex-direction: column;
@@ -86,6 +79,17 @@ export const ResultsPaneOnDashboard = ({
     };
   });
 
+  const resultsTabFallback = getStaleResultsTabFallback(
+    activeTabKey,
+    items.map(({ key }) => key),
+  );
+
+  useEffect(() => {
+    if (resultsTabFallback) {
+      setActiveTabKey(resultsTabFallback);
+    }
+  }, [resultsTabFallback]);
+
   return (
     <Wrapper>
       <Tabs activeKey={activeTabKey} onChange={setActiveTabKey} items={items} />
diff --git a/superset-frontend/src/explore/components/DataTablesPane/test/ResultsPaneOnDashboard.test.tsx b/superset-frontend/src/explore/components/DataTablesPane/test/ResultsPaneOnDashboard.test.tsx
index 145ab3e7896..16a7c2ff0b7 100644
--- a/superset-frontend/src/explore/components/DataTablesPane/test/ResultsPaneOnDashboard.test.tsx
+++ b/superset-frontend/src/explore/components/DataTablesPane/test/ResultsPaneOnDashboard.test.tsx
@@ -20,14 +20,34 @@ import fetchMock from 'fetch-mock';
 import {
   screen,
   render,
+  act,
   waitForElementToBeRemoved,
   waitFor,
 } from 'spec/helpers/testing-library';
 import { ChartMetadata, ChartPlugin, VizType } from '@superset-ui/core';
 import { setupAGGridModules } from '@superset-ui/core/components/ThemedAgGridReact';
+import Tabs from '@superset-ui/core/components/Tabs';
 import { ResultsPaneOnDashboard } from '../components';
+import { useResultsPane } from '../components/useResultsPane';
 import { createResultsPaneOnDashboardProps } from './fixture';
 
+// `fullHeight`'s CSS isn't testable under jsdom (no `importSource` for the
+// `css` prop in jest's babel config), so spy on call args instead.
+jest.mock('@superset-ui/core/components/Tabs', () => {
+  const actual = jest.requireActual('@superset-ui/core/components/Tabs');
+  return { __esModule: true, ...actual, default: jest.fn(actual.default) };
+});
+
+// Wraps the real hook; only overridden below to avoid mounting a second
+// real AG Grid instance, which jsdom doesn't support.
+jest.mock('../components/useResultsPane', () => {
+  const actual = jest.requireActual('../components/useResultsPane');
+  return {
+    __esModule: true,
+    useResultsPane: jest.fn(actual.useResultsPane),
+  };
+});
+
 beforeAll(() => {
   setupAGGridModules();
 });
@@ -106,6 +126,10 @@ describe('ResultsPaneOnDashboard', () => {
     expect(
       await findByText('No results were returned for this query'),
     ).toBeVisible();
+    expect(Tabs).toHaveBeenCalledWith(
+      expect.objectContaining({ fullHeight: true }),
+      expect.anything(),
+    );
   });
 
   test('render errorMessage', async () => {
@@ -219,4 +243,36 @@ describe('ResultsPaneOnDashboard', () => {
     expect(tab2).toBeVisible();
     expect(tab3).toBeNull();
   });
+
+  test('falls back to the first results tab when the active one disappears', async () => {
+    const mockedUseResultsPane = useResultsPane as jest.Mock;
+    mockedUseResultsPane.mockReturnValue([<div key="a" />, <div key="b" />]);
+
+    const props = createResultsPaneOnDashboardProps({ sliceId: 999 });
+    const { rerender } = render(<ResultsPaneOnDashboard {...props} />, {
+      useRedux: true,
+    });
+
+    const latestTabsProps = () => {
+      const { calls } = (Tabs as unknown as jest.Mock).mock;
+      return calls[calls.length - 1][0];
+    };
+    expect(latestTabsProps().items.map((i: { key: string }) => i.key)).toEqual([
+      'results',
+      'results 2',
+    ]);
+
+    act(() => {
+      latestTabsProps().onChange('results 2');
+    });
+    expect(latestTabsProps().activeKey).toBe('results 2');
+
+    // A mixed chart dropped from two query results to one, removing "results 2"
+    mockedUseResultsPane.mockReturnValue([<div key="a" />]);
+    rerender(<ResultsPaneOnDashboard {...props} />);
+
+    await waitFor(() => {
+      expect(latestTabsProps().activeKey).toBe('results');
+    });
+  });
 });
diff --git a/superset-frontend/src/explore/components/DataTablesPane/test/getStaleResultsTabFallback.test.ts b/superset-frontend/src/explore/components/DataTablesPane/test/getStaleResultsTabFallback.test.ts
index d387babf4ee..2b6d8df6aee 100644
--- a/superset-frontend/src/explore/components/DataTablesPane/test/getStaleResultsTabFallback.test.ts
+++ b/superset-frontend/src/explore/components/DataTablesPane/test/getStaleResultsTabFallback.test.ts
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { getStaleResultsTabFallback } from '../DataTablesPane';
+import { getStaleResultsTabFallback } from '../utils';
 import { ResultTypes } from '../types';
 
 test('keeps the active tab when it still exists', () => {
diff --git a/superset-frontend/src/explore/components/DataTablesPane/utils.ts b/superset-frontend/src/explore/components/DataTablesPane/utils.ts
new file mode 100644
index 00000000000..b6e58649382
--- /dev/null
+++ b/superset-frontend/src/explore/components/DataTablesPane/utils.ts
@@ -0,0 +1,35 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { ResultTypes } from './types';
+
+/**
+ * A mixed chart can be reconfigured to return fewer result panes than before
+ * (e.g. dropping a query), which removes the corresponding results tab. If the
+ * selected tab was one of those, the active key goes stale and the data panel
+ * renders blank until the user reselects a valid tab. Returns the first
+ * results tab to fall back to in that case, otherwise undefined.
+ */
+export const getStaleResultsTabFallback = (
+  activeTabKey: string,
+  resultsTabKeys: string[],
+): string | undefined =>
+  activeTabKey.startsWith(ResultTypes.Results) &&
+  !resultsTabKeys.includes(activeTabKey)
+    ? ResultTypes.Results
+    : undefined;