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

Use light or dark theme based on current OS colour scheme #6515

Open
StrangePeanut opened this issue Jul 20, 2024 · 17 comments
Open

Use light or dark theme based on current OS colour scheme #6515

StrangePeanut opened this issue Jul 20, 2024 · 17 comments
Assignees
Labels
client feature-request This issue or PR deals with a new feature good first issue Good for first-time contributors qt ui

Comments

@StrangePeanut
Copy link

Context

No response

Description

Introduce a new default 'Sync with OS' theme option that automatically uses the light or dark theme based on the current OS colour scheme. Users can override the behaviour by explicitly selecting a theme option.

This improves the overall user experience for those who regularly switch between the light and dark colour schemes. It also benefits new users who use the dark colour scheme around the clock as they no longer need to change the theme setting from default.

Mumble component

Client

OS-specific?

No

Additional information

No response

@StrangePeanut StrangePeanut added feature-request This issue or PR deals with a new feature triage This issue is waiting to be triaged by one of the project members labels Jul 20, 2024
@Hartmnt
Copy link
Member

Hartmnt commented Jul 21, 2024

Sounds like a good idea. Should be reasonably easy to implement once Mumble switches to Qt 6.5 https://stackoverflow.com/a/77751363

@Hartmnt Hartmnt added client ui qt and removed triage This issue is waiting to be triaged by one of the project members labels Jul 21, 2024
@Krzmbrzl Krzmbrzl added the good first issue Good for first-time contributors label Jul 25, 2024
@jakub2682-tuke
Copy link

Hello , I’m new to contributing to open-source projects, and I’m looking to get involved as part of a requirement for my studies. I have some experience with C++, but I'm new to Qt. Could I take a crack at this issue?

@Hartmnt
Copy link
Member

Hartmnt commented Oct 1, 2024

Could I take a crack at this issue?

Sure, have a go! If you have any questions feel free to ask here, in a newly created PR or in our Matrix chat room!

@Hartmnt
Copy link
Member

Hartmnt commented Oct 1, 2024

@Krzmbrzl Are we Qt 6.5 yet?

@davidebeatrici
Copy link
Member

Yes.

@Krzmbrzl
Copy link
Member

Krzmbrzl commented Oct 3, 2024

No we are not. We can only require Qt >= 6.2 in order to stay compatible with Ubuntu 22.04 which ships with that version (btw. 24.04 comes with 6.4)

However, I would totally be fine with making this feature available only if Mumble has been compiled with Qt >=6.5 which can be achieved by using the preprocessor like this:

#if QT_VERSION >= QT_VERSION_CHECK(6,5,0)
    // Your code here
#endif

@Krzmbrzl
Copy link
Member

Krzmbrzl commented Oct 3, 2024

As an aside: this issue seems rather closely related to #4420

@davidebeatrici
Copy link
Member

No we are not. We can only require Qt >= 6.2 in order to stay compatible with Ubuntu 22.04 which ships with that version (btw. 24.04 comes with 6.4)

Ah, sorry, I had only checked our vcpkg build environment...

However, I would totally be fine with making this feature available only if Mumble has been compiled with Qt >=6.5 which can be achieved by using the preprocessor like this:

#if QT_VERSION >= QT_VERSION_CHECK(6,5,0)
    // Your code here
#endif

Definitely.

As an aside: this issue seems rather closely related to #4420

Yes, in fact we should first address my comment:

Maybe we could add a flag to each theme that indicates whether it's dark/light/neutral and then decide which one to set depending on the system's.

@jakub2682-tuke
Copy link

jakub2682-tuke commented Oct 4, 2024

Although the official version for this issue will be for Qt 6.5 as discussed, I will also look into trying to implementing this feature for Qt 6.2.

As mentioned in #4420

Switching themes while Mumble is running is not possible either as that requires it to be restarted so I'm not sure if that'd make for a great experience...

Would you like me to explore a solution to avoid requiring a restart when switching themes, or is this behavior expected and not something that needs to be address?

@Hartmnt
Copy link
Member

Hartmnt commented Oct 4, 2024

Would you like me to explore a solution to avoid requiring a restart when switching themes, or is this behavior expected and not something that needs to be address?

I would say this depends on how complicated this gets. The primary goal would be for Mumble to choose the light or dark theme on startup (if the user configures automatic theme detection of course). That would be enough of a feature to be merged for sure.

Of course, this is not ideal for users whose theme switches with the time of day. If you manage to additionally also address this in a neat way - preferably in a separate commit - that would be a nice bonus.

@Krzmbrzl
Copy link
Member

Krzmbrzl commented Oct 4, 2024

My gut tells me that fixing the need for a restart of Mumble in order to change the theme is rather tricky

@Atikrg
Copy link

Atikrg commented Oct 6, 2024

I am new here. Could somebody tell me how can I start this project by cloning

@Krzmbrzl
Copy link
Member

Krzmbrzl commented Oct 8, 2024

@NSHABV
Copy link

NSHABV commented Oct 24, 2024

Greetings, I am looking into implementing this feature with the understanding that it would be enabled only if compiler detects Qt version over 6.5, as mentioned here: #6515 (comment).

One question though: should I default dark theme to Mumble's default dark theme and light theme to Mumble's light theme?

Besides this, should the option be implemented in the themes combobox or as a checkbox? Insertion of the default-by-OS theme can be implemented with the StyleInfo class, but I don't know whether that is correct at all.
I am new to the codebase, and the behavior of these fields confuses me a bit.
QFileInfo defaultQss; QMap< QString, QFileInfo > qssFiles;
Technically speaking, the code trying to pull the theme would be overriden (by me) and forced to use the chosen themes, but should I change QFileInfo to point to a different file (the default dark theme file etc) and swap qssFiles out for different ones each time a theme gets refreshed?

@jakub2682-tuke
Copy link

@NSHABV Hello , could I ask you to hold off on you working on this? I am currently working on this. Could you give me 1-2 weeks to try to finish this.

@Hartmnt
Copy link
Member

Hartmnt commented Oct 24, 2024

@NSHABV This issue was indeed already assigned to @jakub2682-tuke

@jakub2682-tuke No pressure :)

@jakub2682-tuke
Copy link

Little update on how it's going.

It took me a few days to build dependencies, as I never did so for such a project, and I struggled a little. I found out your vcpkg got a problem with boost ports as several components got wrong hashes.

On my Windows 11, it currently works for qt 6.2, and it is dynamic, so it doesn't have to restart to change the theme when auto would be enabled. I wrote code for the qt6.5 version, but I haven't tested it yet, as I don't know how to upgrade the qt to qt6.5, as when I build with qt6.5, it has problems on the last building step.

Also, currently it's always auto, as I haven't figured out how to properly add the option to choose "Auto" and then detect its on auto. But I think when I look at that part of the code properly, I should easily finish that.

jakub2682-tuke added a commit to jakub2682-tuke/mumble that referenced this issue Oct 28, 2024
This update introduces an automatic theme switch that changes based on the OS color scheme at startup and during runtime. This feature is implemented for Qt 6.2 and includes compatibility for Qt 6.5 once Mumble upgrades.

Implements mumble-voip#6515
jakub2682-tuke added a commit to jakub2682-tuke/mumble that referenced this issue Oct 29, 2024
This update introduces an automatic theme switch that changes based on the OS color scheme at startup and during runtime. This feature is implemented for Qt 6.2 and includes compatibility for Qt 6.5 once Mumble upgrades. -fix

Implements mumble-voip#6515
jakub2682-tuke added a commit to jakub2682-tuke/mumble that referenced this issue Oct 31, 2024
This update introduces an automatic theme switch that changes based on the OS color scheme at startup and during runtime. This feature is implemented for Qt 6.2 and includes compatibility for Qt 6.5 once Mumble upgrades.

Implements mumble-voip#6515
jakub2682-tuke added a commit to jakub2682-tuke/mumble that referenced this issue Nov 1, 2024
This update introduces an automatic theme switch that changes based on the OS color scheme at startup and during runtime. This feature is implemented for Qt 6.2 and includes compatibility for Qt 6.5 once Mumble upgrades.

Implements mumble-voip#6515
jakub2682-tuke added a commit to jakub2682-tuke/mumble that referenced this issue Nov 1, 2024
This update introduces an automatic theme switch that changes based on the OS color scheme at startup and during runtime. This feature is implemented for Qt 6.2 and includes compatibility for Qt 6.5 once Mumble upgrades.

Implements mumble-voip#6515
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client feature-request This issue or PR deals with a new feature good first issue Good for first-time contributors qt ui
Projects
None yet
Development

No branches or pull requests

7 participants