Skip to content
Snippets Groups Projects
Unverified Commit c1d70d36 authored by github-automation-metabase's avatar github-automation-metabase Committed by GitHub
Browse files

Fix pdf exports issues (#51491) (#51540)


* fix horizontally trimmed content on short pages

* include filters

* update specs

Co-authored-by: default avatarAleksandr Lesnenko <alxnddr@users.noreply.github.com>
parent c2f9e583
No related branches found
No related tags found
No related merge requests found
...@@ -165,22 +165,6 @@ export const ParametersAndCardsContainer = styled.div<{ ...@@ -165,22 +165,6 @@ export const ParametersAndCardsContainer = styled.div<{
/* Makes sure it doesn't use all the height, so the actual content height could be used in embedding #37437 */ /* Makes sure it doesn't use all the height, so the actual content height could be used in embedding #37437 */
align-self: ${({ shouldMakeDashboardHeaderStickyAfterScrolling }) => align-self: ${({ shouldMakeDashboardHeaderStickyAfterScrolling }) =>
!shouldMakeDashboardHeaderStickyAfterScrolling && "flex-start"}; !shouldMakeDashboardHeaderStickyAfterScrolling && "flex-start"};
&.${SAVING_DOM_IMAGE_CLASS} {
${ParametersWidgetContainer} {
background-color: transparent;
border-bottom: none;
margin-top: 1rem;
legend {
top: -12px;
}
}
${CardsContainer} {
padding-bottom: 20px;
}
}
`; `;
export const FIXED_WIDTH = "1048px"; export const FIXED_WIDTH = "1048px";
...@@ -195,6 +179,12 @@ export const FixedWidthContainer = styled.div<{ ...@@ -195,6 +179,12 @@ export const FixedWidthContainer = styled.div<{
margin: 0 auto; margin: 0 auto;
max-width: ${FIXED_WIDTH}; max-width: ${FIXED_WIDTH};
`} `}
.${SAVING_DOM_IMAGE_CLASS} & {
legend {
top: -9px;
}
}
`; `;
export const ParametersFixedWidthContainer = styled(FixedWidthContainer)` export const ParametersFixedWidthContainer = styled(FixedWidthContainer)`
......
import { DASHBOARD_PARAMETERS_PDF_EXPORT_NODE_ID } from "metabase/dashboard/constants";
import { import {
getDashboardComplete, getDashboardComplete,
getIsEditing, getIsEditing,
...@@ -70,6 +71,7 @@ export function DashboardParameterPanel({ ...@@ -70,6 +71,7 @@ export function DashboardParameterPanel({
data-testid="dashboard-parameters-widget-container" data-testid="dashboard-parameters-widget-container"
> >
<ParametersFixedWidthContainer <ParametersFixedWidthContainer
id={DASHBOARD_PARAMETERS_PDF_EXPORT_NODE_ID}
isFixedWidth={dashboard?.width === "fixed"} isFixedWidth={dashboard?.width === "fixed"}
data-testid="fixed-width-filters" data-testid="fixed-width-filters"
> >
......
...@@ -53,6 +53,8 @@ export const DASHBOARD_SLOW_TIMEOUT = 15 * 1000; ...@@ -53,6 +53,8 @@ export const DASHBOARD_SLOW_TIMEOUT = 15 * 1000;
export const DASHBOARD_PDF_EXPORT_ROOT_ID = export const DASHBOARD_PDF_EXPORT_ROOT_ID =
"Dashboard-Parameters-And-Cards-Container"; "Dashboard-Parameters-And-Cards-Container";
export const DASHBOARD_PARAMETERS_PDF_EXPORT_NODE_ID =
"Dashboard-Parameters-Content";
export const DEFAULT_DASHBOARD_DISPLAY_OPTIONS: EmbedDisplayParams = { export const DEFAULT_DASHBOARD_DISPLAY_OPTIONS: EmbedDisplayParams = {
background: true, background: true,
......
...@@ -12,6 +12,7 @@ import Link from "metabase/core/components/Link"; ...@@ -12,6 +12,7 @@ import Link from "metabase/core/components/Link";
import Tooltip from "metabase/core/components/Tooltip"; import Tooltip from "metabase/core/components/Tooltip";
import CS from "metabase/css/core/index.css"; import CS from "metabase/css/core/index.css";
import { DashboardTabs } from "metabase/dashboard/components/DashboardTabs"; import { DashboardTabs } from "metabase/dashboard/components/DashboardTabs";
import { DASHBOARD_PARAMETERS_PDF_EXPORT_NODE_ID } from "metabase/dashboard/constants";
import { Dashboard } from "metabase/dashboard/containers/Dashboard"; import { Dashboard } from "metabase/dashboard/containers/Dashboard";
import { DashboardData } from "metabase/dashboard/hoc/DashboardData"; import { DashboardData } from "metabase/dashboard/hoc/DashboardData";
import { getIsHeaderVisible, getTabs } from "metabase/dashboard/selectors"; import { getIsHeaderVisible, getTabs } from "metabase/dashboard/selectors";
...@@ -169,6 +170,7 @@ class AutomaticDashboardAppInner extends Component { ...@@ -169,6 +170,7 @@ class AutomaticDashboardAppInner extends Component {
{parameters && parameters.length > 0 && ( {parameters && parameters.length > 0 && (
<div className={cx(CS.px1, CS.pt1)}> <div className={cx(CS.px1, CS.pt1)}>
<FixedWidthContainer <FixedWidthContainer
id={DASHBOARD_PARAMETERS_PDF_EXPORT_NODE_ID}
data-testid="fixed-width-filters" data-testid="fixed-width-filters"
isFixedWidth={dashboard?.width === "fixed"} isFixedWidth={dashboard?.width === "fixed"}
> >
......
...@@ -11,7 +11,10 @@ import { ...@@ -11,7 +11,10 @@ import {
ParametersFixedWidthContainer, ParametersFixedWidthContainer,
} from "metabase/dashboard/components/Dashboard/Dashboard.styled"; } from "metabase/dashboard/components/Dashboard/Dashboard.styled";
import { ExportAsPdfButton } from "metabase/dashboard/components/DashboardHeader/buttons/ExportAsPdfButton"; import { ExportAsPdfButton } from "metabase/dashboard/components/DashboardHeader/buttons/ExportAsPdfButton";
import { DASHBOARD_PDF_EXPORT_ROOT_ID } from "metabase/dashboard/constants"; import {
DASHBOARD_PARAMETERS_PDF_EXPORT_NODE_ID,
DASHBOARD_PDF_EXPORT_ROOT_ID,
} from "metabase/dashboard/constants";
import { getDashboardType } from "metabase/dashboard/utils"; import { getDashboardType } from "metabase/dashboard/utils";
import { initializeIframeResizer, isSmallScreen } from "metabase/lib/dom"; import { initializeIframeResizer, isSmallScreen } from "metabase/lib/dom";
import { useSelector } from "metabase/lib/redux"; import { useSelector } from "metabase/lib/redux";
...@@ -211,6 +214,7 @@ export const EmbedFrame = ({ ...@@ -211,6 +214,7 @@ export const EmbedFrame = ({
data-testid="dashboard-parameters-widget-container" data-testid="dashboard-parameters-widget-container"
> >
<ParametersFixedWidthContainer <ParametersFixedWidthContainer
id={DASHBOARD_PARAMETERS_PDF_EXPORT_NODE_ID}
data-testid="fixed-width-filters" data-testid="fixed-width-filters"
isFixedWidth={dashboard?.width === "fixed"} isFixedWidth={dashboard?.width === "fixed"}
> >
......
import { t } from "ttag"; import { t } from "ttag";
import _ from "underscore"; import _ from "underscore";
import { DASHBOARD_PARAMETERS_PDF_EXPORT_NODE_ID } from "metabase/dashboard/constants";
import type { Dashboard } from "metabase-types/api"; import type { Dashboard } from "metabase-types/api";
import { SAVING_DOM_IMAGE_CLASS } from "./save-chart-image"; import { SAVING_DOM_IMAGE_CLASS } from "./save-chart-image";
...@@ -75,6 +76,7 @@ export const getPageBreaks = ( ...@@ -75,6 +76,7 @@ export const getPageBreaks = (
sortedCards: DashCardBounds[], sortedCards: DashCardBounds[],
optimalPageHeight: number, optimalPageHeight: number,
totalHeight: number, totalHeight: number,
minPageHeight: number,
offset = 0, offset = 0,
): number[] => { ): number[] => {
if (sortedCards.length === 0) { if (sortedCards.length === 0) {
...@@ -88,7 +90,6 @@ export const getPageBreaks = ( ...@@ -88,7 +90,6 @@ export const getPageBreaks = (
return []; return [];
} }
const minPageSize = optimalPageHeight * 0.7;
const result: number[] = []; const result: number[] = [];
let currentPageStart = 0; let currentPageStart = 0;
let candidateIndex = 0; let candidateIndex = 0;
...@@ -98,7 +99,7 @@ export const getPageBreaks = ( ...@@ -98,7 +99,7 @@ export const getPageBreaks = (
while ( while (
candidateIndex < pageBreakCandidates.length && candidateIndex < pageBreakCandidates.length &&
pageBreakCandidates[candidateIndex] <= currentPageStart + minPageSize pageBreakCandidates[candidateIndex] <= currentPageStart + minPageHeight
) { ) {
candidateIndex++; candidateIndex++;
} }
...@@ -141,7 +142,8 @@ const createHeaderElement = (dashboardName: string, marginBottom: number) => { ...@@ -141,7 +142,8 @@ const createHeaderElement = (dashboardName: string, marginBottom: number) => {
return header; return header;
}; };
const HEADER_MARGIN_BOTTOM = 8; const HEADER_MARGIN_BOTTOM = 12;
const PARAMETERS_MARGIN_BOTTOM = 12;
const PAGE_PADDING = 16; const PAGE_PADDING = 16;
export const saveDashboardPdf = async ( export const saveDashboardPdf = async (
...@@ -149,25 +151,37 @@ export const saveDashboardPdf = async ( ...@@ -149,25 +151,37 @@ export const saveDashboardPdf = async (
dashboardName: string, dashboardName: string,
) => { ) => {
const fileName = `${dashboardName}.pdf`; const fileName = `${dashboardName}.pdf`;
const node = document const dashboardRoot = document.querySelector(selector);
.querySelector(selector) const gridNode = dashboardRoot?.querySelector(".react-grid-layout");
?.querySelector(".react-grid-layout");
if (!node || !(node instanceof HTMLElement)) { if (!gridNode || !(gridNode instanceof HTMLElement)) {
console.warn("No dashboard content found", selector); console.warn("No dashboard content found", selector);
return; return;
} }
const cardsBounds = getSortedDashCardBounds(node); const cardsBounds = getSortedDashCardBounds(gridNode);
const pdfHeader = createHeaderElement(dashboardName, HEADER_MARGIN_BOTTOM); const pdfHeader = createHeaderElement(dashboardName, HEADER_MARGIN_BOTTOM);
const parametersNode = dashboardRoot
?.querySelector(`#${DASHBOARD_PARAMETERS_PDF_EXPORT_NODE_ID}`)
?.cloneNode(true);
let parametersHeight = 0;
if (parametersNode instanceof HTMLElement) {
gridNode.append(parametersNode);
parametersNode.style.cssText = `margin-bottom: ${PARAMETERS_MARGIN_BOTTOM}px`;
parametersHeight =
parametersNode.getBoundingClientRect().height + PARAMETERS_MARGIN_BOTTOM;
gridNode.removeChild(parametersNode);
}
node.appendChild(pdfHeader); gridNode.appendChild(pdfHeader);
const headerHeight = const headerHeight =
pdfHeader.getBoundingClientRect().height + HEADER_MARGIN_BOTTOM; pdfHeader.getBoundingClientRect().height + HEADER_MARGIN_BOTTOM;
node.removeChild(pdfHeader); gridNode.removeChild(pdfHeader);
const contentWidth = node.offsetWidth; const verticalOffset = headerHeight + parametersHeight;
const contentHeight = node.offsetHeight + headerHeight; const contentWidth = gridNode.offsetWidth;
const contentHeight = gridNode.offsetHeight + verticalOffset;
const width = contentWidth + PAGE_PADDING * 2; const width = contentWidth + PAGE_PADDING * 2;
const backgroundColor = getComputedStyle(document.documentElement) const backgroundColor = getComputedStyle(document.documentElement)
...@@ -175,7 +189,7 @@ export const saveDashboardPdf = async ( ...@@ -175,7 +189,7 @@ export const saveDashboardPdf = async (
.trim(); .trim();
const { default: html2canvas } = await import("html2canvas-pro"); const { default: html2canvas } = await import("html2canvas-pro");
const image = await html2canvas(node, { const image = await html2canvas(gridNode, {
height: contentHeight, height: contentHeight,
width: contentWidth, width: contentWidth,
useCORS: true, useCORS: true,
...@@ -183,18 +197,25 @@ export const saveDashboardPdf = async ( ...@@ -183,18 +197,25 @@ export const saveDashboardPdf = async (
node.classList.add(SAVING_DOM_IMAGE_CLASS); node.classList.add(SAVING_DOM_IMAGE_CLASS);
node.style.height = `${contentHeight}px`; node.style.height = `${contentHeight}px`;
node.style.backgroundColor = backgroundColor; node.style.backgroundColor = backgroundColor;
if (parametersNode instanceof HTMLElement) {
node.insertBefore(parametersNode, node.firstChild);
}
node.insertBefore(pdfHeader, node.firstChild); node.insertBefore(pdfHeader, node.firstChild);
}, },
}); });
const { default: jspdf } = await import("jspdf"); const { default: jspdf } = await import("jspdf");
// Page page height cannot be smaller than page width otherwise the content will be cut off
// or the page should have a landscape orientation.
const minPageHeight = contentWidth;
const optimalPageHeight = Math.round(width * TARGET_ASPECT_RATIO); const optimalPageHeight = Math.round(width * TARGET_ASPECT_RATIO);
const pageBreaks = getPageBreaks( const pageBreaks = getPageBreaks(
cardsBounds, cardsBounds,
optimalPageHeight - PAGE_PADDING * 2, optimalPageHeight - PAGE_PADDING * 2,
contentHeight, contentHeight,
headerHeight, minPageHeight,
verticalOffset,
); );
const pdf = new jspdf({ const pdf = new jspdf({
......
...@@ -94,7 +94,7 @@ describe("save-dashboard-pdf", () => { ...@@ -94,7 +94,7 @@ describe("save-dashboard-pdf", () => {
}, },
]; ];
const breaks = getPageBreaks(cards, 300, 200); const breaks = getPageBreaks(cards, 300, 200, 100);
expect(breaks).toEqual([]); expect(breaks).toEqual([]);
}); });
...@@ -111,7 +111,7 @@ describe("save-dashboard-pdf", () => { ...@@ -111,7 +111,7 @@ describe("save-dashboard-pdf", () => {
]; ];
const optimalPageHeight = 270; const optimalPageHeight = 270;
const breaks = getPageBreaks(cards, optimalPageHeight, 500); const breaks = getPageBreaks(cards, optimalPageHeight, 500, 100);
expect(breaks).toEqual([250]); // The page break at 250 is the closest to the optimal page height of 270 expect(breaks).toEqual([250]); // The page break at 250 is the closest to the optimal page height of 270
}); });
...@@ -132,11 +132,11 @@ describe("save-dashboard-pdf", () => { ...@@ -132,11 +132,11 @@ describe("save-dashboard-pdf", () => {
}, },
]; ];
const breaks = getPageBreaks(cards, 200, 600); const breaks = getPageBreaks(cards, 200, 600, 100);
expect(breaks).toEqual([200, 400]); expect(breaks).toEqual([200, 400]);
}); });
it("should respect minimum page size constraint", () => { it("should allow breaks that respect minimum page size", () => {
const cards = [ const cards = [
{ top: 0, bottom: 100, height: 100, allowedBreaks: new Set<number>() }, { top: 0, bottom: 100, height: 100, allowedBreaks: new Set<number>() },
{ {
...@@ -148,8 +148,25 @@ describe("save-dashboard-pdf", () => { ...@@ -148,8 +148,25 @@ describe("save-dashboard-pdf", () => {
{ top: 250, bottom: 300, height: 50, allowedBreaks: new Set<number>() }, { top: 250, bottom: 300, height: 50, allowedBreaks: new Set<number>() },
]; ];
const breaks = getPageBreaks(cards, 200, 300); const breaks = getPageBreaks(cards, 200, 300, 100);
expect(breaks).toEqual([250]); expect(breaks).toEqual([250]);
}); });
it("should not break if it would create pages smaller than minimum size", () => {
const cards = [
{ top: 0, bottom: 150, height: 150, allowedBreaks: new Set<number>() },
{
top: 150,
bottom: 400,
height: 250,
allowedBreaks: new Set<number>(),
},
];
// With minPageHeight of 200, breaking at 150 would create a first page
// of only 150px height, which is less than minimum, so no breaks should occur
const breaks = getPageBreaks(cards, 250, 400, 200);
expect(breaks).toEqual([]);
});
}); });
}); });
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment