-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
JS: add PrintOptions with predefined and custom sizes; margins; tests (#15072) #16465
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
base: trunk
Are you sure you want to change the base?
JS: add PrintOptions with predefined and custom sizes; margins; tests (#15072) #16465
Conversation
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
PR Code Suggestions ✨Explore these optional code suggestions:
|
// specific language governing permissions and limitations | ||
// under the License. | ||
|
||
goog.module('webdriver.print_options'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason why we need these ?
// under the License. | ||
|
||
goog.module('webdriver.print_options'); | ||
goog.module.declareLegacyNamespace(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here, idk what it is doing in this file ?
this.left = left; | ||
} | ||
|
||
static get Default() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defaults are set by W3C automatically, i dont think we need defaults here
* @param {string} orientation - Must be either "portrait" or "landscape". | ||
*/ | ||
setOrientation(orientation) { | ||
if (!Object.values(PrintOrientation).includes(orientation)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even errors thrown by W3C and we dont need duplicate check in here.
I think you can just export default values as options and webdriver printPage
can use it
* @param {{ top: number, bottom: number, left: number, right: number }} margins - The margins object. | ||
*/ | ||
setMargins(margins) { | ||
if (!margins || margins.top < 0 || margins.bottom < 0 || margins.left < 0 || margins.right < 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above!
we dont need value checks here
see https://www.w3.org/TR/webdriver2/#print-page P-18
If any of marginTop, marginBottom, marginLeft, or marginRight is not a Number, or is less then 0, return error with error code invalid argument.
// under the License. | ||
|
||
|
||
goog.provide('webdriver.PrintOptionsTest'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason why we need atoms here ?
you can move this test to mocha, which is already in here
https://github.com/SeleniumHQ/selenium/blob/trunk/javascript/selenium-webdriver/test/print_pdf_test.js
User description
Summary
PrintOptions
in JS mirroring other bindings (Python, Java, .NET, Ruby).What’s in this PR
javascript/webdriver/print_options.js
{ orientation, scale, background, shrinkToFit, page: { width, height }, margin: { top, right, bottom, left }, pageRanges }
javascript/webdriver/test/print_options_test.js
Why
Scope / Impact
Validation
bazel test //javascript/webdriver:test --test_output=errors
References
PR Type
Enhancement
Description
Implement PrintOptions class mirroring other language bindings
Support predefined paper sizes (A4, Letter, Legal, Tabloid)
Add custom page size, margins, orientation, scale configuration
Include comprehensive test suite for all PrintOptions features
Diagram Walkthrough
File Walkthrough
print_options.js
PrintOptions class with paper sizes and margins
javascript/webdriver/print_options.js
orientation, 1.0 scale, A4 page size)
centimeters
with input validation
format
communication
print_options_test.js
Comprehensive test suite for PrintOptions
javascript/webdriver/test/print_options_test.js