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

feat: include all standard paper sizes for webContents.print() #37159

Merged
merged 1 commit into from Feb 14, 2023
Merged

feat: include all standard paper sizes for webContents.print() #37159

merged 1 commit into from Feb 14, 2023

Conversation

codebytere
Copy link
Member

Description of Change

Closes #37127. Adds missing standard page sizes.

Checklist

Release Notes

Notes: Added several standard page size options to webContents.print().

@codebytere codebytere added semver/minor backwards-compatible functionality target/22-x-y PR should also be added to the "22-x-y" branch. target/23-x-y PR should also be added to the "23-x-y" branch. labels Feb 7, 2023
@electron-cation electron-cation bot added new-pr 🌱 PR opened in the last 24 hours api-review/requested 🗳 labels Feb 7, 2023
deepak1556
deepak1556 reviewed Feb 7, 2023
View reviewed changes
Copy link
Member

@deepak1556 deepak1556 left a comment

Choose a reason for hiding this comment

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

Should we support all ISO paper sizes https://source.chromium.org/chromium/chromium/src/+/main:chrome/common/printing/print_media_l10n.cc;l=35-589;bpv=0 ?

@codebytere
Copy link
Member Author

@deepak1556 I defaulted to what's currently supported in the Chrome DevTools Protocol - we could expand the list but then imo we should do so for both webContents.printToPDF() and webContents.print() so there's at least some consistency. Personally, I think at least adding these is likely to address the vast majority of use cases for now!

jkleinsc
jkleinsc reviewed Feb 7, 2023
View reviewed changes
Copy link
Contributor

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

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

@codebytere does A6 need to be declared in PDFPageSizes?

@deepak1556
Copy link
Member

I defaulted to what's currently supported in the Chrome DevTools Protocol

Sounds good, thanks for clarifying.

@codebytere
Copy link
Member Author

@jkleinsc oops, yes, missed that! addressed.

jkleinsc
jkleinsc approved these changes Feb 8, 2023
View reviewed changes
Copy link
Contributor

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

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

API LGTM

zcbenz
zcbenz approved these changes Feb 9, 2023
View reviewed changes
Copy link
Member

@zcbenz zcbenz left a comment

Choose a reason for hiding this comment

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

API LGTM

@jkleinsc jkleinsc added the target/24-x-y PR should also be added to the "24-x-y" branch. label Feb 9, 2023
nornagon
nornagon reviewed Feb 13, 2023
View reviewed changes
Copy link
Member

@nornagon nornagon left a comment •
edited

Choose a reason for hiding this comment

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

Is there a source list for this somewhere in Chrome, or did we invent this? nvm, already answered

nornagon
nornagon approved these changes Feb 13, 2023
View reviewed changes
Copy link
Member

@nornagon nornagon left a comment

Choose a reason for hiding this comment

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

API LGTM

@codebytere codebytere merged commit 889859d into main Feb 14, 2023
@codebytere codebytere deleted the fixup-printing-paper-sizes branch February 14, 2023 10:44
@release-clerk
Copy link

release-clerk bot commented Feb 14, 2023

Release Notes Persisted

Added several standard page size options to webContents.print().

This was referenced Feb 14, 2023
Merged
Merged
Merged
@trop
Copy link
Contributor

trop bot commented Feb 14, 2023

I have automatically backported this PR to "22-x-y", please check out #37263

@trop trop bot added in-flight/22-x-y and removed target/22-x-y PR should also be added to the "22-x-y" branch. labels Feb 14, 2023
@trop
Copy link
Contributor

trop bot commented Feb 14, 2023

I have automatically backported this PR to "23-x-y", please check out #37264

@trop
Copy link
Contributor

trop bot commented Feb 14, 2023

I have automatically backported this PR to "24-x-y", please check out #37265

@trop trop bot added in-flight/23-x-y merged/24-x-y PR was merged to the "24-x-y" branch merged/23-x-y PR was merged to the "23-x-y" branch. merged/22-x-y PR was merged to the "22-x-y" branch. and removed target/23-x-y PR should also be added to the "23-x-y" branch. target/24-x-y PR should also be added to the "24-x-y" branch. in-flight/24-x-y labels Feb 14, 2023
khalwa pushed a commit to solarwindscloud/electron that referenced this pull request Feb 22, 2023
@codebytere @khalwa
feat: include all standard paper sizes for webContents.print() (elect…
0c163d9 
…ron#37159)
gecko19 pushed a commit to brightsign/electron that referenced this pull request Feb 28, 2023
@codebytere @gecko19
feat: include all standard paper sizes for webContents.print() (elect…
c1036d7 
…ron#37159)
@commit-lint commit-lint bot mentioned this pull request May 15, 2023
Open
@origani
Copy link

origani commented Oct 18, 2023

@codebytere can add 8K and 16K in paper sizes?thinks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nornagon nornagon nornagon approved these changes

@jkleinsc jkleinsc jkleinsc approved these changes

@zcbenz zcbenz zcbenz approved these changes

@deepak1556 deepak1556 deepak1556 approved these changes

@VerteDinde VerteDinde Awaiting requested review from VerteDinde

Assignees
No one assigned
Labels
api-review/approved ✅ merged/22-x-y PR was merged to the "22-x-y" branch. merged/23-x-y PR was merged to the "23-x-y" branch. merged/24-x-y PR was merged to the "24-x-y" branch semver/minor backwards-compatible functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: A6 not accepted as pageSize in settings of webContents.print
6 participants
@codebytere @deepak1556 @origani @nornagon @jkleinsc @zcbenz