-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Skins: style main menubar #3788
Conversation
35b9363
to
6328c7d
Compare
May I recommend to increase the contrast of the check box border. On view angle dependend LCD displays, it can be difficult to see that there is a check box. |
@JoergAtGithub Even though I see your demand I think we shouldn't bend over too much to compensate mediocre hardware, because the additional contrast may not be desired when using contemporary IPS panels. |
To be honest I like more dim (original) version much better. |
Me too. I can try to brighten the border a bit, but your proposal is going too far for PaleMoon |
I have also stumbled over this thinking the menu bar is grayed out. Sure am biased as a embedded developer, but I think we are on the wrong track here. Please consider the following: The overall impression of a skin depends on a the picked output rgb values and the monitor it is displayed on. If we consider a sRGB Monitor the colors are calibrated to produce the same look at all devices at a 80 cd/m² white luminance at center position of the monitors brightness control. Only a minimum of users have monitor capable of displaying all sRGB colors AND use it with the sRGB brilliance settings. Some have a weak device where there is no visual diffence between two colours in the RGB neighborhood. If you like pale skin you have the option to reduce the contrast of the input RGB value or lower the brilliance settings to get the same color in the sRGB color space. There is also another issue with sourrunding apps if you are not in full screen mode. Using PaleMoon the other apps are kind of dazzling. This is extrem if you open a web page with s white backrounds. After going back to Mixxx PaleMoon you cannot see anything for some seconds. This can already an issue in Mixxx if you display for instance a white cover art. lt;dr |
No, not at all. The original issue was only the checkbox border in the menu. I can imagine that it is not visible at all on some screens. All elements of a skin should be consistent for a full screen view. My long text was more a general advice how to design displays for night use. For my understanding PaleMoon focuses dark booths, where the display should be eye pleasant, when looking to other objects in the booth and back to the display. I am afraid it fails for this use case when I see the bright default windows border decoration. It works only in windowed mode if the windows decorations also dark and no bright app is shown. So if Pale Moon actually focusses this use case, it should be tested with the display brightness at minimum. And we should verify if the contrast ratio is still good enough to be readable even on poor displays. I have not find time to check. For verification, we need a photo including some sourrunding objects. A screenshot did not work, because it is displayed with the background light of the viewing device. |
I understand, thanks. I'm using PaleMoon with very low backlight in club environment since I started working on it, and it feels very pleasant for 8h+ with fog and drinks and all the disturbing 'real live effects' ; ) From my experience I'd say my IPS panel is somewhere in the midfield between old, dim, low-contrast panels and contemporary office panels with good or high brightness and contrast and much better sRGB coverage, let alone Macbook panels.
Photos are not suitable to judge this. Digital cameras can hardly reproduce the dynamic range of the human eye, also photo results vary from camera to camera, let alone implicite image post-processing. I can test again with my 2012 laptop with an TN panel to adjust label colors in order to find a better compromise. I'll take WCAG 2.0 level AA recommendations as orientation (though we already did that check in the initial PaleMoon PR IIRC). sidenote: I find the contrast in Deere very poor, even now with IPS, and that was our default skin for some years with receiving complaints. |
I suggest we continue this discussion on Zulip and restrict this PR to the main menu bar styling. |
The linked thread issues another topic. For my understanding the menu bar contrast topic is fully explored here. If @ronso0 can confirm my findings, I think we have a plan. |
I can confirm the menu bar looks good now. I would prefer a bit warmer grey though, but that is a matter of taste. Did you also consider to fix the MIN/BAL lable? Or is this a topic of another PR. |
#888 for the main menu was just a quick shot. I'll adapt that to be warmer. There'll be a separate PR fixing the broken SVGs and the visibility issues when I've thested with my old laptop. |
dc30711
to
83056d9
Compare
This is ready for final testing!
Please check those menus for consistent appearance
I'll give this a shot on Windows again soonish and force-push if necessary. |
@poelzi Oh saw your comment only now. May your fix went into |
commit eb3b2fd was the fix |
ah okay. that was for the hotcue menu, here it's about the color picker in the track menu. The issue seems to be that adjustSize() is doing what it should only after the first show event. |
Ahh, my bad. I would guess same fix. call updateLayout() in the init method so the size in correct when show() is called. I will look into this this week |
I tried the fix already, it doesn't work. The issue is really about resizing parent (m_pColorMenu) to content (adjustSize()) is not working for QMenu unitl it's shown the first time. Seems there are possibly ugly workaounds with a fake resizeEvent(). |
How does it look like in 2.3?
Yes, the 'internal' menus use |
I'll check tomorrow. The menu bar checkboxes look nice, can we use them in context menus, too? Or is this too much hassle? |
hmm.. describe "nice" please. |
|
Maybe :D The blur makes the checkbox border appear slighly rounded (like 1px radius or something), maybe that is what I find appealing. Anyway, I think the last remaining major issue is the wide gap between the checkbox and the label in the menu. Can you take care of that? Then we can merge IMHO. |
sure, when I'm back home I'll finish this. I guess the required stylesheet differences arw mostly due to different Qt versions, so the distinction Linux / Windows is probably not the ultimate solution, just a workaround that seemed to solve it in the initial wtrackmenu PR (2.1 / 2.2?) Checkbox tweaking may happen in a follow-up as soon as I established a somewhat ciherent appearance here. |
Maybe we should file a feature request to add something like the CSS media selectors to QSS, like:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, looks fine. I think the risk that something for other OSes broke is small and it would be a cosmetic issue anyway. I'll merge now to unblock your follow-up PRs. If more patches are needed, these can go into follow-up PRs.
let's merge this tomorrow. |
Alright, looks good on Windows and Linux. 1322088 is the only c++ change that slipped in -- the Crates submenu has an id now which allows to style it individually. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
Thank you very much for all the work on this. |
Deere (I made the toolbar a bit less dark so it blends with the menubar)