stylix icon indicating copy to clipboard operation
stylix copied to clipboard

treewide: optionalize wallpaper functionality

Open trueNAHO opened this issue 1 year ago • 5 comments

About

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

Steps

  • [ ] https://github.com/danth/stylix/issues/200
  • [ ] treewide: add 'options.stylix.targets.<TARGET>.wallpaper' options
    • hyprland
    • kde
    • [ ] Where else are wallpapers enforced?

Unresolved Questions

  • [ ] How to optionalize the wallpaper functionality?

trueNAHO avatar Jun 19 '24 09:06 trueNAHO

So, an option like enableWallpaper under each target?

danth avatar Jun 19 '24 11:06 danth

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;
}

trueNAHO avatar Jun 19 '24 12:06 trueNAHO

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.

NovaViper avatar Jun 19 '24 15:06 NovaViper

This is my current thinking which depends on https://github.com/danth/stylix/issues/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.

compactcode avatar Jun 20 '24 02:06 compactcode

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:

  • Get color scheme from stylix.image without setting stylix.image as wallpaper: stylix = { enableWallpaper = false; image = image; }.

  • Get color scheme from stylix.image and set stylix.image as wallpaper: stylix = { enableWallpaper = true; image = image; }.

  • Manually declare color scheme (stylix.base16Scheme) and entirely ignore stylix.image: stylix = { base16Scheme = base16Scheme; enableWallpaper = false; image = null; }.

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.

trueNAHO avatar Jun 23 '24 14:06 trueNAHO