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

gtk: Allow user defined overrides #322

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

Conversation

SpeDAllen
Copy link

  • allows users to assign colors from theme
  • simplifies the gtk mustache

Allows users to assign colors from theme
Simplifies the gtk mustache
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.

This is already possible by writing ${config.lib.stylix.colors.baseXX} within the definition of extraCss, and that can be simplified using with config.lib.stylix.colors; to avoid repetition.

Does having this as a variable within the CSS provide any additional benefits?

@SpeDAllen
Copy link
Author

SpeDAllen commented Apr 3, 2024

I had not considered using ${config.lib.stylix.colors.baseXX} in the extra css. I did try #{{baseXX-hex}} and #${baseXX} and several other patterns that did not work. I had been using the full hashed value such as #959dcb and wanted a way for the value to change with a theme change. As far as does having the variable in the css provide any benefit. I believe it's more readable. A pattern of lines ending in @baseXX; with baseXX defined in the first few lines is easier to read and understand then a pattern of lines ending in #{{baseXX-hex}}; I guess reasonable people could disagree on such a thing though. I took my inspiration from the resulting css provided by the waybar module. Perhaps I could claim the benefit is consistency.

@danth
Copy link
Owner

danth commented Apr 17, 2024

I'm not sure which way to go with this.

On one hand, having variables defined in the CSS makes the output file more readable.

On the other, we could argue that you should be reading the source code and not the generated file. Removing a level of indirection also makes the file smaller and possibly more efficient to load, depending on the details of the parser.

When we make a decision we should update either this or the Waybar module to follow a standard format.

@trueNAHO
Copy link
Collaborator

Related: #230

@SpeDAllen
Copy link
Author

I guess this opened a bigger can of worms then I expected, my apologies. As someone with much much less experience in this space then you it appears to me that having the variables defined in the CSS would make using stylix more approachable for users less experienced then me. Regardless of what you decide I'd like to assist in standardizing the solution across the modules.

@danth
Copy link
Owner

danth commented Apr 20, 2024

No worries, this is something we should try to improve.

Having different methods may actually be more confusing for inexperienced users - they could read @baseXX in a string and assume this works everywhere, then have issues when they try it with a string which doesn't contain CSS. The ${baseXX} method works in all cases (with the appropriate with config.lib.stylix.colors; statement at the top of the file).

@SpeDAllen
Copy link
Author

I think I'm missing something. When you say the ${baseXX} method are you referring to using no mustache file similar to the zathura module?

@danth
Copy link
Owner

danth commented Apr 22, 2024

Yes, I'm referring to strings written directly within a Nix file, not a separate template.

The current state (ignoring the Waybar module) is that:

  • ${baseXX} is used in Nix files
  • {{baseXX}} is used in Mustache files

Which has a clear separation of syntax by the different file types.

If we add CSS variables too, it becomes:

  • ${baseXX} can be used in Nix files
  • {{baseXX}} can used in Mustache files
  • @baseXX can be used in either, but only if it's CSS

@SpeDAllen
Copy link
Author

Ok, yes, it's taking advantage of a feature of css given the fact that not every product uses css.

If consistency across all the modules is desired and simple user defined overrides are also desired perhaps a third alternative is worth considering. The alternative that springs to mind is converting all color assignments to nix attribute sets. I realize this option would mean a major rewrite to stylix but it would achieve both goals. Unless you expect adoption of stylix outside the context of nixos users should already be familiar with using nix attribute sets and it would align stylix with most home manager modules.

@trueNAHO
Copy link
Collaborator

If consistency across all the modules is desired and simple user defined overrides are also desired perhaps a third alternative is worth considering. The alternative that springs to mind is converting all color assignments to nix attribute sets. I realize this option would mean a major rewrite to stylix but it would achieve both goals. Unless you expect adoption of stylix outside the context of nixos users should already be familiar with using nix attribute sets and it would align stylix with most home manager modules.

This would also most likely improve evaluation time: #159 (comment).

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