-
-
Notifications
You must be signed in to change notification settings - Fork 166
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
treewide: swatch proposal #270
base: master
Are you sure you want to change the base?
Conversation
First working example push
Added blend factor
Added swatch loading to hm and darwin modules
Removed swatch options from swatches Added base16scheme option to swatches Added override option to swatches
Major rewrite no.1 has been done to bring this closer in line with how stylix's config and option workflow, and better integrates base16 support into the swatch system. User facing changesThis means that this: stylix.swatches.terminal = {
base16Scheme = "${pkgs.base16-schemes}/share/themes/dracula.yaml";
}; Is all that would be needed by end users to apply a specific scheme to all items tagged with 'terminal' on their swatch imports. This also now uses the override pattern meaning that: stylix.swatches.terminal = {
override = {
base00 = "000000";
tabActiveHover = ({ colors, mkSwatch, ... }: with colors; mkSwatch base00 base05 base01);
tabInactiveHover = import ./userSwatchFile.nix;
};
}; Users can use the swatches' overrides to modify both the base16 scheme as well as individual swatch generators giving them granular control when desired. Dev facing changes:This rewrite now allows for minimal effort migration by keeping hold of the scheme and colors. colors = (config.lib.stylix.getSwatches [ "alacritty" "terminal" ]).colors; This is the only change the majority of current modules need applied to support the swatch system's color schemes, which addresses #208. This snippet will check if the user has created swatches under "alacritty" or "terminal" and then falls back to the default colors from stylix. This means migration to using the swatches is only necessary for the better readability and more granular controls. |
#102 might be worth looking over - there may be some overlap with what you're doing here.
I prefer verbose naming since it's a lot more understandable to people who aren't familiar with the codebase.
These would be nice to have. Perhaps they could be an alias on top of the
I think it could be useful to group variations of the same swatch in some way. This would include standard, hover and active variations of a button, for example.
Both of these would be useful if implemented, but I would suggest leaving them for a followup PR. |
Thanks for the pointer, there certainly is some overlap in approach. Do we have any status on those changes? I'm happy to rebase and work on a solution based off of #102 since these changes are both smaller and will definitely conflict. Looks like similar concerns on how caching/memoizing is going to play into performance. Also looks like this has the same issue of being a lib.stylix function that needs a config ref. I have a similar issue in my personal flake where I shade my shared functions directly into lib and ended up feeding config as a param into the offending functions which is more of an eyesore. After addressing the other points I'll have another go at how the data is stored/passed around and see if I can get rid of the config req in the lib functions. I've been meaning to anyways since I'm not content with how the current impl will silently ignore invalid inputs for swatches and overrides. I will say those #102 black box types are looking quite nice for that.
Doneskis
Sounds good to me, I can just shade the aliases + base colors as wrapped colors into the top level of the getSwatches attrs. I'll also set them up to feed into the swatch generation function so that the swatch gens also benefit from the aliases. That said adding more to the top level of swatches overloads the implied contract of 'swatch' which would make it unintuitive for newcomers. I'd lean towards swatches being their own nested group like colors scheme, and change the getSwatches command to indicate it's different. {
scheme = base16Scheme;
swatches.button.active.hovered = wrappedSwatch;
base00 = wrappedColor;
red = wrappedColor;
}; 'palette' has been out of rotation for a while, but could also use something like 'colorway' or just 'colors', but I think the latter would also lead to the overloaded terminology issue.
So nest them in attrs? eg
or
depending on what you think works best
Good call, #248 is definitely a separate treewide issue, I can't see anything that would be blocking a separate pr since it's essentially adding options to be implemented during refactor. |
Nested attribute sets would work well and allow us to write functions such as this: cssForButton = class: swatch: ''
.${class} {
background-color: ${swatch.background};
color: ${swatch.foreground};
}
.${class}:hover {
background-color: ${swatch.hovered.background};
color: ${swatch.hovered.foreground};
}
'' We should consider whether to provide I'm not sure on the file naming. One one hand, I don't see any reason we would need to include additional files alongside a swatch, so having a separate directory for each one may be unnecessary. However, I see it being easier to traverse and import the file structure with a recursive function if we're only looking for files specifically named |
I'd definitely avoid permuting the states, some element types can balloon eg We'd probably want something like [ Class > Subtype > App/Container State > Element State > Ui State ] as a base line.
Another thing to consider is when people want to check which swatches are available. It'll probably be easier in a flat directory. |
Fixed missed renames from previous commit
Assuming we implement potentially all of VIM's highlight groups
|
I believe
Considering the vast amount of highlight groups we may have in the future, it might be better to use nested directories. IMHO, project navigation is mind-blowingly simple and fast with modern tools. |
TLDR: Color swatch (highlight group) replacement for base16 calls
This is a proposal to supersede the current base## model with a swatch model inspired by vim highlight group as I mentioned in #249. I'm not attached to any changes here and this is 100% geared towards improving both end user ergonomics and general devex so any and all feed back is welcome. This also is currently built so that no breaking changes happen, which means each module can be migrated in separate PRs, and end users will not need to change their configurations.
Addresses Issues
Todos for this PR
Discussion topics
Drawbacks/Concerns
Swatches
Swatches are a selection of 3 color components that correspond to the foreground, background, and outline colors of specific UI elements. This means that there will be separate swatches for different elements and their states, such as ButtonActivehovered, ButtonInactiveUnhovered, etc. By breaking these into their own components, it makes it both easier for developers to follow the style guide, and for end users to override particular elements.
This currently works off a set of .nix files in the new swatches directory. Each file returns a generator that creates a swatch from the base16 color space. These modules also generate options submodules that can be overriden by end users. This individual file approach was selected to reduce merge clashes on changes to and new implementations of swatches.
This swatch file example is currently fed a set of colors corresponding to the base16 colors converted to u8 rgb ints, pkgs.lib, and a mkSwatch function for convenience. They return a set consisting of fg bg and ol attrs. Each of these components have r g b u8 ints.
Currently the swatch names are derived from the file names, where the files are in kebab case and the names used in stylix are camel case. This is due to the way their loading is structured, their names are used to construct the stylix's options, which needs to happen before they can be imported.
Usage
Redone, see below comment for details
NOTE: the pr's example colors and assignments are made up. I did not rtfm for them.