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 support for platform-dependent skin styles + fix WEffectSelector styling #1590

Merged
merged 5 commits into from Apr 5, 2018

Conversation

Be-ing
Copy link
Contributor

@Be-ing Be-ing commented Apr 4, 2018

This will allow us to work around platform-dependent quirks in styling. For 2.1 this is needed to work around the issue of the checkmark pushing text to the right and getting cut off in WEffectSelector only on macOS.

@Be-ing Be-ing mentioned this pull request Apr 4, 2018
@Be-ing Be-ing added this to the 2.1.0 milestone Apr 4, 2018
@daschuer
Copy link
Member

daschuer commented Apr 4, 2018

How does it work? Please add a short description to the source code and the wiki.

@Be-ing
Copy link
Contributor Author

Be-ing commented Apr 4, 2018

This allows the <Style> element in skin.xml (or for any other element in the skin but I don't intend to use that) to have src-mac, src-windows, and src-linux attributes to specify a platform-specific QSS file to override anything in the style.qss file for all platforms. Do you think this is a good approach for working around the issue of effect names getting cut off?

@Be-ing
Copy link
Contributor Author

Be-ing commented Apr 4, 2018

This may turn out to be useful for other problems in the future as well as the present issue with WEffectSelector.

@ronso0
Copy link
Member

ronso0 commented Apr 4, 2018

Could you test it for WEffectSelector?

This will allow us to work around platform-dependent quirks
in styling. For 2.1 this is needed to work around the issue of
the checkmark pushing text to the right and getting cut off
in WEffectSelector only on macOS.
@Be-ing Be-ing changed the title add support for platform-dependent skin styles add support for platform-dependent skin styles + fix WEffectSelector styling Apr 5, 2018
@Be-ing
Copy link
Contributor Author

Be-ing commented Apr 5, 2018

macOS 1440 x 900 screenshots:
screen shot 2018-04-04 at 7 37 37 pm
screen shot 2018-04-04 at 7 33 43 pm
screen shot 2018-04-04 at 7 33 20 pm
screen shot 2018-04-04 at 7 38 03 pm

@Be-ing
Copy link
Contributor Author

Be-ing commented Apr 5, 2018

I double checked with #1592 merged in and the drop down menus still look good.

@Be-ing
Copy link
Contributor Author

Be-ing commented Apr 5, 2018

Defining the height and font size for WEffectSelector::item in Shade was breaking skin scaling. After that, I had to fix it again on macOS by hiding the checkmark:
screen shot 2018-04-04 at 8 27 58 pm

@Be-ing
Copy link
Contributor Author

Be-ing commented Apr 5, 2018

GNOME at 250%:
image
image
image
image

@Be-ing
Copy link
Contributor Author

Be-ing commented Apr 5, 2018

KDE at 250%:
interestingly here the menus show below the widget instead of expanding above and below. This makes the font color of the loaded effect (WEffectSelector::checked) a little odd, but it's no big deal.
image
image
image
image
The font color of the selected item in Shade is difficult to read. I tried explicitly setting the font color of WEffectSelector::item:selected but it seemed the system color overrode that.

@Be-ing
Copy link
Contributor Author

Be-ing commented Apr 5, 2018

I'll merge this now to ensure the skins are in their final state for the screenshots in #1525 and so we can get builds to test on macOS with Retina screens and Windows.

@Be-ing Be-ing merged commit e666330 into mixxxdj:2.1 Apr 5, 2018
@ronso0
Copy link
Member

ronso0 commented Apr 9, 2018

So I can remove the hack from Tango and bring back the tick mark?

@ronso0
Copy link
Member

ronso0 commented Apr 9, 2018

This went to RC1, right?
Text is still/again cut off in LateNight & Shade with minimal window width, scaled 100%.

@Be-ing
Copy link
Contributor Author

Be-ing commented Apr 9, 2018

So I can remove the hack from Tango and bring back the tick mark?

I think so, now that we have a way to hide it only on macOS.

This went to RC1, right?

Yes.

Text is still/again cut off in LateNight & Shade with minimal window width, scaled 100%.

🤦‍♂️ You are right. Would you like to take care of that?

@Be-ing
Copy link
Contributor Author

Be-ing commented Apr 10, 2018

Windows 7, 1366 x 768:
image
image
image
image

@ronso0
Copy link
Member

ronso0 commented Apr 10, 2018

@Be-ing This is how it looks right now on windows or is this your WIP fix?
I'm confused since on Linux the checkmarks are there. Tested with french locale.
In Tango it looks good (except the missing tick mark) because the effect selector has a right padding to work around issues with scaling, and is hidden underneath the effect toggle and effect name. That padding is apparently applied to QAbstractItemView and makes space for long names.

rc1-fx-names-linux-deere

rc1-fx-names-linux-latenight

rc1-fx-names-linux-shade

rc1-fx-names-linux-tango

So I move the current qss snippets to style-win.qss as it looks good on Windows, and put the Mac fixes into style-linux.qss, as well. Alright?

@Be-ing
Copy link
Contributor Author

Be-ing commented Apr 10, 2018

Text is still/again cut off in LateNight & Shade with minimal window width, scaled 100%.

I can't reproduce this at 100% scaling on my 3840 x 2160 screen, neither in GNOME nor KDE... 😵

@Be-ing
Copy link
Contributor Author

Be-ing commented Apr 10, 2018

This is how it looks right now on windows or is this your WIP fix?

Those screenshots were taken on Windows 7 with RC build 6663.

@Be-ing
Copy link
Contributor Author

Be-ing commented Apr 10, 2018

So I move the current qss snippets to style-win.qss as it looks good on Windows, and put the Mac fixes into style-linux.qss, as well. Alright?

Do you mean keep the Mac fixes in style-mac.qss and make new style-linux.qss files?

@ronso0
Copy link
Member

ronso0 commented Apr 10, 2018

Those screenshots were taken on Windows 7 with RC build 6663.

Mine were taken with the same build. ubuntu studio 14.04 with xfce4.
Can try with vanilla Ubuntu and maybe up-to-date Ubuntu Studio later on..

@Be-ing
Copy link
Contributor Author

Be-ing commented Apr 10, 2018

Note that if you add other platform-specific style sheets you need to list them in the <Style> element in skin.xml as described above.

@ronso0
Copy link
Member

ronso0 commented Apr 10, 2018

fixed in #1596

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