diff --git a/superset-frontend/src/features/home/Menu.test.tsx b/superset-frontend/src/features/home/Menu.test.tsx index 6cbacebcb40a..1d712fb66637 100644 --- a/superset-frontend/src/features/home/Menu.test.tsx +++ b/superset-frontend/src/features/home/Menu.test.tsx @@ -796,3 +796,105 @@ test('brand link falls back to brand.path when theme brandLogoUrl is absent', as // ensureAppRoot must have been applied: /welcome/ → /superset/welcome/ expect(brandLink).toHaveAttribute('href', '/superset/welcome/'); }); + +// --- Active tab highlighting (regression tests for issue #36403) --- +// +// The active top-level tab is highlighted by matching the current route to a +// menu item. The matching must rely on a stable identifier (the FAB `name`), +// not the displayed label, otherwise highlighting breaks for any non-English +// locale where the label is translated. + +// Returns the top-level
  • that contains the given visible text, so we can +// assert whether antd marked it as the selected menu item. +const getMenuItemByText = (text: string): HTMLElement | null => + screen.getByText(text).closest('li'); + +afterEach(() => { + // Reset the route so a pushed path does not leak into the next test. + window.history.pushState({}, '', '/'); +}); + +test('highlights the active top-level tab on a matching route (English)', async () => { + useSelectorMock.mockReturnValue({ roles: user.roles }); + window.history.pushState({}, '', '/dashboard/list/'); + + render(, { + useRedux: true, + useQueryParams: true, + useRouter: true, + useTheme: true, + }); + + await screen.findByText('Dashboards'); + expect(getMenuItemByText('Dashboards')).toHaveClass('ant-menu-item-selected'); +}); + +test('highlights the active top-level tab when the label is localized', async () => { + // Russian locale: the FAB `name` stays the stable English identifier while + // the displayed `label` is translated. Highlighting must still work. + const localizedProps = { + ...mockedProps, + data: { + ...mockedProps.data, + menu: mockedProps.data.menu.map(item => + item.name === 'Dashboards' ? { ...item, label: 'Дашборды' } : item, + ), + }, + }; + + useSelectorMock.mockReturnValue({ roles: user.roles }); + window.history.pushState({}, '', '/dashboard/list/'); + + render(, { + useRedux: true, + useQueryParams: true, + useRouter: true, + useTheme: true, + }); + + await screen.findByText('Дашборды'); + expect(getMenuItemByText('Дашборды')).toHaveClass('ant-menu-item-selected'); +}); + +test('highlights the active SQL tab when the label is localized', async () => { + // The SQL Lab top-level entry is a FAB category: its stable `name` is + // "SQL Lab" while its label ("SQL") is localized. + const localizedProps = { + ...mockedProps, + data: { + ...mockedProps.data, + menu: [ + ...mockedProps.data.menu, + { + name: 'SQL Lab', + icon: 'fa-flask', + label: 'SQL запросы', + childs: [ + { + name: 'SQL Editor', + label: 'SQL Lab', + url: '/sqllab/', + index: 1, + }, + ], + }, + ], + }, + }; + + useSelectorMock.mockReturnValue({ roles: user.roles }); + window.history.pushState({}, '', '/sqllab/'); + + render(, { + useRedux: true, + useQueryParams: true, + useRouter: true, + useTheme: true, + }); + + await screen.findByText('SQL запросы'); + // SQL Lab renders as a submenu, so antd marks it with the submenu variant. + expect(getMenuItemByText('SQL запросы')).toHaveClass( + 'ant-menu-submenu-selected', + ); +}); diff --git a/superset-frontend/src/features/home/Menu.tsx b/superset-frontend/src/features/home/Menu.tsx index 6b950f9fff36..a7e8d2dcdf39 100644 --- a/superset-frontend/src/features/home/Menu.tsx +++ b/superset-frontend/src/features/home/Menu.tsx @@ -211,6 +211,16 @@ export function Menu({ SavedQueries = '/savedqueryview', } + // Stable Flask-AppBuilder menu identifiers (`name`), used as menu item keys. + // These are locale-independent, unlike the displayed labels, so matching the + // active tab against them keeps highlighting working in every language. + enum MenuKeys { + Dashboards = 'Dashboards', + Charts = 'Charts', + Datasets = 'Datasets', + SqlLab = 'SQL Lab', + } + const defaultTabSelection: string[] = []; const [activeTabs, setActiveTabs] = useState(defaultTabSelection); const location = useLocation(); @@ -218,16 +228,16 @@ export function Menu({ const path = location.pathname; switch (true) { case path.startsWith(Paths.Dashboard): - setActiveTabs([t('Dashboards')]); + setActiveTabs([MenuKeys.Dashboards]); break; case path.startsWith(Paths.Chart) || path.startsWith(Paths.Explore): - setActiveTabs([t('Charts')]); + setActiveTabs([MenuKeys.Charts]); break; case path.startsWith(Paths.Datasets): - setActiveTabs([datasetsLabel()]); + setActiveTabs([MenuKeys.Datasets]); break; case path.startsWith(Paths.SqlLab) || path.startsWith(Paths.SavedQueries): - setActiveTabs(['SQL']); + setActiveTabs([MenuKeys.SqlLab]); break; default: setActiveTabs(defaultTabSelection); @@ -242,10 +252,14 @@ export function Menu({ childs, url, isFrontendRoute, + name, }: MenuObjectProps): MenuItem => { + // Key items by the stable FAB `name` so active-tab matching is independent + // of the localized label. Fall back to the label when no name is provided. + const key = name ?? label; if (url && isFrontendRoute) { return { - key: label, + key, label: ( {label} @@ -256,7 +270,7 @@ export function Menu({ if (url) { return { - key: label, + key, label: {label}, }; } @@ -268,7 +282,11 @@ export function Menu({ } else if (typeof child !== 'string') { Object.assign(child, { label: t(child.label) }); childItems.push({ - key: `${child.label}`, + // Key children by the stable FAB `name` as well, so a child whose + // localized label coincides with a parent key (e.g. the "SQL Editor" + // child labeled "SQL Lab" under the "SQL Lab" category) doesn't + // collide with that parent. Fall back to the label when no name. + key: child.name ?? `${child.label}`, label: child.isFrontendRoute ? ( {child.label} @@ -281,7 +299,7 @@ export function Menu({ }); return { - key: label, + key, label, ...(screens.md && { icon: ,