-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
hyprland: add direct support for animation configuration #7424
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine to me, and the changes are simple enough.
@@ -216,6 +216,43 @@ in | |||
''; | |||
}; | |||
|
|||
animations = lib.mkOption { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My first gut reaction is not liking introducing a random option when we have a freeform settings option and an extraConfig explicitly for things that don't get generated how you want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same, I had to think it over for a few hours before approving, but I'm not fully sure we want to implement this in HM. There's https://github.com/hyprland-community/hyprnix/ for all the bells and whistles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My first gut reaction is not liking introducing a random option when we have a freeform settings option and an extraConfig explicitly for things that don't get generated how you want.
I do agree this may not be suited for integration into HM, for the reasons you stated above.
Just to make my point clear, let me state why this option in particular has a purpose that cannot be reached by using the current settings
or extraConfig
Yesterday when working on my personal config (C0Florent/config@c33b8fd), I found the need to "repeat" an animation which was written in settings.animation
, which is a list along these lines:
animation = [
"windows, 1, 3.5, easeOutBack, slide"
"windowsOut, 1, 5, easeOutBack, slide"
];
In this case, the names windows
and windowsOut
are special identifiers recognised by Hyprland to configure particular animations.
Later in configuration (in a completely separate file), I need to reference a specific animation configuration (which in this case is a list element). Lists are not suited to find elements in this way; so in this particular case I wish it was an attribute set rather than a list, so that I could lookup by name instead of by string prefix.
My proposed change adds another option for defining animations, which does allow defining this as an attrset, in the following fashion:
animations = {
windows = "1, 3.5, easeOutBack, slide";
windowsOut = "1, 5, easeOutBack, slide";
};
This way, I can reference animations in other parts of my config by using config.wayland.windowManager.hyprland.animations.<name>
, which gives me back the string I wrote in config.
Sorry for the repetition of my point, I simply wanted to give you a clear overview of the pros of this change, given you've already understood the cons of it. Now that this point is clear I'm curious of your definitive opinion on the subject
Description
This PR adds a specific support for Hyprland animation configuration.
Note it is already possible to configure animations through
programs.services.window-managers.hyprland.settings
, but I recently found a use case which I described in my added option's example in which it would be useful to have a dedicated option.I'm not sure we want this change or not: it has the advantage of covering my niche use case and provides a more readable, less repetitive configuration option; but has the disadvantage of proposing several ways of achieving the same thing, potentially making it harder to understand for the module users. Whether or not this is considered valuable for HM, I still need something for my use case, so I would just use it as a custom module in my config if it does not land here.
Checklist
Change is backwards compatible.
Code formatted with
nix fmt
ornix-shell -p treefmt nixfmt-rfc-style deadnix keep-sorted --run treefmt
.Code tested through
nix-shell --pure tests -A run.all
or
nix build --reference-lock-file flake.lock ./tests#test-all
using Flakes.Test cases updated/added. See example.
Commit messages are formatted like
See CONTRIBUTING for more information and recent commit messages for examples.
If this PR adds a new module
Maintainer CC
@fufexan