Skip to content

fix(render): viewport not set when only one dimension specified — causes 2x width in generated images #13

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

Open
CompilError-bts wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(render): viewport not set when only one dimension specified — causes 2x width in generated images #13

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/render.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,9 @@ async def html2pic(
width = viewport_width if viewport_width is not None else 800
height = viewport_height if viewport_height is not None else 720
# Set viewport size if either width or height is specified
if viewport_width is not None and viewport_height is not None:
# Default values if one dimension not specified
await page.set_viewport_size({"width": width, "height": height})
logger.info(f"html2pic: set viewport size to {width}x{height}")
# Default values if one dimension not specified
await page.set_viewport_size({"width": width, "height": height})
logger.info(f"html2pic: set viewport size to {width}x{height}")
Comment on lines +239 to +241

@gemini-code-assist gemini-code-assist Bot May 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The comments in this block are now misleading because the if condition was removed, meaning the viewport is now set unconditionally. The comment on line 239 is redundant as the default logic is already handled in lines 236-237. Furthermore, the comment on line 238 (which is a context line in this diff) now contradicts the implementation as it states the viewport is only set if a dimension is specified. Consider updating the comments to accurately reflect that the viewport is now always set to ensure the library's default dimensions (800x720) are applied when no overrides are provided.

Suggested change
# Default values if one dimension not specified
await page.set_viewport_size({"width": width, "height": height})
logger.info(f"html2pic: set viewport size to {width}x{height}")
# Set viewport size using specified values or defaults
await page.set_viewport_size({"width": width, "height": height})
logger.info(f"html2pic: set viewport size to {width}x{height}")


Comment on lines +241 to 242

@sourcery-ai sourcery-ai Bot May 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

question (bug_risk): Consider whether forcing a default viewport even when no dimensions are provided changes existing behavior unintentionally.

Previously, the viewport was only set when both dimensions were provided; now it’s always set to 800×720, even when both are None. This could break callers that relied on the browser’s default. If that change isn’t intentional, consider guarding with something like if viewport_width is not None or viewport_height is not None: so the default is only applied when at least one dimension is specified.

try:
await page.goto(
Expand Down