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

Controller preferences: add :hwbtn: markup parser #13046

Merged
merged 2 commits into from
Apr 5, 2024

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Apr 4, 2024

Allows using :hwbtn:LABEL in xml and moves the styling to c++
image

It's just bold now (dropped the tt monospace font tag).

The only way I figured to get horizontal padding was to wrap the label in  
Padding is not applied with padding: 2px; alone. Might be one of those qss quirks where we need to set an unrelated property, though with span neither display: inline-block; nor box-sizing: border-box; or border work

@acolombier Mind taking a look?

Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

small code nitpick

src/controllers/legacycontrollersettings.cpp Outdated Show resolved Hide resolved
src/controllers/legacycontrollersettings.cpp Outdated Show resolved Hide resolved
@Holzhaus
Copy link
Member

Holzhaus commented Apr 4, 2024

Padding is not applied with padding: 2px; alone. Might be one of those qss quirks where we need to set an unrelated property, though with span neither display: inline-block; nor box-sizing: border-box; or border work

It works, but only for block elements or table cells. Unfortunately, using table cells also introduces a linebreak, so we can't use it here:
Screenshot from 2024-04-04 20-37-52

@Holzhaus
Copy link
Member

Holzhaus commented Apr 4, 2024

ronso0#66

@ronso0
Copy link
Member Author

ronso0 commented Apr 4, 2024

Thanks for looking into it @Holzhaus

I think we're good with the   solution. I expect it to look odd only if users have a monospace font configured e.g. with Qt6ct.

@ronso0
Copy link
Member Author

ronso0 commented Apr 4, 2024

oh, you already have fix ...

ronso0 and others added 2 commits April 4, 2024 21:15
Co-authored-by: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com>
Co-authored-by: Jan Holthuis <jholthuis@mixxx.org>
@Holzhaus
Copy link
Member

Holzhaus commented Apr 4, 2024

oh, you already have fix ...

No, just some minor improvement unfortunately 😢

Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

LGTM in terms of code. Can't comment much on the formatting because I don't know HTML&CSS well enough.

@JoergAtGithub
Copy link
Member

LGTM! Thank you!

@JoergAtGithub JoergAtGithub merged commit cbd280f into mixxxdj:main Apr 5, 2024
13 checks passed
@ronso0
Copy link
Member Author

ronso0 commented Apr 5, 2024

Thanks everyone for the quick response!

@ronso0 ronso0 deleted the controller-prefs-markup branch April 5, 2024 16:42
@acolombier
Copy link
Member

Sorry I didn't have time test this - thanks for looking into this @ronso0 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants