Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix: Wrong TypeScript type in jsPDF.table (#3086) #3087

Merged
merged 3 commits into from Feb 10, 2021
Merged

Fix: Wrong TypeScript type in jsPDF.table (#3086) #3087

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5834c1f
Fix: Wrong TypeScript type in jsPDF.table (#3086)
jabaa Feb 8, 2021
e3b1f2c
Fix: Unit tests for array of strings in table header
jabaa Feb 9, 2021
72aae93
Fix: Enable autoSize for array of strings in table header
jabaa Feb 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/modules/cell.js
View file
Open in desktop
Expand Up @@ -442,7 +442,7 @@ import { jsPDF } from "../jspdf.js";
});
}

if (autoSize) {
if (autoSize || (Array.isArray(headers) && typeof headers[0] === "string")) {
var headerName;
for (i = 0; i < headerNames.length; i += 1) {
headerName = headerNames[i];
Expand Down
Binary file added test/reference/table-autoSize-headerNames.pdf
View file
Open in desktop
Binary file not shown.
28 changes: 25 additions & 3 deletions test/specs/cell.spec.js
View file
Open in desktop
Expand Up @@ -71,16 +71,18 @@ describe("Module: Cell", () => {
}));
}

var header = createHeaders([
var headerNames = [
"coin",
"game_group",
"game_name",
"game_version",
"machine",
"vlt"
]);
];

it("table", () => {
var header = createHeaders(headerNames);

it("table with CellConfig[]", () => {
var doc = new jsPDF({
putOnlyUsedFonts: true,
orientation: "landscape",
Expand All @@ -90,6 +92,26 @@ describe("Module: Cell", () => {
comparePdf(doc.output(), "table.pdf");
});

it("table with string[] and without autoSize", () => {
var doc = new jsPDF({
putOnlyUsedFonts: true,
orientation: "landscape",
floatPrecision: 2
});
doc.table(1, 1, generateData(100), headerNames);
comparePdf(doc.output(), "table-autoSize-headerNames.pdf");
});

it("table with string[] and autoSize", () => {
var doc = new jsPDF({
putOnlyUsedFonts: true,
orientation: "landscape",
floatPrecision: 2
});
doc.table(1, 1, generateData(100), headerNames, { autoSize: true });
comparePdf(doc.output(), "table-autoSize-headerNames.pdf");
});

it("table-autoSize", () => {
var doc = new jsPDF({
putOnlyUsedFonts: true,
Expand Down
18 changes: 15 additions & 3 deletions types/index.d.ts
View file
Open in desktop
Expand Up @@ -563,6 +563,18 @@ declare module "jspdf" {
y: number;
}

export interface TableConfig {
printHeaders?: boolean;
autoSize?: boolean;
margins?: number;
fontSize?: number;
padding?: number;
headerBackgroundColor?: string;
css?: {
"font-size": number;
};
}

export interface CellConfig {
name: string;
prompt: string;
Expand Down Expand Up @@ -981,9 +993,9 @@ declare module "jspdf" {
table(
x: number,
y: number,
data: any,
headers: string[],
config: any
data: { [key: string]: string }[],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should also allow numbers and booleans as values here. AFAIK, they work, too.

Copy link
Contributor Author

@jabaa jabaa Feb 9, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think numbers and booleans don't work. At the beginning I used numbers and I had to convert all numbers to strings otherwise the cells were not even drawn. I'm currently checking it using the unit tests. If I use numbers the cells are not drawn. If I use booleans I get the error message getTextDimensions expects text-parameter to be of type String or type Number or an Array of Strings. It looks like numbers should work but don't work and booleans shouldn't work.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright. Thanks for checking. I just remembered that numbers are planned to be fixed in #2875. Then I think we can leave it as is.

headers: string[] | CellConfig[],
config: TableConfig
): jsPDF;
calculateLineHeight(
headerNames: string[],
Expand Down