This is an automated email from the ASF dual-hosted git repository. elizabeth pushed a commit to branch elizabeth/fix-resize-bug in repository https://gitbox.apache.org/repos/asf/superset.git
commit c60f9e4cc9750064c0dcb9d5cb0ecbede9090442 Author: Mehmet Salih Yavuz <[email protected]> AuthorDate: Wed Jul 30 23:32:32 2025 +0300 fix: Console errors from various sources (#34178) Co-authored-by: Diego Pucci <[email protected]> --- .../cypress-base/cypress/e2e/explore/chart.test.js | 6 +- .../components/TimeComparisonVisibility.tsx | 60 +-- .../plugins/plugin-chart-table/src/TableChart.tsx | 87 ++-- .../src/components/Chart/DrillDetail/index.ts | 1 + .../Chart/DrillDetail/useDrillDetailMenuItems.tsx | 269 ++++++++++++ .../Header/useHeaderActionsDropdownMenu.tsx | 358 +++++++++------- .../PropertiesModal/PropertiesModal.test.tsx | 3 +- .../components/SliceHeaderControls/index.tsx | 370 ++++++++-------- .../DownloadMenuItems/DownloadMenuItems.test.tsx | 22 +- .../components/menu/DownloadMenuItems/index.tsx | 114 +++-- .../menu/ShareMenuItems/ShareMenuItems.test.tsx | 49 ++- .../components/menu/ShareMenuItems/index.tsx | 43 +- .../useExploreAdditionalActionsMenu/index.jsx | 475 +++++++++++---------- .../HeaderReportDropdown/index.test.tsx | 58 +-- .../ReportModal/HeaderReportDropdown/index.tsx | 274 +++++------- 15 files changed, 1264 insertions(+), 925 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/e2e/explore/chart.test.js b/superset-frontend/cypress-base/cypress/e2e/explore/chart.test.js index b4cc177def..72fb1b3dfe 100644 --- a/superset-frontend/cypress-base/cypress/e2e/explore/chart.test.js +++ b/superset-frontend/cypress-base/cypress/e2e/explore/chart.test.js @@ -68,11 +68,13 @@ function verifyDashboardSearch() { function verifyDashboardLink() { interceptDashboardGet(); openDashboardsAddedTo(); - cy.get('.ant-dropdown-menu-submenu-popup').trigger('mouseover'); + cy.get('.ant-dropdown-menu-submenu-popup').trigger('mouseover', { + force: true, + }); cy.get('.ant-dropdown-menu-submenu-popup a') .first() .invoke('removeAttr', 'target') - .click(); + .click({ force: true }); cy.wait('@get'); } diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/TimeComparisonVisibility.tsx b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/TimeComparisonVisibility.tsx index 8e3d0fbe8e..4546c54391 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/TimeComparisonVisibility.tsx +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/TimeComparisonVisibility.tsx @@ -18,7 +18,7 @@ */ /* eslint-disable import/no-extraneous-dependencies */ import { useState } from 'react'; -import { Dropdown, Menu } from 'antd'; +import { Dropdown } from 'antd'; import { TableOutlined, DownOutlined, CheckOutlined } from '@ant-design/icons'; import { t } from '@superset-ui/core'; import { InfoText, ColumnLabel, CheckIconWrapper } from '../../styles'; @@ -69,34 +69,42 @@ const TimeComparisonVisibility: React.FC<TimeComparisonVisibilityProps> = ({ return ( <Dropdown placement="bottomRight" - visible={showComparisonDropdown} - onVisibleChange={(flag: boolean) => { + open={showComparisonDropdown} + onOpenChange={(flag: boolean) => { setShowComparisonDropdown(flag); }} - overlay={ - <Menu - multiple - onClick={handleOnClick} - onBlur={handleOnBlur} - selectedKeys={selectedComparisonColumns} - > - <InfoText> - {t( - 'Select columns that will be displayed in the table. You can multiselect columns.', - )} - </InfoText> - {comparisonColumns.map((column: ComparisonColumn) => ( - <Menu.Item key={column.key}> - <ColumnLabel>{column.label}</ColumnLabel> - <CheckIconWrapper> - {selectedComparisonColumns.includes(column.key) && ( - <CheckOutlined /> + menu={{ + multiple: true, + onClick: handleOnClick, + onBlur: handleOnBlur, + selectedKeys: selectedComparisonColumns, + items: [ + { + key: 'all', + label: ( + <InfoText> + {t( + 'Select columns that will be displayed in the table. You can multiselect columns.', )} - </CheckIconWrapper> - </Menu.Item> - ))} - </Menu> - } + </InfoText> + ), + type: 'group', + children: comparisonColumns.map((column: ComparisonColumn) => ({ + key: column.key, + label: ( + <> + <ColumnLabel>{column.label}</ColumnLabel> + <CheckIconWrapper> + {selectedComparisonColumns.includes(column.key) && ( + <CheckOutlined /> + )} + </CheckIconWrapper> + </> + ), + })), + }, + ], + }} trigger={['click']} > <span> diff --git a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx index b701e2a356..ced1e23232 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx @@ -59,7 +59,6 @@ import { Space, RawAntdSelect as Select, Dropdown, - Menu, Tooltip, } from '@superset-ui/core/components'; import { @@ -564,52 +563,62 @@ export default function TableChart<D extends DataRecord = DataRecord>( return ( <Dropdown placement="bottomRight" - visible={showComparisonDropdown} - onVisibleChange={(flag: boolean) => { + open={showComparisonDropdown} + onOpenChange={(flag: boolean) => { setShowComparisonDropdown(flag); }} - overlay={ - <Menu - multiple - onClick={handleOnClick} - onBlur={handleOnBlur} - selectedKeys={selectedComparisonColumns} - > - <div - css={css` - max-width: 242px; - padding: 0 ${theme.sizeUnit * 2}px; - color: ${theme.colorText}; - font-size: ${theme.fontSizeSM}px; - `} - > - {t( - 'Select columns that will be displayed in the table. You can multiselect columns.', - )} - </div> - {comparisonColumns.map(column => ( - <Menu.Item key={column.key}> - <span + menu={{ + multiple: true, + onClick: handleOnClick, + onBlur: handleOnBlur, + selectedKeys: selectedComparisonColumns, + items: [ + { + key: 'all', + label: ( + <div css={css` + max-width: 242px; + padding: 0 ${theme.sizeUnit * 2}px; color: ${theme.colorText}; - `} - > - {column.label} - </span> - <span - css={css` - float: right; font-size: ${theme.fontSizeSM}px; `} > - {selectedComparisonColumns.includes(column.key) && ( - <CheckOutlined /> + {t( + 'Select columns that will be displayed in the table. You can multiselect columns.', )} - </span> - </Menu.Item> - ))} - </Menu> - } + </div> + ), + type: 'group', + children: comparisonColumns.map( + (column: { key: string; label: string }) => ({ + key: column.key, + label: ( + <> + <span + css={css` + color: ${theme.colorText}; + `} + > + {column.label} + </span> + <span + css={css` + float: right; + font-size: ${theme.fontSizeSM}px; + `} + > + {selectedComparisonColumns.includes(column.key) && ( + <CheckOutlined /> + )} + </span> + </> + ), + }), + ), + }, + ], + }} trigger={['click']} > <span> diff --git a/superset-frontend/src/components/Chart/DrillDetail/index.ts b/superset-frontend/src/components/Chart/DrillDetail/index.ts index cf154680be..7911551479 100644 --- a/superset-frontend/src/components/Chart/DrillDetail/index.ts +++ b/superset-frontend/src/components/Chart/DrillDetail/index.ts @@ -18,3 +18,4 @@ */ 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/DrillDetail/useDrillDetailMenuItems.tsx new file mode 100644 index 0000000000..f89d949770 --- /dev/null +++ b/superset-frontend/src/components/Chart/DrillDetail/useDrillDetailMenuItems.tsx @@ -0,0 +1,269 @@ +/** + * 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 { MenuItem } from '@superset-ui/core/components/Menu'; +import { RootState } from 'src/dashboard/types'; +import { getSubmenuYOffset } from '../utils'; +import { MenuItemTooltip } from '../DisabledMenuItemTooltip'; +import { useMenuItemWithTruncation } from '../MenuItemWithTruncation'; + +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.', + ), +}; + +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, +}: { + 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 DrillDetailMenuItemsArgs = { + 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; +}; + +export const useDrillDetailMenuItems = ({ + formData, + filters = [], + isContextMenu = false, + contextMenuY = 0, + onSelection = () => null, + onClick = () => null, + submenuIndex = 0, + setFilters, + setShowModal, + key, + ...props +}: DrillDetailMenuItemsArgs) => { + 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: MenuItem = drillDisabled + ? getDisabledMenuItem( + <> + {DRILL_TO_DETAIL} + <MenuItemTooltip title={drillDisabled} /> + </>, + 'drill-to-detail-disabled', + props, + ) + : { + key: 'drill-to-detail', + label: DRILL_TO_DETAIL, + onClick: openModal.bind(null, []), + ...props, + }; + + 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]), + key: `drill-detail-filter-${i}`, + children: ( + <> + {`${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> + </> + ), + }), + }, + ].filter(Boolean) as MenuItem[], + onClick: openModal.bind(null, filters), + forceSubmenuRender: true, + popupOffset: [0, submenuYOffset], + popupClassName: 'chart-context-submenu', + ...props, + }; + if (isContextMenu) { + return { + drillToDetailMenuItem, + drillToDetailByMenuItem, + }; + } + return { + drillToDetailMenuItem, + }; +}; diff --git a/superset-frontend/src/dashboard/components/Header/useHeaderActionsDropdownMenu.tsx b/superset-frontend/src/dashboard/components/Header/useHeaderActionsDropdownMenu.tsx index 511e70a91e..9900d29b0a 100644 --- a/superset-frontend/src/dashboard/components/Header/useHeaderActionsDropdownMenu.tsx +++ b/superset-frontend/src/dashboard/components/Header/useHeaderActionsDropdownMenu.tsx @@ -18,16 +18,16 @@ */ import { useState, useEffect, useCallback, useMemo } from 'react'; import { useSelector, useDispatch } from 'react-redux'; -import { Menu } from '@superset-ui/core/components/Menu'; +import { Menu, MenuItem } from '@superset-ui/core/components/Menu'; import { t } from '@superset-ui/core'; import { isEmpty } from 'lodash'; import { URL_PARAMS } from 'src/constants'; -import ShareMenuItems from 'src/dashboard/components/menu/ShareMenuItems'; -import DownloadMenuItems from 'src/dashboard/components/menu/DownloadMenuItems'; +import { useShareMenuItems } from 'src/dashboard/components/menu/ShareMenuItems'; +import { useDownloadMenuItems } from 'src/dashboard/components/menu/DownloadMenuItems'; +import { useHeaderReportMenuItems } from 'src/features/reports/ReportModal/HeaderReportDropdown'; import CssEditor from 'src/dashboard/components/CssEditor'; import RefreshIntervalModal from 'src/dashboard/components/RefreshIntervalModal'; import SaveModal from 'src/dashboard/components/SaveModal'; -import HeaderReportDropdown from 'src/features/reports/ReportModal/HeaderReportDropdown'; import injectCustomCss from 'src/dashboard/util/injectCustomCss'; import { SAVE_TYPE_NEWDASHBOARD } from 'src/dashboard/util/constants'; import FilterScopeModal from 'src/dashboard/components/filterscope/FilterScopeModal'; @@ -74,9 +74,6 @@ export const useHeaderActionsMenu = ({ }: HeaderDropdownProps) => { const dispatch = useDispatch(); const [css, setCss] = useState(customCss || ''); - const [showReportSubMenu, setShowReportSubMenu] = useState<boolean | null>( - null, - ); const [isDropdownVisible, setIsDropdownVisible] = useState(false); const directPathToChild = useSelector( (state: RootState) => state.dashboardState.directPathToChild, @@ -172,163 +169,220 @@ export const useHeaderActionsMenu = ({ [directPathToChild], ); + const shareMenuItems = useShareMenuItems({ + title: t('Share'), + disabled: isLoading, + url, + dashboardId, + dashboardComponentId, + copyMenuItemTitle: t('Copy permalink to clipboard'), + emailMenuItemTitle: t('Share permalink by email'), + emailSubject, + emailBody: t('Check out this dashboard: '), + addSuccessToast, + addDangerToast, + }); + + const downloadMenuItem = useDownloadMenuItems({ + pdfMenuItemTitle: t('Export to PDF'), + imageMenuItemTitle: t('Download as Image'), + dashboardTitle, + dashboardId, + title: t('Download'), + disabled: isLoading, + logEvent, + }); + + const reportMenuItem = useHeaderReportMenuItems({ + dashboardId: dashboardInfo?.id, + showReportModal, + setCurrentReportDeleting, + }); + + // Helper function to create menu items for components with triggerNode + const createModalMenuItem = ( + key: string, + modalComponent: React.ReactElement, + ): MenuItem => ({ + key, + label: modalComponent, + }); + const menu = useMemo(() => { const isEmbedded = !dashboardInfo?.userId; const refreshIntervalOptions = - dashboardInfo.common?.conf?.DASHBOARD_AUTO_REFRESH_INTERVALS; + dashboardInfo?.common?.conf?.DASHBOARD_AUTO_REFRESH_INTERVALS; + + const menuItems: MenuItem[] = []; + + // Refresh dashboard + if (!editMode) { + menuItems.push({ + key: MenuKeys.RefreshDashboard, + label: t('Refresh dashboard'), + disabled: isLoading, + }); + } + + // Toggle fullscreen + if (!editMode && !isEmbedded) { + menuItems.push({ + key: MenuKeys.ToggleFullscreen, + label: getUrlParam(URL_PARAMS.standalone) + ? t('Exit fullscreen') + : t('Enter fullscreen'), + }); + } + + // Edit properties + if (editMode) { + menuItems.push({ + key: MenuKeys.EditProperties, + label: t('Edit properties'), + }); + } + + // Edit CSS + if (editMode) { + menuItems.push( + createModalMenuItem( + MenuKeys.EditCss, + <CssEditor + triggerNode={<div>{t('Theme & CSS')}</div>} + initialCss={css} + onChange={changeCss} + addDangerToast={addDangerToast} + currentThemeId={dashboardInfo.theme?.id || null} + onThemeChange={handleThemeChange} + />, + ), + ); + } + + // Divider + menuItems.push({ type: 'divider' }); + + // Save as + if (userCanSave) { + menuItems.push( + createModalMenuItem( + MenuKeys.SaveModal, + <SaveModal + addSuccessToast={addSuccessToast} + addDangerToast={addDangerToast} + dashboardId={dashboardId} + dashboardTitle={dashboardTitle} + dashboardInfo={dashboardInfo} + saveType={SAVE_TYPE_NEWDASHBOARD} + layout={layout} + expandedSlices={expandedSlices} + refreshFrequency={refreshFrequency} + shouldPersistRefreshFrequency={shouldPersistRefreshFrequency} + lastModifiedTime={lastModifiedTime} + customCss={customCss} + colorNamespace={colorNamespace} + colorScheme={colorScheme} + onSave={onSave} + triggerNode={ + <div data-test="save-as-menu-item">{t('Save as')}</div> + } + canOverwrite={userCanEdit} + />, + ), + ); + } + + // Download submenu + menuItems.push(downloadMenuItem); + + // Share submenu + if (userCanShare) { + menuItems.push(shareMenuItems); + } + + // Embed dashboard + if (!editMode && userCanCurate) { + menuItems.push({ + key: MenuKeys.ManageEmbedded, + label: t('Embed dashboard'), + }); + } + + // Divider + menuItems.push({ type: 'divider' }); + + // Report dropdown + if (!editMode && reportMenuItem) { + menuItems.push(reportMenuItem); + } + + // Set filter mapping + if (editMode && !isEmpty(dashboardInfo?.metadata?.filter_scopes)) { + menuItems.push( + createModalMenuItem( + MenuKeys.SetFilterMapping, + <FilterScopeModal + triggerNode={<div>{t('Set filter mapping')}</div>} + />, + ), + ); + } + + // Auto-refresh interval + menuItems.push( + createModalMenuItem( + MenuKeys.AutorefreshModal, + <RefreshIntervalModal + addSuccessToast={addSuccessToast} + refreshFrequency={refreshFrequency} + refreshLimit={refreshLimit} + refreshWarning={refreshWarning} + onChange={changeRefreshInterval} + editMode={editMode} + refreshIntervalOptions={refreshIntervalOptions} + triggerNode={<div>{t('Set auto-refresh interval')}</div>} + />, + ), + ); + return ( <Menu selectable={false} data-test="header-actions-menu" onClick={handleMenuClick} - > - {!editMode && ( - <Menu.Item - key={MenuKeys.RefreshDashboard} - data-test="refresh-dashboard-menu-item" - disabled={isLoading} - > - {t('Refresh dashboard')} - </Menu.Item> - )} - {!editMode && !isEmbedded && ( - <Menu.Item key={MenuKeys.ToggleFullscreen}> - {getUrlParam(URL_PARAMS.standalone) - ? t('Exit fullscreen') - : t('Enter fullscreen')} - </Menu.Item> - )} - {editMode && ( - <Menu.Item key={MenuKeys.EditProperties}> - {t('Edit properties')} - </Menu.Item> - )} - {editMode && ( - <Menu.Item key={MenuKeys.EditCss}> - <CssEditor - triggerNode={<div>{t('Theme & CSS')}</div>} - initialCss={css} - onChange={changeCss} - addDangerToast={addDangerToast} - currentThemeId={dashboardInfo.theme?.id || null} - onThemeChange={handleThemeChange} - /> - </Menu.Item> - )} - <Menu.Divider /> - {userCanSave && ( - <Menu.Item key={MenuKeys.SaveModal}> - <SaveModal - addSuccessToast={addSuccessToast} - addDangerToast={addDangerToast} - dashboardId={dashboardId} - dashboardTitle={dashboardTitle} - dashboardInfo={dashboardInfo} - saveType={SAVE_TYPE_NEWDASHBOARD} - layout={layout} - expandedSlices={expandedSlices} - refreshFrequency={refreshFrequency} - shouldPersistRefreshFrequency={shouldPersistRefreshFrequency} - lastModifiedTime={lastModifiedTime} - customCss={customCss} - colorNamespace={colorNamespace} - colorScheme={colorScheme} - onSave={onSave} - triggerNode={ - <div data-test="save-as-menu-item">{t('Save as')}</div> - } - canOverwrite={userCanEdit} - /> - </Menu.Item> - )} - <DownloadMenuItems - submenuKey={MenuKeys.Download} - disabled={isLoading} - title={t('Download')} - pdfMenuItemTitle={t('Export to PDF')} - imageMenuItemTitle={t('Download as Image')} - dashboardTitle={dashboardTitle} - dashboardId={dashboardId} - logEvent={logEvent} - /> - {userCanShare && ( - <ShareMenuItems - disabled={isLoading} - data-test="share-dashboard-menu-item" - title={t('Share')} - url={url} - copyMenuItemTitle={t('Copy permalink to clipboard')} - emailMenuItemTitle={t('Share permalink by email')} - emailSubject={emailSubject} - emailBody={t('Check out this dashboard: ')} - addSuccessToast={addSuccessToast} - addDangerToast={addDangerToast} - dashboardId={dashboardId} - dashboardComponentId={dashboardComponentId} - /> - )} - {!editMode && userCanCurate && ( - <Menu.Item key={MenuKeys.ManageEmbedded}> - {t('Embed dashboard')} - </Menu.Item> - )} - <Menu.Divider /> - {!editMode ? ( - showReportSubMenu ? ( - <> - <HeaderReportDropdown - submenuTitle={t('Manage email report')} - dashboardId={dashboardInfo.id} - setShowReportSubMenu={setShowReportSubMenu} - showReportModal={showReportModal} - showReportSubMenu={showReportSubMenu} - setCurrentReportDeleting={setCurrentReportDeleting} - useTextMenu - /> - <Menu.Divider /> - </> - ) : ( - <HeaderReportDropdown - dashboardId={dashboardInfo.id} - setShowReportSubMenu={setShowReportSubMenu} - showReportModal={showReportModal} - setCurrentReportDeleting={setCurrentReportDeleting} - useTextMenu - /> - ) - ) : null} - {editMode && !isEmpty(dashboardInfo?.metadata?.filter_scopes) && ( - <Menu.Item key={MenuKeys.SetFilterMapping}> - <FilterScopeModal - triggerNode={<div>{t('Set filter mapping')}</div>} - /> - </Menu.Item> - )} - <Menu.Item key={MenuKeys.AutorefreshModal}> - <RefreshIntervalModal - addSuccessToast={addSuccessToast} - refreshFrequency={refreshFrequency} - refreshLimit={refreshLimit} - refreshWarning={refreshWarning} - onChange={changeRefreshInterval} - editMode={editMode} - refreshIntervalOptions={refreshIntervalOptions} - triggerNode={<div>{t('Set auto-refresh interval')}</div>} - /> - </Menu.Item> - </Menu> + items={menuItems} + /> ); }, [ + addDangerToast, + addSuccessToast, + changeRefreshInterval, + changeCss, + colorNamespace, + colorScheme, css, - showReportSubMenu, - isDropdownVisible, - directPathToChild, + customCss, + dashboardId, + dashboardInfo, + dashboardTitle, + downloadMenuItem, + editMode, + expandedSlices, handleMenuClick, - changeCss, - changeRefreshInterval, - emailSubject, - url, - dashboardComponentId, + isLoading, + lastModifiedTime, + layout, + onSave, + refreshFrequency, + refreshLimit, + refreshWarning, + reportMenuItem, + shareMenuItems, + shouldPersistRefreshFrequency, + userCanCurate, + userCanEdit, + userCanSave, + userCanShare, ]); return [menu, isDropdownVisible, setIsDropdownVisible]; diff --git a/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx b/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx index 3e31277b27..599d271d36 100644 --- a/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx +++ b/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx @@ -438,10 +438,9 @@ describe('PropertiesModal', () => { const props = createProps(); const propsWithDashboardInfo = { ...props, dashboardInfo }; - const open = () => waitFor(() => userEvent.click(getSelect())); const getSelect = () => screen.getByRole('combobox', { name: SupersetCore.t('Owners') }); - + const open = () => waitFor(() => userEvent.click(getSelect())); const getElementsByClassName = (className: string) => document.querySelectorAll(className)! as NodeListOf<HTMLElement>; diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx index 312b20ec86..b43941ad82 100644 --- a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx @@ -41,20 +41,20 @@ import { QueryFormData, } from '@superset-ui/core'; import { useSelector } from 'react-redux'; -import { Menu } from '@superset-ui/core/components/Menu'; +import { Menu, MenuItem } from '@superset-ui/core/components/Menu'; import { NoAnimationDropdown, Tooltip, Button, ModalTrigger, } from '@superset-ui/core/components'; -import ShareMenuItems from 'src/dashboard/components/menu/ShareMenuItems'; +import { useShareMenuItems } from 'src/dashboard/components/menu/ShareMenuItems'; import downloadAsImage from 'src/utils/downloadAsImage'; 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 { DrillDetailMenuItems } from 'src/components/Chart/DrillDetail'; +import { useDrillDetailMenuItems } from 'src/components/Chart/DrillDetail'; 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'; @@ -334,183 +334,199 @@ const SliceHeaderControls = ( animationDuration: '0s', }; - const menu = ( - <Menu - onClick={handleMenuClick} - data-test={`slice_${slice.slice_id}-menu`} - id={`slice_${slice.slice_id}-menu`} - selectable={false} - > - <Menu.Item - key={MenuKeys.ForceRefresh} - disabled={props.chartStatus === 'loading'} - style={{ height: 'auto', lineHeight: 'initial' }} - data-test="refresh-chart-menu-item" - > - {t('Force refresh')} - <RefreshTooltip data-test="dashboard-slice-refresh-tooltip"> - {refreshTooltip} - </RefreshTooltip> - </Menu.Item> - - <Menu.Item key={MenuKeys.Fullscreen}>{fullscreenLabel}</Menu.Item> - - <Menu.Divider /> - - {slice.description && ( - <Menu.Item key={MenuKeys.ToggleChartDescription}> - {props.isDescriptionExpanded - ? t('Hide chart description') - : t('Show chart description')} - </Menu.Item> - )} - - {canExplore && ( - <Menu.Item - key={MenuKeys.ExploreChart} - data-test-edit-chart-name={slice.slice_name} - > - <Tooltip title={getSliceHeaderTooltip(props.slice.slice_name)}> - {t('Edit chart')} - </Tooltip> - </Menu.Item> - )} + const newMenuItems: MenuItem[] = [ + { + key: MenuKeys.ForceRefresh, + label: ( + <> + {t('Force refresh')} + <RefreshTooltip data-test="dashboard-slice-refresh-tooltip"> + {refreshTooltip} + </RefreshTooltip> + </> + ), + disabled: props.chartStatus === 'loading', + style: { height: 'auto', lineHeight: 'initial' }, + ...{ 'data-test': 'refresh-chart-menu-item' }, // Typescript hack to get around MenuItem type + }, + { + key: MenuKeys.Fullscreen, + label: fullscreenLabel, + }, + { + type: 'divider', + }, + ]; + + if (slice.description) { + newMenuItems.push({ + key: MenuKeys.ToggleChartDescription, + label: props.isDescriptionExpanded + ? t('Hide chart description') + : t('Show chart description'), + }); + } - {canEditCrossFilters && ( - <Menu.Item key={MenuKeys.CrossFilterScoping}> - {t('Cross-filtering scoping')} - </Menu.Item> - )} + if (canExplore) { + newMenuItems.push({ + key: MenuKeys.ExploreChart, + label: ( + <Tooltip title={getSliceHeaderTooltip(props.slice.slice_name)}> + {t('Edit chart')} + </Tooltip> + ), + ...{ 'data-test-edit-chart-name': slice.slice_name }, + }); + } - {(canExplore || canEditCrossFilters) && <Menu.Divider />} - - {(canExplore || canViewQuery) && ( - <Menu.Item key={MenuKeys.ViewQuery}> - <ModalTrigger - triggerNode={ - <div data-test="view-query-menu-item">{t('View query')}</div> - } - modalTitle={t('View query')} - modalBody={<ViewQueryModal latestQueryFormData={props.formData} />} - draggable - resizable - responsive - ref={queryMenuRef} - /> - </Menu.Item> - )} + if (canEditCrossFilters) { + newMenuItems.push({ + key: MenuKeys.CrossFilterScoping, + label: t('Cross-filtering scoping'), + }); + } - {(canExplore || canViewTable) && ( - <Menu.Item key={MenuKeys.ViewResults}> - <ViewResultsModalTrigger - canExplore={props.supersetCanExplore} - exploreUrl={props.exploreUrl} - triggerNode={ - <div data-test="view-query-menu-item">{t('View as table')}</div> - } - modalRef={resultsMenuRef} - modalTitle={t('Chart Data: %s', slice.slice_name)} - modalBody={ - <ResultsPaneOnDashboard - queryFormData={props.formData} - queryForce={false} - dataSize={20} - isRequest - isVisible - canDownload={!!props.supersetCanCSV} - /> - } - /> - </Menu.Item> - )} + if (canExplore || canEditCrossFilters) { + newMenuItems.push({ type: 'divider' }); + } - {isFeatureEnabled(FeatureFlag.DrillToDetail) && canDrillToDetail && ( - <DrillDetailMenuItems - setFilters={setFilters} - filters={modalFilters} - formData={props.formData} - key={MenuKeys.DrillToDetail} - setShowModal={setDrillModalIsOpen} + if (canExplore || canViewQuery) { + newMenuItems.push({ + key: MenuKeys.ViewQuery, + label: ( + <ModalTrigger + triggerNode={ + <div data-test="view-query-menu-item">{t('View query')}</div> + } + modalTitle={t('View query')} + modalBody={<ViewQueryModal latestQueryFormData={props.formData} />} + draggable + resizable + responsive + ref={queryMenuRef} /> - )} + ), + }); + } - {(slice.description || canExplore) && <Menu.Divider />} - - {supersetCanShare && ( - <ShareMenuItems - dashboardId={dashboardId} - dashboardComponentId={componentId} - copyMenuItemTitle={t('Copy permalink to clipboard')} - emailMenuItemTitle={t('Share chart by email')} - emailSubject={t('Superset chart')} - emailBody={t('Check out this chart: ')} - addSuccessToast={addSuccessToast} - addDangerToast={addDangerToast} - title={t('Share')} + if (canExplore || canViewTable) { + newMenuItems.push({ + key: MenuKeys.ViewResults, + label: ( + <ViewResultsModalTrigger + canExplore={props.supersetCanExplore} + exploreUrl={props.exploreUrl} + triggerNode={ + <div data-test="view-query-menu-item">{t('View as table')}</div> + } + modalRef={resultsMenuRef} + modalTitle={t('Chart Data: %s', slice.slice_name)} + modalBody={ + <ResultsPaneOnDashboard + queryFormData={props.formData} + queryForce={false} + dataSize={20} + isRequest + isVisible + canDownload={!!props.supersetCanCSV} + /> + } /> - )} + ), + }); + } + + const { drillToDetailMenuItem, drillToDetailByMenuItem } = + useDrillDetailMenuItems({ + formData: props.formData, + filters: modalFilters, + setFilters, + setShowModal: setDrillModalIsOpen, + key: MenuKeys.DrillToDetail, + }); + + const shareMenuItems = useShareMenuItems({ + dashboardId, + dashboardComponentId: componentId, + copyMenuItemTitle: t('Copy permalink to clipboard'), + emailMenuItemTitle: t('Share chart by email'), + emailSubject: t('Superset chart'), + emailBody: t('Check out this chart: '), + addSuccessToast, + addDangerToast, + title: t('Share'), + }); + + if (isFeatureEnabled(FeatureFlag.DrillToDetail) && canDrillToDetail) { + newMenuItems.push(drillToDetailMenuItem); + if (drillToDetailByMenuItem) { + newMenuItems.push(drillToDetailByMenuItem); + } + } + + if (slice.description || canExplore) { + newMenuItems.push({ type: 'divider' }); + } + + if (supersetCanShare) { + newMenuItems.push(shareMenuItems); + } + + if (props.supersetCanCSV) { + newMenuItems.push({ + type: 'submenu', + key: MenuKeys.Download, + label: t('Download'), + children: [ + { + key: MenuKeys.ExportCsv, + label: t('Export to .CSV'), + icon: <Icons.FileOutlined css={dropdownIconsStyles} />, + }, + ...(isPivotTable + ? [ + { + key: MenuKeys.ExportPivotCsv, + label: t('Export to Pivoted .CSV'), + icon: <Icons.FileOutlined css={dropdownIconsStyles} />, + }, + { + key: MenuKeys.ExportPivotXlsx, + label: t('Export to Pivoted Excel'), + icon: <Icons.FileOutlined css={dropdownIconsStyles} />, + }, + ] + : []), + { + key: MenuKeys.ExportXlsx, + label: t('Export to Excel'), + icon: <Icons.FileOutlined css={dropdownIconsStyles} />, + }, + ...(isFeatureEnabled(FeatureFlag.AllowFullCsvExport) && + props.supersetCanCSV && + isTable + ? [ + { + key: MenuKeys.ExportFullCsv, + label: t('Export to full .CSV'), + icon: <Icons.FileOutlined css={dropdownIconsStyles} />, + }, + { + key: MenuKeys.ExportFullXlsx, + label: t('Export to full Excel'), + icon: <Icons.FileOutlined css={dropdownIconsStyles} />, + }, + ] + : []), + { + key: MenuKeys.DownloadAsImage, + label: t('Download as image'), + icon: <Icons.FileImageOutlined css={dropdownIconsStyles} />, + }, + ], + }); + } - {props.supersetCanCSV && ( - <Menu.SubMenu title={t('Download')} key={MenuKeys.Download}> - <Menu.Item - key={MenuKeys.ExportCsv} - icon={<Icons.FileOutlined css={dropdownIconsStyles} />} - > - {t('Export to .CSV')} - </Menu.Item> - {isPivotTable && ( - <Menu.Item - key={MenuKeys.ExportPivotCsv} - icon={<Icons.FileOutlined css={dropdownIconsStyles} />} - > - {t('Export to Pivoted .CSV')} - </Menu.Item> - )} - <Menu.Item - key={MenuKeys.ExportXlsx} - icon={<Icons.FileOutlined css={dropdownIconsStyles} />} - > - {t('Export to Excel')} - </Menu.Item> - - {isPivotTable && ( - <Menu.Item - key={MenuKeys.ExportPivotXlsx} - icon={<Icons.FileOutlined css={dropdownIconsStyles} />} - > - {t('Export to Pivoted Excel')} - </Menu.Item> - )} - - {isFeatureEnabled(FeatureFlag.AllowFullCsvExport) && - props.supersetCanCSV && - isTable && ( - <> - <Menu.Item - key={MenuKeys.ExportFullCsv} - icon={<Icons.FileOutlined css={dropdownIconsStyles} />} - > - {t('Export to full .CSV')} - </Menu.Item> - <Menu.Item - key={MenuKeys.ExportFullXlsx} - icon={<Icons.FileOutlined css={dropdownIconsStyles} />} - > - {t('Export to full Excel')} - </Menu.Item> - </> - )} - - <Menu.Item - key={MenuKeys.DownloadAsImage} - icon={<Icons.FileImageOutlined css={dropdownIconsStyles} />} - > - {t('Download as image')} - </Menu.Item> - </Menu.SubMenu> - )} - </Menu> - ); return ( <> {isFullSize && ( @@ -522,7 +538,15 @@ const SliceHeaderControls = ( /> )} <NoAnimationDropdown - popupRender={() => menu} + popupRender={() => ( + <Menu + onClick={handleMenuClick} + data-test={`slice_${slice.slice_id}-menu`} + id={`slice_${slice.slice_id}-menu`} + selectable={false} + items={newMenuItems} + /> + )} overlayStyle={dropdownOverlayStyle} trigger={['click']} placement="bottomRight" diff --git a/superset-frontend/src/dashboard/components/menu/DownloadMenuItems/DownloadMenuItems.test.tsx b/superset-frontend/src/dashboard/components/menu/DownloadMenuItems/DownloadMenuItems.test.tsx index 1f45af9722..f83ec0e377 100644 --- a/superset-frontend/src/dashboard/components/menu/DownloadMenuItems/DownloadMenuItems.test.tsx +++ b/superset-frontend/src/dashboard/components/menu/DownloadMenuItems/DownloadMenuItems.test.tsx @@ -17,8 +17,8 @@ * under the License. */ import { render, screen } from 'spec/helpers/testing-library'; -import { Menu } from '@superset-ui/core/components/Menu'; -import DownloadMenuItems from '.'; +import { Menu, MenuItem } from '@superset-ui/core/components/Menu'; +import { useDownloadMenuItems } from '.'; const createProps = () => ({ pdfMenuItemTitle: 'Export to PDF', @@ -30,19 +30,17 @@ const createProps = () => ({ submenuKey: 'download', }); -const renderComponent = () => { - render( - <Menu forceSubMenuRender> - <DownloadMenuItems {...createProps()} /> - </Menu>, - { - useRedux: true, - }, - ); +const MenuWrapper = () => { + const downloadMenuItem = useDownloadMenuItems(createProps()); + const menuItems: MenuItem[] = [downloadMenuItem]; + return <Menu forceSubMenuRender items={menuItems} />; }; test('Should render menu items', () => { - renderComponent(); + render(<MenuWrapper />, { + useRedux: true, + }); + expect(screen.getByText('Export to PDF')).toBeInTheDocument(); expect(screen.getByText('Download as Image')).toBeInTheDocument(); }); diff --git a/superset-frontend/src/dashboard/components/menu/DownloadMenuItems/index.tsx b/superset-frontend/src/dashboard/components/menu/DownloadMenuItems/index.tsx index cbbc6823b7..b3f66cce8b 100644 --- a/superset-frontend/src/dashboard/components/menu/DownloadMenuItems/index.tsx +++ b/superset-frontend/src/dashboard/components/menu/DownloadMenuItems/index.tsx @@ -16,16 +16,21 @@ * specific language governing permissions and limitations * under the License. */ -import { FeatureFlag, isFeatureEnabled } from '@superset-ui/core'; -import { Menu } from '@superset-ui/core/components/Menu'; +import { SyntheticEvent } from 'react'; +import { FeatureFlag, isFeatureEnabled, logging, t } from '@superset-ui/core'; +import { MenuItem } from '@superset-ui/core/components/Menu'; import { useDownloadScreenshot } from 'src/dashboard/hooks/useDownloadScreenshot'; -import { ComponentProps } from 'react'; +import { MenuKeys } from 'src/dashboard/types'; +import downloadAsPdf from 'src/utils/downloadAsPdf'; +import downloadAsImage from 'src/utils/downloadAsImage'; +import { + LOG_ACTIONS_DASHBOARD_DOWNLOAD_AS_PDF, + LOG_ACTIONS_DASHBOARD_DOWNLOAD_AS_IMAGE, +} from 'src/logger/LogUtils'; +import { useToasts } from 'src/components/MessageToasts/withToasts'; import { DownloadScreenshotFormat } from './types'; -import DownloadAsPdf from './DownloadAsPdf'; -import DownloadAsImage from './DownloadAsImage'; -export interface DownloadMenuItemProps - extends ComponentProps<typeof Menu.SubMenu> { +export interface UseDownloadMenuItemsProps { pdfMenuItemTitle: string; imageMenuItemTitle: string; dashboardTitle: string; @@ -33,56 +38,81 @@ export interface DownloadMenuItemProps dashboardId: number; title: string; disabled?: boolean; - submenuKey: string; } -const DownloadMenuItems = (props: DownloadMenuItemProps) => { +export const useDownloadMenuItems = ( + props: UseDownloadMenuItemsProps, +): MenuItem => { const { pdfMenuItemTitle, imageMenuItemTitle, logEvent, dashboardId, dashboardTitle, - submenuKey, disabled, title, - ...rest } = props; + + const { addDangerToast } = useToasts(); + const SCREENSHOT_NODE_SELECTOR = '.dashboard'; + const isWebDriverScreenshotEnabled = isFeatureEnabled(FeatureFlag.EnableDashboardScreenshotEndpoints) && isFeatureEnabled(FeatureFlag.EnableDashboardDownloadWebDriverScreenshot); const downloadScreenshot = useDownloadScreenshot(dashboardId, logEvent); - return isWebDriverScreenshotEnabled ? ( - <Menu.SubMenu key={submenuKey} title={title} disabled={disabled} {...rest}> - <Menu.Item - key={DownloadScreenshotFormat.PDF} - onClick={() => downloadScreenshot(DownloadScreenshotFormat.PDF)} - > - {pdfMenuItemTitle} - </Menu.Item> - <Menu.Item - key={DownloadScreenshotFormat.PNG} - onClick={() => downloadScreenshot(DownloadScreenshotFormat.PNG)} - > - {imageMenuItemTitle} - </Menu.Item> - </Menu.SubMenu> - ) : ( - <Menu.SubMenu key={submenuKey} title={title} disabled={disabled} {...rest}> - <DownloadAsPdf - text={pdfMenuItemTitle} - dashboardTitle={dashboardTitle} - logEvent={logEvent} - /> - <DownloadAsImage - text={imageMenuItemTitle} - dashboardTitle={dashboardTitle} - logEvent={logEvent} - /> - </Menu.SubMenu> - ); -}; + const onDownloadPdf = async (e: SyntheticEvent) => { + try { + downloadAsPdf(SCREENSHOT_NODE_SELECTOR, dashboardTitle, true)(e); + } catch (error) { + logging.error(error); + addDangerToast(t('Sorry, something went wrong. Try again later.')); + } + logEvent?.(LOG_ACTIONS_DASHBOARD_DOWNLOAD_AS_PDF); + }; + + const onDownloadImage = async (e: SyntheticEvent) => { + try { + downloadAsImage(SCREENSHOT_NODE_SELECTOR, dashboardTitle, true)(e); + } catch (error) { + logging.error(error); + addDangerToast(t('Sorry, something went wrong. Try again later.')); + } + logEvent?.(LOG_ACTIONS_DASHBOARD_DOWNLOAD_AS_IMAGE); + }; -export default DownloadMenuItems; + const children: MenuItem[] = isWebDriverScreenshotEnabled + ? [ + { + key: DownloadScreenshotFormat.PDF, + label: pdfMenuItemTitle, + onClick: () => downloadScreenshot(DownloadScreenshotFormat.PDF), + }, + { + key: DownloadScreenshotFormat.PNG, + label: imageMenuItemTitle, + onClick: () => downloadScreenshot(DownloadScreenshotFormat.PNG), + }, + ] + : [ + { + key: 'download-pdf', + label: pdfMenuItemTitle, + onClick: (e: any) => onDownloadPdf(e.domEvent), + }, + { + key: 'download-image', + label: imageMenuItemTitle, + onClick: (e: any) => onDownloadImage(e.domEvent), + }, + ]; + + return { + key: MenuKeys.Download, + type: 'submenu', + label: title, + disabled, + children, + }; +}; diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx index 4b6db26030..c0a84448df 100644 --- a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx +++ b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx @@ -17,7 +17,7 @@ * under the License. */ -import { Menu } from '@superset-ui/core/components/Menu'; +import { Menu, MenuItem } from '@superset-ui/core/components/Menu'; import { render, screen, @@ -26,7 +26,8 @@ import { } from 'spec/helpers/testing-library'; import * as copyTextToClipboard from 'src/utils/copy'; import fetchMock from 'fetch-mock'; -import ShareMenuItems from '.'; +import { ComponentProps } from 'react'; +import { useShareMenuItems, ShareMenuItemProps } from '.'; const spy = jest.spyOn(copyTextToClipboard, 'default'); @@ -69,17 +70,23 @@ afterAll((): void => { window.location = location; }); +const MenuWrapper = ( + props: ComponentProps<typeof Menu> & { shareProps: ShareMenuItemProps }, +) => { + const shareMenuItems = useShareMenuItems(props.shareProps); + const menuItems: MenuItem[] = [shareMenuItems]; + return <Menu {...props} items={menuItems} />; +}; + test('Should render menu items', () => { - const props = createProps(); render( - <Menu + <MenuWrapper onClick={jest.fn()} selectable={false} data-test="main-menu" forceSubMenuRender - > - <ShareMenuItems {...props} /> - </Menu>, + shareProps={createProps()} + />, { useRedux: true }, ); expect(screen.getByText('Copy dashboard URL')).toBeInTheDocument(); @@ -90,14 +97,13 @@ test('Click on "Copy dashboard URL" and succeed', async () => { spy.mockResolvedValue(undefined); const props = createProps(); render( - <Menu + <MenuWrapper onClick={jest.fn()} selectable={false} data-test="main-menu" forceSubMenuRender - > - <ShareMenuItems {...props} /> - </Menu>, + shareProps={props} + />, { useRedux: true }, ); @@ -123,14 +129,13 @@ test('Click on "Copy dashboard URL" and fail', async () => { spy.mockRejectedValue(undefined); const props = createProps(); render( - <Menu + <MenuWrapper onClick={jest.fn()} selectable={false} data-test="main-menu" forceSubMenuRender - > - <ShareMenuItems {...props} /> - </Menu>, + shareProps={props} + />, { useRedux: true }, ); @@ -157,14 +162,13 @@ test('Click on "Copy dashboard URL" and fail', async () => { test('Click on "Share dashboard by email" and succeed', async () => { const props = createProps(); render( - <Menu + <MenuWrapper onClick={jest.fn()} selectable={false} data-test="main-menu" forceSubMenuRender - > - <ShareMenuItems {...props} /> - </Menu>, + shareProps={props} + />, { useRedux: true }, ); @@ -191,14 +195,13 @@ test('Click on "Share dashboard by email" and fail', async () => { ); const props = createProps(); render( - <Menu + <MenuWrapper onClick={jest.fn()} selectable={false} data-test="main-menu" forceSubMenuRender - > - <ShareMenuItems {...props} /> - </Menu>, + shareProps={props} + />, { useRedux: true }, ); diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx index 9d663958e3..ed1cacc5cd 100644 --- a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx +++ b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx @@ -19,12 +19,13 @@ import { ComponentProps, RefObject } from 'react'; import copyTextToClipboard from 'src/utils/copy'; import { t, logging } from '@superset-ui/core'; -import { Menu } from '@superset-ui/core/components/Menu'; +import { Menu, MenuItem } from '@superset-ui/core/components/Menu'; import { getDashboardPermalink } from 'src/utils/urlUtils'; import { MenuKeys, RootState } from 'src/dashboard/types'; import { shallowEqual, useSelector } from 'react-redux'; -interface ShareMenuItemProps extends ComponentProps<typeof Menu.SubMenu> { +export interface ShareMenuItemProps + extends ComponentProps<typeof Menu.SubMenu> { url?: string; copyMenuItemTitle: string; emailMenuItemTitle: string; @@ -40,9 +41,10 @@ interface ShareMenuItemProps extends ComponentProps<typeof Menu.SubMenu> { setOpenKeys?: Function; title: string; disabled?: boolean; + [key: string]: any; } -const ShareMenuItems = (props: ShareMenuItemProps) => { +export const useShareMenuItems = (props: ShareMenuItemProps): MenuItem => { const { copyMenuItemTitle, emailMenuItemTitle, @@ -96,20 +98,23 @@ const ShareMenuItems = (props: ShareMenuItemProps) => { } } - return ( - <Menu.SubMenu - title={title} - key={MenuKeys.Share} - disabled={disabled} - {...rest} - > - <Menu.Item key={MenuKeys.CopyLink} onClick={() => onCopyLink()}> - {copyMenuItemTitle} - </Menu.Item> - <Menu.Item key={MenuKeys.ShareByEmail} onClick={() => onShareByEmail()}> - {emailMenuItemTitle} - </Menu.Item> - </Menu.SubMenu> - ); + return { + ...rest, + type: 'submenu', + label: title, + key: MenuKeys.Share, + disabled, + children: [ + { + key: MenuKeys.CopyLink, + label: copyMenuItemTitle, + onClick: onCopyLink, + }, + { + key: MenuKeys.ShareByEmail, + label: emailMenuItemTitle, + onClick: onShareByEmail, + }, + ], + }; }; -export default ShareMenuItems; diff --git a/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.jsx b/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.jsx index 38d43caf96..264f01dc99 100644 --- a/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.jsx +++ b/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.jsx @@ -34,7 +34,7 @@ import { exportChart, getChartKey } from 'src/explore/exploreUtils'; import downloadAsImage from 'src/utils/downloadAsImage'; import { getChartPermalink } from 'src/utils/urlUtils'; import copyTextToClipboard from 'src/utils/copy'; -import HeaderReportDropDown from 'src/features/reports/ReportModal/HeaderReportDropdown'; +import { useHeaderReportMenuItems } from 'src/features/reports/ReportModal/HeaderReportDropdown'; import { logEvent } from 'src/logger/actions'; import { LOG_ACTIONS_CHART_DOWNLOAD_AS_IMAGE, @@ -123,12 +123,18 @@ export const useExploreAdditionalActionsMenu = ( const theme = useTheme(); const { addDangerToast, addSuccessToast } = useToasts(); const dispatch = useDispatch(); - const [showReportSubMenu, setShowReportSubMenu] = useState(null); const [isDropdownVisible, setIsDropdownVisible] = useState(false); const chart = useSelector( state => state.charts?.[getChartKey(state.explore)], ); + // Use the updated report menu items hook + const reportMenuItem = useHeaderReportMenuItems({ + chart, + showReportModal, + setCurrentReportDeleting, + }); + const { datasource } = latestQueryFormData; const shareByEmail = useCallback(async () => { @@ -203,14 +209,106 @@ export const useExploreAdditionalActionsMenu = ( } }, [addDangerToast, addSuccessToast, latestQueryFormData]); - const handleMenuClick = useCallback( - ({ key, domEvent }) => { - switch (key) { - case MENU_KEYS.EDIT_PROPERTIES: + const menu = useMemo(() => { + const menuItems = []; + + // Edit chart properties + if (slice) { + menuItems.push({ + key: MENU_KEYS.EDIT_PROPERTIES, + label: t('Edit chart properties'), + onClick: () => { onOpenPropertiesModal(); setIsDropdownVisible(false); - break; - case MENU_KEYS.EXPORT_TO_CSV: + }, + }); + } + + // On dashboards submenu + menuItems.push({ + key: MENU_KEYS.DASHBOARDS_ADDED_TO, + type: 'submenu', + label: t('On dashboards'), + children: [ + { + key: 'dashboards-content', + label: ( + <DashboardsSubMenu + chartId={slice?.slice_id} + dashboards={dashboards} + /> + ), + }, + ], + }); + + // Divider + menuItems.push({ type: 'divider' }); + + // Download submenu + const downloadChildren = []; + + if (VIZ_TYPES_PIVOTABLE.includes(latestQueryFormData.viz_type)) { + downloadChildren.push( + { + key: MENU_KEYS.EXPORT_TO_CSV, + label: t('Export to original .CSV'), + icon: <Icons.FileOutlined />, + disabled: !canDownloadCSV, + onClick: () => { + exportCSV(); + setIsDropdownVisible(false); + dispatch( + logEvent(LOG_ACTIONS_CHART_DOWNLOAD_AS_CSV, { + chartId: slice?.slice_id, + chartName: slice?.slice_name, + }), + ); + }, + }, + { + key: MENU_KEYS.EXPORT_TO_CSV_PIVOTED, + label: t('Export to pivoted .CSV'), + icon: <Icons.FileOutlined />, + disabled: !canDownloadCSV, + onClick: () => { + exportCSVPivoted(); + setIsDropdownVisible(false); + dispatch( + logEvent(LOG_ACTIONS_CHART_DOWNLOAD_AS_CSV_PIVOTED, { + chartId: slice?.slice_id, + chartName: slice?.slice_name, + }), + ); + }, + }, + { + key: MENU_KEYS.EXPORT_TO_PIVOT_XLSX, + label: t('Export to Pivoted Excel'), + icon: <Icons.FileOutlined />, + disabled: !canDownloadCSV, + onClick: () => { + exportPivotExcel( + '.pvtTable', + slice?.slice_name ?? t('pivoted_xlsx'), + ); + setIsDropdownVisible(false); + dispatch( + logEvent(LOG_ACTIONS_CHART_DOWNLOAD_AS_XLS, { + chartId: slice?.slice_id, + chartName: slice?.slice_name, + }), + ); + }, + }, + ); + } else { + downloadChildren.push({ + key: MENU_KEYS.EXPORT_TO_CSV, + label: t('Export to .CSV'), + icon: <Icons.FileOutlined />, + disabled: !canDownloadCSV, + onClick: () => { exportCSV(); setIsDropdownVisible(false); dispatch( @@ -219,18 +317,17 @@ export const useExploreAdditionalActionsMenu = ( chartName: slice?.slice_name, }), ); - break; - case MENU_KEYS.EXPORT_TO_CSV_PIVOTED: - exportCSVPivoted(); - setIsDropdownVisible(false); - dispatch( - logEvent(LOG_ACTIONS_CHART_DOWNLOAD_AS_CSV_PIVOTED, { - chartId: slice?.slice_id, - chartName: slice?.slice_name, - }), - ); - break; - case MENU_KEYS.EXPORT_TO_JSON: + }, + }); + } + + downloadChildren.push( + { + key: MENU_KEYS.EXPORT_TO_JSON, + label: t('Export to .JSON'), + icon: <Icons.FileOutlined />, + disabled: !canDownloadCSV, + onClick: () => { exportJson(); setIsDropdownVisible(false); dispatch( @@ -239,19 +336,34 @@ export const useExploreAdditionalActionsMenu = ( chartName: slice?.slice_name, }), ); - break; - case MENU_KEYS.EXPORT_TO_XLSX: - exportExcel(); + }, + }, + { + key: MENU_KEYS.DOWNLOAD_AS_IMAGE, + label: t('Download as image'), + icon: <Icons.FileImageOutlined />, + onClick: e => { + downloadAsImage( + '.panel-body .chart-container', + slice?.slice_name ?? t('New chart'), + true, + )(e.domEvent); setIsDropdownVisible(false); dispatch( - logEvent(LOG_ACTIONS_CHART_DOWNLOAD_AS_XLS, { + logEvent(LOG_ACTIONS_CHART_DOWNLOAD_AS_IMAGE, { chartId: slice?.slice_id, chartName: slice?.slice_name, }), ); - break; - case MENU_KEYS.EXPORT_TO_PIVOT_XLSX: - exportPivotExcel('.pvtTable', slice?.slice_name ?? t('pivoted_xlsx')); + }, + }, + { + key: MENU_KEYS.EXPORT_TO_XLSX, + label: t('Export to Excel'), + icon: <Icons.FileOutlined />, + disabled: !canDownloadCSV, + onClick: () => { + exportExcel(); setIsDropdownVisible(false); dispatch( logEvent(LOG_ACTIONS_CHART_DOWNLOAD_AS_XLS, { @@ -259,215 +371,128 @@ export const useExploreAdditionalActionsMenu = ( chartName: slice?.slice_name, }), ); - break; - case MENU_KEYS.DOWNLOAD_AS_IMAGE: - downloadAsImage( - '.panel-body .chart-container', - // eslint-disable-next-line camelcase - slice?.slice_name ?? t('New chart'), - true, - )(domEvent); - setIsDropdownVisible(false); - dispatch( - logEvent(LOG_ACTIONS_CHART_DOWNLOAD_AS_IMAGE, { - chartId: slice?.slice_id, - chartName: slice?.slice_name, - }), - ); - break; - case MENU_KEYS.COPY_PERMALINK: + }, + }, + ); + + menuItems.push({ + key: MENU_KEYS.DOWNLOAD_SUBMENU, + type: 'submenu', + label: t('Download'), + children: downloadChildren, + }); + + // Share submenu + const shareChildren = [ + { + key: MENU_KEYS.COPY_PERMALINK, + label: t('Copy permalink to clipboard'), + onClick: () => { copyLink(); setIsDropdownVisible(false); - break; - case MENU_KEYS.EMBED_CODE: - setIsDropdownVisible(false); - break; - case MENU_KEYS.SHARE_BY_EMAIL: + }, + }, + { + key: MENU_KEYS.SHARE_BY_EMAIL, + label: t('Share chart by email'), + onClick: () => { shareByEmail(); setIsDropdownVisible(false); - break; - case MENU_KEYS.VIEW_QUERY: - setIsDropdownVisible(false); - break; - case MENU_KEYS.RUN_IN_SQL_LAB: - onOpenInEditor(latestQueryFormData, domEvent.metaKey); - setIsDropdownVisible(false); - break; - default: - break; - } - }, - [ - copyLink, - exportCSV, - exportCSVPivoted, - exportJson, - latestQueryFormData, - onOpenInEditor, - onOpenPropertiesModal, - shareByEmail, - slice?.slice_name, - ], - ); + }, + }, + ]; - const menu = useMemo( - () => ( - <Menu onClick={handleMenuClick} selectable={false} {...rest}> - <> - {slice && ( - <Menu.Item key={MENU_KEYS.EDIT_PROPERTIES}> - {t('Edit chart properties')} - </Menu.Item> - )} - <Menu.SubMenu - title={t('On dashboards')} - key={MENU_KEYS.DASHBOARDS_ADDED_TO} - > - <DashboardsSubMenu - chartId={slice?.slice_id} - dashboards={dashboards} - /> - </Menu.SubMenu> - <Menu.Divider /> - </> - <Menu.SubMenu title={t('Download')} key={MENU_KEYS.DOWNLOAD_SUBMENU}> - {VIZ_TYPES_PIVOTABLE.includes(latestQueryFormData.viz_type) ? ( - <> - <Menu.Item - key={MENU_KEYS.EXPORT_TO_CSV} - icon={<Icons.FileOutlined />} - disabled={!canDownloadCSV} - > - {t('Export to original .CSV')} - </Menu.Item> - <Menu.Item - key={MENU_KEYS.EXPORT_TO_CSV_PIVOTED} - icon={<Icons.FileOutlined />} - disabled={!canDownloadCSV} - > - {t('Export to pivoted .CSV')} - </Menu.Item> - </> - ) : ( - <Menu.Item - key={MENU_KEYS.EXPORT_TO_CSV} - icon={<Icons.FileOutlined />} - disabled={!canDownloadCSV} - > - {t('Export to .CSV')} - </Menu.Item> - )} - <Menu.Item - key={MENU_KEYS.EXPORT_TO_JSON} - icon={<Icons.FileOutlined />} - disabled={!canDownloadCSV} - > - {t('Export to .JSON')} - </Menu.Item> - <Menu.Item - key={MENU_KEYS.DOWNLOAD_AS_IMAGE} - icon={<Icons.FileImageOutlined />} - > - {t('Download as image')} - </Menu.Item> - <Menu.Item - key={MENU_KEYS.EXPORT_TO_XLSX} - icon={<Icons.FileOutlined />} - disabled={!canDownloadCSV} - > - {t('Export to Excel')} - </Menu.Item> - <Menu.Item - key={MENU_KEYS.EXPORT_TO_PIVOT_XLSX} - icon={<Icons.FileOutlined />} - disabled={!canDownloadCSV} - > - {t('Export to Pivoted Excel')} - </Menu.Item> - </Menu.SubMenu> - <Menu.SubMenu title={t('Share')} key={MENU_KEYS.SHARE_SUBMENU}> - <Menu.Item key={MENU_KEYS.COPY_PERMALINK}> - {t('Copy permalink to clipboard')} - </Menu.Item> - <Menu.Item key={MENU_KEYS.SHARE_BY_EMAIL}> - {t('Share chart by email')} - </Menu.Item> - {isFeatureEnabled(FeatureFlag.EmbeddableCharts) ? ( - <Menu.Item key={MENU_KEYS.EMBED_CODE}> - <ModalTrigger - triggerNode={ - <div data-test="embed-code-button">{t('Embed code')}</div> - } - modalTitle={t('Embed code')} - modalBody={ - <EmbedCodeContent - formData={latestQueryFormData} - addDangerToast={addDangerToast} - /> - } - maxWidth={`${theme.sizeUnit * 100}px`} - destroyOnHidden - responsive - /> - </Menu.Item> - ) : null} - </Menu.SubMenu> - <Menu.Divider /> - {showReportSubMenu ? ( - <> - <HeaderReportDropDown - submenuTitle={t('Manage email report')} - chart={chart} - setShowReportSubMenu={setShowReportSubMenu} - showReportSubMenu={showReportSubMenu} - showReportModal={showReportModal} - setCurrentReportDeleting={setCurrentReportDeleting} - useTextMenu - /> - <Menu.Divider /> - </> - ) : ( - <HeaderReportDropDown - chart={chart} - setShowReportSubMenu={setShowReportSubMenu} - showReportModal={showReportModal} - setCurrentReportDeleting={setCurrentReportDeleting} - useTextMenu - /> - )} - <Menu.Item key={MENU_KEYS.VIEW_QUERY}> + if (isFeatureEnabled(FeatureFlag.EmbeddableCharts)) { + shareChildren.push({ + key: MENU_KEYS.EMBED_CODE, + label: ( <ModalTrigger triggerNode={ - <div data-test="view-query-menu-item">{t('View query')}</div> + <div data-test="embed-code-button">{t('Embed code')}</div> } - modalTitle={t('View query')} + modalTitle={t('Embed code')} modalBody={ - <ViewQueryModal latestQueryFormData={latestQueryFormData} /> + <EmbedCodeContent + formData={latestQueryFormData} + addDangerToast={addDangerToast} + /> } - draggable - resizable + maxWidth={`${theme.sizeUnit * 100}px`} + destroyOnHidden responsive /> - </Menu.Item> - {datasource && ( - <Menu.Item key={MENU_KEYS.RUN_IN_SQL_LAB}> - {t('Run in SQL Lab')} - </Menu.Item> - )} - </Menu> - ), - [ - addDangerToast, - canDownloadCSV, - chart, - dashboards, - handleMenuClick, - isDropdownVisible, - latestQueryFormData, - showReportSubMenu, - slice, - theme.sizeUnit, - ], - ); + ), + onClick: () => setIsDropdownVisible(false), + }); + } + + menuItems.push({ + key: MENU_KEYS.SHARE_SUBMENU, + type: 'submenu', + label: t('Share'), + children: shareChildren, + }); + + // Divider + menuItems.push({ type: 'divider' }); + + // Report menu item + if (reportMenuItem) { + menuItems.push(reportMenuItem); + } + + // View query + menuItems.push({ + key: MENU_KEYS.VIEW_QUERY, + label: ( + <ModalTrigger + triggerNode={ + <div data-test="view-query-menu-item">{t('View query')}</div> + } + modalTitle={t('View query')} + modalBody={ + <ViewQueryModal latestQueryFormData={latestQueryFormData} /> + } + draggable + resizable + responsive + /> + ), + onClick: () => setIsDropdownVisible(false), + }); + + // Run in SQL Lab + if (datasource) { + menuItems.push({ + key: MENU_KEYS.RUN_IN_SQL_LAB, + label: t('Run in SQL Lab'), + onClick: e => { + onOpenInEditor(latestQueryFormData, e.domEvent.metaKey); + setIsDropdownVisible(false); + }, + }); + } + + return <Menu selectable={false} items={menuItems} {...rest} />; + }, [ + addDangerToast, + canDownloadCSV, + copyLink, + dashboards, + datasource, + dispatch, + exportCSV, + exportCSVPivoted, + exportExcel, + exportJson, + latestQueryFormData, + onOpenInEditor, + onOpenPropertiesModal, + reportMenuItem, + shareByEmail, + slice, + theme.sizeUnit, + ]); + return [menu, isDropdownVisible, setIsDropdownVisible]; }; diff --git a/superset-frontend/src/features/reports/ReportModal/HeaderReportDropdown/index.test.tsx b/superset-frontend/src/features/reports/ReportModal/HeaderReportDropdown/index.test.tsx index 2a2e392c22..a14aa91c58 100644 --- a/superset-frontend/src/features/reports/ReportModal/HeaderReportDropdown/index.test.tsx +++ b/superset-frontend/src/features/reports/ReportModal/HeaderReportDropdown/index.test.tsx @@ -18,12 +18,11 @@ */ import { act, render, screen, userEvent } from 'spec/helpers/testing-library'; import { FeatureFlag, isFeatureEnabled } from '@superset-ui/core'; -import { Menu } from '@superset-ui/core/components/Menu'; -import HeaderReportDropdown, { HeaderReportProps } from '.'; +import { Menu, MenuItem } from '@superset-ui/core/components/Menu'; +import { useHeaderReportMenuItems, HeaderReportProps } from './index'; const createProps = () => ({ dashboardId: 1, - useTextMenu: false, setShowReportSubMenu: jest.fn, showReportModal: jest.fn, setCurrentReportDeleting: jest.fn, @@ -115,13 +114,14 @@ const stateWithUserAndReport = { }, }; +const MenuWrapper = (props: HeaderReportProps) => { + const reportMenuItems = useHeaderReportMenuItems(props); + const menuItems: MenuItem[] = [reportMenuItems]; + return <Menu items={menuItems} forceSubMenuRender />; +}; + function setup(props: HeaderReportProps, initialState = {}) { - render( - <Menu> - <HeaderReportDropdown {...props} /> - </Menu>, - { useRedux: true, initialState }, - ); + render(<MenuWrapper {...props} />, { useRedux: true, initialState }); } jest.mock('@superset-ui/core', () => ({ @@ -147,7 +147,7 @@ describe('Header Report Dropdown', () => { act(() => { setup(mockedProps, stateWithUserAndReport); }); - expect(screen.getByRole('menuitem')).toBeInTheDocument(); + expect(screen.getAllByRole('menuitem')[0]).toBeInTheDocument(); }); it('renders the dropdown correctly', async () => { @@ -155,8 +155,6 @@ describe('Header Report Dropdown', () => { act(() => { setup(mockedProps, stateWithUserAndReport); }); - const emailReportModalButton = screen.getByRole('menuitem'); - userEvent.hover(emailReportModalButton); expect(await screen.findByText('Email reports active')).toBeInTheDocument(); expect(screen.getByText('Edit email report')).toBeInTheDocument(); expect(screen.getByText('Delete email report')).toBeInTheDocument(); @@ -168,8 +166,6 @@ describe('Header Report Dropdown', () => { act(() => { setup(mockedProps, stateWithUserAndReport); }); - const emailReportModalButton = screen.getByRole('menuitem'); - userEvent.click(emailReportModalButton); const editModal = await screen.findByText('Edit email report'); userEvent.click(editModal); expect(mockedProps.showReportModal).toHaveBeenCalled(); @@ -181,49 +177,34 @@ describe('Header Report Dropdown', () => { act(() => { setup(mockedProps, stateWithUserAndReport); }); - const emailReportModalButton = screen.getByRole('menuitem'); - userEvent.click(emailReportModalButton); const deleteModal = await screen.findByText('Delete email report'); userEvent.click(deleteModal); expect(mockedProps.setCurrentReportDeleting).toHaveBeenCalled(); }); - it('renders Manage Email Reports Menu if textMenu is set to true and there is a report', async () => { - let mockedProps = createProps(); - mockedProps = { - ...mockedProps, - useTextMenu: true, - }; + it('renders Manage Email Reports Menu if there is a report', async () => { + const mockedProps = createProps(); act(() => { setup(mockedProps, stateWithUserAndReport); }); - userEvent.click(screen.getByRole('menuitem')); expect(await screen.findByText('Email reports active')).toBeInTheDocument(); expect(screen.getByText('Edit email report')).toBeInTheDocument(); expect(screen.getByText('Delete email report')).toBeInTheDocument(); }); - it('renders Schedule Email Reports if textMenu is set to true and there is a report', async () => { - let mockedProps = createProps(); - mockedProps = { - ...mockedProps, - useTextMenu: true, - }; + it('renders Schedule Email Reports if there is a report', async () => { + const mockedProps = createProps(); + act(() => { setup(mockedProps, stateWithOnlyUser); }); - userEvent.click(screen.getByRole('menuitem')); expect( await screen.findByText('Set up an email report'), ).toBeInTheDocument(); }); it('renders Schedule Email Reports as long as user has permission through any role', async () => { - let mockedProps = createProps(); - mockedProps = { - ...mockedProps, - useTextMenu: true, - }; + const mockedProps = createProps(); act(() => { setup(mockedProps, stateWithNonAdminUser); }); @@ -234,11 +215,8 @@ describe('Header Report Dropdown', () => { }); it('do not render Schedule Email Reports if user no permission', () => { - let mockedProps = createProps(); - mockedProps = { - ...mockedProps, - useTextMenu: true, - }; + const mockedProps = createProps(); + act(() => { setup(mockedProps, stateWithNonMenuAccessOnManage); }); diff --git a/superset-frontend/src/features/reports/ReportModal/HeaderReportDropdown/index.tsx b/superset-frontend/src/features/reports/ReportModal/HeaderReportDropdown/index.tsx index a094cd3f65..ad80f98205 100644 --- a/superset-frontend/src/features/reports/ReportModal/HeaderReportDropdown/index.tsx +++ b/superset-frontend/src/features/reports/ReportModal/HeaderReportDropdown/index.tsx @@ -16,24 +16,20 @@ * specific language governing permissions and limitations * under the License. */ -import { ReactNode, useEffect } from 'react'; +import { useEffect } from 'react'; import { useSelector, useDispatch } from 'react-redux'; -import { isEmpty } from 'lodash'; import { t, - SupersetTheme, - css, styled, FeatureFlag, isFeatureEnabled, getExtensionsRegistry, usePrevious, + css, } from '@superset-ui/core'; -import { Icons } from '@superset-ui/core/components/Icons'; -import { Switch } from '@superset-ui/core/components/Switch'; -import { AlertObject } from 'src/features/alerts/types'; -import { Menu } from '@superset-ui/core/components/Menu'; +import { MenuItem } from '@superset-ui/core/components/Menu'; import { Checkbox } from '@superset-ui/core/components'; +import { AlertObject } from 'src/features/alerts/types'; import { noOp } from 'src/utils/common'; import { ChartState } from 'src/explore/types'; import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes'; @@ -41,35 +37,14 @@ import { fetchUISpecificReport, toggleActive, } from 'src/features/reports/ReportModal/actions'; -import { reportSelector } from 'src/views/CRUD/hooks'; import { MenuItemWithCheckboxContainer } from 'src/explore/components/useExploreAdditionalActionsMenu/index'; const extensionsRegistry = getExtensionsRegistry(); -const deleteColor = (theme: SupersetTheme) => css` - color: ${theme.colorError}; -`; - -const onMenuHover = (theme: SupersetTheme) => css` - & .ant-menu-item { - padding: 5px 12px; - margin-top: 0px; - margin-bottom: 4px; - :hover { - color: ${theme.colorText}; - } - } - :hover { - background-color: ${theme.colorPrimaryBg}; - } -`; - -const onMenuItemHover = (theme: SupersetTheme) => css` - &:hover { - color: ${theme.colorText}; - background-color: ${theme.colorPrimaryBg}; - } -`; +export enum CreationMethod { + Charts = 'charts', + Dashboards = 'dashboards', +} const StyledDropdownItemWithIcon = styled.div` display: flex; @@ -85,63 +60,59 @@ const DropdownItemExtension = extensionsRegistry.get( 'report-modal.dropdown.item.icon', ); -export enum CreationMethod { - Charts = 'charts', - Dashboards = 'dashboards', -} export interface HeaderReportProps { dashboardId?: number; chart?: ChartState; - useTextMenu?: boolean; - setShowReportSubMenu?: (show: boolean) => void; - showReportSubMenu?: boolean; - submenuTitle?: string; showReportModal: () => void; setCurrentReportDeleting: (report: AlertObject | null) => void; } -// Same instance to be used in useEffects -const EMPTY_OBJECT = {}; - -export default function HeaderReportDropDown({ +export const useHeaderReportMenuItems = ({ dashboardId, chart, - useTextMenu = false, - setShowReportSubMenu, - submenuTitle, showReportModal, setCurrentReportDeleting, -}: HeaderReportProps) { +}: HeaderReportProps): MenuItem | null => { const dispatch = useDispatch(); - const report = useSelector<any, AlertObject>(state => { - const resourceType = dashboardId - ? CreationMethod.Dashboards - : CreationMethod.Charts; - return ( - reportSelector(state, resourceType, dashboardId || chart?.id) || - EMPTY_OBJECT - ); + const resourceId = dashboardId || chart?.id; + const resourceType = dashboardId + ? CreationMethod.Dashboards + : CreationMethod.Charts; + + // Select the reports state and specific report with proper reactivity + const report = useSelector<any, AlertObject | null>(state => { + if (!resourceId) return null; + // Select directly from the reports state to ensure reactivity + const reportsState = state.reports || {}; + const resourceTypeReports = reportsState[resourceType] || {}; + const reportData = resourceTypeReports[resourceId]; + + // Debug logging to understand what's happening + console.log('Report selector called:', { + resourceId, + resourceType, + reportsState: Object.keys(reportsState), + resourceTypeReports: Object.keys(resourceTypeReports), + reportData: reportData + ? { id: reportData.id, name: reportData.name } + : null, + }); + + return reportData || null; }); - const isReportActive: boolean = report?.active || false; const user: UserWithPermissionsAndRoles = useSelector< any, UserWithPermissionsAndRoles >(state => state.user); - const canAddReports = () => { - if (!isFeatureEnabled(FeatureFlag.AlertReports)) { - return false; - } - if (!user?.userId) { - // this is in the case that there is an anonymous user. - return false; - } + const prevDashboard = usePrevious(dashboardId); - // Cannot add reports if the resource is not saved - if (!(dashboardId || chart?.id)) { - return false; - } + // Check if user can add reports + const canAddReports = () => { + if (!isFeatureEnabled(FeatureFlag.AlertReports)) return false; + if (!user?.userId) return false; + if (!resourceId) return false; const roles = Object.keys(user.roles || []); const permissions = roles.map(key => @@ -152,17 +123,11 @@ export default function HeaderReportDropDown({ return permissions.some(permission => permission.length > 0); }; - const prevDashboard = usePrevious(dashboardId); - const toggleActiveKey = async (data: AlertObject, checked: boolean) => { - if (data?.id) { - dispatch(toggleActive(data, checked)); - } - }; - const shouldFetch = canAddReports() && !!((dashboardId && prevDashboard !== dashboardId) || chart?.id); + // Fetch report data when needed useEffect(() => { if (shouldFetch) { dispatch( @@ -170,113 +135,82 @@ export default function HeaderReportDropDown({ userId: user.userId, filterField: dashboardId ? 'dashboard_id' : 'chart_id', creationMethod: dashboardId ? 'dashboards' : 'charts', - resourceId: dashboardId || chart?.id, + resourceId, }), ); } - }, []); + }, [dispatch, shouldFetch, user?.userId, dashboardId, resourceId]); - const showReportSubMenu = report && setShowReportSubMenu && canAddReports(); + // Don't show anything if user can't add reports + if (!canAddReports()) { + return null; + } - useEffect(() => { - if (showReportSubMenu) { - setShowReportSubMenu(true); - } else if (!report && setShowReportSubMenu) { - setShowReportSubMenu(false); + // Handler functions + const handleShowModal = () => showReportModal(); + const handleDeleteReport = () => setCurrentReportDeleting(report); + const handleToggleActive = () => { + if (report?.id) { + dispatch(toggleActive(report, !report.active)); } - }, [report]); - - const handleShowMenu = () => { - showReportModal(); - }; - - const handleDeleteMenuClick = () => { - setCurrentReportDeleting(report); }; - const textMenu = () => - isEmpty(report) ? ( - <Menu.SubMenu title={submenuTitle} css={onMenuHover}> - <Menu.Item onClick={handleShowMenu}> - {DropdownItemExtension ? ( + // If no report exists, show "Set up email report" option + if (!report || !report.id) { + return { + key: 'email-report-setup', + type: 'submenu', + label: t('Manage email report'), + children: [ + { + key: 'set-up-report', + label: DropdownItemExtension ? ( <StyledDropdownItemWithIcon> <div>{t('Set up an email report')}</div> <DropdownItemExtension /> </StyledDropdownItemWithIcon> ) : ( t('Set up an email report') - )} - </Menu.Item> - <Menu.Divider /> - </Menu.SubMenu> - ) : ( - <Menu.SubMenu - title={submenuTitle} - css={css` - border: none; - `} - > - <Menu.Item - css={onMenuItemHover} - onClick={() => toggleActiveKey(report, !isReportActive)} - > + ), + onClick: handleShowModal, + }, + ], + }; + } + + // If report exists, show management options + return { + key: 'email-report-manage', + type: 'submenu', + label: t('Manage email report'), + children: [ + { + key: 'toggle-active', + label: ( <MenuItemWithCheckboxContainer> - <Checkbox checked={isReportActive} onChange={noOp} /> + <Checkbox + checked={report.active || false} + onChange={noOp} + css={theme => css` + margin-right: ${theme.sizeUnit}px; + `} + /> {t('Email reports active')} </MenuItemWithCheckboxContainer> - </Menu.Item> - <Menu.Item css={onMenuItemHover} onClick={handleShowMenu}> - {t('Edit email report')} - </Menu.Item> - <Menu.Item css={onMenuItemHover} onClick={handleDeleteMenuClick}> - {t('Delete email report')} - </Menu.Item> - </Menu.SubMenu> - ); - const menu = (title: ReactNode) => ( - <Menu.SubMenu - title={title} - css={css` - width: 200px; - `} - > - <Menu.Item> - {t('Email reports active')} - <Switch - data-test="toggle-active" - checked={isReportActive} - onClick={(checked: boolean) => toggleActiveKey(report, checked)} - size="small" - css={theme => css` - margin-left: ${theme.sizeUnit * 2}px; - `} - /> - </Menu.Item> - <Menu.Item onClick={() => showReportModal()}> - {t('Edit email report')} - </Menu.Item> - <Menu.Item - onClick={() => setCurrentReportDeleting(report)} - css={deleteColor} - > - {t('Delete email report')} - </Menu.Item> - </Menu.SubMenu> - ); - - const iconMenu = () => - isEmpty(report) ? ( - <span - role="button" - title={t('Schedule email report')} - tabIndex={0} - className="action-button action-schedule-report" - onClick={() => showReportModal()} - > - <Icons.CalendarOutlined /> - </span> - ) : ( - menu(<Icons.CalendarOutlined />) - ); - return <>{canAddReports() && (useTextMenu ? textMenu() : iconMenu())}</>; -} + ), + onClick: handleToggleActive, + }, + { + key: 'edit-report', + label: t('Edit email report'), + onClick: handleShowModal, + }, + { + key: 'delete-report', + label: t('Delete email report'), + onClick: handleDeleteReport, + danger: true, + }, + ], + }; +};
