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

UI scaling improvements #1553

Merged
merged 7 commits into from Apr 30, 2024
Merged

Conversation

lilyCalla
Copy link
Contributor

@lilyCalla lilyCalla commented Apr 21, 2024

Fix general UI scaling bugs including

  • Top bar elements drift downwards when scale goes under 1.0
  • Background for "Actions" bar didn't automatically adjust to rescaled sizing (required second click on "Actions" menu item)
  • Prevent BPM "ratchet" issues where scaling up increases keypad button height, but scaling down leaves it at the maximum height scaled to
  • Beat Indicator / Stop all button not scaling with UI
  • interface jitter in UI Setting editor
  • Width of text elements in UI settings editor, but font size changes
  • Undo/Redo buttons to layout to scale properly
  • Prevent text overflow in about dialog

Current (Beat generator panel has been scaled up, then returned to smaller size):
pre-pr

Pull Request:
post-pr

@@ -30,13 +30,17 @@ Rectangle
anchors.fill: parent
color: "transparent"

property real origItemHeight: UISettings.listItemHeight
property real origIconMedium: UISettings.iconSizeMedium
property real origItemHeight: { origItemHeight = UISettings.listItemHeight }
Copy link
Owner

Choose a reason for hiding this comment

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

Are you sure this syntax makes sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This syntax is meant to prevent a binding from being created on origItemHeight. To my knowledge this is the cleanest way to do this inline, although I'm open to being wrong

Copy link
Owner

Choose a reason for hiding this comment

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

TBH I've never seen that syntax before.
I'll merge this as it is. Thanks

@mcallegari
Copy link
Owner

Hey, thanks for this.
Just 2 comments but the rest looks fine to me.

@coveralls
Copy link

coveralls commented Apr 28, 2024

Coverage Status

coverage: 32.074% (-0.007%) from 32.081%
when pulling cbd5487 on lilyCalla:ui-scaling-improvement
into 8e695ef on mcallegari:master.

@mcallegari mcallegari merged commit 73349d7 into mcallegari:master Apr 30, 2024
7 checks passed
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.

None yet

3 participants