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

Library: resize the Played checkbox and BPM lock with the library font #4050

Merged
merged 1 commit into from
Jul 8, 2021

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Jul 2, 2021

This should resize the 'Played' checkbox and the BPM lock icon to match the library font.
(another snippet to fix https://bugs.launchpad.net/mixxx/+bug/1736001)
This works well for library font sizes up to 200% of the original system font's size. Above that, the indicators start to overlap the item text because the painter rectangle is not resized (also depending on the system font size).

Admittedly this can be considered a stupid hack, but I have not found a more efficient way, and the solution should be fine for now.
For the BPM delegate this could be done in paintItem() by resizing the painter rectangle, but I didn't find a way to access the Played indicator like that.

System 11pt (Qt5 Settings 11pt), library 22pt
This PR
image

2.3
image

@ronso0 ronso0 added this to the 2.3.1 milestone Jul 2, 2021
@github-actions github-actions bot added the ui label Jul 2, 2021
@ronso0
Copy link
Member Author

ronso0 commented Jul 2, 2021

Please test this on Linux, Windows (@JoergAtGithub ?) and macOS (@foss- ?) if this works in general.

Meanwhile I'll test if the Played checkbox needs some adjustment.

@foss-
Copy link
Contributor

foss- commented Jul 3, 2021

Build artifact 2.3.0-14-g89fd8ec131 (HEAD)
Size of lock and played check mark unchanged:
2.3
2 3
This PR
This PR

@ronso0
Copy link
Member Author

ronso0 commented Jul 3, 2021

@foss- Do you have any screens showing the previous state?
Indeed the BPM icon appears to be a tiny bit smaller than in my screens. I could just make it a bit bigger than the checkbox.

@foss-
Copy link
Contributor

foss- commented Jul 3, 2021

I do not want to go back to pre 2.3 versions to avoid problems. But I am certain that lock icon previously had the size from your screenshot for this PR.

@ronso0
Copy link
Member Author

ronso0 commented Jul 6, 2021

I do not want to go back to pre 2.3 versions to avoid problems

Of course, I thought maybe you have still some screenshots somewere.

Can you please reset the mac app scaling to 100% and share a screenshot with that?
Also I'd like to see a screenshot with your usual app scaling, just to see how certain widgets are displayed (for example, I see that gap in between the BPM lock / Played checkmark and the respective items labels only if I use a scale factor >100% but don't scale up the library font so it matches the size all other text labels)

Then, if you don't mind, please try the current build artifact and share a screenshot: I made the BPM and Played icons as big as those in the main menus on other OS, and the Played checkbox is now also a SVG instead of a css border.
For me this works really well for all (reasonable) combinations of system font size and library font size.

@ronso0
Copy link
Member Author

ronso0 commented Jul 6, 2021

I'll test this on Windows myself soonish.

@ronso0 ronso0 force-pushed the lib-icons-scaling branch from 51ceef4 to 4b3494c Compare July 6, 2021 13:13
@foss-
Copy link
Contributor

foss- commented Jul 6, 2021

back to expected size on macOS 11.4, mixxx 2.3.0-21-g26af2937c5 (HEAD)

Played column however has changed and is now showing a play counter. I am not sure if that is expected. that seems to be the total plays while in earlier versions I thought it was an indicator of whether a track has been playing during current set or not (see previous screenshots).

lock size ok

@JoergAtGithub
Copy link
Member

On Windows7 it looks the same:
grafik

@ronso0 ronso0 force-pushed the lib-icons-scaling branch from 4b3494c to a528ec1 Compare July 6, 2021 18:19
@ronso0
Copy link
Member Author

ronso0 commented Jul 6, 2021

Played column however has changed and is now showing a play counter. .... it was an indicator of whether a track has been playing during current set or not

It's both, like ever since : ) you're column was to narrow to show the counter.

Played checkbox is now restored, I did too much cleanup ;)

If you can confirm it looks fine on macOS and Windows (also try changing the library font size as this is the actual purpose of this PR) I'll change the other skins, too.

@foss-
Copy link
Contributor

foss- commented Jul 6, 2021

getting closer: check mark still missing:
Screenshot 2021-07-06 at 23 12 20

Deere is fine though:
Screenshot 2021-07-06 at 23 19 49

@JoergAtGithub
Copy link
Member

Now it looks on Win7 like this:
20px (Default):
grafik
24px:
grafik
16px:
grafik

@ronso0 ronso0 force-pushed the lib-icons-scaling branch from a528ec1 to ea37e7d Compare July 7, 2021 01:17
@ronso0
Copy link
Member Author

ronso0 commented Jul 7, 2021

check mark still missing

uff, embarassing, that was caused by a left-over comma in the wrong place...
sorry for not getting this tiny PR done in one go (tried to fix during my son's siesta, but didn't test properly : )
it's fine now. the other skins are tweaked, too.

@JoergAtGithub Looks like you adjusted the lib row height, not the font size ; )

@ronso0 ronso0 marked this pull request as ready for review July 7, 2021 01:19
@foss-
Copy link
Contributor

foss- commented Jul 7, 2021

...and we have a winner. Looking good. Thanks for tackling this.

Screenshot 2021-07-07 at 10 12 15

@ronso0
Copy link
Member Author

ronso0 commented Jul 7, 2021

okay, thanks for testing!
If someone has a few minutes to test the other skins/color schemes we can merge.

@foss-
Copy link
Contributor

foss- commented Jul 7, 2021

Deere
Screenshot 2021-07-07 at 17 38 00

Shade
Screenshot 2021-07-07 at 17 38 42

Tango
Screenshot 2021-07-07 at 17 39 32

All good on macOS.

@Holzhaus Holzhaus merged commit a05daa5 into mixxxdj:2.3 Jul 8, 2021
@ronso0 ronso0 deleted the lib-icons-scaling branch July 8, 2021 15:26
@ywwg
Copy link
Member

ywwg commented Jul 27, 2021

@ronso0

This broke the checkboxes in the right-click menu of track header view:

image

Narrowed down with git bisect:

ea37e7df74216378741c427c80c04ebbd34e5188 is the first bad commit
commit ea37e7df74216378741c427c80c04ebbd34e5188
Author: ronso0 <ronso0@mixxx.org>
Date:   Wed Jul 7 03:17:24 2021 +0200

    Skins / library: resize the Played checkbox and BPM lock with the library font

@ronso0
Copy link
Member Author

ronso0 commented Jul 27, 2021

whoopsy..
fixed with 49b08aa

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