xiaoyifang / goldendict-ng

The Next Generation GoldenDict
https://xiaoyifang.github.io/goldendict-ng/
Other
1.56k stars 82 forks source link

feature: darkmode ,darkreader,custom style refinement #339

Closed xiaoyifang closed 1 year ago

xiaoyifang commented 1 year ago

when user choose a custom style ,the background is different in the following image image

the above image is caused by removing the custom qt-* style.css.

This has caused some confusions. I think the combination of the darkmode ,darkreader,custom style should be considered more closely.

Such as : 1, put darkmode as a custom style in the dropdownlist(Windows only). image by this way,the darkmode will not conflict with custom style. 2, when user choose darkmode,

xiaoyifang commented 1 year ago

@shenlebantongying can you check on this

shenlebantongying commented 1 year ago

On Linux, we shouldn't override UI's background & text colour. It looks insanely good if users choose to use a global theme that is comfortable for reading.

image


On windows, I have a better idea: let the user choose an arbitrary background colour rather than yellow only. It will override both article's background and the side panels' background colour.

image

The article display style will only change the style of the widgets.

image

Pro: Satisfy all users Con: One extra setting option :sweat_smile:

If the dark mode is enabled, then the side panel's background colour overriding will be disabled.

If the dark reader mode is enabled, then the article view's background colour overriding will be disabled.

Some extra thoughts from serious Windows users maybe help.

xiaoyifang commented 1 year ago

I think we should not grant users too much options.

On windows, I have a better idea: let the user choose an arbitrary background colour rather than yellow only. It will override both article's background and the side panels' background colour.

currently we have about 6 custom styles , if implemented the above feature ,we should have too much styles . this will make the situation worse.

If the dark mode is enabled, then the side panel's background colour overriding will be disabled.

If the dark reader mode is enabled, then the article view's background colour overriding will be disabled.

on Windows ,this is much better. and dark reader mode can be remove . it should be enabled together with darkmode.

on Linux/Macos, darkreaer can be enabled automaticly when the system has enabled the dark mode. which means ,the two options(darkmode ,darkreader) should not appear on Linux.

// Enable when the system color scheme is dark.
DarkReader.auto({
    brightness: 100,
    contrast: 90,
    sepia: 10
});
shenlebantongying commented 1 year ago

We shouldn't use the auto dark reader, because "light text on dark background" is unusable for some people with low vision.

The white text on the black background will become blurry for them. Using dark UI and light text in article view is a possible use case.

https://www.google.com/search?q=accessibility+light+text+on+dark+background

xiaoyifang commented 1 year ago

The white text on the black background will become blurry for them. Using dark UI and light text in article view is a possible use case.

then ,on linux, just keep the dark reader mode. Let's users to choose . and same behavior as this

If the dark reader mode is enabled, then the article view's background colour overriding will be disabled.

If the dark reader mode is not enabled ,the application's custom style should be enabled.

shenlebantongying commented 1 year ago

To prevent dark reader mode from messing with the yellow background, We can simply append <style> body .gdarticle { background: white; } </style> when the dark reader is enabled.

https://github.com/xiaoyifang/goldendict/blob/bd736d7f37a3993f92c3f3557be5a52e7386f92c/article_maker.cc#L152-L165


However, the "correct" solution is just one step away from using arbitrary background colour:

We will remove body { background: yellow } from the CSS files. When the dark reader is not used, body .gdarticle {background: #{custom_color}} will be appended to the CSS file.

https://github.com/xiaoyifang/goldendict/blob/bd736d7f37a3993f92c3f3557be5a52e7386f92c/article-style-st-classic.css#L7-L12

xiaoyifang commented 1 year ago

in screenshot ,the darkmode has not been enabled , a custom style has been choosen. the custom style has stop to work.

I think 587aad14716b0421c8bdcd1f31d61eed014ccd34 can be reverted and make some minor changes to satisfy the custom style.

if custom style+darkmode ,the custom style will not applied. only apply dark mode. if custom style has been choosen , custom style will take effect.

shenlebantongying commented 1 year ago

If we revert that, there will be glitches on linux & mac because the old custom .css isn't complete (only some styles are changed. If the user chooses another global qt theme, then some weird visual glitch will happen.).

In other words: the struggles of those issues:

https://github.com/goldendict/goldendict/issues/719 https://github.com/goldendict/goldendict/issues/679 https://github.com/xiaoyifang/goldendict/issues/271#issue-1510215538 https://github.com/xiaoyifang/goldendict/issues/188#issuecomment-1309593039

They cannot figure out a nice solution because the old .css files only change some of the styles.

To avoid those glitches:

One path is to avoid theming UI elements as https://github.com/xiaoyifang/goldendict/commit/587aad14716b0421c8bdcd1f31d61eed014ccd34 did.

Another path is fully theming everything, but this approach's maintenance cost is too high.

However, we can bring back those qt style sheets only for Windows.

xiaoyifang commented 1 year ago

the glitches only occured when enabled dark mode. when darkmode is enabled , do not apply the custom style can avoid these glitch.

when dark mode is not enabled ,they should also work.

shenlebantongying commented 1 year ago

There is no concept of "dark theme" for Linux. The theme can be any colour, like purple or green or orange.

I think the only reasonable path is to bring them back to Windows only.

Sample glitch of reverting the commit and using the Lingoes-blue (there is no "Dark mode" involved at all):

image

xiaoyifang commented 1 year ago

There is no concept of "dark theme" for Linux.

I kind of confused.

Sample glitch of reverting the commit and using the Lingoes-blue (there is no "Dark mode" involved at all):

is the screenshort taken when both darkmode and lignoes-blue been enabled? in such a case , I think only darkmode should be applied and do not apply lingoes-blue scheme.

xiaoyifang commented 1 year ago

maybe the goal can be split into several steps: 1, make darkmode and custom style can work on Windows. 2, keep the linux ,macos unchanged for now before a new solution come up.

shenlebantongying commented 1 year ago

There is no concept of "dark theme" for Linux. I kind of confused.

Linux users may use any color. The black background is only one of the possibilities. Check the screen below: I can install Green/Blue/Brown backgrounds, and they are not "dark".

In fact, the Qt's style engine on Linux is replaceable,

Such as Kvantum which uses SVG to theme Qt widgets, a theme is just a SVG picture.

There is also QtCurve which is built on top of GTK so that Qt and GTK apps look exactly the same. The theme will be determined by the user's GTK theme.

Thus, there is no concept of "dark theme" on Linux.

So, I think the the best solution for Linux users is don't hard code any colours. Just don't touch the defaults unless necessary.

image

xiaoyifang commented 1 year ago

So, I think the the best solution for Linux users is don't hard code any colours. Just don't touch the defaults unless necessary

Does this mean on linux the custom style can be removed?

shenlebantongying commented 1 year ago

Yes, and we already removed them. We just bring them back to Windows Only. https://github.com/xiaoyifang/goldendict-ng/pull/272