Skip to content

Conversation

@rgarcia
Copy link
Contributor

@rgarcia rgarcia commented Oct 15, 2025

@rgarcia rgarcia requested a review from hiroTamada October 15, 2025 14:50
Copy link

@mesa-dot-dev mesa-dot-dev bot left a comment

Choose a reason for hiding this comment

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

Performed full review of ed27333...ed9ee4a

Analysis

  1. The parseViewport function lacks proper bounds validation for viewport dimensions, which could lead to invalid configurations or unexpected behavior at runtime.

  2. The hardcoded list of available viewports needs better documentation explaining their source and selection criteria, hampering future maintenance.

  3. Error handling for viewport configuration is insufficient, missing specific guidance on valid input formats which could create a poor user experience.

  4. The renaming from UploadExtensions to LoadExtensions represents an undocumented breaking API change that could disrupt existing integrations.

Tip

⚡ Quick Actions

This review was generated by Mesa.

Actions:

Slash Commands:

  • /review - Request a full code review
  • /review latest - Review only changes since the last review
  • /describe - Generate PR description. This will update the PR body or issue comment depending on your configuration
  • /help - Get help with Mesa commands and configuration options

4 files reviewed | 0 comments | Review on Mesa | Edit Reviewer Settings

@mesa-dot-dev
Copy link

mesa-dot-dev bot commented Oct 15, 2025

Mesa Description

This PR introduces the ability to specify a browser's viewport resolution upon creation via new CLI flags.

Description

Users can now control the initial screen size of a browser instance using the --viewport <width>x<height>[@<refreshRate>] flag. An interactive --viewport-interactive flag is also included to select from a list of common resolutions.

Changes

  • Feature: Added --viewport and --viewport-interactive flags to the browsers create command.
  • Implementation:
    • Introduced a Viewport field to the browser creation input.
    • Added robust parsing and validation for the viewport string format.
  • Refactor: Renamed UploadExtensions to LoadExtensions for improved clarity in the service interface.
  • Testing: Added comprehensive unit tests to validate viewport parsing for various valid, partial, and invalid inputs.

Description generated by Mesa. Update settings

{"2560x1440@10", 2560, 1440, 10},
{"1024x768@60", 1024, 768, 60},
{"1920x1080", 1920, 1080, 0},
{"800x600", 800, 600, 0},
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: i know it is a test, but we dont support this resolution technically

@rgarcia rgarcia merged commit 09b6ba1 into main Oct 15, 2025
1 check passed
@rgarcia rgarcia deleted the add-resolution-switches branch October 15, 2025 16:10
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