danth / stylix

System-wide colorscheming and typography for NixOS
https://stylix.danth.me/
MIT License
1.22k stars 159 forks source link

treewide: optionalize wallpaper functionality #442

Open trueNAHO opened 5 months ago

trueNAHO commented 5 months ago

About

This is a tracking issue for optionalizing wallpaper functionality, when supported.

Steps

Unresolved Questions

danth commented 5 months ago

So, an option like enableWallpaper under each target?

trueNAHO commented 5 months ago

So, an option like enableWallpaper under each target?

Yes, I had the same idea. However, I would simplify the option name to wallpaper.

For consistency, we could implement and use a helper function, like mkEnableWallpaper, with an appropriate description, similar to mkEnableTarget:

options.stylix.targets.hyprland = let
  target = "Hyprland";
in {
  enable = lib.stylix.mkEnableTarget target true;
  wallpaper = lib.stylix.mkEnableWallpaper target false;
}
NovaViper commented 5 months ago

Hey @trueNAHO would this be an option that can be set globally for stylix? I actually want to disable stylix's wallpaper functionality in favor of plasma-manager's wallpaper module as that module lets me set multiple wallpapers for KDE Plasma.

compactcode commented 5 months ago

This is my current thinking which depends on https://github.com/danth/stylix/issues/200

trueNAHO commented 5 months ago

would this be an option that can be set globally for stylix?

The options.stylix.targets.<TARGET>.wallpaper options would be <TARGET> specific, similar to options.stylix.targets.<TARGET>.enable.

Based on the following use cases, it makes sense to add a stylix.enableWallpaper option complementary to stylix.autoEnable:

For reference, it has been considered to better rename the stylix.autoEnable option. Additionally, these use cases somewhat correspond to the slideshow feature suggestion.

I actually want to disable stylix's wallpaper functionality in favor of plasma-manager's wallpaper module as that module lets me set multiple wallpapers for KDE Plasma.

My previously mentioned use cases and an additional options.stylix.targets.kde.wallpaper would resolve this scenario. For reference, I added the kde module to the Steps required to resolve this tracking issue.

This is my current thinking which depends on #200

  • If sylix.image is set (currently mandatory) then we configure all enabled wallpaper services.
    • It shouldn't be the responsibility of sylix to decide (and maintain code for) which wallpaper service should be used.
  • If sylix.image is not set then we do not configure any wallpaper services.
    • If the user doesn't want wallpaper then this is the way.
    • If the user wants to do something custom (e.g. a folder with rotating images) they now have unobstructed control to configure their chosen service.

This corresponds to my previously mentioned use cases, except that stylix.enableWallpaper provides a default implementation for setting wallpapers:

  • It shouldn't be the responsibility of sylix to decide (and maintain code for) which wallpaper service should be used.

Regarding the implementation of stylix.enableWallpaper, we could add additional guards, similar to lib.stylix.mkEnableTarget:

https://github.com/danth/stylix/blob/97dcf3c216fe5fb19c406e39f265d3bc9b851377/stylix/target.nix#L42

Considering that the scope of wallpaper optionality is larger than initially anticipated, it might be better to extend the scope of this tracking issue. Consequently, https://github.com/danth/stylix/issues/200 is included and the following statement is removed from this issue's top-level description:

Unlike https://github.com/danth/stylix/issues/200, this does not optionalize the stylix.image declaration. Instead, it should be possible to disable wallpapers being set, when supported.