chore: Refactor Menu.Item and cleanup console errors (#34536)

This commit is contained in:
Geido
2025-08-12 16:57:23 +03:00
committed by GitHub
parent 57d0e78d40
commit 3c17ff8445
21 changed files with 864 additions and 1334 deletions

View File

@@ -16,69 +16,180 @@
* specific language governing permissions and limitations
* under the License.
*/
import {
render,
screen,
userEvent,
waitFor,
} from 'spec/helpers/testing-library';
import { Menu } from '@superset-ui/core/components/Menu';
import DashboardItems from './DashboardsSubMenu';
import { render, screen } from 'spec/helpers/testing-library';
import type { MenuItemType } from '@superset-ui/core/components';
import { useDashboardsMenuItems } from './DashboardsSubMenu';
import { SEARCH_THRESHOLD } from './index';
const asyncRender = (numberOfItems: number) => {
const TestDashboardsMenuItems = ({
chartId,
dashboards,
searchTerm,
}: {
chartId?: number;
dashboards: { id: number; dashboard_title: string }[];
searchTerm?: string;
}) => {
const menuItems = useDashboardsMenuItems({
chartId,
dashboards,
searchTerm,
}) as MenuItemType[];
return (
<div data-test="menu-items">
{menuItems.map(item => (
<div key={item.key} data-test={`menu-item-${item!.key}`}>
{typeof item.label === 'string' ? item!.label : 'Complex Label'}
{item!.disabled && <span data-test="disabled">disabled</span>}
</div>
))}
</div>
);
};
const createDashboards = (numberOfItems: number) => {
const dashboards = [];
for (let i = 1; i <= numberOfItems; i += 1) {
dashboards.push({ id: i, dashboard_title: `Dashboard ${i}` });
}
render(
<Menu openKeys={['menu']}>
<Menu.SubMenu title="On dashboards" key="menu">
<DashboardItems key="menu" dashboards={dashboards} />
</Menu.SubMenu>
</Menu>,
{
useRouter: true,
},
);
return dashboards;
};
test('renders a submenu', async () => {
asyncRender(3);
await waitFor(() => {
expect(screen.getByText('Dashboard 1')).toBeInTheDocument();
expect(screen.getByText('Dashboard 2')).toBeInTheDocument();
expect(screen.getByText('Dashboard 3')).toBeInTheDocument();
describe('DashboardsSubMenu', () => {
test('exports SEARCH_THRESHOLD constant', () => {
expect(SEARCH_THRESHOLD).toBe(10);
});
test('renders menu items for dashboards', () => {
const dashboards = createDashboards(3);
render(
<TestDashboardsMenuItems
chartId={123}
dashboards={dashboards}
searchTerm=""
/>,
{ useRouter: true },
);
expect(screen.getByTestId('menu-item-1')).toBeInTheDocument();
expect(screen.getByTestId('menu-item-2')).toBeInTheDocument();
expect(screen.getByTestId('menu-item-3')).toBeInTheDocument();
});
test('filters dashboards based on search term', () => {
const dashboards = createDashboards(20);
render(
<TestDashboardsMenuItems
chartId={123}
dashboards={dashboards}
searchTerm="2"
/>,
{ useRouter: true },
);
// Should show Dashboard 2, Dashboard 12, and Dashboard 20
expect(screen.getByTestId('menu-item-2')).toBeInTheDocument();
expect(screen.getByTestId('menu-item-12')).toBeInTheDocument();
expect(screen.getByTestId('menu-item-20')).toBeInTheDocument();
// Should not show Dashboard 1
expect(screen.queryByTestId('menu-item-1')).not.toBeInTheDocument();
expect(screen.queryByTestId('menu-item-3')).not.toBeInTheDocument();
});
test('returns "No results found" when search has no matches', () => {
const dashboards = createDashboards(20);
render(
<TestDashboardsMenuItems
chartId={123}
dashboards={dashboards}
searchTerm="unknown"
/>,
{ useRouter: true },
);
expect(screen.getByTestId('menu-item-no-results')).toBeInTheDocument();
expect(screen.getByText('No results found')).toBeInTheDocument();
expect(screen.getByTestId('disabled')).toBeInTheDocument();
});
test('returns "None" when no dashboards provided', () => {
render(
<TestDashboardsMenuItems chartId={123} dashboards={[]} searchTerm="" />,
{ useRouter: true },
);
expect(screen.getByTestId('menu-item-no-dashboards')).toBeInTheDocument();
expect(screen.getByText('None')).toBeInTheDocument();
expect(screen.getByTestId('disabled')).toBeInTheDocument();
});
test('handles missing chart ID gracefully', () => {
const dashboards = createDashboards(1);
render(<TestDashboardsMenuItems dashboards={dashboards} searchTerm="" />, {
useRouter: true,
});
expect(screen.getByTestId('menu-item-1')).toBeInTheDocument();
});
test('case-insensitive search filtering', () => {
const dashboards = [
{ id: 1, dashboard_title: 'Sales Dashboard' },
{ id: 2, dashboard_title: 'Marketing Dashboard' },
{ id: 3, dashboard_title: 'Analytics Dashboard' },
];
render(
<TestDashboardsMenuItems
chartId={123}
dashboards={dashboards}
searchTerm="SALES"
/>,
{ useRouter: true },
);
expect(screen.getByTestId('menu-item-1')).toBeInTheDocument();
expect(screen.queryByTestId('menu-item-2')).not.toBeInTheDocument();
expect(screen.queryByTestId('menu-item-3')).not.toBeInTheDocument();
});
test('empty search term shows all dashboards', () => {
const dashboards = createDashboards(5);
render(
<TestDashboardsMenuItems
chartId={123}
dashboards={dashboards}
searchTerm=""
/>,
{ useRouter: true },
);
expect(screen.getByTestId('menu-item-1')).toBeInTheDocument();
expect(screen.getByTestId('menu-item-2')).toBeInTheDocument();
expect(screen.getByTestId('menu-item-3')).toBeInTheDocument();
expect(screen.getByTestId('menu-item-4')).toBeInTheDocument();
expect(screen.getByTestId('menu-item-5')).toBeInTheDocument();
});
test('partial string search works correctly', () => {
const dashboards = [
{ id: 1, dashboard_title: 'Revenue Report' },
{ id: 2, dashboard_title: 'User Engagement' },
{ id: 3, dashboard_title: 'Product Performance' },
];
render(
<TestDashboardsMenuItems
chartId={123}
dashboards={dashboards}
searchTerm="port"
/>,
{ useRouter: true },
);
expect(screen.getByTestId('menu-item-1')).toBeInTheDocument(); // Revenue Report
expect(screen.queryByTestId('menu-item-2')).not.toBeInTheDocument();
expect(screen.queryByTestId('menu-item-3')).not.toBeInTheDocument();
});
});
test('renders a submenu with search', async () => {
asyncRender(20);
expect(await screen.findByPlaceholderText('Search')).toBeInTheDocument();
});
test('displays a searched value', async () => {
asyncRender(20);
userEvent.type(screen.getByPlaceholderText('Search'), '2');
expect(await screen.findByText('Dashboard 2')).toBeInTheDocument();
expect(await screen.findByText('Dashboard 20')).toBeInTheDocument();
});
test('renders a "No results found" message when searching', async () => {
asyncRender(20);
userEvent.type(screen.getByPlaceholderText('Search'), 'unknown');
expect(await screen.findByText('No results found')).toBeInTheDocument();
});
test('renders a submenu with no dashboards', async () => {
asyncRender(0);
expect(await screen.findByText('None')).toBeInTheDocument();
});
test('shows link icon when hovering', async () => {
asyncRender(3);
expect(screen.queryByRole('img', { name: 'full' })).not.toBeInTheDocument();
userEvent.hover(await screen.findByText('Dashboard 1'));
expect(
(await screen.findAllByRole('img', { name: 'full' }))[0],
).toBeInTheDocument();
});

View File

@@ -16,130 +16,94 @@
* specific language governing permissions and limitations
* under the License.
*/
import { useState } from 'react';
import { useMemo } from 'react';
import { css, t, useTheme } from '@superset-ui/core';
import { Input } from '@superset-ui/core/components';
import { MenuItem } from '@superset-ui/core/components/Menu';
import { Icons } from '@superset-ui/core/components/Icons';
import { Menu } from '@superset-ui/core/components/Menu';
import { Link } from 'react-router-dom';
export interface DashboardsSubMenuProps {
export interface DashboardsMenuProps {
chartId?: number;
dashboards?: { id: number; dashboard_title: string }[];
searchTerm?: string;
}
const WIDTH = 220;
const HEIGHT = 300;
const SEARCH_THRESHOLD = 10;
const DashboardsSubMenu = ({
export const useDashboardsMenuItems = ({
chartId,
dashboards = [],
...menuProps
}: DashboardsSubMenuProps) => {
searchTerm = '',
}: DashboardsMenuProps): MenuItem[] => {
const theme = useTheme();
const [dashboardSearch, setDashboardSearch] = useState<string>();
const [hoveredItem, setHoveredItem] = useState<number | null>();
const showSearch = dashboards.length > SEARCH_THRESHOLD;
const filteredDashboards = dashboards.filter(
dashboard =>
!dashboardSearch ||
const filteredDashboards = useMemo(() => {
if (!searchTerm) return dashboards;
return dashboards.filter(dashboard =>
dashboard.dashboard_title
.toLowerCase()
.includes(dashboardSearch.toLowerCase()),
);
const noResults = dashboards.length === 0;
const noResultsFound = dashboardSearch && filteredDashboards.length === 0;
.includes(searchTerm.toLowerCase()),
);
}, [dashboards, searchTerm]);
const urlQueryString = chartId ? `?focused_chart=${chartId}` : '';
return (
<>
{showSearch && (
<Input
allowClear
placeholder={t('Search')}
prefix={<Icons.StarOutlined iconSize="l" />}
css={css`
width: ${WIDTH}px;
margin: ${theme.sizeUnit * 2}px ${theme.sizeUnit * 3}px;
`}
value={dashboardSearch}
onChange={e => setDashboardSearch(e.currentTarget.value)}
/>
)}
<div
css={css`
max-height: ${HEIGHT}px;
overflow: auto;
`}
>
{filteredDashboards.map(dashboard => (
<Menu.Item
key={String(dashboard.id)}
onMouseEnter={() => setHoveredItem(dashboard.id)}
onMouseLeave={() => {
if (hoveredItem === dashboard.id) {
setHoveredItem(null);
}
}}
{...menuProps}
>
const noResults = dashboards.length === 0;
const noResultsFound = searchTerm && filteredDashboards.length === 0;
return useMemo(() => {
const items: MenuItem[] = [];
if (noResults) {
items.push({
key: 'no-dashboards',
label: t('None'),
disabled: true,
});
} else if (noResultsFound) {
items.push({
key: 'no-results',
label: t('No results found'),
disabled: true,
});
} else {
filteredDashboards.forEach(dashboard => {
items.push({
key: String(dashboard.id),
label: (
<Link
target="_blank"
rel="noreferer noopener"
to={`/superset/dashboard/${dashboard.id}${urlQueryString}`}
css={css`
display: flex;
flex-direction: row;
align-items: center;
width: 200px;
justify-self: center;
`}
>
<div
css={css`
display: flex;
flex-direction: row;
align-items: center;
max-width: ${WIDTH}px;
white-space: normal;
flex: 1;
`}
>
<div
css={css`
white-space: normal;
`}
>
{dashboard.dashboard_title}
</div>
<Icons.Full
iconSize="l"
css={css`
margin-left: ${theme.sizeUnit * 2}px;
visibility: ${hoveredItem === dashboard.id
? 'visible'
: 'hidden'};
`}
/>
{dashboard.dashboard_title}
</div>
<Icons.Full
iconSize="l"
css={{ marginLeft: theme.sizeUnit * 2 }}
/>
</Link>
</Menu.Item>
))}
{noResultsFound && (
<div
css={css`
margin-left: ${theme.sizeUnit * 3}px;
margin-bottom: ${theme.sizeUnit}px;
`}
>
{t('No results found')}
</div>
)}
{noResults && (
<Menu.Item
disabled
css={css`
min-width: ${WIDTH}px;
`}
{...menuProps}
>
{t('None')}
</Menu.Item>
)}
</div>
</>
);
};
),
});
});
}
export default DashboardsSubMenu;
return items;
}, [
filteredDashboards,
urlQueryString,
noResults,
noResultsFound,
theme.sizeUnit,
]);
};

View File

@@ -18,6 +18,7 @@
*/
import { useCallback, useMemo, useState } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import { useDebounceValue } from 'src/hooks/useDebounceValue';
import {
css,
isFeatureEnabled,
@@ -27,7 +28,12 @@ import {
useTheme,
VizType,
} from '@superset-ui/core';
import { Icons, ModalTrigger, Button } from '@superset-ui/core/components';
import {
Icons,
ModalTrigger,
Button,
Input,
} from '@superset-ui/core/components';
import { Menu } from '@superset-ui/core/components/Menu';
import { useToasts } from 'src/components/MessageToasts/withToasts';
import { exportChart, getChartKey } from 'src/explore/exploreUtils';
@@ -46,7 +52,9 @@ import {
import exportPivotExcel from 'src/utils/downloadAsPivotExcel';
import ViewQueryModal from '../controls/ViewQueryModal';
import EmbedCodeContent from '../EmbedCodeContent';
import DashboardsSubMenu from './DashboardsSubMenu';
import { useDashboardsMenuItems } from './DashboardsSubMenu';
export const SEARCH_THRESHOLD = 10;
const MENU_KEYS = {
EDIT_PROPERTIES: 'edit_properties',
@@ -124,6 +132,11 @@ export const useExploreAdditionalActionsMenu = (
const { addDangerToast, addSuccessToast } = useToasts();
const dispatch = useDispatch();
const [isDropdownVisible, setIsDropdownVisible] = useState(false);
const [dashboardSearchTerm, setDashboardSearchTerm] = useState('');
const debouncedDashboardSearchTerm = useDebounceValue(
dashboardSearchTerm,
300,
);
const chart = useSelector(
state => state.charts?.[getChartKey(state.explore)],
);
@@ -137,6 +150,15 @@ export const useExploreAdditionalActionsMenu = (
const { datasource } = latestQueryFormData;
// Get dashboard menu items using the hook
const dashboardMenuItems = useDashboardsMenuItems({
chartId: slice?.slice_id,
dashboards,
searchTerm: debouncedDashboardSearchTerm,
});
const showDashboardSearch = dashboards?.length > SEARCH_THRESHOLD;
const shareByEmail = useCallback(async () => {
try {
const subject = t('Superset Chart');
@@ -225,21 +247,44 @@ export const useExploreAdditionalActionsMenu = (
}
// On dashboards submenu
const dashboardsChildren = [];
// Add search input if needed
if (showDashboardSearch) {
dashboardsChildren.push({
key: 'dashboard-search',
label: (
<Input
allowClear
placeholder={t('Search')}
prefix={<Icons.StarOutlined iconSize="l" />}
css={css`
width: 220px;
margin: ${theme.sizeUnit * 2}px ${theme.sizeUnit * 3}px;
`}
value={dashboardSearchTerm}
onChange={e => setDashboardSearchTerm(e.currentTarget.value)}
onClick={e => e.stopPropagation()}
/>
),
disabled: true, // Prevent clicks on the search input from closing menu
});
}
// Add dashboard items
dashboardMenuItems.forEach(item => {
dashboardsChildren.push(item);
});
menuItems.push({
key: MENU_KEYS.DASHBOARDS_ADDED_TO,
type: 'submenu',
label: t('On dashboards'),
children: [
{
key: 'dashboards-content',
label: (
<DashboardsSubMenu
chartId={slice?.slice_id}
dashboards={dashboards}
/>
),
},
],
children: dashboardsChildren,
popupStyle: {
maxHeight: '300px',
overflow: 'auto',
},
});
// Divider
@@ -479,6 +524,9 @@ export const useExploreAdditionalActionsMenu = (
canDownloadCSV,
copyLink,
dashboards,
dashboardMenuItems,
dashboardSearchTerm,
debouncedDashboardSearchTerm,
datasource,
dispatch,
exportCSV,
@@ -490,6 +538,7 @@ export const useExploreAdditionalActionsMenu = (
onOpenPropertiesModal,
reportMenuItem,
shareByEmail,
showDashboardSearch,
slice,
theme.sizeUnit,
]);