kiwix-desktop icon indicating copy to clipboard operation
kiwix-desktop copied to clipboard

UI not respecting user/OS color theme

Open Boruch-Baum opened this issue 4 years ago • 35 comments

The linux appimage for kiwix 2.0-rc4 does not respect the user's choice of theme (eg. gtk/qt)

Boruch-Baum avatar Dec 02 '19 16:12 Boruch-Baum

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

stale[bot] avatar Jan 31 '20 17:01 stale[bot]

I confirm that on Ubuntu 20.04. Just changing the the default color theme has no impact on Kiwix, it should.

kelson42 avatar May 02 '20 10:05 kelson42

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

stale[bot] avatar Jul 01 '20 12:07 stale[bot]

Seems we need to include some Qt style plugin into our AppImage:

https://wiki.archlinux.org/index.php/Uniform_look_for_Qt_and_GTK_applications#QGtkStyle

asashnov avatar Dec 31 '20 22:12 asashnov

@asashnov I don't believe this is a problem (only) with appimage. This is a general problem (with self-compiled, deb packages, flatak), right?

kelson42 avatar Jan 14 '21 10:01 kelson42

https://github.com/kiwix/kiwix-desktop/blob/cb5cfdceaf7840c456de67814e7395a7dd6c75a5/src/kiwixapp.cpp#L70

This is the part of the code that causing problem on Linux.

  1. It forces the "Windows" qt widget style on Linux, while normally this is configured by desktop
  2. The next few lines set an custom stylesheet, this overrides the widget background color, but it does not set the text color. https://github.com/kiwix/kiwix-desktop/blob/cb5cfdceaf7840c456de67814e7395a7dd6c75a5/resources/css/style.css#L2 here's my suggestion:
  3. only set widget style for windows system.
  4. As for color:
    • Only set style related to color on windows
    • Or set the widget text color in stylesheet to totally override system color (but I guess people still going to complain like this issue)
    • Or have an option: "Use system color", if disabled, apply the color related css.

Here's a screenshot under KDE's Breeze dark after I tried to remove the setStyle and all color related settings in css. 图片

wengxt avatar Jan 31 '21 06:01 wengxt

Another option is only set stylesheet against kiwix's main window (still if you want to use the same css, the text color need to be set otherwise it still use the system text color which may conflict with the background), instead of applying to QApplication.

wengxt avatar Jan 31 '21 06:01 wengxt

@wengxt Thank you for your in-depth analysis.

The ticket does not really details how Kiwix is expected to work:

  • First it should have a night/day theme depending how the OS is configured. Not sure this option exists on both windows/linux. Not sure either if it fundamentaly differs from have a custom theme.
  • We have a UI theme for Kiwix, as far as possible it should be used.
  • If the user as a custom theme (I think in particular to people have extra large fonts because being visually impaired), then it should be as much as possible respected.

It might be possible that, to some extend, these requirements are conflicting.

What I fear a bit, if we are fully using the user theme, is that we get an ugly rendering (I know current rendering is not always convincing either, but at least we have a plan), in particular per default (for people not having customized there desktop).

As far as I understand the problem can not be only the text color, this all works together (pictures, text size/font/color, background colors). So we should apply the same approach for all these properties right?

Would that be a good approach to:

  • Have a builtin day/dark mode and apply accordingly?
  • Have an option to activate/deactivate our theme (and in place fully use the default Desktop theme)?

kelson42 avatar Feb 04 '21 11:02 kelson42

It could be a three-state setting:

  • Use Kiwix white theme
  • Use Kiwix dark theme
  • Use system default theme

asashnov avatar Feb 08 '21 12:02 asashnov

From readability point of view, if you apply custom background, then you should also apply the custom color setting to text, this is a bug and should be fixed. Easiest way is to set a color for the text.

As for whether to use "system" or "kiwix" custom theme, I can only say that there are many linux users who prefer to use their own theme for the whole system level consistency, while as for windows it is not that possible to have the same level of customization on linux, so I guess window user does not care that too much.

For example: Chrome on linux has its "blue frame" theme, but it also provides an option: "using system color and title". Another counter example, telegram-desktop only option for its own themes (dark / light).

So it is really up to developer to decide what works best for themselves.

As for the problem that exposed in my screenshot: the icon is not very readable, I would suggest if you could try to use the QIcon::fromTheme and use system icon (E.g. "the plus sign icon with QIcon::fromTheme("list-add") instead. On KDE the symbolic color from theme could be automatically changed color based on system theme. A guide for using fromTheme to support on non-Linux platform: https://openapplibrary.org/dev-tutorials/qt-icon-themes

wengxt avatar Feb 08 '21 17:02 wengxt

From readability point of view, if you apply custom background, then you should also apply the custom color setting to text, this is a bug and should be fixed. Easiest way is to set a color for the text.

Agree, I have open a ticket for that, see https://github.com/kiwix/kiwix-desktop/issues/592

kelson42 avatar Feb 10 '21 09:02 kelson42

As for the problem that exposed in my screenshot: the icon is not very readable, I would suggest if you could try to use the QIcon::fromTheme and use system icon (E.g. "the plus sign icon with QIcon::fromTheme("list-add") instead. On KDE the symbolic color from theme could be automatically changed color based on system theme. A guide for using fromTheme to support on non-Linux platform: https://openapplibrary.org/dev-tutorials/qt-icon-themes

Thank you for the suggestion, but this is only one small part of the problem. All the other icons have exactly the same problem. We should find a solution working for all of them.

kelson42 avatar Feb 10 '21 09:02 kelson42

any chance there is a working solution in the near future (as new release without having to alter / compile it for yourself)? the application in its current form is nearly unusable on dark themes currently (cinnamon, lxde). do you really want users to switch to the browser-extension?

slynobody avatar May 04 '21 16:05 slynobody

@slynobody I think #592 is the main problem and I have put it in next release.

kelson42 avatar May 04 '21 17:05 kelson42

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

stale[bot] avatar Jul 08 '21 08:07 stale[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

stale[bot] avatar Apr 16 '22 15:04 stale[bot]

@kelson42 Should this remain open? :)

Jonta avatar Apr 21 '22 15:04 Jonta

@Jonta Yes, we probably need to develop a night mode... but not sure we should not better open another ticket for that.

kelson42 avatar Apr 21 '22 15:04 kelson42

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

stale[bot] avatar Aug 14 '22 01:08 stale[bot]

  • "night" gives 0 results in GitHub's search for code
  • "dark" only gives 1, which doesn't seem relevant enough
  • Therefore: I think this should remain open

Jonta avatar Aug 15 '22 14:08 Jonta

Just to say that it's very easy to implement an "inversion" dark mode as a simple CSS file inserted in (i.e. linked form) the HTML of any article. Just add inversion CSS rules:

html,
img,
video {
    filter: invert(1) hue-rotate(180deg);
}

There are some subtleties that can be added for known sites such as Wikipedia. See https://github.com/kiwix/kiwix-js/blob/master/www/css/kiwixJS_mwInvert.css for how we did it in Kiwix JS.

Jaifroid avatar Aug 30 '22 06:08 Jaifroid

PS The reason I posted this now is because on the Reddit feature request there is mention of Dark Mode, though I think that user wants it for Kiwix Serve. The same principle would apply: need to inject the CSS. This could/should be part of the JS API in the ZIM.

Jaifroid avatar Aug 30 '22 06:08 Jaifroid

+1 for request to use my system theme.

ghost avatar Nov 18 '22 08:11 ghost

do you really want users to switch to the browser-extension?

He, he, why not use both?! (Full disclosure, I developed the dark theme for the browser extension versions...)

Jaifroid avatar Nov 21 '22 07:11 Jaifroid

I think it should work like this (I'm talking for the UI, not the rendered page):

Windows

There should be 3 configuration options:

  • Use Light theme
  • Use Dark theme
  • Use system Dark Mode setting

The last one chooses between the Light and Dark theme according to the system settings and should be the default.

GNU/LInux

There should be 4 configuration options:

  • Use Kiwix Light theme
  • Use Kiwix Dark theme
  • Use system Dark Mode setting
  • Use system theme

The first 3 options work exactly the same as on Windows. The last one uses the system's QT theme, allowing Kiwix's UI to be consistent with the rest of the desktop. This should be the default.

SuperDuperDeou avatar Jan 21 '23 14:01 SuperDuperDeou

We found (over at Kiwix JS) that there is really very little use for a dark-themed UI if the content is not also dark-themed. Obviously, it's a start to provide dark UI, but it's very garish (and a bit unsettling) to have dark skin, and then the ZIM contents glaring white.

Jaifroid avatar Jan 21 '23 16:01 Jaifroid

We found (over at Kiwix JS) that there is really very little use for a dark-themed UI if the content is not also dark-themed. Obviously, it's a start to provide dark UI, but it's very garish (and a bit unsettling) to have dark skin, and then the ZIM contents glaring white.

If we could get Kiwix to use a default css file (similar to Mozilla's userContent.css) that could alleviate the problem.

ghost avatar Jan 21 '23 23:01 ghost

is there any solution for this? i often use offline Wikipedia in kiwix and with the light theme i get hurt in my eyes i hope that there is a solution for that so as i can use it in dark mode

dzwdev avatar Nov 24 '23 03:11 dzwdev

I think there's some confusion over whether this issue is about the Kiwix Desktop UI only, or also about the content displayed in the webview. Those are clearly two somewhat different issues, even though developing a dark theme for one doesn't make sense without developing a dark theme for the other.

Personally, I would say that the most urgent thing is actually the contents. The UI should be easy. I think the main line of thought for the contents is that there should be an API in each scraper (and especially in mwOffliner, where some work was started). While that would be the "ideal" solution, it would take so long to implement an API in each scraper, that I think it's unfortunately somewhat unrealistic. The only practical option (especially with upcoming support of Zimit 2.0 files, for which there is unlikely to be a standardized dark-theme API), is to provide support for dark-themed content in each reader. This is the solution adopted in Kiwix JS and Kiwix PWA.

FWIW, here is my experience wrt contents. There are two main options:

  • either use a simple inversion CSS like the one used in Kiwix JS (which ensures that images and video are re-inverted);
  • or else use a plugin like darkreader.js (offered as an experimental option in the PWA) - this is open source and MIT licensed.

I found darkreader was needed especially for Zimit-based ZIMs, where simple inversion often produces poor results. Darkreader was surprisingly easy to configure and with default settings supports pretty much all ZIMs (including Zimit). We have an open issue to backport it to the Browser Extension (which currently only offers inversion, since up to now the darkreader feature was considered experimental).

Jaifroid avatar Nov 24 '23 09:11 Jaifroid

https://github.com/kiwix/kiwix-desktop/blob/cb5cfdceaf7840c456de67814e7395a7dd6c75a5/src/kiwixapp.cpp#L70

This is the part of the code that causing problem on Linux.

1. It forces the "Windows" qt widget style on Linux, while normally this is configured by desktop

2. The next few lines set an custom stylesheet, this overrides the widget background color, but it does not set the text color.
   https://github.com/kiwix/kiwix-desktop/blob/cb5cfdceaf7840c456de67814e7395a7dd6c75a5/resources/css/style.css#L2
   
   here's my suggestion:

3. only set widget style for windows system.

4. As for color:
   
   * Only set style related to color on windows
   * Or set the widget text color in stylesheet to totally override system color (but I guess people still going to complain like this issue)
   * Or have an option: "Use system color", if disabled, apply the color related css.

Here's a screenshot under KDE's Breeze dark after I tried to remove the setStyle and all color related settings in css. 图片

i removed this line: setStyleSheet(parseStyleFromFile(":/css/style.css") from this file: kiwixapp.cpp and now the main windows is dark. thank you. but the content is still in white theme, so is there any other edit i can do to make the content in dark mode?

dzwdev avatar Dec 04 '23 07:12 dzwdev