-
-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[java] Enhance PageSize class to support for predefined and custom Paper Sizes #15052
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
Thanks for the PR @yvsvarma, this is a great suggestion! Could you please add a test to ensure this works as intended? |
Hi @shbenzer, Thank you for your review and feedback! To address your request, I’d like to point out that we already have an example to test PageSize functionality in the seleniumhq.github.io repository. Specifically, the example is in the PrintOptionsTest class. I’ve verified the enhanced functionality locally by updating the existing example, printed the sizes and compared, everything is working as intended with the changes in this PR. For your reference, I’ve attached the example, I intend to update after this PR - PrintOptionsTest.txt Once this PR is merged, I’d be happy to enhance the PrintOptionsTest example in the seleniumhq.github.io repository to reflect the updated PageSize implementation. Please note that the existing example remains valid with this update. Let me know if there’s anything else you’d like me to address. Thank you! |
We need a test inside this repo, and can’t rely on the examples in the docs to test our code. Since this is new functionality it needs a new test. Nothing complex, just something simple like:
|
Let’s make this more Java standard, and like the Keys class, so instead it’ll be setPageSize(PageSize.LEGAL). We want to reference a constant instead of calling a method inside setPageSize() |
f584082
to
b251728
Compare
@shbenzer , Fixed as per your review comments. |
62d9369
to
f24cf74
Compare
Updated test cases also in this repo. Please review. |
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.
Thank you! @yvsvarma
Excellent, thank you. |
@yvsvarma please run scripts/format.sh to pass |
@pujagani , Thank you for reviewing and approving the changes. |
This is the current integration with PrintOptions class, correct? printOptions.setPageSize(PageSize.setPageSize(PageSize.ISO_A4)); because if so, it should be: printOptions.setPageSize(PageSize.ISO_A4); Let’s remove the redundant function call - once that’s done I think it’ll be good to go. |
@shbenzer , Thanks again for reviewing. Updated as per review comments. Also, ran scripts/format.sh. All good. |
@VietND96 , can you please review. |
The changes look good to me. Will merge once the CI passes. |
User description
PR Description
Current Behavior
The
PageSize
class in Selenium provides basic functionality to manage page dimensions but lacks the ability to:Set predefined paper sizes like "A4", "A6", "Legal", "Tabloid," etc.
This is also a discrepancy between the documented functionality in the examples Print Page which says user can set the paper size as - “A0”, “A6”, “Legal”, “Tabloid”, etc.
The updated
PageSize
class introduces:Predefined Sizes:
Added an internal
PaperSize
enum that includes common paper sizes:A4
: 27.94 cm x 21.59 cmA6
: 14.8 cm x 10.5 cmLegal
: 35.56 cm x 21.59 cmTabloid
: 43.18 cm x 27.94 cmConvenient Factory Methods:
New static methods for creating predefined sizes:
PageSize.A4()
,PageSize.A6()
,PageSize.LEGAL()
, andPageSize.TABLOID()
.Custom Sizes:
Added support for users to create PageSize objects with custom dimensions:
Example: new PageSize(30.0, 20.0).
Enhanced Usability:
Users can now:
Set a predefined paper size directly:
printOptions.setPageSize(PageSize.TABLOID());
Retrieve height and width using getHeight() and getWidth():
Define custom paper sizes for flexibility:
How to Use:
1. Predefined Sizes:
2. Custom Sizes:
3. Retrieving Dimensions:
Note: Examples will be updated on this Print Page once this PR is approved and merged.
PR Type
Enhancement
Description
Added predefined paper sizes using an internal
PaperSize
enum.Introduced factory methods for common paper sizes (e.g.,
PageSize.A4()
).Enabled custom page size creation with height and width parameters.
Enhanced usability with
toMap
serialization andtoString
representation.Changes walkthrough 📝
PageSize.java
Added predefined and custom paper size support
java/src/org/openqa/selenium/print/PageSize.java
PaperSize
enum for predefined sizes (A4, A6, Legal, Tabloid).toMap
and addedtoString
method.