Skip to content
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

Enable native Windows 10 dark theme support by default #13952

Closed
wants to merge 2 commits into from

Conversation

FranciscoPombal
Copy link
Member

@FranciscoPombal FranciscoPombal commented Dec 11, 2020

Settings related to workarounds to HiDPI issues
are still commented out by default.


See #13675 (comment) and Ctrl+F darkmode here: https://doc.qt.io/qt-5/qguiapplication.html

I did not test on a Windows version lower than Windows 10 1903, but I think in such cases Qt simply ignores this. However, it would be nice for someone to confirm this before merging, since we still support those versions.

The new font related argument that is commented out comes from here: #13749 (comment) and this whole discussion: #12295

Screenshots:

Click to expand

dark mode ss1
dark mode ss5
dark mode ss4
dark mode ss3
dark mode ss2

It respects title bars and window borders color changes:

Go to Settings->Personalisation->Colours->"Choose your accent colour"
Show the accent colour on the following surfaces->Check/Enable Title bars and windows borders

dark mode ss7
dark mode ss6

@FranciscoPombal FranciscoPombal added OS: Windows Issues specific to Windows Core labels Dec 11, 2020
@FranciscoPombal FranciscoPombal mentioned this pull request Dec 11, 2020
@xavier2k6
Copy link
Member

darkmode=[1|2] controls how Qt responds to the activation of the Dark Mode for applications introduced in Windows 10 1903 (since Qt 5.15).

A value of 1 causes Qt to switch the window borders to black when Dark Mode for applications is activated and no High Contrast Theme is in use. This is intended for applications that implement their own theming.

A value of 2 will in addition cause the Windows Vista style to be deactivated and switch to the Windows style using a simplified palette in dark mode. This is currently experimental pending the introduction of new style that properly adapts to dark mode.

https://doc.qt.io/qt-5/qguiapplication.html#platform-specific-arguments

@glassez
Copy link
Member

glassez commented Dec 11, 2020

How does it interact with custom themes when both custom theme and Windows dark mode are enabled?

@glassez glassez added Look and feel Affect UI "Look and feel" only without changing the logic and removed Core labels Dec 11, 2020
@glassez
Copy link
Member

glassez commented Dec 11, 2020

Also please provide screenshots in PR description.

@Chocobo1
Copy link
Member

Chocobo1 commented Dec 11, 2020

I don't like the fontengine introduced, it is a temporary workaround at best. Its usage should be documented in wiki instead.

Adding fontengine should have its own commit.

@ghost
Copy link

ghost commented Dec 11, 2020

Doesn't work for me.
20H2.

@xavier2k6
Copy link
Member

Doesn't work for me.
20H2.

Go to Settings->Personalisation->Colours->"Dark"

or

Go to Settings->Personalisation->Colours->Custom->Choose your default app mode->Check/Enable "Dark"

@xavier2k6
Copy link
Member

xavier2k6 commented Dec 11, 2020

controls how Qt responds to the activation of the Dark Mode for applications

Title is probably a bit misleading.....

EDIT (FranciscoPombal): Thanks for the screenshots, moved to a collapsible section in the OP.

@jagannatharjun
Copy link
Member

image

This is clearly not ready for production

@xavier2k6
Copy link
Member

xavier2k6 commented Dec 11, 2020

some more screenshots:

EDIT (FranciscoPombal): Thanks for the screenshots, moved to a collapsible section in the OP.

fontengine=freetype may solve font rendering problems in some cases
related to usage of high DPI scaling settings on Windows.
@FranciscoPombal
Copy link
Member Author

@jagannatharjun

This is clearly not ready for production

As the Qt docs state, it is currently experimental, so it can use some polish, but it's usable at least. I'm sure that even in this state, some users will prefer it. I'm expecting it to improve in future Qt versions of course. The user can always disable it for qBittorrent even if they have dark theme system-wide.

@xavier2k6

controls how Qt responds to the activation of the Dark Mode for applications

Title is probably a bit misleading.....

How so? Enabling Qt to respond to the activation/deactivation of "dark mode for applications" is the same as saying that we enable support for the native dark theme.

@Chocobo1

Adding fontengine should have its own commit.

Fixed. Let me know if you have a better suggestion for the commit message.

@xavier2k6
Copy link
Member

How so? Enabling Qt to respond to the activation/deactivation of "dark mode for applications" is the same as saying that we enable support for the native dark theme.

Apologies! that should've been part of this #13952 (comment) (I think. there was a presumption that dark mode would've been on/active by default) not that there is a requirement of user interaction/choice....(my interpretation of the comment may be wrong too)

@jagannatharjun
Copy link
Member

The user can always disable it for qBittorrent even if they have dark theme system-wide.

How? going and editing the .conf file is not good UX. IMHO darkmode=1; is more than enough for now.

@FranciscoPombal
Copy link
Member Author

@xavier2k6 @jagannatharjun

Apologies! that should've been part of this #13952 (comment) (I think. there was a presumption that dark mode would've been on/active by default) not that there is a requirement of user interaction/choice....(my interpretation of the comment may be wrong too)

Dark mode won't be on by default. Support for it will be active by default. This means that if the user selects "Dark mode for applications" in the Windows settings, qBittorrent will use dark mode. As soon as the user turns off "Dark mode for applications", qBittorrent automagically reverts back to the default native look & feel, without even needing to be restarted.

How? going and editing the .conf file is not good UX. IMHO darkmode=1; is more than enough for now.

I doubt that a user who turns on "Dark mode for applications" in the Windows settings, which is meant to apply systemwide, prefers the non-dark native look to the dark theme, even if it does not have the greatest polish yet. Also, if they are truly unhappy with Qt's dark theme, I'm guessing they'd still much rather hunt for a custom dark theme first before resorting to deactivating the dark theme and use the non-dark native look. But hey, if they really want qBittorrent to be non-dark mode in their dark-mode system, the escape hatch is there in any case.

Also, again, as I've said before, darkmode=1 is basically useless, it only affects the window borders, which is almost unnoticeable.

@jagannatharjun
Copy link
Member

I doubt that a user who turns on "Dark mode for applications" in the Windows settings, which is meant to apply systemwide, prefers the non-dark native look to the dark theme,

you know how bad it looks, right? and I assume you know we have our own theme system right? the whole point of our theming system is to overcome qt's limitations. why you want to eat half-baked cake when the chef himself advised not to eat.

Also, again, as I've said before, darkmode=1 is basically useless, it only affects the window borders, which is almost unnoticeable

no with the custom theme this is more than enough jagannatharjun/qbt-theme#40 I was thinking to open a PR over the weekend, huh

@xavier2k6
Copy link
Member

Dark mode won't be on by default. Support for it will be active by default.

I am aware of that. (we are getting our wires crossed, lol!) as explained - it should've been in response to an0n666's comment.

@xavier2k6
Copy link
Member

xavier2k6 commented Dec 11, 2020

you know how bad it looks, right?

I think it's not too bad except for a few hiccups/colour changes required ("stalled arrows")

What has to be taken in to account as well is there will be users who won't want to use a 3rd party theme....

No harm in giving the user another choice!

@jagannatharjun What do you find so bad about it?

also that windows title bar that you linked to wouldn't be a problem...see OP for screenshots.

@FranciscoPombal
Copy link
Member Author

FranciscoPombal commented Dec 11, 2020

@glassez

How does it interact with custom themes when both custom theme and Windows dark mode are enabled?

As suspected in #13952 (comment), it "just works" as expected: custom themes completely override the base dark theme. I tested a bit with themes from https://github.com/jagannatharjun/qbt-theme, and It looks identical to the screenshots.

As far as I can see, this is a straight improvement to the current situation.

Previously (3 out of 4 cases work as expected):

  • User doesn't have Windows dark mode active: all good, qBittorrent uses default non-dark native look
  • User doesn't have Windows dark mode active and they want to use custom themes: all good, qBittorrent uses custom theme
  • User has Windows dark mode active and they want to use custom themes: all good, qBittorrent uses custom theme
  • User has Windows dark mode active and they don't want to use custom themes, they want qBittorrent to use Window's native dark theme: user is SOL

Now (4 out of 4 cases work as expected):

  • User doesn't have Windows dark mode active: all good, qBittorrent uses default non-dark native look
  • User doesn't have Windows dark mode active and they want to use custom themes: all good, qBittorrent uses custom theme
  • User has Windows dark mode active and they want to use custom themes: all good, qBittorrent uses custom theme
  • User has Windows dark mode active and they don't want to use custom themes, they want qBittorrent to use Window's native dark theme: all good

The only "regression" is the following case, which I don't think is realistic:

  • User has Windows dark mode active, don't want to use custom themes, and don't want qBittorrent to use Window's native dark theme (instead, they want the default non-dark native look, which would be massively jarring).

But even in this case, there is the conf file escape hatch (these users can just set darkmode=0 in the conf file). Other applications don't even cover this case with a conf file. It is sanely assumed that if users turn on the systemwide dark theme, they don't want a single application to be massively different from anything else. So I don't think claiming the "UX of disabling dark theme support is bad" is a relevant criticism of this change.

Final note: since with darkmode=2 the theme switch happens without application restarts, this will work seamlessly for users who have dark mode periodically active on a schedule in Windows.

@jagannatharjun
Copy link
Member

personally, I don't want darkmode=2 in the repo, this will just add another "feature" that doesn't work correctly.

@FranciscoPombal
Copy link
Member Author

@jagannatharjun

personally, I don't want darkmode=2 in the repo, this will just add another "feature" that doesn't work correctly.

Disagree. Native dark theme support, even if unpolished, is better than none at all, especially when we have custom themeing support to make up for any deficiencies. As my previous comment states, now 4/4 cases are covered, without this change only 3/4. If the user really dislikes the native dark theme, they can still use custom themes, as they were forced to do previously anyway.

@FranciscoPombal
Copy link
Member Author

@glassez

@FranciscoPombal
Do you use Windows dark theme daily?

Yes, both in a machine where I have Windows installed on bare-metal, and on a VM that I have on one of my Linux systems, which is also running a dark theme (xfce + adwaita-dark). I have also been running AMOLED-dark themes on my phones as soon as that became an option.

@FranciscoPombal
Copy link
Member Author

FranciscoPombal commented Jul 4, 2021

@jagannatharjun

Francis don't even use windows.

Unfortunately, this is not true - I am a sinner because of both gaming (though Proton has been chunking away at that particular stronghold at a steady pace) and the occasional cross-platform stint (as is the case for qBittorrent).

@glassez
Copy link
Member

glassez commented Jul 4, 2021

@FranciscoPombal
Do you use Windows dark theme daily?

Francis don't even use windows.

@jagannatharjun, do you?

I use UI theme bundle.

Then it's like an argument between a mute and a deaf person, sorry...
For me personally, it deserves nothing more than to be mentioned in the Wiki. But I can only approve it in order to collect some feedback statistics from real users.

@FranciscoPombal
Copy link
Member Author

@glassez

For me personally, it deserves nothing more than to be mentioned in the Wiki. But I can only approve it in order to collect some feedback statistics from real users.

Merging this would go a long way to get users to provide feedback... If you are concerned about users complaining, the responsibility is not solely yours, you can tell them (and they can see) that it was originally my idea. But perhaps I'm more optimistic in thinking that users would be most likely to provide constructive feedback, rather than come at us with pitchforks in case they don't like it. Anyway, you know I'll do my best to be present and active in addressing feedback from the theme. This is Free Software made by the people, for the people, one either engages productively in the discussion or gets their post closed & locked and we all move on.

@FranciscoPombal
Copy link
Member Author

@glassez

Then it's like an argument between a mute and a deaf person, sorry...

Not sure what he meant by "UI theme bundle", but contrary to popular belief, I do sometimes find myself using Windows, and when I do so, I always use the native dark theme FWIW: #13952 (comment), #13952 (comment)

@glassez
Copy link
Member

glassez commented Jul 4, 2021

In any case, my approval and its reasons are set out above. But this is not enough, as you know. And I have no reason to put pressure on someone else to get this PR approved.

@jagannatharjun
Copy link
Member

Then it's like an argument between a mute and a deaf person, sorry...

My main OS is always been windows and I only started using linux recently because of work.

I can only approve it in order to collect some feedback statistics from real users.

This PR would've been fine if it were to be not make this the default behavior. My concern is providing an experimental feature as a default behavior with no clear way to change it. Plus it doesn't look good.

it deserves nothing more than to be mentioned in the Wiki.

👍

@FranciscoPombal

Not sure what he meant by "UI theme bundle"

I meant by qbittorrent's UI theme system.

@sakkamade
Copy link

Sorry, I don't see the point in this argument...

If someone wishes a dark theme, presently their only choice is to try the custom theme.
If someone uses dark theme in Windows, they will undoubtedly try the custom dark theme—for what is the point in having only qbit blindly white, right?

Whereas after this PR, if someone uses dark theme in Windows, they would be furnished with power to choose:

  1. Use ugly default dark theme
    • And if someone hates it from the bottom of the heart, it may even be disabled via config file.
  2. Try out a custom dark theme

Then why be against it at all?


Not sure what he meant by "UI theme bundle"

I meant by qbittorrent's UI theme system.

I guess it means custom theme...?

@sakkamade
Copy link

sakkamade commented Jul 4, 2021

Then why be against it at all?

Simultaneously, I can understand that there is no point in feature which no one intends to make use of.
EDIT: But I am in deep doubts that this should be like so, in this case.


EDIT2:

This is not an unrealistic scenario, I repeat no one in their right mind would ever use this

Well, in my opinion, no one in right mind will use dark theme at all, unless lighting in their chambers is broken 😃
But I see that people actually using it, and using a lot.
We should speak only for ourselves.

@sakkamade
Copy link

sakkamade commented Jul 4, 2021

Someone may say "You don't use dark theme, how you could speak of it?" however I need not be a dark-theme specialist to understand that to improve something afterwards is much easier than to create something flawless right from the start...

@sakkamade
Copy link

from #13952 (comment)

if you want this themeing you can run qbittorrent like qbittorrent -platform windows:darkmode=2

@jagannatharjun,
However easy it may seem to you, here is some example of workaround which people suggests #11917 (comment).

@xavier2k6
Copy link
Member

If this was to be merged, I presume it would only be in master for 4.4.x beta builds & not backported to 4.3.x branchfor upcoming release?

@FranciscoPombal
Copy link
Member Author

@xavier2k6

If this was to be merged, I presume it would only be in master for 4.4.x beta builds & not backported to 4.3.x branchfor upcoming release?

No reason why it couldn't be backported.

@sakkamade
Copy link

sakkamade commented Jul 7, 2021

It seems my commentaries left no as deep impression here as I wanted them to, so I shall make myself more plain and elaborate on this point.

Whilst Windows user has the custom themes as the only native option of dark theming, they will not even dare to dwell upon the thought that there might be another direction, and even more native, too.
And whereas no common knowledge therein, sparsely there would ever be any progress to it, as the contributors would also be gravitated to improve but custom themes, for this have become a default route by now.

This was referenced Aug 1, 2021
@github-actions
Copy link

github-actions bot commented Sep 6, 2021

This PR is stale because it has been 60 days with no activity. This PR will be automatically closed within 7 days if there is no further activity.

@github-actions github-actions bot added the Stale label Sep 6, 2021
@xavier2k6
Copy link
Member

If we were to make this work to some degree.....we would have to fix icon colors etc. that would work for both normal & dark mode.

See a WIP example:

my icons light
my icons dark

@ArcticGems
Copy link

If we were to make this work to some degree.....we would have to fix icon colors etc. that would work for both normal & dark mode.

See a WIP example:

Having 2 default themes would be great (+ a toggle option) and it would make it easier for @nowshed-imran since he could make 2 sets, one for light/normal and one for dark.

@xavier2k6
Copy link
Member

Having 2 default themes would be great (+ a toggle option) and it would make it easier for @nowshed-imran since he could make 2 sets, one for light/normal and one for dark.

There's no need for two sets of icons.......if we get the coloring right with icons & text

The toggle option you refer to is what this PR does.......activates windows dark mode for qBittorrent.

@ArcticGems
Copy link

There's no need for two sets of icons.......if we get the coloring right with icons & text

Wasn't that one of the reasons why people couldn't agree, because it's hard to make it work for both light & dark themes?

@xavier2k6
Copy link
Member

Wasn't that one of the reasons why people couldn't agree, because it's hard to make it work for both light & dark themes?

Look at the screen shots from my previous post........the icon colors are the same.

I use a different source to check out colors which give an example of what it looks like with white background/black background.

Example:
Forest Green

@github-actions github-actions bot removed the Stale label Sep 12, 2021
@github-actions
Copy link

This PR is stale because it has been 60 days with no activity. This PR will be automatically closed within 7 days if there is no further activity.

@github-actions github-actions bot added the Stale label Nov 11, 2021
@github-actions
Copy link

This PR was closed because it has been stalled for some time with no activity.

@github-actions github-actions bot closed this Nov 19, 2021
@debiedowner
Copy link

Wow... For the last two years, my tedious routine has been to enable a custom dark theme every evening and restart qBittorrent, and disable it in the morning and then again restart. Except sometimes in the evening I didn't want to stop a download by restarting, so I had to suffer the blinding white UI until I could restart. I never knew that all that effort and discomfort was unnecessary, that qBittorrent can actually change automatically according to Windows theme, without even needing to restart... And some qBittorrent contributors were blocking this feature, because they personally disliked the aesthetics of this dark theme... Incredible.

Anyway, I was happy to see that this change doesn't require recompiling and I can simply edit the qt.conf file on my system. Which I did immediately, and it looks and works great. The only problem is that when transitioning from dark to light theme, some text in qBittorrent options dialog remain white and difficult to read, which requires a restart to correct. But it's no big deal as I don't look at settings that often, and even if I did actually need to restart to see them, it is still a very significant improvement over the existing situation where I need to restart every time for theme change to take effect. Besides, restarting in the morning is less of a problem for me; it was having to restart in the evening (i.e. transitioning from light to dark theme) that was annoying, since I am more likely to be in the middle of something.

Also, interestingly, this dark theme is slightly more compact than qBittorrent's default theme, but only if qBittorrent was started while Windows is already set to dark mode, not transitioned while it is open. I figured this is due to the changes in version 4.2.2, so I tried 4.2.1's compact theme to see if that's the case, but 4.2.1 theme is even more compact; dark theme is somewhere in between. Anyway, it's a minor difference, I only noticed it because I took screenshots to compare it with the default theme and the custom dark theme I used to use until today.

Also, I am using Qt 5.15.2; I didn't test with Qt 6 version of qBittorrent 4.4.0, maybe that makes improvements to the aesthetics aspect that might satisfy qBittorrent contributors. I didn't check as I think this version, as seen in the screenshots above, is already perfectly fine, much better than all the unofficial themes listed in the wiki (except for the "legacy-qBittorrent" and "qbittorrent-black-theme"), in addition to the ease-of-use advantages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Look and feel Affect UI "Look and feel" only without changing the logic OS: Windows Issues specific to Windows Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants