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

kde: apply Qt theme on non-KDE systems and add Qt6 support #367

Merged
merged 26 commits into from
Jul 19, 2024

Conversation

Jackaed
Copy link
Contributor

@Jackaed Jackaed commented May 15, 2024

Fixes plasma theme not applying on non-KDE desktop environments and on window managers. See #142.

@danth
Copy link
Owner

danth commented May 15, 2024

Here's Dolphin, fully themed under GNOME:

Screenshot of Dolphin

The missing file previews are because I didn't install the thumbnail service, nothing to do with this PR.

I didn't have much luck getting this to work with non-KDE Qt apps. Is it meant to theme those too?

@Jackaed
Copy link
Contributor Author

Jackaed commented May 15, 2024

It worked with PCManFM-Qt when I tried it, so it works with some. What apps was it not working for?

@danth
Copy link
Owner

danth commented May 15, 2024

You're right, it works straight away with PCManFM. I just tried some XFCE apps and they work, too.

I was trying it with KeepassXC before, which does include its own theming menu, so it may just be that KeepassXC is doing its own thing even when I set it to use the system theme.

@Jackaed
Copy link
Contributor Author

Jackaed commented May 15, 2024

If you set keepassxc to use the "Classic (platform native)" theme it should follow your system theme

@danth
Copy link
Owner

danth commented May 15, 2024

I tried that, but it kept a default white theme for some reason.

@danth
Copy link
Owner

danth commented May 15, 2024

Side note: This PR adds around 250 MiB downloaded and 800 MiB unpacked on a GNOME system, due to the dependency on plasma-workspace. Although it's not small, I think that's a reasonable price to pay for consistent theming across a number of different apps.

@danth danth changed the title Fix plasma theme not applying on non-KDE systems kde: apply Qt theme on non-KDE systems May 15, 2024
@Jackaed
Copy link
Contributor Author

Jackaed commented May 15, 2024

I tried that, but it kept a default white theme for some reason.

Interesting - works fine on my machine. I think there'll always be some strange edge cases with how weird Qt theming is in general - probably best to deal with these cases as they come up.

@Jackaed
Copy link
Contributor Author

Jackaed commented May 15, 2024

Side note: This PR adds around 250 MiB downloaded and 800 MiB unpacked on a GNOME system, due to the dependency on plasma-workspace. Although it's not small, I think that's a reasonable price to pay for consistent theming across a number of different apps.

We could try and remove the plasma-workspace dependency if it becomes a problem that people complain about by trying to write to kdeglobals directly, but I doubt it'll be an issue.

@Jackaed
Copy link
Contributor Author

Jackaed commented May 16, 2024

In it's current iteration, this PR doesn't seem to work if home-manager.useUserPackages = true is set. Not sure why, will look into it when I have time.

@Jackaed
Copy link
Contributor Author

Jackaed commented May 16, 2024

I've edited the PR to replace the dependency on plasma-workspace with just directly writing to kdeglobals instead.

Copy link
Owner

@danth danth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using kdeglobals rather than a look and feel package makes the user experience worse when KDE is used, since the settings menu no longer shows that it's using a Stylix theme.

Perhaps we can use environment variables to force plasma-apply-lookandfeel to find the package in all situations?

modules/kde/hm.nix Outdated Show resolved Hide resolved
@niksingh710
Copy link

plasma-workspace taking 800 Mib.
It should not be an issue but using kvantum (kvconfig) and qt5ct/qt6ct themes templates to generate and set them.

My Templates does contain a lot of pywal based templates but they are not in moustache format (they are in somthing similar).
Rn m in mid of transition Arch->Nix porting them is not possible rn for me.

modules/kde/hm.nix Outdated Show resolved Hide resolved
@dzervas
Copy link

dzervas commented Jul 12, 2024

Dolphin (and I imagine other programs too) doesn't get correctly themed:
image

@Jackaed
Copy link
Contributor Author

Jackaed commented Jul 12, 2024

Dolphin (and I imagine other programs too) doesn't get correctly themed:
Dolphin is being correctly themed on my system with my branch. Can you show me some more of your config?

@dzervas
Copy link

dzervas commented Jul 12, 2024

@Jackaed https://github.com/dzervas/dotfiles/tree/f2b784c3ef018b7693861140d8250bbbef3fdeb9
(ignore the flake lock that points to stylix, I did nix flake update --override-input stylix github:Jackaed/stylix locally)

only the nix folder is of interest - anything else isn't used for nixos
the effective machine is laptop and the files of most interest should be system/display.nix, system/theme.nix and home/desktop/sway.nix

@Jackaed
Copy link
Contributor Author

Jackaed commented Jul 13, 2024

The issue is that this did not previously have qt6 support, which it now does. You were using dolphin qt6 because you were using kdePackages.dolphin instead of just dolphin.

@Jackaed Jackaed changed the title kde: apply Qt theme on non-KDE systems kde: apply Qt theme on non-KDE systems and add Qt6 support Jul 13, 2024
@Jackaed
Copy link
Contributor Author

Jackaed commented Jul 13, 2024

Okay, after a lot of meddling, this now works with Qt6 everything (at least on my system, please test independently).

@Jackaed
Copy link
Contributor Author

Jackaed commented Jul 13, 2024

Here is dolphin qt5 and qt6 both being themed correctly under my branch, side by side. Qt6 on the left, Qt5 on the right.
image
Same goes for okular:
image

@dzervas
Copy link

dzervas commented Jul 13, 2024

OMG YES!
image

EDIT: wow even the icons are magically fixed. did you do something about it?

@Jackaed
Copy link
Contributor Author

Jackaed commented Jul 14, 2024

EDIT: wow even the icons are magically fixed. did you do something about it?

Yeah icons were a pain in the arse, installing kdePackages.full fixed them (note this doesn't install all of kde, just all of the qt6 dependency and extension libraries. Not sure what individual package fixed them, but glad it's working. Just need to try and get this merged now.

Copy link
Owner

@danth danth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works on my system too, as well as in the testbed VM - where KDE theming no longer requires any manual steps to apply.

Although this isn't a perfect solution:

  • It makes kdeglobals read-only, which I believe may cause issues with a small number of programs
  • It installs several potentially large packages

It's a huge improvement over the current situation, and I'm happy to merge this now.

@danth danth merged commit 6bbae4f into danth:master Jul 19, 2024
10 checks passed
trueNAHO added a commit to trueNAHO/stylix that referenced this pull request Jul 25, 2024
Revert commit 6bbae4f ("kde: apply Qt theme on non-KDE systems and
add Qt6 support (danth#367)") due to unexpected issues.

Follow-up work is tracked in danth#480.

Closes: danth#480
Closes: danth#485
Closes: danth#487
trueNAHO added a commit to trueNAHO/stylix that referenced this pull request Jul 25, 2024
Revert commit 6bbae4f ("kde: apply Qt theme on non-KDE systems and
add Qt6 support (danth#367)") due to unexpected issues.

Follow-up work is tracked in danth#489.

Closes: danth#480
Closes: danth#485
Closes: danth#487
danth pushed a commit that referenced this pull request Jul 26, 2024
Revert commit 6bbae4f ("kde: apply Qt theme on non-KDE systems and
add Qt6 support (#367)") due to unexpected issues.

Follow-up work is tracked in #489.

Closes: #480
Closes: #485
Closes: #487
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants