From c1aacde3041e9a7b5f42acb97e092b906954c9ee Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Sat, 20 Feb 2021 18:45:03 +0100 Subject: [PATCH] fix(explore): Fix downloading as image charts which use Mapbox (#13181) * Fix downloading as image charts which use Mapbox * Remove unused prop * Bump deckgl version --- superset-frontend/package-lock.json | 19 ++++++++++++------- superset-frontend/package.json | 2 +- .../explore/components/DisplayQueryButton.jsx | 10 ++++------ .../components/ExploreActionButtons.tsx | 3 --- .../explore/components/ExploreChartHeader.jsx | 2 -- .../explore/components/ExploreChartPanel.jsx | 1 - .../src/utils/downloadAsImage.ts | 15 +++++++++++++-- 7 files changed, 30 insertions(+), 22 deletions(-) diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index 4e178154f..cc750c31d 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -35,7 +35,7 @@ "@superset-ui/legacy-plugin-chart-treemap": "^0.17.9", "@superset-ui/legacy-plugin-chart-world-map": "^0.17.9", "@superset-ui/legacy-preset-chart-big-number": "^0.17.9", - "@superset-ui/legacy-preset-chart-deckgl": "^0.4.1", + "@superset-ui/legacy-preset-chart-deckgl": "^0.4.2", "@superset-ui/legacy-preset-chart-nvd3": "^0.17.9", "@superset-ui/plugin-chart-echarts": "^0.17.9", "@superset-ui/plugin-chart-table": "^0.17.9", @@ -14579,9 +14579,9 @@ } }, "node_modules/@superset-ui/legacy-preset-chart-deckgl": { - "version": "0.4.1", - "resolved": "https://registry.npmjs.org/@superset-ui/legacy-preset-chart-deckgl/-/legacy-preset-chart-deckgl-0.4.1.tgz", - "integrity": "sha512-3THN+WM8HUU1NlV3VNXRVS1j2jH33CmVAdyPNB35RqtwkY+udgFGOxm0lXumcUElht/3ROGMPvcwo1SXijVuLA==", + "version": "0.4.2", + "resolved": "https://registry.npmjs.org/@superset-ui/legacy-preset-chart-deckgl/-/legacy-preset-chart-deckgl-0.4.2.tgz", + "integrity": "sha512-lr9K0KihVgaCjE72Zu/nd3YoU/q2EDX7qd73y95vzoXBgIA2cMfIkAmEnOe2Komb51ed9hzz0MnudfZHZk1cfw==", "dependencies": { "@math.gl/web-mercator": "^3.2.2", "@types/d3-array": "^2.0.0", @@ -14601,6 +14601,11 @@ "underscore": "^1.8.3", "urijs": "^1.18.10", "xss": "^1.0.6" + }, + "peerDependencies": { + "@superset-ui/chart-controls": "^0.16.3", + "@superset-ui/core": "^0.16.3", + "react": "^15 || ^16" } }, "node_modules/@superset-ui/legacy-preset-chart-nvd3": { @@ -68832,9 +68837,9 @@ } }, "@superset-ui/legacy-preset-chart-deckgl": { - "version": "0.4.1", - "resolved": "https://registry.npmjs.org/@superset-ui/legacy-preset-chart-deckgl/-/legacy-preset-chart-deckgl-0.4.1.tgz", - "integrity": "sha512-3THN+WM8HUU1NlV3VNXRVS1j2jH33CmVAdyPNB35RqtwkY+udgFGOxm0lXumcUElht/3ROGMPvcwo1SXijVuLA==", + "version": "0.4.2", + "resolved": "https://registry.npmjs.org/@superset-ui/legacy-preset-chart-deckgl/-/legacy-preset-chart-deckgl-0.4.2.tgz", + "integrity": "sha512-lr9K0KihVgaCjE72Zu/nd3YoU/q2EDX7qd73y95vzoXBgIA2cMfIkAmEnOe2Komb51ed9hzz0MnudfZHZk1cfw==", "requires": { "@math.gl/web-mercator": "^3.2.2", "@types/d3-array": "^2.0.0", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index a2aaf3c69..6c58457b1 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -87,7 +87,7 @@ "@superset-ui/legacy-plugin-chart-treemap": "^0.17.9", "@superset-ui/legacy-plugin-chart-world-map": "^0.17.9", "@superset-ui/legacy-preset-chart-big-number": "^0.17.9", - "@superset-ui/legacy-preset-chart-deckgl": "^0.4.1", + "@superset-ui/legacy-preset-chart-deckgl": "^0.4.2", "@superset-ui/legacy-preset-chart-nvd3": "^0.17.9", "@superset-ui/plugin-chart-echarts": "^0.17.9", "@superset-ui/plugin-chart-table": "^0.17.9", diff --git a/superset-frontend/src/explore/components/DisplayQueryButton.jsx b/superset-frontend/src/explore/components/DisplayQueryButton.jsx index 16bbf1d51..ae24ee3e5 100644 --- a/superset-frontend/src/explore/components/DisplayQueryButton.jsx +++ b/superset-frontend/src/explore/components/DisplayQueryButton.jsx @@ -47,7 +47,6 @@ const propTypes = { onOpenPropertiesModal: PropTypes.func, onOpenInEditor: PropTypes.func, chartStatus: PropTypes.string, - chartHeight: PropTypes.string.isRequired, latestQueryFormData: PropTypes.object.isRequired, slice: PropTypes.object, }; @@ -100,7 +99,7 @@ export const DisplayQueryButton = props => { }; const handleMenuClick = ({ key, domEvent }) => { - const { chartHeight, slice, onOpenInEditor, latestQueryFormData } = props; + const { slice, onOpenInEditor, latestQueryFormData } = props; switch (key) { case MENU_KEYS.EDIT_PROPERTIES: props.onOpenPropertiesModal(); @@ -110,12 +109,11 @@ export const DisplayQueryButton = props => { break; case MENU_KEYS.DOWNLOAD_AS_IMAGE: downloadAsImage( - '.chart-container', + '.panel-body > .chart-container', // eslint-disable-next-line camelcase slice?.slice_name ?? t('New chart'), - { - height: parseInt(chartHeight, 10), - }, + {}, + true, )(domEvent); break; default: diff --git a/superset-frontend/src/explore/components/ExploreActionButtons.tsx b/superset-frontend/src/explore/components/ExploreActionButtons.tsx index 60f7301bb..cbf038b47 100644 --- a/superset-frontend/src/explore/components/ExploreActionButtons.tsx +++ b/superset-frontend/src/explore/components/ExploreActionButtons.tsx @@ -41,7 +41,6 @@ type ActionButtonProps = { type ExploreActionButtonsProps = { actions: { redirectSQLLab: Function; openPropertiesModal: Function }; canDownload: boolean; - chartHeight: number; chartStatus: string; latestQueryFormData: {}; queriesResponse: {}; @@ -85,7 +84,6 @@ const ExploreActionButtons = (props: ExploreActionButtonsProps) => { const { actions, canDownload, - chartHeight, chartStatus, latestQueryFormData, queriesResponse, @@ -189,7 +187,6 @@ const ExploreActionButtons = (props: ExploreActionButtonsProps) => { )} diff --git a/superset-frontend/src/explore/components/ExploreChartPanel.jsx b/superset-frontend/src/explore/components/ExploreChartPanel.jsx index 61916984d..12fb78417 100644 --- a/superset-frontend/src/explore/components/ExploreChartPanel.jsx +++ b/superset-frontend/src/explore/components/ExploreChartPanel.jsx @@ -241,7 +241,6 @@ const ExploreChartPanel = props => { addHistory={props.addHistory} can_overwrite={props.can_overwrite} can_download={props.can_download} - chartHeight={props.height} isStarred={props.isStarred} slice={props.slice} sliceName={props.sliceName} diff --git a/superset-frontend/src/utils/downloadAsImage.ts b/superset-frontend/src/utils/downloadAsImage.ts index a315e1df8..ad23d366e 100644 --- a/superset-frontend/src/utils/downloadAsImage.ts +++ b/superset-frontend/src/utils/downloadAsImage.ts @@ -43,16 +43,20 @@ const generateFileStem = (description: string, date = new Date()) => * @param selector css selector of the parent element which should be turned into image * @param description name or a short description of what is being printed. * Value will be normalized, and a date as well as a file extension will be added. - * @param backgroundColor background color to apply to screenshot document + * @param domToImageOptions dom-to-image Options object. + * @param isExactSelector if false, searches for the closest ancestor that matches selector. * @returns event handler */ export default function downloadAsImage( selector: string, description: string, domToImageOptions: Options = {}, + isExactSelector = false, ) { return (event: SyntheticEvent) => { - const elementToPrint = event.currentTarget.closest(selector); + const elementToPrint = isExactSelector + ? document.querySelector(selector) + : event.currentTarget.closest(selector); if (!elementToPrint) { return addWarningToast( @@ -64,6 +68,10 @@ export default function downloadAsImage( .toJpeg(elementToPrint, { quality: 0.95, bgcolor: GRAY_BACKGROUND_COLOR, + // Mapbox controls are loaded from different origin, causing CORS error + // See https://developer.mozilla.org/en-US/docs/Web/API/HTMLCanvasElement/toDataURL#exceptions + filter: (node: Element) => + node.className !== 'mapboxgl-control-container', ...domToImageOptions, }) .then(dataUrl => { @@ -71,6 +79,9 @@ export default function downloadAsImage( link.download = `${generateFileStem(description)}.jpg`; link.href = dataUrl; link.click(); + }) + .catch(e => { + console.error('Creating image failed', e); }); }; }