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 zoom option to make OSD big in small screens #3252

Merged
merged 2 commits into from
Jan 20, 2023

Conversation

McGiverGim
Copy link
Member

Alternative to #3241

This PR fits the OSD preview into the screen for small devices. The problem is that is not usable to reorganice elements because the small size, specially in OSD HD format. But if the user needs to make it bigger, it adds a "zoom" check to make it usable. It overflows, so the user needs to do a scroll, but is possible to reorganize elements.

Zoom disabled (default):
image

Zoom enabled:
image

@github-actions

This comment has been minimized.

@haslinghuis
Copy link
Member

Would love to see the zoom button for desktop while viewpoint is above 1455px :)

On Nokia G60 the right side has some overflow past the screen.

@McGiverGim
Copy link
Member Author

It has no sense to have it for over 1455px, the max-width of the OSD preview is smaller than that ;)

For Nokia G60, it does not appear like in my screenshot? My Pixel 4 has a similar resolution to yours. Maybe can you paste a screenshot?

@haslinghuis
Copy link
Member

image

@McGiverGim
Copy link
Member Author

Yes, it overflows, but only when the "zoom" option is pressed. This is the expected behaviour. It's ugly but it works.

The idea is default to false, to fit nice, but make it overflow when needed to reorganize elements. We can add real scrolls to show only the part that fits and not overflow, but I think it will not be very usable.

@blckmn
Copy link
Member

blckmn commented Jan 16, 2023

AUTOMERGE: (FAIL)

  • github identifies PR as mergeable -> FAIL
  • assigned to a milestone -> PASS
  • cooling off period lapsed -> PASS
  • commit count less or equal to three -> PASS
  • Don't merge label NOT found -> PASS
  • at least one RN: label found -> PASS
  • Tested label found -> FAIL
  • assigned to an approver -> PASS
  • approver count at least three -> FAIL

@haslinghuis
Copy link
Member

After playing more with this PR I found a way to add right margin:

Adding padding-right: 1em on line 382 does the trick 🎉

https://stackoverflow.com/questions/32765039/using-margin-on-flex-items

@haslinghuis
Copy link
Member

image

@McGiverGim
Copy link
Member Author

After playing more with this PR I found a way to add right margin:

Adding padding-right: 1em on line 382 does the trick 🎉

https://stackoverflow.com/questions/32765039/using-margin-on-flex-items

I didn't saw this message. Now all have more sense. I will change it ;)

@McGiverGim
Copy link
Member Author

Added the padding to the "zoom" style only. If not, it affects the desktop too and gives different space between left and right (left only gap, right gap and padding).

@sonarcloud
Copy link

sonarcloud bot commented Jan 16, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
3.9% 3.9% Duplication

@github-actions
Copy link
Contributor

Do you want to test this code? Here you have an automated build:
Betaflight-Configurator-Android
Betaflight-Configurator-Linux
Betaflight-Configurator-macOS
Betaflight-Configurator-Windows
WARNING: It may be unstable and result in corrupted configurations or data loss. Use only for testing!

Copy link
Member

@haslinghuis haslinghuis left a comment

Choose a reason for hiding this comment

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

Perfect now - zoom with analog does nothing :). But this looks good!

@McGiverGim
Copy link
Member Author

It depends on the size of your phone. On mine it makes it bigger with analog too 😉

@@ -2985,6 +2983,14 @@ osd.initialize = function(callback) {
// Hide custom if not used
$('.osdfont-selector option[value=-1]').toggle(osdFontSelectorElement.val() === -1);

// Zoom option for the preview only for mobile devices
if (GUI.isCordova()) {
$('.osd-preview-zoom-group').css({display: 'inherit'});
Copy link
Member

Choose a reason for hiding this comment

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

Why not just using css media query for that classname instead of js?

Copy link
Member Author

Choose a reason for hiding this comment

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

The media query was my first attempt, using resolution, but finally I wanted to restrict to Android only. Is a way in the media query to limit to Android specifically?

@blckmn blckmn merged commit a5fad5c into betaflight:master Jan 20, 2023
Finalizing Firmware 4.4 Release automation moved this from In progress to Done Jan 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

4 participants