Copilot commented on code in PR #39417:
URL: https://github.com/apache/superset/pull/39417#discussion_r3278099453
##########
superset-frontend/src/dashboard/components/nativeFilters/state.ts:
##########
@@ -180,17 +182,60 @@ export function useDashboardHasTabs() {
);
}
-function useActiveDashboardTabs() {
- return useSelector<RootState, ActiveTabs>(
+function useActiveDashboardTabs(): ActiveTabs {
+ const reduxTabs = useSelector<RootState, ActiveTabs>(
state => state.dashboardState?.activeTabs,
);
+ const dashboardLayout = useDashboardLayout();
+
+ return useMemo(() => {
+ if (reduxTabs?.length > 0) return reduxTabs;
+
Review Comment:
`useActiveDashboardTabs` returns early whenever `reduxTabs.length > 0`, but
in embedded `hideTab:true` only the *top-level* Tabs component is skipped.
Nested Tabs can still mount and dispatch `setActiveTab`, making `activeTabs`
non-empty while missing the top-level active tab id. In that case, filters
scoped to charts under the top-level tab will still be treated out-of-scope.
Consider deriving the top-level default tab from the layout and merging it with
`reduxTabs` unless `reduxTabs` already contains one of the top-level tab ids
(or otherwise verifying `reduxTabs` includes the needed ancestors).
##########
superset-frontend/src/dashboard/components/nativeFilters/state.test.ts:
##########
@@ -601,3 +619,431 @@ test('useChartCustomizationConfiguration ignores
undefined items in metadata', (
expect.objectContaining({ id: 'CHART_CUSTOMIZATION-1' }),
);
});
+
+// --- Embedded / hideTab: activeTabs is empty ---
+// When an embedded dashboard uses hideTab:true, the Tabs component never
+// mounts, so setActiveTab never fires and activeTabs stays []. The same
+// empty state occurs transiently on first render of any tabbed dashboard.
+//
+// useActiveDashboardTabs derives the default first tab from the layout when
+// Redux activeTabs is empty, so scope evaluation uses the correct default
+// tab instead of either "no tabs active" (blank filter bar) or "all tabs"
+// (showing out-of-scope filters).
Review Comment:
The new embedded/hideTab tests cover the `activeTabs: []` fallback, but they
don’t cover the case where `activeTabs` is non-empty due to nested tabs
mounting while the top-level tab id is missing (possible when `hideTab:true`).
Adding a test for this scenario would prevent regressions once
`useActiveDashboardTabs` is adjusted to merge/complete the active tab set.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
