diff --git a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx index 98e7fb8a894..750167c8eec 100644 --- a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx +++ b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx @@ -18,7 +18,6 @@ */ import { forwardRef, - Key, ReactNode, RefObject, useCallback, @@ -43,18 +42,18 @@ import { 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'; +import { MenuItemTooltip } from '../DisabledMenuItemTooltip'; export enum ContextMenuItem { CrossFilter, @@ -94,8 +93,8 @@ const ChartContextMenu = ( }: ChartContextMenuProps, ref: RefObject, ) => { - const theme = useTheme(); const dispatch = useDispatch(); + const theme = useTheme(); const { canDrillToDetail, canDrillBy, canDownload } = usePermissions(); const crossFiltersEnabled = useSelector( @@ -104,7 +103,6 @@ const ChartContextMenu = ( const dashboardId = useSelector( ({ dashboardInfo }) => dashboardInfo.id, ); - const [openKeys, setOpenKeys] = useState([]); const [modalFilters, setFilters] = useState( [], @@ -160,7 +158,6 @@ const ChartContextMenu = ( const closeContextMenu = useCallback(() => { setVisible(false); - setOpenKeys([]); onClose(); }, [onClose]); @@ -177,7 +174,7 @@ const ChartContextMenu = ( setShowDrillByModal(false); }, []); - const menuItems: React.JSX.Element[] = []; + const menuItems: MenuItem[] = []; const showDrillToDetail = isFeatureEnabled(FeatureFlag.DrillToDetail) && @@ -264,6 +261,20 @@ const ChartContextMenu = ( itemsCount = 1; // "No actions" appears if no actions in menu } + 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 || @@ -305,74 +316,65 @@ const ChartContextMenu = ( ); } + menuItems.push( - <> - { - if (filters?.crossFilter) { - dispatch(updateDataMask(id, filters.crossFilter.dataMask)); - } - }} - > - {filters?.crossFilter?.isCurrentValueSelected ? ( - t('Remove cross-filter') - ) : ( -
- {t('Add cross-filter')} - -
- )} - - {itemsCount > 1 && } - , + { + 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( - , - ); + menuItems.push(...drillDetailMenuItems); } + if (showDrillBy) { - let submenuIndex = 0; - if (showCrossFilters) { - submenuIndex += 1; + if (menuItems.length > 0) { + menuItems.push({ key: 'divider-drill-by', type: 'divider' as const }); } - if (showDrillToDetail) { - submenuIndex += 2; - } - menuItems.push( - , - ); + + 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 to support react-window + menuItems.push({ + key: 'drill-by-submenu', + disabled: isDrillByDisabled, + label: ( + + ), + }); } const open = useCallback( @@ -404,30 +406,22 @@ const ChartContextMenu = ( return ReactDOM.createPortal( <> ( - { - setVisible(false); - setOpenKeys([]); - onClose(); - }} - > - {menuItems.length ? ( - menuItems - ) : ( - {t('No actions')} - )} - + menu={{ + items: + menuItems.length > 0 + ? menuItems + : [{ key: 'no-actions', label: t('No actions'), disabled: true }], + onClick: () => { + setVisible(false); + onClose(); + }, + }} + dropdownRender={menu => ( +
{menu}
)} 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 85b6af61ec7..00000000000 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx +++ /dev/null @@ -1,277 +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(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 ( - -
- {t('Drill by')} - -
- - ); - } - - const Row = ({ - index, - data, - style, - }: { - index: number; - data: { columns: Column[] }; - style: CSSProperties; - }) => { - const { columns, ...rest } = data; - const column = columns[index]; - return ( - handleSelection(e, column)} - style={style} - {...rest} - > - {column.verbose_name || column.column_name} - - ); - }; - - // Don't render drill by menu items when matrixify is enabled - if ( - formData.matrixify_enable_vertical_layout === true || - formData.matrixify_enable_horizontal_layout === true - ) { - return null; - } - - return ( - <> - -
- {showSearch && ( - - } - 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 ? ( -
- -
- ) : filteredColumns.length ? ( - - {Row} - - ) : ( - - {t('No columns found')} - - )} -
- - - ); -}; diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.test.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.test.tsx similarity index 60% rename from superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.test.tsx rename to superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.test.tsx index 13410f80316..8bcfc7959b5 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) => +}: Partial) => render( - - - , + , { 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,17 @@ 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(); + userEvent.hover(drillByButton); + + const popover = await screen.findByRole('menu'); + expect(popover).toBeInTheDocument(); }; getChartMetadataRegistry().registerValue( @@ -149,7 +137,7 @@ afterEach(() => { }); test('render disabled menu item for unsupported chart', async () => { - renderMenu({ + renderSubmenu({ formData: { ...defaultFormData, viz_type: 'unsupported_viz' }, }); await expectDrillByDisabled( @@ -158,89 +146,75 @@ 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'); + + const noColumnsText = await screen.findByText('No columns found'); + expect(noColumnsText).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 + const col1Element = await screen.findByText('col1'); + expect(col1Element).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 first column to ensure menu is loaded + await screen.findByText('col1'); - const searchbox = await waitFor( - () => screen.getAllByPlaceholderText('Search columns')[0], - ); + // Then check all columns are visible + defaultColumns.forEach(column => { + expect(screen.getByText(column.column_name)).toBeInTheDocument(); + }); + + const searchbox = screen.getByPlaceholderText('Search columns'); expect(searchbox).toBeInTheDocument(); userEvent.type(searchbox, 'col1'); const expectedFilteredColumnNames = ['col1', 'col10', 'col11']; - // Wait for filtered results + // Wait for filtering to take effect by checking for first filtered item await waitFor(() => { - const submenus = screen.getAllByTestId('drill-by-submenu'); - const submenu = submenus[0]; - expectedFilteredColumnNames.forEach(colName => { - expect(within(submenu).getByText(colName)).toBeInTheDocument(); - }); + // Check that non-matching columns are not visible + expect(screen.queryByText('col2')).not.toBeInTheDocument(); }); - const submenus = screen.getAllByTestId('drill-by-submenu'); - const submenu = submenus[0]; + // Then verify all expected columns are visible + expectedFilteredColumnNames.forEach(colName => { + expect(screen.getByText(colName)).toBeInTheDocument(); + }); + // 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 +226,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, @@ -261,32 +235,24 @@ 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 }, - ); + // Wait for first column to ensure menu is loaded + await screen.findByText('col1'); + + // Then check all non-excluded columns are visible + 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 +260,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 screen.findByText('col1'); userEvent.click(col1Element); expect(onSelectionMock).toHaveBeenCalledWith( @@ -309,3 +271,10 @@ test('When menu item is clicked, call onSelection with clicked column and drill { filters: defaultFilters, groupbyFieldName: 'groupby' }, ); }); + +test('matrixify_enable_vertical_layout should not render component', () => { + const { container } = renderSubmenu({ + formData: { ...defaultFormData, matrixify_enable_vertical_layout: 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 00000000000..08c23001db1 --- /dev/null +++ b/superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.tsx @@ -0,0 +1,342 @@ +/** + * 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(null); + const menuItemRef = useRef(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 as MouseEvent); + onSelection(column, drillByConfig); + if (openNewModal && onDrillBy && dataset) { + onDrillBy(column, dataset); + } + setPopoverOpen(false); + onCloseMenu(); + }, + [ + drillByConfig, + onClick, + onSelection, + openNewModal, + onDrillBy, + dataset, + onCloseMenu, + ], + ); + + useEffect(() => { + let timeoutId: NodeJS.Timeout; + if (popoverOpen) { + // Small delay to ensure popover is rendered + timeoutId = setTimeout(() => { + ref.current?.input?.focus({ preventScroll: true }); + }, 100); + } else { + // Reset search input when menu is closed + setSearchInput(''); + setDebouncedSearchInput(''); + } + return () => { + if (timeoutId) clearTimeout(timeoutId); + }; + }, [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'); + } + + if ( + formData.matrixify_enable_vertical_layout === true || + formData.matrixify_enable_horizontal_layout === true + ) { + 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 ( + handleSelection(e, column)} + style={style} + > + {column.verbose_name || column.column_name} + + ); + }; + + const popoverContent = ( +
e.stopPropagation()} + > + {showSearch && ( + + } + 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 ? ( +
+ +
+ ) : filteredColumns.length ? ( + + {Row} + + ) : ( +
+ {t('No columns found')} +
+ )} +
+ ); + + const menuItem = ( +
!isDisabled && setPopoverOpen(!popoverOpen)} + onKeyDown={e => { + if (!isDisabled && (e.key === 'Enter' || e.key === ' ')) { + e.preventDefault(); + setPopoverOpen(!popoverOpen); + } + }} + > + {t('Drill by')} + {isDisabled ? ( + + ) : ( + + )} +
+ ); + + if (isDisabled) { + return menuItem; + } + + return ( + + {menuItem} + + ); +}; 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 69d9d20cfa9..00000000000 --- 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; -}) => ( - -
- {children} -
- -); - -const Filter = ({ - children, - stripHTML = false, -}: { - children: ReactNode; - stripHTML: boolean; -}) => { - const content = - stripHTML && typeof children === 'string' - ? removeHTMLTags(children) - : children; - return {content}; -}; - -const StyledFilter = styled(Filter)` - ${({ theme }) => ` - font-weight: ${theme.fontWeightStrong}; - color: ${theme.colorPrimary}; - `} -`; - -export type DrillDetailMenuItemsProps = { - formData: QueryFormData; - filters?: BinaryQueryObjectFilterClause[]; - setFilters: Dispatch>; - 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( - ({ 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 ? ( - - {DRILL_TO_DETAIL} - - - ) : ( - - {DRILL_TO_DETAIL} - - ); - - const drillToDetailByMenuItem = drillByDisabled ? ( - - {DRILL_TO_DETAIL_BY} - - - ) : ( - -
- {filters.map((filter, i) => ( - - {`${DRILL_TO_DETAIL_BY} `} - {filter.formattedVal} - - ))} - {filters.length > 1 && ( - -
- {`${DRILL_TO_DETAIL_BY} `} - {t('all')} -
- - )} -
- - ); - - 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 79115514797..00000000000 --- 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 61% rename from superset-frontend/src/components/Chart/DrillDetail/useDrillDetailMenuItems.tsx rename to superset-frontend/src/components/Chart/useDrillDetailMenuItems/index.tsx index f89d9497701..e8c47054dcb 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: ( -
- {children} -
- ), - ...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>; @@ -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( ({ 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,110 @@ export const useDrillDetailMenuItems = ({ drillByDisabled = DISABLED_REASONS.NOT_SUPPORTED; } - const drillToDetailMenuItem: MenuItem = drillDisabled - ? getDisabledMenuItem( - <> - {DRILL_TO_DETAIL} - - , - 'drill-to-detail-disabled', - props, - ) + const drillToDetailMenuItem: ItemType = drillDisabled + ? { + key: 'drill-to-detail-disabled', + disabled: true, + label: ( +
+ {DRILL_TO_DETAIL} + +
+ ), + ...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} - - , - '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: ( +
+ {DRILL_TO_DETAIL_BY} + +
+ ), + ...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: ( - <> - {`${DRILL_TO_DETAIL_BY} `} - {filter.formattedVal} - + onClick: openModal.bind(null, [filter]), + label: ( +
+ + + {DRILL_TO_DETAIL_BY}{' '} + + {filter.formattedVal} + + + +
), - }), - })), - 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} `} - {t('all')} - - ), - }), - }, - ].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: ( +
+ {`${DRILL_TO_DETAIL_BY} `} + + {t('all')} + +
+ ), + }, + ] + : []), + ], + ...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 84% rename from superset-frontend/src/components/Chart/DrillDetail/DrillDetailMenuItems.test.tsx rename to superset-frontend/src/components/Chart/useDrillDetailMenuItems/useDrillDetailMenuItems.test.tsx index db5cd63b986..ea7450153a0 100644 --- a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailMenuItems.test.tsx +++ b/superset-frontend/src/components/Chart/useDrillDetailMenuItems/useDrillDetailMenuItems.test.tsx @@ -22,6 +22,7 @@ import { render, screen, userEvent, + waitFor, within, } from 'spec/helpers/testing-library'; import setupPlugins from 'src/setup/setupPlugins'; @@ -29,15 +30,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 +86,17 @@ const MockRenderChart = ({ BinaryQueryObjectFilterClause[] | undefined >(filters); + const menuItems = useDrillDetailMenuItems({ + setFilters, + formData: formData ?? defaultFormData, + filters: modalFilters, + isContextMenu, + setShowModal: setShowMenu, + }); + return ( <> - - - + { +const setupMenu = async (filters: BinaryQueryObjectFilterClause[]) => { cleanup(); + // Small delay to ensure DOM cleanup is complete + await new Promise(resolve => setTimeout(resolve, 10)); renderMenu({ chartId: defaultChartId, formData: defaultFormData, @@ -235,11 +236,11 @@ const expectDrillToDetailByEnabled = async () => { .getAllByRole('menuitem') .find(menuItem => within(menuItem).queryByText('Drill to detail by')); await expectMenuItemEnabled(drillToDetailBy!); + userEvent.hover(drillToDetailBy!); - const submenus = await screen.findAllByTestId('drill-to-detail-by-submenu'); - - expect(submenus.length).toEqual(2); + const submenu = await screen.findByRole('menu', {}); + expect(submenu).toBeInTheDocument(); }; /** @@ -258,17 +259,37 @@ const expectDrillToDetailByDisabled = async (tooltipContent?: string) => { 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', - ); + const formattedVal = filter.formattedVal as string; + const drillByMenuItem = screen.getByRole('menuitem', { + name: 'Drill to detail by', + }); + + userEvent.hover(drillByMenuItem); + + const submenuPopup = (await waitFor(() => + screen + .getAllByRole('menu', { hidden: true }) + .find(menu => + (menu.textContent ?? '') + .replace(/\s+/g, ' ') + .trim() + .includes(formattedVal), + ), + )) as HTMLElement; + + const drillToDetailBySubmenuItem = (await waitFor(() => { + const items = within(submenuPopup).getAllByRole('menuitem'); + return items.find(item => + (item.textContent ?? '') + .replace(/\s+/g, ' ') + .trim() + .includes(`Drill to detail by ${filter.formattedVal}`), + ); + })) as HTMLElement; const menuItemName = `Drill to detail by ${filter.formattedVal}`; - const drillToDetailBySubmenuItems = await within( - drillToDetailBySubMenus[1], - ).findAllByRole('menuitem'); - await expectMenuItemEnabled(drillToDetailBySubmenuItems[0]); + await expectMenuItemEnabled(drillToDetailBySubmenuItem); await expectDrillToDetailModal(menuItemName, [filter]); }; @@ -278,14 +299,15 @@ const expectDrillToDetailByDimension = async ( 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 drillByMenuItem = screen.getByRole('menuitem', { + name: 'Drill to detail by', + }); - const drillToDetailBySubmenuItem = await within( - drillToDetailBySubMenus[1], - ).findByText(/all/i); + userEvent.hover(drillByMenuItem); + + await screen.findByRole('menu'); + + const drillToDetailBySubmenuItem = await screen.findByText(/all/i, {}); await expectMenuItemEnabled(drillToDetailBySubmenuItem); @@ -386,20 +408,20 @@ test('context menu for supported chart, dimensions, 1 filter', async () => { test('context menu for supported chart, dimensions, filter A', async () => { const filters = [filterA, filterB]; - setupMenu(filters); + await setupMenu(filters); await expectDrillToDetailByEnabled(); await expectDrillToDetailByDimension(filterA); }); test('context menu for supported chart, dimensions, filter B', async () => { const filters = [filterA, filterB]; - setupMenu(filters); + await setupMenu(filters); await expectDrillToDetailByEnabled(); await expectDrillToDetailByDimension(filterB); }); test.skip('context menu for supported chart, dimensions, all filters', async () => { const filters = [filterA, filterB]; - setupMenu(filters); + await setupMenu(filters); await expectDrillToDetailByAll(filters); }); diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx b/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx index 6829ff7eb24..f9d845e094d 100644 --- a/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx +++ b/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx @@ -86,16 +86,6 @@ const StyledFilterCount = styled.div` const StyledBadge = styled(Badge)` ${({ theme }) => ` margin-left: ${theme.sizeUnit * 2}px; - &>sup.ant-badge-count { - padding: 0 ${theme.sizeUnit}px; - min-width: ${theme.sizeUnit * 4}px; - height: ${theme.sizeUnit * 4}px; - line-height: 1.5; - font-weight: ${theme.fontWeightStrong}; - font-size: ${theme.fontSizeSM - 1}px; - box-shadow: none; - padding: 0 ${theme.sizeUnit}px; - } `} `; @@ -314,6 +304,7 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => { data-test="applied-filter-count" className="applied-count" count={filterCount} + size="small" showZero /> diff --git a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx index 9abb544c0e8..49745bb5da9 100644 --- a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx @@ -85,7 +85,7 @@ const ChartHeaderStyles = styled.div` & > .header-title { overflow: hidden; text-overflow: ellipsis; - max-width: 100%; + max-width: calc(100% - ${theme.sizeUnit * 4}px); flex-grow: 1; display: -webkit-box; -webkit-line-clamp: 2; diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx index 24ff1ceb6e6..cd68a328012 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/dashboard/components/menu/HoverMenu.tsx b/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx index 959b27778e2..b34917b4e2f 100644 --- a/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx +++ b/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx @@ -58,7 +58,7 @@ const HoverStyleOverrides = styled.div` top: ${({ theme }) => theme.sizeUnit * -6}px; left: 50%; transform: translate(-50%); - padding: 0 ${({ theme }) => theme.sizeUnit * 2}px; + padding: 0; display: flex; flex-direction: row; justify-content: center; diff --git a/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.ts b/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.ts index 3beb62d57aa..a84d2211aea 100644 --- a/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.ts +++ b/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.ts @@ -33,7 +33,7 @@ const useFilterFocusHighlightStyles = (chartId: number) => { () => ({ borderColor: theme.colorPrimaryBorder, opacity: 1, - boxShadow: `0px 0px ${theme.sizeUnit * 2}px ${theme.colorPrimary}`, + boxShadow: `0px 0px ${theme.sizeUnit * 3}px ${theme.colorPrimary}`, pointerEvents: 'auto', }), [theme], diff --git a/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.tsx b/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.tsx index 161c856d669..df9c21d9e15 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` @@ -162,7 +158,7 @@ export const DataTablesPane = ({ ); return ( - +
{panelOpen ? ( )} - +
); }, [handleCollapseChange, panelOpen]);