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

stylix: adding option for other wallpapers and improvements (DRAFT) #508

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

no-mood
Copy link

@no-mood no-mood commented Aug 16, 2024

I have adapted the function to generate a wallpaper with the Nix(OS) logo, taken from misterio77/nix-colors.

The code still needs some fixes, but the general idea should be there.

I thought of adding an additional file (nix-wallpaper.nix), which will be imported instead of the previous pixel.nix. This file will contain all the function calls that generate wallpapers based on a scheme, such as pixel-wallpaper and flake-wallpaper.

As I said, it still needs some fixes, and I haven’t changed the content of pixel-wallpaper.nix because it's not clear to me how to adapt it; in my humble opinion, we could follow the structure of flake-wallpaper.

@trueNAHO trueNAHO marked this pull request as draft August 19, 2024 18:13
@trueNAHO
Copy link
Collaborator

trueNAHO commented Aug 19, 2024

I have adapted the function to generate a wallpaper with the Nix(OS) logo, taken from misterio77/nix-colors.

Based on the patch, I am unsure what the purpose or functionality of this PR is. Could you clarify how it is trying to solve what problem?

Is it trying to resolve #242 or complement #477?

@no-mood
Copy link
Author

no-mood commented Aug 20, 2024

Actually, it's none of them if I understood correctly; it's more like a different wallpaper generator compared to the 'pixel' one. My idea is to extend the range of possible wallpapers generated from a color scheme.
Here is the wallpaper based on the color scheme I'm using.
EDIT: I'm gonna add a working commit soon
2024-08-20-114438_hyprshot

@trueNAHO
Copy link
Collaborator

Actually, it's none of them if I understood correctly; it's more like a different wallpaper generator compared to the 'pixel' one. My idea is to extend the range of possible wallpapers generated from a color scheme.

Actually, I am unfamiliar with the exact extent of the pixel function. @danth might be more helpful here.

@danth
Copy link
Owner

danth commented Aug 21, 2024

The pixel function was not originally intended to generate a wallpaper, rather, it's a workaround for when we want a solid color but the application only accepts images.

It is currently only used in a single place within Stylix, in the GRUB module. (We could consider deprecating the pixel function and inlining it directly into the GRUB module instead?)

@trueNAHO
Copy link
Collaborator

trueNAHO commented Aug 21, 2024

The pixel function was not originally intended to generate a wallpaper, rather, it's a workaround for when we want a solid color but the application only accepts images.

We should consider better renaming the pixel function and adding a comment clarifying its purpose.

What should we do about this PR then?

It is currently only used in a single place within Stylix, in the GRUB module. (We could consider deprecating the pixel function and inlining it directly into the GRUB module instead?)

It might be better to keep the pixel function in the internal lib to prevent future code duplication: #327. Considering its current implementation is fairly isolated, it does not introduce any major code complexity.

@no-mood
Copy link
Author

no-mood commented Aug 27, 2024

The pixel function was not originally intended to generate a wallpaper, rather, it's a workaround for when we want a solid color but the application only accepts image

Still, I think it would be a nice addition :)
If you're interested I can add a commit asap

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.

3 participants