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.

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

Updating Types #1509

Merged
merged 3 commits into from
Jan 9, 2020
Merged

Updating Types #1509

merged 3 commits into from
Jan 9, 2020

Conversation

jpinkster
Copy link
Contributor

There are two missing types that I am requesting to add. jpg support and on the NodeCanvasRenderingContext2D

There are two missing types that I am requesting to add. `jpg` support and on the NodeCanvasRenderingContext2D
Copy link
Collaborator

@zbjornson zbjornson left a comment

Choose a reason for hiding this comment

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

Thanks for finding the addPage mistake! :)

/**
* An additional method .addPage() is then available to create multiple page PDFs
*/
addPage(width?: number, height?: number): void
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oops, good catch. Can you delete the incorrect declaration from line 130-134 (I had it on the Canvas instance instead of the rendering context) please? You could also copy the text from it that describes the default width/height values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated!

types/index.d.ts Outdated
@@ -78,7 +78,7 @@ export class Canvas {
/** Constant used in PNG encoding methods. */
readonly PNG_FILTER_PAETH: number

constructor(width: number, height: number, type?: 'pdf'|'svg')
constructor(width: number, height: number, type?: 'pdf'|'svg'|'jpg')
Copy link
Collaborator

Choose a reason for hiding this comment

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

'jpg' isn't a valid Canvas type, just 'pdf' and 'svg'. Anything else is ignored and creates an image canvas. If you want you could add 'image' here to maybe improve clarity...

node-canvas/src/Canvas.cc

Lines 105 to 110 in 37d556c

if (0 == strcmp("pdf", *Nan::Utf8String(info[2])))
backend = new PdfBackend(width, height);
else if (0 == strcmp("svg", *Nan::Utf8String(info[2])))
backend = new SvgBackend(width, height);
else
backend = new ImageBackend(width, height);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh thanks for that clarity! Updated

Copy link
Collaborator

@zbjornson zbjornson left a comment

Choose a reason for hiding this comment

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

Awesome, thanks! :)

@zbjornson zbjornson merged commit c025143 into Automattic:master Jan 9, 2020
@zbjornson zbjornson mentioned this pull request Jan 29, 2020
@eventualbuddha
Copy link

Could you do a release to include this? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants