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.

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

add 1024x576 as an intermediate GUI resolution for smaller screens #3811

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

levkropp
Copy link
Contributor

@levkropp levkropp commented Dec 4, 2024

Some Macbooks and other small laptops have resolutions such as 1440x900 or 1366x768 that are below the threshold for the "large" GUI resolution of 1400x822's minimum screen size of 1600x900, but the default (minimum) of 750x450 is too small.

This PR updates the startup application window size logic in main.dart to add an intermediate 1024x576 resolution for smaller screens,

It now sets the window size to 1400x822 for screens 1600x900 or larger, 1024x576 for screens 1280x720 or larger, and defaults to 750x450 otherwise.

: (screenSize.width >= 1280 && screenSize.height >= 720)
? const Size(1024, 576) // For screens 1280x720 or larger
: const Size(750, 450) // Default window size
: const Size(750, 450); // Default window size if screenSize is null
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does it mean if it is null? Should we fallback to the default or middle resolution in that case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If screenSize is null it means that the plugin cannot get the current screen resolution for some reason. Falling back to the minimum resolution is probably safest if this happens

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why though? Isn't it safer to make a compromise?

Copy link
Contributor Author

@levkropp levkropp Dec 4, 2024

Choose a reason for hiding this comment

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

I can't think of any situations where serving a larger resolution (when you don't know the size of the screen) is safer than serving the minimum resolution

Copy link
Collaborator

@ricab ricab Dec 4, 2024

Choose a reason for hiding this comment

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

Safety is perhaps not the best concept here, but when "the plugin cannot get the current screen resolution for some reason", the most reasonable expectation is still that the user either doesn't have graphics at all, or they have 720p or higher (since most screens today have that). OTOH, the smallest size is our least preferred.

We can deal with this better later.

Copy link

codecov bot commented Dec 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.94%. Comparing base (f01004e) to head (52f139f).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3811   +/-   ##
=======================================
  Coverage   88.94%   88.94%           
=======================================
  Files         256      256           
  Lines       14584    14584           
=======================================
  Hits        12972    12972           
  Misses       1612     1612           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@ricab ricab left a comment

Choose a reason for hiding this comment

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

OK I don't want to delay this any further, so approving.

@ricab
Copy link
Collaborator

ricab commented Dec 4, 2024

For some reason GH is encountering some internal error (seen in other PRs too). The runs otherwise succeeded, so merging manually.

https://github.com/canonical/multipass-private/actions/runs/12165023258

@ricab ricab merged commit 6b5aebe into main Dec 4, 2024
11 of 14 checks passed
@ricab ricab deleted the flutter-small-screen-resolution branch December 4, 2024 20:27
ricab added a commit that referenced this pull request Dec 4, 2024
add 1024x576 as an intermediate GUI resolution for smaller screens
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.

2 participants