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
Feature request: allow using a separate icon theme for lxqt panel/menu #470
Comments
I dont know, this is true for Breeze (which is incomplete right now) but there is more icon themes with this problem? Most themes include dark panel variants (Ubuntu, Numix, Elementary XFCE, Faenza). One option could be to hard code the icons to the lxqt theme, like plasma themes do. But Im not sure. |
When you say those themes include dark panel variants, do you mean they assume panels are dark, or they actually have versions of icons for bright panels and for dark panels? How does the right one get selected? |
I'm +1 on this as long as it's discrete. |
@jleclanche could you clarify what you mean by discrete? What do you think about this outline of a possible solution?
A flaw with this is that it's also possible to set the background color from Configure Panel, and then the icon theme replacement may not make sense if the user changes a dark theme to use a bright background. However I don't think this would be very common, and this background color setting doesn't seem to apply to menus either. |
That's the opposite of discrete, that's extremely invasive. An option in lxqt-panel config to choose to override all icons in the panel (menu, systray and other plugins) would be the only solution I'd be ok with, personally. |
My suggestion was to override icons in menu, systray and plugins, except for configuration dialogs which use plain Qt styles. Let's break possible solutions into decisions:
|
Either way, I suppose it's best to start from the basics. hagabaka/lxqt-panel@c7fc81b allows using a separate icon theme for the entire lxqt-panel application. It can only be set by editing the configuration file for now. It could be added to the Configure Panel dialog, but I think if it has to be user-configurable, I would prefer for it to be set in the LXQt Theme section of lxqt-config-appearance instead, since changing the LXQt theme is likely to require changing the panel theme too. Besides the fact that config dialogs also using the panel icon theme, another problem is that when the system icon theme is changed, the panel icons will be updated to the system icon theme. I don't think either problem can be fixed if the panel needs to continue to use XdgIcon::fromTheme, but I'm open to suggestions. |
I agree with @jleclanche. And the option really has to be on lxqt-panel's config dialog as lxqt-panel is supposed to be independent. |
@paulolieuthier The panel is independent, but it already uses LXQt theme settings which are defined in the lxqt-common repository, and configured in lxqt-config-appearance. |
You're right. |
@ALL - two and a half year later we still have no solution - so i guess we should implement it as soon as possible. We have exactly two choices right now: i would suggest right now a) Quick and dirty and do it right later - distributions like Lubuntu desperatly need it and should not be delivered without as it would make us look bad as project. |
@agaida I know this isn't déjà vu; we had a discussion elsewhere. When this issue was reported, people thought, "an icon theme that looks good over a dark background doesn't necessarily look good on a bright background" (quoted from the first comment) because they didn't know about symbolic SVG icons and how they change color. To have a light color for SVG symbolic icons on a dark panel, you should assign a dark Qt theme to that panel -- and conversely. IMO, changing the icon theme is a bad idea and its implementation would have serious side effects... Murphy's law. |
A detailed explanation (which I might have given in another form elsewhere): SVG symbolic icons solve the problem of contrast against different backgrounds. That's exactly the problem brought up here. However, symbolic SVG icons only follow the widget style and its palette; they can't know anything about QSS or how dark/light it may be. Unfortunately, there's no general way of assigning different Qt palettes to different apps. Kvantum can do it; other Qt styles can't. Therefore, IMO, a solution can be like this: Make it possible for each LXQt theme to have a simple Qt palette file (= color scheme) and add dark palettes to themes whose QSS files are dark, and conversely. Such a solution will be without side effects. How it can be done is another question. Moreover, it is necessary because of the dichotomy between stylesheets and BTW, KDE's panel has exactly the same issue, at least, in its systray. It just tries to avoid it by using external icons in other places. |
It's possible but makes Mr. Murphy angry. When I think only in terms of what I want, I may easily neglect other possible scenarios and break them. On second thought, I withdraw my above suggestion because, in the last analysis, it means making a special style for LXQt -- like Breeze for KDE but with the difference that it should be able to assign different color schemes to different apps. That's simply impractical for 2 reasons: it requires a lot of work (who wants to do it?); and users may simply ignore it and use another style (a lot of effort for nothing). The only remaining solution that comes to my mind is the possibility of setting the icons of widgets like volume control and removable media in the panel style -- like the main menu icon and like what KDE does (?). It shouldn't be so hard and won't have side effects either.
No, it's the icon set. The symbolic icons of most sets are limited to ≤ 22-24 px. Last but not least, think why others don't do that; are they stupid? |
Is qt5ct an option? |
erm - re side effects - see it the other way round - this will happend everytime - even with no changes to the current state. But that doesn't change anything. Let me be more verbose:
Now the suggestion is: override some things for several applications - no problem technically. I'ts proven to work, in that special case with two different icon themes. Might be nuts or not, it is possible. A second thing that i don't have implemented right now is the switch that is implemented right now in config apperance. Overriding this switch for applications might help a lot too. And this is exactly what i suggested times ago. Will implement it the hard way right now and see what happend. And to be honest - i don't see any pitfalls right now. It will not be worse than the current situation and don't change the general behaviour, because if implemented it will change the behaviour only for selected applications - and that's all about. Finding a fine theme(s) for any use case will be up to the user - and thats it. 🕶️ |
You mean adding it to the tooltip, or something else? |
If you mean adding a check box for switching that option here, I think it's neither good nor possible. A hint is enough, IMO. |
The new tooltip:
|
ok |
Closes lxqt/lxqt#470 When a light widget style is used with a dark panel, the symbolic SVG icons follow the style and become dark on the panel. The right way of solving such issues is assigning a dark widget style to the panel but neither Fusion nor Breeze can do that. This patch is about the "wrong way". It gives a separate icon set to the panel with unavoidable side effects but is optional and may be used as the last resort.
@agaida: Frankly speaking, I'd never had such a bad PR: lxqt/lxqt-panel#674 Do hesitate to merge it! I just wanted to see what was possible and what not. On the "bright" side, it's relatively small and can be reversed easily. |
BTW, overriding icon colorizing only for Panel is "somehow" possible but only with private headers and then, if the global option for colorizing is turned off and on, everything will be ruined. So, it can be forgotten peacefully ;) |
@palinek That's what I suggested at #470 (comment) but couldn't do it from inside Panel's code. It can be done easily from a Qt style code (like in Kvantum) though; I don't understand why Breeze doesn't have it. If you know of a way of doing it, please make a PR. |
@palinek Actually, giving palettes to panel parts is easy but, when I tried it, it didn't work in that way.... |
How about http://doc.qt.io/qt-5/qapplication.html#setPalette and adding it into |
Tried it to no avail but, maybe, without patience. I suggest you try palettes too. |
Tried with following patch for liblxqt: diff --git a/lxqtapplication.cpp b/lxqtapplication.cpp
index dfb88f3..2218591 100644
--- a/lxqtapplication.cpp
+++ b/lxqtapplication.cpp
@@ -131,6 +131,9 @@ void Application::updateTheme()
{
const QString styleSheetKey = QFileInfo(applicationFilePath()).fileName();
setStyleSheet(lxqtTheme.qss(styleSheetKey));
+ const QScopedPointer<QPalette> pal{lxqtTheme.palette(styleSheetKey)};
+ if (pal)
+ setPalette(*pal);
emit themeChanged();
}
diff --git a/lxqtsettings.cpp b/lxqtsettings.cpp
index 84bec7d..c496f40 100644
--- a/lxqtsettings.cpp
+++ b/lxqtsettings.cpp
@@ -35,6 +35,7 @@
#include <QFileSystemWatcher>
#include <QSharedData>
#include <QTimerEvent>
+#include <QPalette>
#include <XdgDirs>
#if QT_VERSION < QT_VERSION_CHECK(5, 6, 0)
@@ -113,6 +114,7 @@ class LXQt::LXQtThemeData: public QSharedData {
public:
LXQtThemeData(): mValid(false) {}
QString loadQss(const QString& qssFile) const;
+ QPalette * loadPalette(const QString& qssFile) const;
QString findTheme(const QString &themeName);
QString mName;
@@ -555,6 +557,27 @@ QString LXQtThemeData::loadQss(const QString& qssFile) const
}
+/************************************************
+
+ ************************************************/
+QPalette * LXQtTheme::palette(const QString& module) const
+{
+ return d->loadPalette(QStringLiteral("%1/%2.pal").arg(d->mPath, module));
+}
+
+/************************************************
+
+ ************************************************/
+QPalette * LXQtThemeData::loadPalette(const QString& paletteFile) const
+{
+ QSettings pal_s{paletteFile, QSettings::IniFormat};
+ QVariant pal = pal_s.value(QStringLiteral("palette"));
+ if (pal.isNull() || !pal.canConvert<QPalette>())
+ return nullptr;
+
+ return new QPalette{qvariant_cast<QPalette>(pal)};
+}
+
/************************************************
************************************************/
diff --git a/lxqtsettings.h b/lxqtsettings.h
index e3a5d8c..8167a19 100644
--- a/lxqtsettings.h
+++ b/lxqtsettings.h
@@ -147,6 +147,10 @@ public:
*/
QString qss(const QString& module) const;
+ /*! \brief Returns palette (if exists and is successfully read) of the called module.
+ */
+ QPalette * palette(const QString& module) const;
+
/*! \brief A full path to image used as a wallpaper
\param screen is an ID of the screen like in Qt. -1 means default (any) screen.
Any other value greater than -1 is the exact screen (in dualhead). With this and the "palette" settings file for e.g. ambiance generated like: QSettings pal_file{QLatin1String("panel.pal"), QSettings::IniFormat};
QPalette pal{QColor{0x51, 0x50, 0x48}};
pal.setColor(QPalette::Base, QColor{0x3c, 0x3b, 0x37});
pal.setColor(QPalette::Disabled, QPalette::Base, QColor{0x4c, 0x4b, 0x47});
pal.setColor(QPalette::Highlight, QColor{0xeb, 0x71, 0x40});
pal.setColor(QPalette::Disabled, QPalette::Highlight, QColor{0xfb, 0x81, 0x50});
qDebug() << Q_FUNC_INFO << pal;
pal_file.setValue(QStringLiteral("palette"), pal); which gives
we're getting (with breeze icon theme): |
@palinek I'd put hard-coded palettes in those source files and they didn't work. The style plugin has the upper hand and can enforce its own palettes. Only style sheets can override what QStyle does. I've written codes here and there in Kvantum, so that the style palette will be respected as far as possible. With your patch and with the Dark LXQt theme, if the widget style is Kvantum, nothing will happen when I give all apps -- panel included -- a light Kvantum theme: Kvantum ensures the correct palettes, which aren't what the So, this solution has its side effects too: QStyle can neutralize it. Kvantum does and who knows what other style plugins (will) exist? Since Kvantum doesn't need anything to do what is wanted here (assign a dark Kvantum theme to However, even in that case, I really don't know whether it would be more acceptable than simply changing the icon theme with lxqt/lxqt-panel#674 because, if the user disables icon colorizong, he/she won't need to do anything (making a palette file, making a theme with it, etc.); he just changes panel's icon theme. Until now, @agaida wins, IMO. |
Maybe I'm a little suggestible but, after all, it seems logical to be able to change the icon theme of a widget that can have its own style(-sheet). It should have had its own icons too in the first place. Runner may need that too. |
Yes. And with the colorized icons we have problem, that the default (qApp) palette (which is used in xdgiconloader) doesn't match with the configured colors/palette in .qss (panel, runner ...). If we approximate the colors that are defined in .qss (in particular theme) into the assigned qApp palette, the colorization can work. |
No, it won't make any difference because we want freedom with style-sheets :D Limiting their colors isn't an option. Let's have icon set overriding! |
Everyone: Please also note that stylesheets with dark and light elements are possible: the panel can be dark while the main menu can be light. It'll be up to the user to choose an appropriate icon set for such cases (and they exist). |
@tsujan - it was not about win or lose - i was just a bit pissed because the only avialable choices was pest and cholera - now we have added flu. :P |
To me, this was a fruitful discussion. I still believe that all style plugins "should" be able to give different themes/color schemes to different apps but that's not the case in the real world. The more I tested, the more I learned that no other approach would work; being able to give a different icon set to the panel is the best approach, albeit with side effects. |
@tsujan - and you are right, in a perfect world this should work. So we could consider the state now as a imperfect solution in an imperfect world 😎 And maybe we could poke some people to make our world more perfect. |
@tsujan - and yes, the runner need this too |
@agaida Because of #470 (comment) , a perfect world is impossible -- even assigning a different theme wouldn't work in that case. Style-sheets add flexibility and create problems at the same time. We can ignore the latter because of the former. Advanced users can always make simple icon themes by inheriting/using the existing ones and put them in |
Funny Fact: one of the icon themes that work out of the box without any magic is the good old faenza-darker |
The side effects I mentioned were all about symbolic SVG icons on config dialogs and other dialogs they may call. They won't exist if the theme has no symbolic SVG icon; it should just be good enough on both dark and light backgrounds. |
right - and now we have a problem - teach users about these things |
Closes lxqt/lxqt#470 When a light widget style is used with a dark panel, the symbolic SVG icons follow the style and become dark on the panel. The right way of solving such issues is assigning a dark widget style to the panel but neither Fusion nor Breeze can do that. This patch is about the "wrong way". It gives a separate icon set to the panel with unavoidable side effects but is optional and may be used as the last resort.
lxqt's panel, application menu, and applets follow a common theme which sets their background color, among other things. The lxqt theme is configured separately from Qt application styles, so it's possible for lxqt's panel and menu etc to have dark backgrounds, and for regular Qt applications to have bright backgrounds, at the same time. In fact this is the default with the "frost" lxqt theme and "Breeze" or "Oxygen" Qt style. An icon theme that looks good over a dark background doesn't necessarily look good on a bright background, so I think it's appropriate to have a separate setting for the icon theme used in lxqt panel etc.
To demonstrate the problem, below are two screenshot showing PCManFM-Qt and part of the panel, using different icon themes.
Using the Breeze icon theme, icons in PCManFM-Qt are clearly visible, but the sound mixer in the bottom right is very dim.
Using the Breeze Dark icon theme, the sound mixer icon in the panel has nice contrast, but most of the icons in PCManFM-Qt are barely visible except when highlighted with blue background.
The text was updated successfully, but these errors were encountered: