EnxDev commented on code in PR #37558:
URL: https://github.com/apache/superset/pull/37558#discussion_r2797828016
##########
superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx:
##########
@@ -270,6 +275,61 @@ const SliceHeaderControls = (
});
break;
}
+ case MenuKeys.DownloadAsPngTransparent:
+ case MenuKeys.DownloadAsPngSolid: {
+ const menu = document.querySelector(
+ '.ant-dropdown:not(.ant-dropdown-hidden)',
+ ) as HTMLElement;
+ if (menu) {
+ menu.style.visibility = 'hidden';
+ }
+
+ const backgroundType =
+ key === MenuKeys.DownloadAsPngTransparent ? 'transparent' : 'solid';
+
+ downloadAsImage(
+ getScreenshotNodeSelector(props.slice.slice_id),
+ props.slice.slice_name,
+ true,
+ theme,
+ { format: 'png', backgroundType },
+ )(domEvent).then(() => {
+ if (menu) {
+ menu.style.visibility = 'visible';
+ }
+ });
+
+ props.logEvent?.(LOG_ACTIONS_CHART_DOWNLOAD_AS_PNG, {
+ chartId: props.slice.slice_id,
+ backgroundType,
+ });
+ break;
+ }
+ case MenuKeys.DownloadAsPdf: {
+ const menu = document.querySelector(
+ '.ant-dropdown:not(.ant-dropdown-hidden)',
+ ) as HTMLElement;
+ if (menu) {
+ menu.style.visibility = 'hidden';
+ }
+
+ downloadAsPdf(
+ getScreenshotNodeSelector(props.slice.slice_id),
+ props.slice.slice_name,
+ true,
+ )(domEvent);
+
+ setTimeout(() => {
+ if (menu) {
+ menu.style.visibility = 'visible';
+ }
+ }, 100);
Review Comment:
Could you clarify why a timeout is needed here?
##########
superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx:
##########
@@ -270,6 +275,61 @@ const SliceHeaderControls = (
});
break;
}
+ case MenuKeys.DownloadAsPngTransparent:
+ case MenuKeys.DownloadAsPngSolid: {
+ const menu = document.querySelector(
+ '.ant-dropdown:not(.ant-dropdown-hidden)',
+ ) as HTMLElement;
+ if (menu) {
Review Comment:
Since this depends on Ant Design’s internal CSS classes, it may be somewhat
fragile. Updates to the dropdown library or multiple visible dropdowns could
cause the selector to behave unexpectedly
##########
superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx:
##########
@@ -550,6 +610,27 @@ const SliceHeaderControls = (
label: t('Download as image'),
icon: <Icons.FileImageOutlined css={dropdownIconsStyles} />,
},
+ {
+ type: 'submenu',
+ key: 'download_as_png_submenu',
+ label: t('Download as image (png)'),
Review Comment:
The Explore view uses ‘Export screenshot (PNG)’. Would it make sense to
standardize the terminology across the UI?
##########
superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx:
##########
@@ -726,6 +735,75 @@ export const useExploreAdditionalActionsMenu = (
);
},
},
+ {
+ type: 'submenu',
+ key: 'export_all_png_submenu',
+ label: t('Export screenshot (png)'),
+ icon: <Icons.FileImageOutlined />,
+ children: [
+ {
+ key: MENU_KEYS.EXPORT_ALL_PNG_TRANSPARENT,
+ label: t('Transparent background'),
+ onClick: e => {
+ downloadAsImage(
+ '.panel-body .chart-container',
+ slice?.slice_name ?? t('New chart'),
+ true,
+ theme,
+ { format: 'png', backgroundType: 'transparent' },
+ )(e.domEvent);
+ setIsDropdownVisible(false);
+ dispatch(
+ logEvent(LOG_ACTIONS_CHART_DOWNLOAD_AS_PNG, {
+ chartId: slice?.slice_id,
+ chartName: slice?.slice_name,
+ backgroundType: 'transparent',
+ }),
+ );
+ },
+ },
+ {
+ key: MENU_KEYS.EXPORT_ALL_PNG_SOLID,
+ label: t('Solid background'),
+ onClick: e => {
+ downloadAsImage(
+ '.panel-body .chart-container',
+ slice?.slice_name ?? t('New chart'),
+ true,
+ theme,
+ { format: 'png', backgroundType: 'solid' },
+ )(e.domEvent);
+ setIsDropdownVisible(false);
+ dispatch(
+ logEvent(LOG_ACTIONS_CHART_DOWNLOAD_AS_PNG, {
+ chartId: slice?.slice_id,
+ chartName: slice?.slice_name,
+ backgroundType: 'solid',
+ }),
+ );
+ },
+ },
+ ],
+ },
+ {
+ key: MENU_KEYS.EXPORT_ALL_PDF,
+ label: t('Export as PDF'),
+ icon: <Icons.FileOutlined />,
+ onClick: e => {
+ downloadAsPdf(
+ '.panel-body .chart-container',
+ slice?.slice_name ?? t('New chart'),
+ true,
+ )(e.domEvent);
+ setIsDropdownVisible(false);
+ dispatch(
+ logEvent(LOG_ACTIONS_CHART_DOWNLOAD_AS_PDF, {
+ chartId: slice?.slice_id,
+ chartName: slice?.slice_name,
+ }),
+ );
+ },
+ },
Review Comment:
It seems there’s quite a bit of duplication here, with the only difference
being the MENU_KEYS. Could we consider extracting this into a shared helper?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
