Skip to content

LateNight: style main menubar#3704

Merged
Holzhaus merged 6 commits intomixxxdj:2.3from
ronso0:menubar-styles-23
Apr 4, 2021
Merged

LateNight: style main menubar#3704
Holzhaus merged 6 commits intomixxxdj:2.3from
ronso0:menubar-styles-23

Conversation

@ronso0
Copy link
Copy Markdown
Member

@ronso0 ronso0 commented Mar 14, 2021

Wooh, I finally managed to tame the Qt stylesheet madness and untangle my previous attempts.
This first implementation is for LateNight only.
It is tested on Ubuntu with xfce. If that works okay on Win and Mac, too, I will start to port the styles to all skins.

Confirmed to work in

  • *ubuntu with Xfce
  • *ubuntu with Unity/Gnome
  • Fedora / Arch with Gnome
  • Windows10 (tested with 100% and 125% native app scaling)
  • Windows7
  • macOS (not affected due to global menu)

Linux, with xfce
image
image

It allows to save some vertical space but it also makes it obvious how much space is wasted by the menubar.

Consider this:
To style the main menubar we use the skin's font but
we try to preserve most other font properties set by
the operating system, such as the font weight, size and style.
This is because the user may use a HiDPI screen with fonts
slightly larger than the defaults but may NOT have scaled Mixxx.
Even though different fonts at the same point size don't necessarily
produce the same effective visual size the difference is
marginal inmy experience, except for some really weird fonts.

@ronso0 ronso0 added the skins label Mar 14, 2021
@ronso0 ronso0 added this to the 2.3.0 milestone Mar 14, 2021
@alhadebe
Copy link
Copy Markdown
Contributor

Before
image

After image

On Windows10 (2560x1440)

  1. Horizontally the menubar feels cramped; menu items are too close together. it looks fine on your screenshot, a windows issue maybe?
  2. The checkboxes are nicely visible now, but super small.
  3. I do feel like I'm missing vertical space. in saying that, I never really use the bar, so may I'll get used to it

@JoergAtGithub
Copy link
Copy Markdown
Member

JoergAtGithub commented Mar 14, 2021

I tried this on Windows7 and in general it works, but I noticed the following:
-At startup the Menu Bar is displayed with Windows system skin after some seconds it switches to LateNight skin. This looks odd to me. Is it possible to hide the menu at all, until itis usable?
-If the main window looses focus, the Windows system skin shows the menus as inactive, the LateNight skin looks allways the same

Aestetical impressions:
-All other fonts in PaleMoon skin look more bold than the menu font
-In fullscreen mode the LateNight menu skin looks much better than before
-If Mixxx is shown as a window, the window decoration is very dominant. IMHO the tiny LateNight menu doesn't fit in this case.
-The style of a context menu differs from the style of a menu from the menu bar

@daschuer
Copy link
Copy Markdown
Member

I can confirm that this work on Ubuntu Bionic with Ubuntu/Unity/Gnome Desktops
Thank you :-)

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Mar 14, 2021

Thanks for testing @NotYourAverageAl
@JoergAtGithub Can you also confirm that it looks like in my screenshot?
If so, this is great! And we just need this confirmed on macOS @foss- maybe?

Then we can get into tweaking the details.

@JoergAtGithub
Copy link
Copy Markdown
Member

This is how it looks on Windows7:
grafik

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Mar 14, 2021

At startup the Menu Bar is displayed with Windows system skin after some seconds it switches to LateNight skin. This looks odd to me. Is it possible to hide the menu at all, until itis usable?

This PR doesn't change that behaviour. The launch screen style is parsed first, before actually loading the skin incl. menu style.
Though, this has multiple aspects:
1 I noticed that opening the preferences from the menu bar before the launch process is finished may leed to incorrect preferences, IIRC not all controllers showed up or the mapping wasn't loaded, yet. This may be confusing. So maybe we delay showing the menubar (or just the Options menu) until the DlgPreferences is fully instantiated? I dunno
2 When a skin fails to load for whatever reason users need the menubar to access the preferences. Otherwise they'd get stuck with an inaccessible UI

For a Consistent Experience™ we could isolate the menubar styles as we do with the launch screen, or even include it there.
I'll take look if that's still feasible for 2.3

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Mar 14, 2021

If the main window looses focus, the Windows system skin shows the menus as inactive, the LateNight skin looks allways the same

Tbh I consider this an improvement. Even with an unfocused window in difficult lighting you can aim for a menu item, instead of clicking an emtpy place first to get a readable menu.
Or do you want to say that dim menus are required to realize the window's focus? If that's the only proper way of a Windows theme to signal the focus that's an issueof its own.

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Mar 14, 2021

Yeah, I see the main menu items could use more spacing.

The checkboxes are nicely visible now, but super small.

should be sized like in the other menus.
Btw the entire row is responsive, not just the checkbox.

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Mar 14, 2021

@daschuer Could you please share a screenshot of the menu with Gnome?

This need some os-specific tweaks to look good, like the checkbox offset is computed differently on linux / windows for example.

@foss-
Copy link
Copy Markdown
Contributor

foss- commented Mar 14, 2021

macOS 11.2.3
mixxx 2.3.0-beta (build HEAD r8076)
2021-03-14

@foss-
Copy link
Copy Markdown
Contributor

foss- commented Mar 14, 2021

Yes that's the artifact build.
Looking fine, fullscreen is ok as well.

@JoergAtGithub
Copy link
Copy Markdown
Member

Is the global menu of MacOS also skined?
I imagine, that in full screen mode, MacOS looks like every other platform, but if I imagine a dark menu bar on top of a light desktop scheme, an a smaller Mixxx window somewhere below, I can't imagine, that this looks good. But I'm not a Mac user.

@foss-
Copy link
Copy Markdown
Contributor

foss- commented Mar 14, 2021

macOS dark mode: dark menu bar
macOS light mode: light menu bar

How would you end up with dark menu bar on light desktop scheme? That will never happen.

The only thing currently off is the light title bar of mixxx window on macOS dark mode. But that is due to some xcode shenaningans which will not be addressed for 2.3.

@daschuer
Copy link
Copy Markdown
Member

Here is the GNOME view on Ubuntu Bionic:
grafik

@poelzi
Copy link
Copy Markdown
Contributor

poelzi commented Mar 15, 2021

I think the checkbox could use a color from the palette. Maybe green or orange.
The difference in win10 spacing could be a result from DPI differences.
Thank you for finishing this madness. When I started with styling the menubar i questioned my sanity with qts strange behaviour :)

@alhadebe
Copy link
Copy Markdown
Contributor

Agree with poelzi about colour. Also I think I liked the the storm trooper look, light menu bar & dark everything else..but that's just me

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Mar 15, 2021

@poelzi May be true. later I'll try to reenact my win10 installation and check on fullhd

@uklotzde
Copy link
Copy Markdown
Contributor

Fedora (before)
latenight before

Fedora (after)
latenight after

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Mar 15, 2021

@uklotzde Your screenshots say this is a no-op PR ;)
please update the 'before' one.

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Mar 15, 2021

Until now it seems in Gnome and Xfce the checkbox is within the item padding, everywhere else (Fedora, Win7/10) it's added.
We can address Win via <Style src-windows="style-windows.qss" ... > but we can't filter for Fedora.
@uklotzde Could you post a screenhot of another Qt app where the main menu has solo items and those with checkboxes?
Do yo use Gnome3?

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Mar 15, 2021

In the end I'd try to make it look good on Linux for the most used desktop distros which appear to be Mint, Ubuntu, ... atm.

@Holzhaus
Copy link
Copy Markdown
Member

This is how it looks on Arch Linux (GNOME Shell 3.38.4):
Screenshot from 2021-03-16 00-00-16

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Mar 17, 2021

Merge?

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Mar 17, 2021

obvioulsy not!
I found another way make it work on Linux (likely for Fedora/Arch as well) and I was fumbling with the styles on Wind'ohs and of course it's again slightly different there, enough to look sh#*tty.

Now picking relevant parts from style_palemoon.qss and splitting styles into styles_linux.qss and styles_windows.qss because theme style sheets would override the platform-specific ones....

Coming closer to make it look like this for both:
image

I believe the folks who implemented the style parsing on certain platforms are not very popular...

@ronso0 ronso0 marked this pull request as draft March 17, 2021 02:41
@poelzi
Copy link
Copy Markdown
Contributor

poelzi commented Mar 17, 2021

Sometimes the different behaviour of QT depending on the OS/Desktop Environment and version of QT is just confusing, the styling system in particular. I have seen the strange spacing problem as well on some desktops, but not others as well and gave up at some point ;)
@ronso0 while you at it ant still remember this mess, maybe your solution could be applied to the context menu as well ?
I think the spacing there is also broken, it is just not that noticeable because we only have checkboxes or normal menu items per submenu and not mixed, but the offsets between the checkboxes seem to be different.

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Mar 17, 2021

I will not mix up the main menu and the context menus for two reasons:
a) reliable test tesults and iteration is super slow, I'd rather not open that can of worms atm (in this PR)
b) the main menu has only items, with or without ::indicators, no QCheckBoxes
Edit appearantly the QCheckBox selector is required for the for WTrackMenu because in the Crates submenu we use QWidgetAction instead of QAction->setCheckable().

But the new styles are quite compact and they are in separate stylesheets, so it's easy to remember what's what. So if I don't loose my mind I can start another PR once this here is merged.

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Mar 27, 2021

perfecto!

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Mar 27, 2021

LateNight Classic is included now:
image

@ronso0 ronso0 force-pushed the menubar-styles-23 branch from 812e2ce to 19e6995 Compare March 27, 2021 23:33
@ronso0 ronso0 marked this pull request as ready for review March 28, 2021 01:57
@ronso0 ronso0 force-pushed the menubar-styles-23 branch from 19e6995 to 48cdc29 Compare March 30, 2021 01:59
@Holzhaus
Copy link
Copy Markdown
Member

On Arch/GNOME 40 the indentation for items without checkboxes is still slighty off but I don't know if that is fixable:

Peek 2021-03-31 00-24

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Mar 30, 2021

oh yeah, I noticed I screwed up some things with the consolidation into common style.qss
also affects Win10 I think.
I'll fix it

@ronso0 ronso0 force-pushed the menubar-styles-23 branch from 48cdc29 to 595e0a9 Compare April 3, 2021 23:46
@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Apr 4, 2021

The main menu should now look the same on all OS, see picture below.

Also, I took the opportunity to consolidate and clean up all QMenu styles and split off a few tweaks into style_windows.qss
Again, tests on ubuntu with xfce and Windows10 passed. Please tell me everything looks as pictured also with Gnome and other compositors.

Please check all menus in

  • main menu bar
  • track menu (tracks table and in decks)
  • editing metadata in the tracks table
  • tree view
  • spinboxes (beatsize, AutoDJ tab, BPM in tracks table)
  • table header
  • cover art
  • label field in cue menu

image

The only weird things I noticed (on linux) is that Select All and Ctrl+A overlap in lineedit menus (english locale), and on Windows10 in PaleMoon the yellow library header sort arrow from the Classic theme is used 🤷

@Swiftb0y
Copy link
Copy Markdown
Member

Swiftb0y commented Apr 4, 2021

Unfortunately, I couldn't compare everything from the list because some references are missing from your composed screenshot. The things that I was able to compare matched and everything that I wasn't able to compare still looked like it was styled in accordance to the latenight theme.
(GNOME 3.38 wayland on fedora btw).

However, I encountered a major issue: I was not able to edit track metadata from within the library table. Selecting a table cell was no problem, but clicking on that cell again so bring up the editor just didn't work.

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Apr 4, 2021

okay, thanks. only the inline edit menu was missing, and that's the same for all spinboxes, track tag editing, cue label editing and the searchbox. Note that on windows there are no icons (same as in all other apps I checked).
image

However, I encountered a major issue: I was not able to edit track metadata from within the library table. Selecting a table cell was no problem, but clicking on that cell again so bring up the editor just didn't work.

@Swiftb0y
Even after you enabled that in Preferences > Library? than it's a regression in 2.3. This branch only chnages xml/qss.

@Swiftb0y
Copy link
Copy Markdown
Member

Swiftb0y commented Apr 4, 2021

Even after you enabled that in Preferences > Library?

I'm sorry for the false alarm, I wasn't aware of this preference option. Works as expected.

Did you provide custom icons for that context menu? The one in your screenshot seem very latenight-ish but Qt seems to use the default ones.
image
weirdly enough, the Select All Ctrl-A seems to be fine for me (english locale as well)

Copy link
Copy Markdown
Member

@Holzhaus Holzhaus left a comment

Choose a reason for hiding this comment

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

LGTM, thank you.

Screenshot from 2021-04-04 21-07-10

System:

gnome-desktop 1:40.0-1
qt5-base 5.15.2-5
xorg-server 1.20.10-3

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Apr 4, 2021

Did you provide custom icons for that context menu?

No. I'd like to try that to have entirely themed, failsafe menus, but that is on hold until we tackle to style all dialogs like the preferences and track properties dialogs, too.
I did some brief research how to do load custom icons but didn't find anything useful, yet (with "qmenu use custom icons / icon set")
This is Papyrus-Dark theme, and it's actually only the inactive icons that match :\

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Apr 4, 2021

Alright, nice!
I guess we can merge this then, and I'll use it as blueprint for the other skins.

@Holzhaus Holzhaus merged commit 3302d95 into mixxxdj:2.3 Apr 4, 2021
@ronso0 ronso0 deleted the menubar-styles-23 branch April 4, 2021 20:25
@ronso0 ronso0 changed the title Skins: style main menubar LateNight: style main menubar Apr 5, 2021
@daschuer
Copy link
Copy Markdown
Member

@ronso0 Great, this really helps to release the pressure form the menu bar topic.

I notice that the menu bar is not styled during the load screen. Is this hard to solve?

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Apr 12, 2021

yes it looks great but I also realized how much space we waste with the menubar..

that it's unstyled initially is a minor annoyance. We could
a) split off the menubar stylesheets and parse them early before/together with the the launch screen, maybe here
b) on Linux and Windows, don't show the menubar until the skin is loaded succesfully (or failed!) because until then the menubar is neither responsive nor needed anyway.

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Apr 12, 2021

as mentioned: the pain with the menubar is only styling checkboxes and items concistently across all OS . so if we omit that we could set a minimal stylesheet (no the submenus, no checkboxes) until the entire skin is parsed.
so even though I'd prefer b we may get a working without too much hazzle (and no risk)

@daschuer
Copy link
Copy Markdown
Member

Thank you for looking into it. Both ideas are reasonable ...

@poelzi
Copy link
Copy Markdown
Contributor

poelzi commented Apr 13, 2021

@ronso0 I already implemented #3189 a solution for the menubar problem. There is only one drawback I was not able to solve: native menubar on linux. QT seems to be very broken when the qaction associated with a menubar item is not owned by the menubar or outlives changes :-/
Shade also needs a menubutton added.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants