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

Small UI button set painting fixes #3519

Closed
wants to merge 2 commits into from
Closed

Small UI button set painting fixes #3519

wants to merge 2 commits into from

Conversation

martincapello
Copy link
Member

@martincapello martincapello commented Sep 14, 2022

This PR contains 2 fixes to the way ButtonSet items are painted.
One fixes the vertical position of the text, and the other fixes the vertical position of the icon (#2676).

@Gasparoken
Copy link
Member

Gasparoken commented Sep 19, 2022

This is not as simple as it seems. This fix fixes the Dynamics popup dialog, but changes other buttons. I think there is something deeper to fix (like the general text/icon alignment of a generic button widget). I'll do some more research tomorrow.
Possible causes of misalignment: buttons border treatment (mainly those with decorative shading added -up to 3 extra pixels on the bottom-), sizes in odd/even pixels of widgets/icons/text, rounding of integers when divided For 2.

The last row of items are a bit taller to make room for the button border, so this compensates that.
@Gasparoken
Copy link
Member

Gasparoken commented Sep 20, 2022

Finally, I think this solution is fine. The text/icon alignment in the last row is consistent with the remaining buttons that are not in the last row.

Comment on lines 99 to 101
if (isLastRow && info.row > 0) {
iconRc.y -= 2*guiscale();
}
Copy link
Member

Choose a reason for hiding this comment

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

Minor modification: omit the { } in the if

@dacap
Copy link
Member

dacap commented Sep 20, 2022

Although some other buttons have changed, I think the new look for the Dynamics is better than before, so I'll merge this as it is. I might add some new properties in the theme to adjust the height of the buttons after main and beta are merged 👍

@dacap dacap closed this Sep 20, 2022
@dacap dacap deleted the fix-2676 branch September 20, 2022 21:08
@dacap
Copy link
Member

dacap commented Sep 20, 2022

Merged 6e57546 and fecfbb1

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