sadpandajoe commented on PR #39417: URL: https://github.com/apache/superset/pull/39417#issuecomment-4615862910
Pushed 049ebda addressing the open feedback: **@aminghadersohi (Jun 2 re-review):** - **MEDIUM — nested-tabs ancestor gap:** `useActiveDashboardTabs` no longer early-returns when `reduxTabs.length > 0`. It walks every TABS container in the layout and for each picks the redux-selected child if present, else the default (first) child. Any redux entries outside the traversed path are appended at the end so nothing marked active is silently dropped. This closes the case where `hideTab:true` skips the top-level Tabs while a nested Tabs dispatches `setActiveTab` and leaves `reduxTabs = ['TAB-Inner1']` without `'TAB-Outer1'`. - **MEDIUM — missing test:** Added a test that mounts `activeTabs: ['TAB-Inner1']` against the outer+inner layout and asserts (a) the outer-tab filter is in scope (ancestor merged) and (b) a filter on the other outer tab stays out of scope (scoping preserved). - **NIT — `useDashboardHasTabs` null guard:** Added. - **NIT — ROOT-first-child invariant:** Documented inline. **@Vitor-Avila:** The layout-derived default-tab fallback from earlier in the PR is preserved (filters from other tabs still don't leak in the basic hideTab case), and the new merge behaviour additionally covers the nested-tabs case where Redux carries only an inner tab id. So scoping is now correct in: (1) `hideTab:true` no permalink, (2) `hideTab:true` with permalink (redux drives selection), (3) `hideTab:true` with nested Tabs (ancestor merged in), (4) regular tabbed dashboards (no behaviour change vs current main). The two Copilot threads (same nested-tabs concern) are resolved. -- 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]
