Created attachment 105736 [details] screenshot Play/pause and volume level icons need more contrast.
Ew, they sure do. Might be able to work up a patch for this in the next few days.
This has been fixed with the latest Gwenview sources somewhere along the way. The button icons are now white, not a muddy light gray.
Which release includes the bug fix? I see the same bug on Arch (plasma 5.11.3) and neon dev unstable.
It still looks exactly like it does in your original screenshot?
Nate: Do you mean https://phabricator.kde.org/R260:b09b216fe7b404dfc12f78834c38f4a0793a5d4d (which you authored, BTW :) contained in 17.08.2? If so, this gives white icons in fullscreen mode indeed, but only for the hover overlays. The video button icons and also the trash icon when comparing two images are still affected in non-fullscreen. Note these have an additional chrome surrounding the actual buttons. We would have to try whether the same fix could be applied or if this would stand out too much on Gwenview's darker background color.
(In reply to Nate Graham from comment #4) > It still looks exactly like it does in your original screenshot? Yes.
I could have sworn they looked better, but maybe I was only looking in Fullscreen mode. Looks like there's more to be done here.
This is due to the icons adapting to the theme/palette. If using a light theme (e.g. Breeze), the icons are dark to contrast, but the rest of the HUD is hard coded dark. The icons switch to white in full screen, because we change the application theme/palette to a dark one. The question is, do we want to keep the HUD a dark theme everywhere, even in non-full screen mode with a light system theme? Or would it be preferable to have the HUD adapt to a light theme in this case? If the former, we could try to force white icons in all cases. If the latter, it would be better to just revamp the HUD completely, given it's a bit outdated.
I think the video HUD makes sense to always be dark, rather than trying to adapt to the active color scheme--at least as long as it remains a semi-translucent overlay. If we wanted it to match the rest of the UI, it would probably have to change to being always visible below the video rather than appearing over it on hover.
(In reply to Nate Graham from comment #9) > I think the video HUD makes sense to always be dark, rather than trying to > adapt to the active color scheme--at least as long as it remains a > semi-translucent overlay. If we wanted it to match the rest of the UI, it > would probably have to change to being always visible below the video rather > than appearing over it on hover. That makes sense. We could just make some small improvements, maybe switch to a flatter style, but keep the framework.
Hm, I think we have a more general problem here: Custom UI elements, not drawing in the chosen widget style and breaking if new features in the underlying libraries are introduced (in this case icon colors automatically adapting to the colour scheme, see http://notmart.org/blog/2016/05/icon-colors/). Either we continue playing catch-up and fix things occasionally, or we stop rolling our own. However, the latter might be a lot of work too. In https://phabricator.kde.org/D7988 we simply switched to normal buttons instead of the custom ones, because we could not find a better way. However, this sadly resulted in the mouse over effect being hardly visible now… If Huoni could actually find a way to only fix the icon colours, that would be good enough for now, IMO, and we could think about changing it back for the hover overlays too (and maybe a flatter look). If the icon colours are unfixable indeed, we'd need to put more thoughts into a new design (style and colour), which would have to be consistent everywhere it is used (hover buttons, video overlay, compare mode, "you have reached the end" message, etc.).
(In reply to Henrik Fehlauer from comment #11) > Hm, I think we have a more general problem here: Custom UI elements, not > drawing in the chosen widget style and breaking if new features in the > underlying libraries are introduced (in this case icon colors automatically > adapting to the colour scheme, see > http://notmart.org/blog/2016/05/icon-colors/). > > Either we continue playing catch-up and fix things occasionally, or we stop > rolling our own. However, the latter might be a lot of work too. > > In https://phabricator.kde.org/D7988 we simply switched to normal buttons > instead of the custom ones, because we could not find a better way. However, > this sadly resulted in the mouse over effect being hardly visible now… > > If Huoni could actually find a way to only fix the icon colours, that would > be good enough for now, IMO, and we could think about changing it back for > the hover overlays too (and maybe a flatter look). I have found a way (patch incoming), but I much prefer the hover buttons as they are now. I would be in favour of switching everything to 'normal' widgets, tweaking them as needed to e.g. fix the non-contrasting hover effect.
> I have found a way (patch incoming) I'm curious ;) > I would be in favour of switching everything to 'normal' widgets Sure, we can try how this pans out, as long as we are careful not to replicate the overall look of Dolphin (i.e. we have a darker background than Dolphin, the URL bar and the crop bar is a bit darker etc.). For example the you-have-reached-the-end HUD should not look weird.
Git commit 3f540441b3940591cf57038a68da284cb176303e by Huon Imberger. Committed on 03/05/2018 at 23:39. Pushed by huoni into branch 'master'. Fix dark HUD icons when using light system theme Summary: The HUD widgets are custom and always dark themed. When using a light system theme like Breeze, the icons were rendered with a dark color, which was bad because it didn't contrast with the dark HUD. This was not an issue in full screen mode, because we force the whole of Gwenview to use a dark theme, and therefore the icons rendered a light color. This affects: # Compare / Light table mode # Video controls popup # The "You are now viewing the new document" message after Save As (Other places the HUD is used do not use icons) So, when setting an icon using `HudButton::setIcon`, this patch now re-loads the icon using the `KIconLoader` global instance, setting a custom palette with a white foreground color, so the loader loads a light icon. `KIconLoader`'s palette is then reset as to not interfere with other icons. FIXED-IN: 18.08.0 Note: we bump KF5 version to 5.39 for `KIconLoader::setCustomPalette` and `KIconLoader::resetPalette`. Compare/Lighttable Before: {F5826780, size=full} Compare/Lighttable After: {F5826781, size=full} Video Controls Before: {F5826782, size=full} Video Controls After: {F5826783, size=full} Save As message Before: {F5826784, size=full} Save As message After: {F5826809, size=full} Test Plan: The following have HUD widgets with icons: # Compare / Light table mode # Video controls popup # The "You are now viewing the new document" message after Save As Note for 3), the HUD only appears for a split second, due to a bug. See D9342. Reviewers: #gwenview, rkflx Reviewed By: #gwenview, rkflx Subscribers: muhlenpfordt Tags: #gwenview Differential Revision: https://phabricator.kde.org/D12595 M +1 -1 CMakeLists.txt M +19 -6 lib/hud/hudbutton.cpp https://commits.kde.org/gwenview/3f540441b3940591cf57038a68da284cb176303e