You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by di...@apache.org on 2025/09/12 16:36:50 UTC
(superset) 02/02: fix: Refactor Drill by context menu items
This is an automated email from the ASF dual-hosted git repository.
diegopucci pushed a commit to branch geido/fix/cosmetic-issues
in repository https://gitbox.apache.org/repos/asf/superset.git
commit c6d4c1cbc841f37e0b6efe3c9cd848d4f57c735a
Author: Diego Pucci <di...@gmail.com>
AuthorDate: Fri Sep 12 19:36:30 2025 +0300
fix: Refactor Drill by context menu items
---
.../Chart/ChartContextMenu/ChartContextMenu.tsx | 203 +++++--------
.../components/Chart/DrillBy/DrillByMenuItems.tsx | 274 -----------------
...yMenuItems.test.tsx => DrillBySubmenu.test.tsx} | 176 +++++------
.../components/Chart/DrillBy/DrillBySubmenu.tsx | 335 +++++++++++++++++++++
.../Chart/DrillDetail/DrillDetailMenuItems.tsx | 253 ----------------
.../src/components/Chart/DrillDetail/index.ts | 21 --
.../index.tsx} | 176 ++++++-----
.../useDrillDetailMenuItems.test.tsx} | 57 ++--
.../components/SliceHeaderControls/index.tsx | 22 +-
.../components/DataTablesPane/DataTablesPane.tsx | 6 +-
10 files changed, 605 insertions(+), 918 deletions(-)
diff --git a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx
index 457636c109..66cf5b7e3d 100644
--- a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx
+++ b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx
@@ -18,8 +18,6 @@
*/
import {
forwardRef,
- Key,
- ReactNode,
RefObject,
useCallback,
useImperativeHandle,
@@ -40,20 +38,18 @@ import {
isFeatureEnabled,
QueryFormData,
t,
- useTheme,
} from '@superset-ui/core';
import { RootState } from 'src/dashboard/types';
-import { Menu } from '@superset-ui/core/components/Menu';
+import { MenuItem } from '@superset-ui/core/components/Menu';
import { usePermissions } from 'src/hooks/usePermissions';
import { Dropdown } from '@superset-ui/core/components';
import { updateDataMask } from 'src/dataMask/actions';
import DrillByModal from 'src/components/Chart/DrillBy/DrillByModal';
import { useDatasetDrillInfo } from 'src/hooks/apiResources/datasets';
import { ResourceStatus } from 'src/hooks/apiResources/apiResources';
-import { DrillDetailMenuItems } from '../DrillDetail';
+import { useDrillDetailMenuItems } from '../useDrillDetailMenuItems';
import { getMenuAdjustedY } from '../utils';
-import { MenuItemTooltip } from '../DisabledMenuItemTooltip';
-import { DrillByMenuItems } from '../DrillBy/DrillByMenuItems';
+import { DrillBySubmenu } from '../DrillBy/DrillBySubmenu';
import DrillDetailModal from '../DrillDetail/DrillDetailModal';
export enum ContextMenuItem {
@@ -94,7 +90,6 @@ const ChartContextMenu = (
}: ChartContextMenuProps,
ref: RefObject<ChartContextMenuRef>,
) => {
- const theme = useTheme();
const dispatch = useDispatch();
const { canDrillToDetail, canDrillBy, canDownload } = usePermissions();
@@ -104,7 +99,6 @@ const ChartContextMenu = (
const dashboardId = useSelector<RootState, number>(
({ dashboardInfo }) => dashboardInfo.id,
);
- const [openKeys, setOpenKeys] = useState<Key[]>([]);
const [modalFilters, setFilters] = useState<BinaryQueryObjectFilterClause[]>(
[],
@@ -160,7 +154,6 @@ const ChartContextMenu = (
const closeContextMenu = useCallback(() => {
setVisible(false);
- setOpenKeys([]);
onClose();
}, [onClose]);
@@ -177,7 +170,7 @@ const ChartContextMenu = (
setShowDrillByModal(false);
}, []);
- const menuItems: React.JSX.Element[] = [];
+ const menuItems: MenuItem[] = [];
const showDrillToDetail =
isFeatureEnabled(FeatureFlag.DrillToDetail) &&
@@ -261,115 +254,79 @@ const ChartContextMenu = (
itemsCount = 1; // "No actions" appears if no actions in menu
}
+ // Call hook unconditionally (React hooks rule)
+ const drillDetailMenuItems = useDrillDetailMenuItems({
+ formData: drillFormData,
+ filters: filters?.drillToDetail,
+ setFilters,
+ isContextMenu: true,
+ contextMenuY: clientY,
+ onSelection,
+ submenuIndex: showCrossFilters ? 2 : 1,
+ setShowModal: setDrillModalIsOpen,
+ dataset: filteredDataset,
+ isLoadingDataset,
+ ...(additionalConfig?.drillToDetail || {}),
+ });
+
if (showCrossFilters) {
const isCrossFilterDisabled =
!isCrossFilteringSupportedByChart ||
!crossFiltersEnabled ||
!filters?.crossFilter;
- let crossFilteringTooltipTitle: ReactNode = null;
- if (!isCrossFilterDisabled) {
- crossFilteringTooltipTitle = (
- <>
- <div>
- {t(
- 'Cross-filter will be applied to all of the charts that use this dataset.',
- )}
- </div>
- <div>
- {t('You can also just click on the chart to apply cross-filter.')}
- </div>
- </>
- );
- } else if (!crossFiltersEnabled) {
- crossFilteringTooltipTitle = (
- <>
- <div>{t('Cross-filtering is not enabled for this dashboard.')}</div>
- </>
- );
- } else if (!isCrossFilteringSupportedByChart) {
- crossFilteringTooltipTitle = (
- <>
- <div>
- {t('This visualization type does not support cross-filtering.')}
- </div>
- </>
- );
- } else if (!filters?.crossFilter) {
- crossFilteringTooltipTitle = (
- <>
- <div>{t(`You can't apply cross-filter on this data point.`)}</div>
- </>
- );
- }
menuItems.push(
- <>
- <Menu.Item
- key="cross-filtering-menu-item"
- disabled={isCrossFilterDisabled}
- onClick={() => {
- if (filters?.crossFilter) {
- dispatch(updateDataMask(id, filters.crossFilter.dataMask));
- }
- }}
- >
- {filters?.crossFilter?.isCurrentValueSelected ? (
- t('Remove cross-filter')
- ) : (
- <div>
- {t('Add cross-filter')}
- <MenuItemTooltip
- title={crossFilteringTooltipTitle}
- color={!isCrossFilterDisabled ? theme.colorIcon : undefined}
- />
- </div>
- )}
- </Menu.Item>
- {itemsCount > 1 && <Menu.Divider />}
- </>,
+ {
+ key: 'cross-filtering-menu-item',
+ label: filters?.crossFilter?.isCurrentValueSelected
+ ? t('Remove cross-filter')
+ : t('Add cross-filter'),
+ disabled: isCrossFilterDisabled,
+ onClick: () => {
+ if (filters?.crossFilter) {
+ dispatch(updateDataMask(id, filters.crossFilter.dataMask));
+ }
+ },
+ },
+ ...(itemsCount > 1
+ ? [{ key: 'divider-1', type: 'divider' as const }]
+ : []),
);
}
if (showDrillToDetail) {
- menuItems.push(
- <DrillDetailMenuItems
- formData={drillFormData}
- filters={filters?.drillToDetail}
- setFilters={setFilters}
- isContextMenu
- contextMenuY={clientY}
- onSelection={onSelection}
- submenuIndex={showCrossFilters ? 2 : 1}
- setShowModal={setDrillModalIsOpen}
- dataset={filteredDataset}
- isLoadingDataset={isLoadingDataset}
- {...(additionalConfig?.drillToDetail || {})}
- />,
- );
+ menuItems.push(...drillDetailMenuItems);
}
+
if (showDrillBy) {
- let submenuIndex = 0;
- if (showCrossFilters) {
- submenuIndex += 1;
+ // Add divider before DrillBy if there are other menu items
+ if (menuItems.length > 0) {
+ menuItems.push({ key: 'divider-drill-by', type: 'divider' as const });
}
- if (showDrillToDetail) {
- submenuIndex += 2;
- }
- menuItems.push(
- <DrillByMenuItems
- drillByConfig={enhancedFilters?.drillBy}
- onSelection={onSelection}
- onCloseMenu={closeContextMenu}
- formData={formData}
- contextMenuY={clientY}
- submenuIndex={submenuIndex}
- open={openKeys.includes('drill-by-submenu')}
- key="drill-by-submenu"
- onDrillBy={handleDrillBy}
- dataset={filteredDataset}
- isLoadingDataset={isLoadingDataset}
- {...(additionalConfig?.drillBy || {})}
- />,
- );
+
+ // Determine if DrillBy should be disabled (same logic as in DrillBySubmenu)
+ const hasDrillBy = enhancedFilters?.drillBy?.groupbyFieldName;
+ const handlesDimensionContextMenu = getChartMetadataRegistry()
+ .get(formData.viz_type)
+ ?.behaviors.find(behavior => behavior === Behavior.DrillBy);
+ const isDrillByDisabled = !handlesDimensionContextMenu || !hasDrillBy;
+
+ // Add a custom render component for DrillBy submenu
+ menuItems.push({
+ key: 'drill-by-submenu',
+ disabled: isDrillByDisabled,
+ label: (
+ <DrillBySubmenu
+ drillByConfig={enhancedFilters?.drillBy}
+ onSelection={onSelection}
+ onCloseMenu={closeContextMenu}
+ formData={formData}
+ onDrillBy={handleDrillBy}
+ dataset={filteredDataset}
+ isLoadingDataset={isLoadingDataset}
+ {...(additionalConfig?.drillBy || {})}
+ />
+ ),
+ });
}
const open = useCallback(
@@ -401,30 +358,22 @@ const ChartContextMenu = (
return ReactDOM.createPortal(
<>
<Dropdown
- popupRender={() => (
- <Menu
- className="chart-context-menu"
- data-test="chart-context-menu"
- onOpenChange={setOpenKeys}
- onClick={() => {
- setVisible(false);
- setOpenKeys([]);
- onClose();
- }}
- >
- {menuItems.length ? (
- menuItems
- ) : (
- <Menu.Item disabled>{t('No actions')}</Menu.Item>
- )}
- </Menu>
+ menu={{
+ items:
+ menuItems.length > 0
+ ? menuItems
+ : [{ key: 'no-actions', label: t('No actions'), disabled: true }],
+ onClick: () => {
+ setVisible(false);
+ onClose();
+ },
+ }}
+ dropdownRender={menu => (
+ <div data-test="chart-context-menu">{menu}</div>
)}
trigger={['click']}
onOpenChange={value => {
setVisible(value);
- if (!value) {
- setOpenKeys([]);
- }
}}
open={visible}
>
diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx
deleted file mode 100644
index 3ac4a6dd8f..0000000000
--- a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx
+++ /dev/null
@@ -1,274 +0,0 @@
-/**
- * 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 {
- CSSProperties,
- ReactNode,
- useCallback,
- useEffect,
- useMemo,
- useRef,
- useState,
-} from 'react';
-import { Menu } from '@superset-ui/core/components/Menu';
-import {
- BaseFormData,
- Behavior,
- Column,
- ContextMenuFilters,
- css,
- ensureIsArray,
- getChartMetadataRegistry,
- t,
- useTheme,
-} from '@superset-ui/core';
-import { Constants, Input, Loading } from '@superset-ui/core/components';
-import { debounce } from 'lodash';
-import { FixedSizeList as List } from 'react-window';
-import { Icons } from '@superset-ui/core/components/Icons';
-import { InputRef } from 'antd';
-import { MenuItemTooltip } from '../DisabledMenuItemTooltip';
-import { getSubmenuYOffset } from '../utils';
-import { VirtualizedMenuItem } from '../MenuItemWithTruncation';
-import { Dataset } from '../types';
-
-const SUBMENU_HEIGHT = 200;
-const SHOW_COLUMNS_SEARCH_THRESHOLD = 10;
-const SEARCH_INPUT_HEIGHT = 48;
-
-export interface DrillByMenuItemsProps {
- drillByConfig?: ContextMenuFilters['drillBy'];
- formData: BaseFormData & { [key: string]: any };
- contextMenuY?: number;
- submenuIndex?: number;
- onSelection?: (...args: any) => void;
- onClick?: (event: MouseEvent) => void;
- onCloseMenu?: () => void;
- openNewModal?: boolean;
- excludedColumns?: Column[];
- open: boolean;
- onDrillBy?: (column: Column, dataset: Dataset) => void;
- dataset?: Dataset;
- isLoadingDataset?: boolean;
-}
-
-export const DrillByMenuItems = ({
- drillByConfig,
- formData,
- contextMenuY = 0,
- submenuIndex = 0,
- onSelection = () => {},
- onClick = () => {},
- onCloseMenu = () => {},
- excludedColumns,
- openNewModal = true,
- open,
- onDrillBy,
- dataset,
- isLoadingDataset = false,
- ...rest
-}: DrillByMenuItemsProps) => {
- const theme = useTheme();
- const [searchInput, setSearchInput] = useState('');
- const [debouncedSearchInput, setDebouncedSearchInput] = useState('');
- const ref = useRef<InputRef>(null);
- const columns = dataset ? ensureIsArray(dataset.drillable_columns) : [];
- const showSearch = columns.length > SHOW_COLUMNS_SEARCH_THRESHOLD;
-
- const handleSelection = useCallback(
- (event, column) => {
- onClick(event);
- onSelection(column, drillByConfig);
- if (openNewModal && onDrillBy && dataset) {
- onDrillBy(column, dataset);
- }
- onCloseMenu();
- },
- [drillByConfig, onClick, onSelection, openNewModal, onDrillBy, dataset],
- );
-
- useEffect(() => {
- if (open) {
- ref.current?.input?.focus({ preventScroll: true });
- } else {
- // Reset search input when menu is closed
- setSearchInput('');
- setDebouncedSearchInput('');
- }
- }, [open]);
-
- const hasDrillBy = drillByConfig?.groupbyFieldName;
-
- const handlesDimensionContextMenu = useMemo(
- () =>
- getChartMetadataRegistry()
- .get(formData.viz_type)
- ?.behaviors.find(behavior => behavior === Behavior.DrillBy),
- [formData.viz_type],
- );
-
- const debouncedSetSearchInput = useMemo(
- () =>
- debounce((value: string) => {
- setDebouncedSearchInput(value);
- }, Constants.FAST_DEBOUNCE),
- [],
- );
-
- const handleInput = (value: string) => {
- setSearchInput(value);
- debouncedSetSearchInput(value);
- };
-
- const filteredColumns = useMemo(
- () =>
- columns.filter(column =>
- (column.verbose_name || column.column_name)
- .toLowerCase()
- .includes(debouncedSearchInput.toLowerCase()),
- ),
- [columns, debouncedSearchInput],
- );
-
- const submenuYOffset = useMemo(
- () =>
- getSubmenuYOffset(
- contextMenuY,
- filteredColumns.length || 1,
- submenuIndex,
- SUBMENU_HEIGHT,
- showSearch ? SEARCH_INPUT_HEIGHT : 0,
- ),
- [contextMenuY, filteredColumns.length, submenuIndex, showSearch],
- );
-
- let tooltip: ReactNode;
-
- if (!handlesDimensionContextMenu) {
- tooltip = t('Drill by is not yet supported for this chart type');
- } else if (!hasDrillBy) {
- tooltip = t('Drill by is not available for this data point');
- }
-
- if (!handlesDimensionContextMenu || !hasDrillBy) {
- return (
- <Menu.Item key="drill-by-disabled" disabled {...rest}>
- <div>
- {t('Drill by')}
- <MenuItemTooltip title={tooltip} />
- </div>
- </Menu.Item>
- );
- }
-
- const Row = ({
- index,
- data,
- style,
- }: {
- index: number;
- data: { columns: Column[] };
- style: CSSProperties;
- }) => {
- const { columns, ...rest } = data;
- const column = columns[index];
- return (
- <VirtualizedMenuItem
- tooltipText={column.verbose_name || column.column_name}
- onClick={e => handleSelection(e, column)}
- style={style}
- {...rest}
- >
- {column.verbose_name || column.column_name}
- </VirtualizedMenuItem>
- );
- };
-
- // Don't render drill by menu items when matrixify is enabled
- if (formData.matrixify_enabled) {
- return null;
- }
-
- return (
- <>
- <Menu.SubMenu
- key="drill-by-submenu"
- title={t('Drill by')}
- popupClassName="chart-context-submenu"
- popupOffset={[0, submenuYOffset]}
- {...rest}
- >
- <div data-test="drill-by-submenu">
- {showSearch && (
- <Input
- ref={ref}
- prefix={
- <Icons.SearchOutlined
- iconSize="l"
- iconColor={theme.colorIcon}
- />
- }
- onChange={e => {
- e.stopPropagation();
- handleInput(e.target.value);
- }}
- placeholder={t('Search columns')}
- onClick={e => {
- // prevent closing menu when clicking on input
- e.nativeEvent.stopImmediatePropagation();
- }}
- allowClear
- css={css`
- width: auto;
- max-width: 100%;
- margin: ${theme.sizeUnit * 2}px ${theme.sizeUnit * 3}px;
- box-shadow: none;
- `}
- value={searchInput}
- />
- )}
- {isLoadingDataset ? (
- <div
- css={css`
- padding: ${theme.sizeUnit * 3}px 0;
- `}
- >
- <Loading position="inline-centered" />
- </div>
- ) : filteredColumns.length ? (
- <List
- width="100%"
- height={SUBMENU_HEIGHT}
- itemSize={35}
- itemCount={filteredColumns.length}
- itemData={{ columns: filteredColumns, ...rest }}
- overscanCount={20}
- >
- {Row}
- </List>
- ) : (
- <Menu.Item disabled key="no-drill-by-columns-found" {...rest}>
- {t('No columns found')}
- </Menu.Item>
- )}
- </div>
- </Menu.SubMenu>
- </>
- );
-};
diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.test.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.test.tsx
similarity index 61%
rename from superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.test.tsx
rename to superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.test.tsx
index 13410f8031..4be07b2c93 100644
--- a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.test.tsx
+++ b/superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.test.tsx
@@ -30,9 +30,8 @@ import {
waitFor,
} from 'spec/helpers/testing-library';
import chartQueries, { sliceId } from 'spec/fixtures/mockChartQueries';
-import { Menu } from '@superset-ui/core/components/Menu';
import { supersetGetCache } from 'src/utils/cachedSupersetGet';
-import { DrillByMenuItems, DrillByMenuItemsProps } from './DrillByMenuItems';
+import { DrillBySubmenu, DrillBySubmenuProps } from './DrillBySubmenu';
/* eslint jest/expect-expect: ["warn", { "assertFunctionNames": ["expect*"] }] */
@@ -79,37 +78,29 @@ const defaultFilters = [
},
];
-const renderMenu = ({
+const renderSubmenu = ({
formData = defaultFormData,
drillByConfig = { filters: defaultFilters, groupbyFieldName: 'groupby' },
dataset = mockDataset,
...rest
-}: Partial<DrillByMenuItemsProps>) =>
+}: Partial<DrillBySubmenuProps>) =>
render(
- <Menu forceSubMenuRender>
- <DrillByMenuItems
- formData={formData ?? defaultFormData}
- drillByConfig={drillByConfig}
- dataset={dataset}
- open
- {...rest}
- />
- </Menu>,
+ <DrillBySubmenu
+ formData={formData ?? defaultFormData}
+ drillByConfig={drillByConfig}
+ dataset={dataset}
+ {...rest}
+ />,
{ useRouter: true, useRedux: true },
);
const expectDrillByDisabled = async (tooltipContent: string) => {
- const drillByMenuItem = screen
- .getAllByRole('menuitem')
- .find(menuItem => within(menuItem).queryByText('Drill by'));
+ const drillByButton = screen.getByRole('button', { name: /drill by/i });
+ expect(drillByButton).toBeInTheDocument();
+ expect(drillByButton).toBeVisible();
+ expect(drillByButton).toHaveAttribute('tabindex', '-1');
- expect(drillByMenuItem).toBeDefined();
- expect(drillByMenuItem).toBeVisible();
- expect(drillByMenuItem).toHaveAttribute('aria-disabled', 'true');
-
- const tooltipTrigger = within(drillByMenuItem!).getByTestId(
- 'tooltip-trigger',
- );
+ const tooltipTrigger = within(drillByButton).getByTestId('tooltip-trigger');
userEvent.hover(tooltipTrigger as HTMLElement);
const tooltip = await screen.findByRole('tooltip', { name: tooltipContent });
@@ -117,20 +108,21 @@ const expectDrillByDisabled = async (tooltipContent: string) => {
};
const expectDrillByEnabled = async () => {
- const drillByMenuItem = screen.getByRole('menuitem', {
- name: 'Drill by',
- });
- expect(drillByMenuItem).toBeInTheDocument();
- await waitFor(() =>
- expect(drillByMenuItem).not.toHaveAttribute('aria-disabled'),
- );
- const tooltipTrigger =
- within(drillByMenuItem).queryByTestId('tooltip-trigger');
+ const drillByButton = screen.getByRole('button', { name: /drill by/i });
+ expect(drillByButton).toBeInTheDocument();
+ expect(drillByButton).not.toHaveAttribute('tabindex', '-1');
+
+ const tooltipTrigger = within(drillByButton).queryByTestId('tooltip-trigger');
expect(tooltipTrigger).not.toBeInTheDocument();
- userEvent.hover(within(drillByMenuItem).getByText('Drill by'));
- const drillBySubmenus = await screen.findAllByTestId('drill-by-submenu');
- expect(drillBySubmenus[0]).toBeInTheDocument();
+ // Hover over the button to open the popover
+ userEvent.hover(drillByButton);
+
+ // Wait for popover to appear
+ await waitFor(() => {
+ const popover = screen.getByRole('menu');
+ expect(popover).toBeInTheDocument();
+ });
};
getChartMetadataRegistry().registerValue(
@@ -149,7 +141,7 @@ afterEach(() => {
});
test('render disabled menu item for unsupported chart', async () => {
- renderMenu({
+ renderSubmenu({
formData: { ...defaultFormData, viz_type: 'unsupported_viz' },
});
await expectDrillByDisabled(
@@ -158,60 +150,54 @@ test('render disabled menu item for unsupported chart', async () => {
});
test('render enabled menu item for supported chart, no filters', async () => {
- renderMenu({ drillByConfig: { filters: [], groupbyFieldName: 'groupby' } });
+ renderSubmenu({
+ drillByConfig: { filters: [], groupbyFieldName: 'groupby' },
+ });
await expectDrillByEnabled();
});
test('render disabled menu item for supported chart, no columns', async () => {
const emptyDataset = { ...mockDataset, columns: [], drillable_columns: [] };
- renderMenu({ dataset: emptyDataset });
+ renderSubmenu({ dataset: emptyDataset });
await expectDrillByEnabled();
- screen.getByText('No columns found');
+
+ // Wait for popover content to load
+ await waitFor(() => {
+ expect(screen.getByText('No columns found')).toBeInTheDocument();
+ });
});
test('render menu item with submenu without searchbox', async () => {
- const slicedColumns = defaultColumns.slice(0, 9);
+ const slicedColumns = defaultColumns.slice(0, 1); // Use only 1 column to avoid search box
const datasetWithSlicedColumns = {
...mockDataset,
columns: slicedColumns,
drillable_columns: slicedColumns,
};
- renderMenu({ dataset: datasetWithSlicedColumns });
+ renderSubmenu({ dataset: datasetWithSlicedColumns });
await expectDrillByEnabled();
- // Check that each column appears in the drill-by submenu
- slicedColumns.forEach(column => {
- const submenus = screen.getAllByTestId('drill-by-submenu');
- const submenu = submenus[0]; // Use the first submenu
- expect(within(submenu).getByText(column.column_name)).toBeInTheDocument();
+ // Check that the column appears in the popover
+ await waitFor(() => {
+ expect(screen.getByText('col1')).toBeInTheDocument();
});
+
+ // Should not have search box for small number of columns
expect(screen.queryByRole('textbox')).not.toBeInTheDocument();
});
-// Add global timeout for all tests
-jest.setTimeout(20000);
-
test('render menu item with submenu and searchbox', async () => {
- renderMenu({ dataset: mockDataset });
+ renderSubmenu({ dataset: mockDataset });
await expectDrillByEnabled();
- // Wait for all columns to be visible
- await waitFor(
- () => {
- const submenus = screen.getAllByTestId('drill-by-submenu');
- const submenu = submenus[0];
- defaultColumns.forEach(column => {
- expect(
- within(submenu).getByText(column.column_name),
- ).toBeInTheDocument();
- });
- },
- { timeout: 10000 },
- );
+ // Wait for all columns to be visible in the popover
+ await waitFor(() => {
+ defaultColumns.forEach(column => {
+ expect(screen.getByText(column.column_name)).toBeInTheDocument();
+ });
+ });
- const searchbox = await waitFor(
- () => screen.getAllByPlaceholderText('Search columns')[0],
- );
+ const searchbox = screen.getByPlaceholderText('Search columns');
expect(searchbox).toBeInTheDocument();
userEvent.type(searchbox, 'col1');
@@ -220,27 +206,17 @@ test('render menu item with submenu and searchbox', async () => {
// Wait for filtered results
await waitFor(() => {
- const submenus = screen.getAllByTestId('drill-by-submenu');
- const submenu = submenus[0];
expectedFilteredColumnNames.forEach(colName => {
- expect(within(submenu).getByText(colName)).toBeInTheDocument();
+ expect(screen.getByText(colName)).toBeInTheDocument();
});
});
- const submenus = screen.getAllByTestId('drill-by-submenu');
- const submenu = submenus[0];
-
+ // Check that non-matching columns are not visible
defaultColumns
.filter(col => !expectedFilteredColumnNames.includes(col.column_name))
.forEach(col => {
- expect(
- within(submenu).queryByText(col.column_name),
- ).not.toBeInTheDocument();
+ expect(screen.queryByText(col.column_name)).not.toBeInTheDocument();
});
-
- expectedFilteredColumnNames.forEach(colName => {
- expect(within(submenu).getByText(colName)).toBeInTheDocument();
- });
});
test('Do not display excluded column in the menu', async () => {
@@ -252,7 +228,7 @@ test('Do not display excluded column in the menu', async () => {
...mockDataset,
drillable_columns: filteredColumns,
};
- renderMenu({
+ renderSubmenu({
dataset: datasetWithFilteredColumns,
excludedColumns: excludedColNames.map(colName => ({
column_name: colName,
@@ -262,31 +238,22 @@ test('Do not display excluded column in the menu', async () => {
await expectDrillByEnabled();
// Wait for menu items to be loaded
- await waitFor(
- () => {
- const submenus = screen.getAllByTestId('drill-by-submenu');
- const submenu = submenus[0];
- defaultColumns
- .filter(column => !excludedColNames.includes(column.column_name))
- .forEach(column => {
- expect(
- within(submenu).getByText(column.column_name),
- ).toBeInTheDocument();
- });
- },
- { timeout: 10000 },
- );
+ await waitFor(() => {
+ defaultColumns
+ .filter(column => !excludedColNames.includes(column.column_name))
+ .forEach(column => {
+ expect(screen.getByText(column.column_name)).toBeInTheDocument();
+ });
+ });
- const submenus = screen.getAllByTestId('drill-by-submenu');
- const submenu = submenus[0];
excludedColNames.forEach(colName => {
- expect(within(submenu).queryByText(colName)).not.toBeInTheDocument();
+ expect(screen.queryByText(colName)).not.toBeInTheDocument();
});
});
test('When menu item is clicked, call onSelection with clicked column and drill by filters', async () => {
const onSelectionMock = jest.fn();
- renderMenu({
+ renderSubmenu({
dataset: mockDataset,
onSelection: onSelectionMock,
});
@@ -294,11 +261,7 @@ test('When menu item is clicked, call onSelection with clicked column and drill
await expectDrillByEnabled();
// Wait for col1 to be visible before clicking
- const col1Element = await waitFor(() => {
- const submenus = screen.getAllByTestId('drill-by-submenu');
- const submenu = submenus[0];
- return within(submenu).getByText('col1');
- });
+ const col1Element = await waitFor(() => screen.getByText('col1'));
userEvent.click(col1Element);
expect(onSelectionMock).toHaveBeenCalledWith(
@@ -309,3 +272,10 @@ test('When menu item is clicked, call onSelection with clicked column and drill
{ filters: defaultFilters, groupbyFieldName: 'groupby' },
);
});
+
+test('matrixify_enabled should not render component', () => {
+ const { container } = renderSubmenu({
+ formData: { ...defaultFormData, matrixify_enabled: true },
+ });
+ expect(container).toBeEmptyDOMElement();
+});
diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.tsx
new file mode 100644
index 0000000000..538be60255
--- /dev/null
+++ b/superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.tsx
@@ -0,0 +1,335 @@
+/**
+ * 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 {
+ CSSProperties,
+ ReactNode,
+ useCallback,
+ useEffect,
+ useMemo,
+ useRef,
+ useState,
+} from 'react';
+import {
+ BaseFormData,
+ Behavior,
+ Column,
+ ContextMenuFilters,
+ css,
+ ensureIsArray,
+ getChartMetadataRegistry,
+ t,
+ useTheme,
+} from '@superset-ui/core';
+import {
+ Constants,
+ Input,
+ Loading,
+ Popover,
+ Icons,
+} from '@superset-ui/core/components';
+import { debounce } from 'lodash';
+import { FixedSizeList as List } from 'react-window';
+import { InputRef } from 'antd';
+import { MenuItemTooltip } from '../DisabledMenuItemTooltip';
+import { VirtualizedMenuItem } from '../MenuItemWithTruncation';
+import { Dataset } from '../types';
+
+const SUBMENU_HEIGHT = 200;
+const SHOW_COLUMNS_SEARCH_THRESHOLD = 10;
+
+export interface DrillBySubmenuProps {
+ drillByConfig?: ContextMenuFilters['drillBy'];
+ formData: BaseFormData & { [key: string]: any };
+ onSelection?: (...args: any) => void;
+ onClick?: (event: MouseEvent) => void;
+ onCloseMenu?: () => void;
+ openNewModal?: boolean;
+ excludedColumns?: Column[];
+ onDrillBy?: (column: Column, dataset: Dataset) => void;
+ dataset?: Dataset;
+ isLoadingDataset?: boolean;
+}
+
+export const DrillBySubmenu = ({
+ drillByConfig,
+ formData,
+ onSelection = () => {},
+ onClick = () => {},
+ onCloseMenu = () => {},
+ openNewModal = true,
+ excludedColumns,
+ onDrillBy,
+ dataset,
+ isLoadingDataset = false,
+ ...rest
+}: DrillBySubmenuProps) => {
+ const theme = useTheme();
+ const [searchInput, setSearchInput] = useState('');
+ const [debouncedSearchInput, setDebouncedSearchInput] = useState('');
+ const [popoverOpen, setPopoverOpen] = useState(false);
+ const ref = useRef<InputRef>(null);
+ const menuItemRef = useRef<HTMLDivElement>(null);
+
+ const columns = useMemo(
+ () => (dataset ? ensureIsArray(dataset.drillable_columns) : []),
+ [dataset],
+ );
+ const showSearch = columns.length > SHOW_COLUMNS_SEARCH_THRESHOLD;
+
+ const handleSelection = useCallback(
+ (event, column) => {
+ onClick(event);
+ onSelection(column, drillByConfig);
+ if (openNewModal && onDrillBy && dataset) {
+ onDrillBy(column, dataset);
+ }
+ setPopoverOpen(false);
+ onCloseMenu();
+ },
+ [
+ drillByConfig,
+ onClick,
+ onSelection,
+ openNewModal,
+ onDrillBy,
+ dataset,
+ onCloseMenu,
+ ],
+ );
+
+ useEffect(() => {
+ if (popoverOpen) {
+ // Small delay to ensure popover is rendered
+ setTimeout(() => {
+ ref.current?.input?.focus({ preventScroll: true });
+ }, 100);
+ } else {
+ // Reset search input when menu is closed
+ setSearchInput('');
+ setDebouncedSearchInput('');
+ }
+ }, [popoverOpen]);
+
+ const hasDrillBy = drillByConfig?.groupbyFieldName;
+
+ const handlesDimensionContextMenu = useMemo(
+ () =>
+ getChartMetadataRegistry()
+ .get(formData.viz_type)
+ ?.behaviors.find(behavior => behavior === Behavior.DrillBy),
+ [formData.viz_type],
+ );
+
+ const debouncedSetSearchInput = useMemo(
+ () =>
+ debounce((value: string) => {
+ setDebouncedSearchInput(value);
+ }, Constants.FAST_DEBOUNCE),
+ [],
+ );
+
+ const handleInput = (value: string) => {
+ setSearchInput(value);
+ debouncedSetSearchInput(value);
+ };
+
+ const filteredColumns = useMemo(
+ () =>
+ columns
+ .filter(column => {
+ // Filter out excluded columns
+ const excludedColumnNames =
+ excludedColumns?.map(col => col.column_name) || [];
+ return !excludedColumnNames.includes(column.column_name);
+ })
+ .filter(column =>
+ (column.verbose_name || column.column_name)
+ .toLowerCase()
+ .includes(debouncedSearchInput.toLowerCase()),
+ ),
+ [columns, debouncedSearchInput, excludedColumns],
+ );
+
+ let tooltip: ReactNode;
+
+ if (!handlesDimensionContextMenu) {
+ tooltip = t('Drill by is not yet supported for this chart type');
+ } else if (!hasDrillBy) {
+ tooltip = t('Drill by is not available for this data point');
+ }
+
+ // Don't render drill by menu items when matrixify is enabled
+ if (formData.matrixify_enabled) {
+ return null;
+ }
+
+ const isDisabled = !handlesDimensionContextMenu || !hasDrillBy;
+
+ const Row = ({
+ index,
+ data,
+ style,
+ }: {
+ index: number;
+ data: { columns: Column[] };
+ style: CSSProperties;
+ }) => {
+ const { columns } = data;
+ const column = columns[index];
+ return (
+ <VirtualizedMenuItem
+ tooltipText={column.verbose_name || column.column_name}
+ onClick={e => handleSelection(e, column)}
+ style={style}
+ >
+ {column.verbose_name || column.column_name}
+ </VirtualizedMenuItem>
+ );
+ };
+
+ const popoverContent = (
+ <div
+ role="menu"
+ tabIndex={0}
+ css={css`
+ width: 220px;
+ max-width: 220px;
+ .ant-input-affix-wrapper {
+ margin-bottom: ${theme.sizeUnit * 2}px;
+ }
+ `}
+ onClick={e => e.stopPropagation()}
+ >
+ {showSearch && (
+ <Input
+ ref={ref}
+ prefix={
+ <Icons.SearchOutlined iconSize="l" iconColor={theme.colorIcon} />
+ }
+ onChange={e => {
+ e.stopPropagation();
+ handleInput(e.target.value);
+ }}
+ placeholder={t('Search columns')}
+ onClick={e => {
+ // prevent closing menu when clicking on input
+ e.nativeEvent.stopImmediatePropagation();
+ }}
+ allowClear
+ css={css`
+ width: 100%;
+ box-shadow: none;
+ `}
+ value={searchInput}
+ />
+ )}
+ {isLoadingDataset ? (
+ <div
+ css={css`
+ padding: ${theme.sizeUnit * 3}px 0;
+ `}
+ >
+ <Loading position="inline-centered" />
+ </div>
+ ) : filteredColumns.length ? (
+ <List
+ width="100%"
+ height={SUBMENU_HEIGHT}
+ itemSize={35}
+ itemCount={filteredColumns.length}
+ itemData={{ columns: filteredColumns }}
+ overscanCount={20}
+ >
+ {Row}
+ </List>
+ ) : (
+ <div
+ css={css`
+ padding: ${theme.sizeUnit * 2}px;
+ color: ${theme.colorTextDisabled};
+ text-align: center;
+ `}
+ >
+ {t('No columns found')}
+ </div>
+ )}
+ </div>
+ );
+
+ const menuItem = (
+ <div
+ ref={menuItemRef}
+ role="button"
+ tabIndex={isDisabled ? -1 : 0}
+ css={css`
+ display: flex;
+ align-items: center;
+ justify-content: space-between;
+ cursor: ${isDisabled ? 'not-allowed' : 'pointer'};
+ color: ${isDisabled ? theme.colorTextDisabled : 'inherit'};
+ &:hover {
+ background: transparent;
+ }
+ `}
+ onClick={() => !isDisabled && setPopoverOpen(!popoverOpen)}
+ onKeyDown={e => {
+ if (!isDisabled && (e.key === 'Enter' || e.key === ' ')) {
+ e.preventDefault();
+ setPopoverOpen(!popoverOpen);
+ }
+ }}
+ >
+ <span>{t('Drill by')}</span>
+ {isDisabled ? (
+ <MenuItemTooltip title={tooltip} />
+ ) : (
+ <Icons.RightOutlined iconSize="s" iconColor={theme.colorTextTertiary} />
+ )}
+ </div>
+ );
+
+ if (isDisabled) {
+ return menuItem;
+ }
+
+ return (
+ <Popover
+ content={popoverContent}
+ placement="rightTop"
+ open={popoverOpen}
+ onOpenChange={setPopoverOpen}
+ trigger={['hover', 'click']}
+ arrow={false}
+ styles={{
+ root: {
+ paddingLeft: 0,
+ },
+ body: {
+ padding: theme.sizeUnit * 2,
+ boxShadow: theme.boxShadow,
+ borderRadius: theme.borderRadius,
+ },
+ }}
+ {...rest}
+ >
+ {menuItem}
+ </Popover>
+ );
+};
diff --git a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailMenuItems.tsx b/superset-frontend/src/components/Chart/DrillDetail/DrillDetailMenuItems.tsx
deleted file mode 100644
index 69d9d20cfa..0000000000
--- a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailMenuItems.tsx
+++ /dev/null
@@ -1,253 +0,0 @@
-/**
- * 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 {
- Dispatch,
- ReactNode,
- SetStateAction,
- useCallback,
- useMemo,
-} from 'react';
-import { isEmpty } from 'lodash';
-import {
- Behavior,
- BinaryQueryObjectFilterClause,
- css,
- extractQueryFields,
- getChartMetadataRegistry,
- QueryFormData,
- removeHTMLTags,
- styled,
- t,
-} from '@superset-ui/core';
-import { useSelector } from 'react-redux';
-import { Menu } from '@superset-ui/core/components/Menu';
-import { RootState } from 'src/dashboard/types';
-import { getSubmenuYOffset } from '../utils';
-import { MenuItemTooltip } from '../DisabledMenuItemTooltip';
-import { MenuItemWithTruncation } from '../MenuItemWithTruncation';
-import { Dataset } from '../types';
-
-const DRILL_TO_DETAIL = t('Drill to detail');
-const DRILL_TO_DETAIL_BY = t('Drill to detail by');
-const DISABLED_REASONS = {
- DATABASE: t(
- 'Drill to detail is disabled for this database. Change the database settings to enable it.',
- ),
- NO_AGGREGATIONS: t(
- 'Drill to detail is disabled because this chart does not group data by dimension value.',
- ),
- NO_FILTERS: t(
- 'Right-click on a dimension value to drill to detail by that value.',
- ),
- NOT_SUPPORTED: t(
- 'Drill to detail by value is not yet supported for this chart type.',
- ),
-};
-
-const DisabledMenuItem = ({
- children,
- menuKey,
- ...rest
-}: {
- children: ReactNode;
- menuKey: string;
-}) => (
- <Menu.Item disabled key={menuKey} {...rest}>
- <div
- css={css`
- white-space: normal;
- max-width: 160px;
- `}
- >
- {children}
- </div>
- </Menu.Item>
-);
-
-const Filter = ({
- children,
- stripHTML = false,
-}: {
- children: ReactNode;
- stripHTML: boolean;
-}) => {
- const content =
- stripHTML && typeof children === 'string'
- ? removeHTMLTags(children)
- : children;
- return <span>{content}</span>;
-};
-
-const StyledFilter = styled(Filter)`
- ${({ theme }) => `
- font-weight: ${theme.fontWeightStrong};
- color: ${theme.colorPrimary};
- `}
-`;
-
-export type DrillDetailMenuItemsProps = {
- formData: QueryFormData;
- filters?: BinaryQueryObjectFilterClause[];
- setFilters: Dispatch<SetStateAction<BinaryQueryObjectFilterClause[]>>;
- isContextMenu?: boolean;
- contextMenuY?: number;
- onSelection?: () => void;
- onClick?: (event: MouseEvent) => void;
- submenuIndex?: number;
- setShowModal: (show: boolean) => void;
- key?: string;
- forceSubmenuRender?: boolean;
- dataset?: Dataset;
- isLoadingDataset?: boolean;
-};
-
-const DrillDetailMenuItems = ({
- formData,
- filters = [],
- isContextMenu = false,
- contextMenuY = 0,
- onSelection = () => null,
- onClick = () => null,
- submenuIndex = 0,
- setFilters,
- setShowModal,
- key,
- ...props
-}: DrillDetailMenuItemsProps) => {
- const drillToDetailDisabled = useSelector<RootState, boolean | undefined>(
- ({ datasources }) =>
- datasources[formData.datasource]?.database?.disable_drill_to_detail,
- );
-
- const openModal = useCallback(
- (filters, event) => {
- onClick(event);
- onSelection();
- setFilters(filters);
- setShowModal(true);
- },
- [onClick, onSelection],
- );
-
- // Check for Behavior.DRILL_TO_DETAIL to tell if plugin handles the `contextmenu`
- // event for dimensions. If it doesn't, tell the user that drill to detail by
- // dimension is not supported. If it does, and the `contextmenu` handler didn't
- // pass any filters, tell the user that they didn't select a dimension.
- const handlesDimensionContextMenu = useMemo(
- () =>
- getChartMetadataRegistry()
- .get(formData.viz_type)
- ?.behaviors.find(behavior => behavior === Behavior.DrillToDetail),
- [formData.viz_type],
- );
-
- // Check metrics to see if chart's current configuration lacks
- // aggregations, in which case Drill to Detail should be disabled.
- const noAggregations = useMemo(() => {
- const { metrics } = extractQueryFields(formData);
- return isEmpty(metrics);
- }, [formData]);
-
- // Ensure submenu doesn't appear offscreen
- const submenuYOffset = useMemo(
- () =>
- getSubmenuYOffset(
- contextMenuY,
- filters.length > 1 ? filters.length + 1 : filters.length,
- submenuIndex,
- ),
- [contextMenuY, filters.length, submenuIndex],
- );
-
- let drillDisabled;
- let drillByDisabled;
- if (drillToDetailDisabled) {
- drillDisabled = DISABLED_REASONS.DATABASE;
- drillByDisabled = DISABLED_REASONS.DATABASE;
- } else if (handlesDimensionContextMenu) {
- if (noAggregations) {
- drillDisabled = DISABLED_REASONS.NO_AGGREGATIONS;
- drillByDisabled = DISABLED_REASONS.NO_AGGREGATIONS;
- } else if (!filters?.length) {
- drillByDisabled = DISABLED_REASONS.NO_FILTERS;
- }
- } else {
- drillByDisabled = DISABLED_REASONS.NOT_SUPPORTED;
- }
-
- const drillToDetailMenuItem = drillDisabled ? (
- <DisabledMenuItem menuKey="drill-to-detail-disabled" {...props}>
- {DRILL_TO_DETAIL}
- <MenuItemTooltip title={drillDisabled} />
- </DisabledMenuItem>
- ) : (
- <Menu.Item key="drill-to-detail" onClick={openModal.bind(null, [])}>
- {DRILL_TO_DETAIL}
- </Menu.Item>
- );
-
- const drillToDetailByMenuItem = drillByDisabled ? (
- <DisabledMenuItem menuKey="drill-to-detail-by-disabled" {...props}>
- {DRILL_TO_DETAIL_BY}
- <MenuItemTooltip title={drillByDisabled} />
- </DisabledMenuItem>
- ) : (
- <Menu.SubMenu
- popupOffset={[0, submenuYOffset]}
- popupClassName="chart-context-submenu"
- title={DRILL_TO_DETAIL_BY}
- key={key}
- {...props}
- >
- <div data-test="drill-to-detail-by-submenu">
- {filters.map((filter, i) => (
- <MenuItemWithTruncation
- tooltipText={`${DRILL_TO_DETAIL_BY} ${filter.formattedVal}`}
- menuKey={`drill-detail-filter-${i}`}
- onClick={openModal.bind(null, [filter])}
- >
- {`${DRILL_TO_DETAIL_BY} `}
- <StyledFilter stripHTML>{filter.formattedVal}</StyledFilter>
- </MenuItemWithTruncation>
- ))}
- {filters.length > 1 && (
- <Menu.Item
- key="drill-detail-filter-all"
- onClick={openModal.bind(null, filters)}
- >
- <div>
- {`${DRILL_TO_DETAIL_BY} `}
- <StyledFilter stripHTML={false}>{t('all')}</StyledFilter>
- </div>
- </Menu.Item>
- )}
- </div>
- </Menu.SubMenu>
- );
-
- return (
- <>
- {drillToDetailMenuItem}
- {isContextMenu && drillToDetailByMenuItem}
- </>
- );
-};
-
-export default DrillDetailMenuItems;
diff --git a/superset-frontend/src/components/Chart/DrillDetail/index.ts b/superset-frontend/src/components/Chart/DrillDetail/index.ts
deleted file mode 100644
index 7911551479..0000000000
--- a/superset-frontend/src/components/Chart/DrillDetail/index.ts
+++ /dev/null
@@ -1,21 +0,0 @@
-/**
- * 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.
- */
-
-export { default as DrillDetailMenuItems } from './DrillDetailMenuItems';
-export { useDrillDetailMenuItems } from './useDrillDetailMenuItems';
diff --git a/superset-frontend/src/components/Chart/DrillDetail/useDrillDetailMenuItems.tsx b/superset-frontend/src/components/Chart/useDrillDetailMenuItems/index.tsx
similarity index 68%
rename from superset-frontend/src/components/Chart/DrillDetail/useDrillDetailMenuItems.tsx
rename to superset-frontend/src/components/Chart/useDrillDetailMenuItems/index.tsx
index f89d949770..ab58de5a8b 100644
--- a/superset-frontend/src/components/Chart/DrillDetail/useDrillDetailMenuItems.tsx
+++ b/superset-frontend/src/components/Chart/useDrillDetailMenuItems/index.tsx
@@ -37,11 +37,12 @@ import {
t,
} from '@superset-ui/core';
import { useSelector } from 'react-redux';
-import { MenuItem } from '@superset-ui/core/components/Menu';
+import { type ItemType } from '@superset-ui/core/components/Menu';
import { RootState } from 'src/dashboard/types';
import { getSubmenuYOffset } from '../utils';
import { MenuItemTooltip } from '../DisabledMenuItemTooltip';
-import { useMenuItemWithTruncation } from '../MenuItemWithTruncation';
+import { TruncatedMenuLabel } from '../MenuItemWithTruncation';
+import { Dataset } from '../types';
const DRILL_TO_DETAIL = t('Drill to detail');
const DRILL_TO_DETAIL_BY = t('Drill to detail by');
@@ -60,28 +61,6 @@ const DISABLED_REASONS = {
),
};
-function getDisabledMenuItem(
- children: ReactNode,
- menuKey: string,
- ...rest: unknown[]
-): MenuItem {
- return {
- disabled: true,
- key: menuKey,
- label: (
- <div
- css={css`
- white-space: normal;
- max-width: 160px;
- `}
- >
- {children}
- </div>
- ),
- ...rest,
- };
-}
-
const Filter = ({
children,
stripHTML = false,
@@ -103,7 +82,7 @@ const StyledFilter = styled(Filter)`
`}
`;
-export type DrillDetailMenuItemsArgs = {
+export type DrillDetailMenuItemsProps = {
formData: QueryFormData;
filters?: BinaryQueryObjectFilterClause[];
setFilters: Dispatch<SetStateAction<BinaryQueryObjectFilterClause[]>>;
@@ -115,6 +94,8 @@ export type DrillDetailMenuItemsArgs = {
setShowModal: (show: boolean) => void;
key?: string;
forceSubmenuRender?: boolean;
+ dataset?: Dataset;
+ isLoadingDataset?: boolean;
};
export const useDrillDetailMenuItems = ({
@@ -129,7 +110,7 @@ export const useDrillDetailMenuItems = ({
setShowModal,
key,
...props
-}: DrillDetailMenuItemsArgs) => {
+}: DrillDetailMenuItemsProps): ItemType[] => {
const drillToDetailDisabled = useSelector<RootState, boolean | undefined>(
({ datasources }) =>
datasources[formData.datasource]?.database?.disable_drill_to_detail,
@@ -142,7 +123,7 @@ export const useDrillDetailMenuItems = ({
setFilters(filters);
setShowModal(true);
},
- [onClick, onSelection],
+ [onClick, onSelection, setFilters, setShowModal],
);
// Check for Behavior.DRILL_TO_DETAIL to tell if plugin handles the `contextmenu`
@@ -191,79 +172,90 @@ export const useDrillDetailMenuItems = ({
drillByDisabled = DISABLED_REASONS.NOT_SUPPORTED;
}
- const drillToDetailMenuItem: MenuItem = drillDisabled
- ? getDisabledMenuItem(
- <>
- {DRILL_TO_DETAIL}
- <MenuItemTooltip title={drillDisabled} />
- </>,
- 'drill-to-detail-disabled',
- props,
- )
+ const drillToDetailMenuItem: ItemType = drillDisabled
+ ? {
+ key: 'drill-to-detail-disabled',
+ disabled: true,
+ label: (
+ <div
+ css={css`
+ white-space: normal;
+ max-width: 160px;
+ `}
+ >
+ {DRILL_TO_DETAIL}
+ <MenuItemTooltip title={drillDisabled} />
+ </div>
+ ),
+ ...props,
+ }
: {
key: 'drill-to-detail',
- label: DRILL_TO_DETAIL,
onClick: openModal.bind(null, []),
- ...props,
+ label: DRILL_TO_DETAIL,
};
- const getMenuItemWithTruncation = useMenuItemWithTruncation();
-
- const drillToDetailByMenuItem: MenuItem = drillByDisabled
- ? getDisabledMenuItem(
- <>
- {DRILL_TO_DETAIL_BY}
- <MenuItemTooltip title={drillByDisabled} />
- </>,
- 'drill-to-detail-by-disabled',
- props,
- )
- : {
- key: key || 'drill-to-detail-by',
- label: DRILL_TO_DETAIL_BY,
- children: [
- ...filters.map((filter, i) => ({
- key: `drill-detail-filter-${i}`,
- label: getMenuItemWithTruncation({
- tooltipText: `${DRILL_TO_DETAIL_BY} ${filter.formattedVal}`,
- onClick: openModal.bind(null, [filter]),
+ const drillToDetailByMenuItem: ItemType | null = !isContextMenu
+ ? null
+ : drillByDisabled
+ ? {
+ key: 'drill-to-detail-by-disabled',
+ disabled: true,
+ label: (
+ <div
+ css={css`
+ white-space: normal;
+ max-width: 160px;
+ `}
+ >
+ {DRILL_TO_DETAIL_BY}
+ <MenuItemTooltip title={drillByDisabled} />
+ </div>
+ ),
+ ...props,
+ }
+ : {
+ key: key || 'drill-to-detail-by',
+ label: DRILL_TO_DETAIL_BY,
+ popupOffset: [0, submenuYOffset],
+ popupClassName: 'chart-context-submenu',
+ children: [
+ ...filters.map((filter, i) => ({
key: `drill-detail-filter-${i}`,
- children: (
- <>
+ onClick: openModal.bind(null, [filter]),
+ label: (
+ <TruncatedMenuLabel
+ tooltipText={`${DRILL_TO_DETAIL_BY} ${filter.formattedVal}`}
+ >
{`${DRILL_TO_DETAIL_BY} `}
<StyledFilter stripHTML>{filter.formattedVal}</StyledFilter>
- </>
- ),
- }),
- })),
- filters.length > 1 && {
- key: 'drill-detail-filter-all',
- label: getMenuItemWithTruncation({
- tooltipText: `${DRILL_TO_DETAIL_BY} ${t('all')}`,
- onClick: openModal.bind(null, filters),
- key: 'drill-detail-filter-all',
- children: (
- <>
- {`${DRILL_TO_DETAIL_BY} `}
- <StyledFilter stripHTML={false}>{t('all')}</StyledFilter>
- </>
+ </TruncatedMenuLabel>
),
- }),
- },
- ].filter(Boolean) as MenuItem[],
- onClick: openModal.bind(null, filters),
- forceSubmenuRender: true,
- popupOffset: [0, submenuYOffset],
- popupClassName: 'chart-context-submenu',
- ...props,
- };
- if (isContextMenu) {
- return {
- drillToDetailMenuItem,
- drillToDetailByMenuItem,
- };
+ })),
+ ...(filters.length > 1
+ ? [
+ {
+ key: 'drill-detail-filter-all',
+ onClick: openModal.bind(null, filters),
+ label: (
+ <div>
+ {`${DRILL_TO_DETAIL_BY} `}
+ <StyledFilter stripHTML={false}>
+ {t('all')}
+ </StyledFilter>
+ </div>
+ ),
+ },
+ ]
+ : []),
+ ],
+ ...props,
+ };
+
+ const menuItems: ItemType[] = [drillToDetailMenuItem];
+ if (drillToDetailByMenuItem) {
+ menuItems.push(drillToDetailByMenuItem);
}
- return {
- drillToDetailMenuItem,
- };
+
+ return menuItems;
};
diff --git a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailMenuItems.test.tsx b/superset-frontend/src/components/Chart/useDrillDetailMenuItems/useDrillDetailMenuItems.test.tsx
similarity index 91%
rename from superset-frontend/src/components/Chart/DrillDetail/DrillDetailMenuItems.test.tsx
rename to superset-frontend/src/components/Chart/useDrillDetailMenuItems/useDrillDetailMenuItems.test.tsx
index db5cd63b98..751b88baca 100644
--- a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailMenuItems.test.tsx
+++ b/superset-frontend/src/components/Chart/useDrillDetailMenuItems/useDrillDetailMenuItems.test.tsx
@@ -29,15 +29,13 @@ import { getMockStoreWithNativeFilters } from 'spec/fixtures/mockStore';
import chartQueries, { sliceId } from 'spec/fixtures/mockChartQueries';
import { BinaryQueryObjectFilterClause, VizType } from '@superset-ui/core';
import { Menu } from '@superset-ui/core/components/Menu';
-import DrillDetailMenuItems, {
- DrillDetailMenuItemsProps,
-} from './DrillDetailMenuItems';
-import DrillDetailModal from './DrillDetailModal';
+import DrillDetailModal from '../DrillDetail/DrillDetailModal';
+import { useDrillDetailMenuItems, DrillDetailMenuItemsProps } from './index';
/* eslint jest/expect-expect: ["warn", { "assertFunctionNames": ["expect*"] }] */
jest.mock(
- './DrillDetailPane',
+ '../DrillDetail/DrillDetailPane',
() =>
({
initialFilters,
@@ -87,17 +85,17 @@ const MockRenderChart = ({
BinaryQueryObjectFilterClause[] | undefined
>(filters);
+ const menuItems = useDrillDetailMenuItems({
+ setFilters,
+ formData: formData ?? defaultFormData,
+ filters: modalFilters,
+ isContextMenu,
+ setShowModal: setShowMenu,
+ });
+
return (
<>
- <Menu forceSubMenuRender>
- <DrillDetailMenuItems
- setFilters={setFilters}
- formData={formData ?? defaultFormData}
- filters={modalFilters}
- isContextMenu={isContextMenu}
- setShowModal={setShowMenu}
- />
- </Menu>
+ <Menu forceSubMenuRender items={menuItems} />
<DrillDetailModal
chartId={chartId ?? defaultChartId}
@@ -237,9 +235,10 @@ const expectDrillToDetailByEnabled = async () => {
await expectMenuItemEnabled(drillToDetailBy!);
userEvent.hover(drillToDetailBy!);
- const submenus = await screen.findAllByTestId('drill-to-detail-by-submenu');
-
- expect(submenus.length).toEqual(2);
+ // With the new Menu component using items array, we need to check differently
+ // The submenu appears as a popup overlay
+ const submenu = await screen.findByRole('menu');
+ expect(submenu).toBeInTheDocument();
};
/**
@@ -259,16 +258,16 @@ const expectDrillToDetailByDimension = async (
filter: BinaryQueryObjectFilterClause,
) => {
userEvent.hover(screen.getByRole('menuitem', { name: 'Drill to detail by' }));
- const drillToDetailBySubMenus = await screen.findAllByTestId(
- 'drill-to-detail-by-submenu',
- );
+
+ // Wait for submenu to appear
+ await screen.findByRole('menu');
const menuItemName = `Drill to detail by ${filter.formattedVal}`;
- const drillToDetailBySubmenuItems = await within(
- drillToDetailBySubMenus[1],
- ).findAllByRole('menuitem');
+ const drillToDetailBySubmenuItem = await screen.findByRole('menuitem', {
+ name: menuItemName,
+ });
- await expectMenuItemEnabled(drillToDetailBySubmenuItems[0]);
+ await expectMenuItemEnabled(drillToDetailBySubmenuItem);
await expectDrillToDetailModal(menuItemName, [filter]);
};
@@ -279,13 +278,11 @@ const expectDrillToDetailByAll = async (
filters: BinaryQueryObjectFilterClause[],
) => {
userEvent.hover(screen.getByRole('menuitem', { name: 'Drill to detail by' }));
- const drillToDetailBySubMenus = await screen.findAllByTestId(
- 'drill-to-detail-by-submenu',
- );
- const drillToDetailBySubmenuItem = await within(
- drillToDetailBySubMenus[1],
- ).findByText(/all/i);
+ // Wait for submenu to appear
+ await screen.findByRole('menu');
+
+ const drillToDetailBySubmenuItem = await screen.findByText(/all/i);
await expectMenuItemEnabled(drillToDetailBySubmenuItem);
diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx
index 24ff1ceb6e..cd68a32801 100644
--- a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx
+++ b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx
@@ -54,7 +54,7 @@ import { getSliceHeaderTooltip } from 'src/dashboard/util/getSliceHeaderTooltip'
import { Icons } from '@superset-ui/core/components/Icons';
import ViewQueryModal from 'src/explore/components/controls/ViewQueryModal';
import { ResultsPaneOnDashboard } from 'src/explore/components/DataTablesPane';
-import { useDrillDetailMenuItems } from 'src/components/Chart/DrillDetail';
+import { useDrillDetailMenuItems } from 'src/components/Chart/useDrillDetailMenuItems';
import { LOG_ACTIONS_CHART_DOWNLOAD_AS_IMAGE } from 'src/logger/LogUtils';
import { MenuKeys, RootState } from 'src/dashboard/types';
import DrillDetailModal from 'src/components/Chart/DrillDetail/DrillDetailModal';
@@ -455,14 +455,13 @@ const SliceHeaderControls = (
});
}
- const { drillToDetailMenuItem, drillToDetailByMenuItem } =
- useDrillDetailMenuItems({
- formData: props.formData,
- filters: modalFilters,
- setFilters,
- setShowModal: setDrillModalIsOpen,
- key: MenuKeys.DrillToDetail,
- });
+ const drillDetailMenuItems = useDrillDetailMenuItems({
+ formData: props.formData,
+ filters: modalFilters,
+ setFilters,
+ setShowModal: setDrillModalIsOpen,
+ key: MenuKeys.DrillToDetail,
+ });
const shareMenuItems = useShareMenuItems({
dashboardId,
@@ -477,10 +476,7 @@ const SliceHeaderControls = (
});
if (isFeatureEnabled(FeatureFlag.DrillToDetail) && canDrillToDetail) {
- newMenuItems.push(drillToDetailMenuItem);
- if (drillToDetailByMenuItem) {
- newMenuItems.push(drillToDetailByMenuItem);
- }
+ newMenuItems.push(...drillDetailMenuItems);
}
if (slice.description || canExplore) {
diff --git a/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.tsx b/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.tsx
index be20c58ac0..df9c21d9e1 100644
--- a/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.tsx
+++ b/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.tsx
@@ -25,11 +25,7 @@ import {
setItem,
LocalStorageKeys,
} from 'src/utils/localStorageHelpers';
-import {
- SamplesPane,
- TableControlsWrapper,
- useResultsPane,
-} from './components';
+import { SamplesPane, useResultsPane } from './components';
import { DataTablesPaneProps, ResultTypes } from './types';
const StyledDiv = styled.div`