search

danth / stylix

System-wide colorscheming and typography for NixOS
https://stylix.danth.me/
MIT License
912 stars 107 forks source link

treewide: swatch proposal #270

Open RANKSHANK opened 4 months ago

RANKSHANK commented 4 months ago

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

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.

#swatches/tab-active-hovered.nix
{ colors, mkSwatch, ... }: with colors; mkSwatch base00 base05 base03

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.

RANKSHANK commented 4 months ago

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 changes

This 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.

danth commented 4 months ago

https://github.com/danth/stylix/pull/102 might be worth looking over - there may be some overlap with what you're doing here.

verbose color handles vs shorthand (eg 'outline' vs 'ol' and 'red' vs 'r')

I prefer verbose naming since it's a lot more understandable to people who aren't familiar with the codebase.

specialized 'pure' color swatches (eg red blue etc)

These would be nice to have. Perhaps they could be an alias on top of the baseXX colors, rather than a full swatch, since a swatch of all the same color could result in a broken appearance if it gets used improperly.

swatch naming conventions

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.

use this pr's sweeping changes to address https://github.com/danth/stylix/issues/248, the option's base implementation is trivial, and we could just require they get implemented in the migration PRs

potential https://github.com/danth/stylix/issues/210 support, could list which swatches a given module uses

Both of these would be useful if implemented, but I would suggest leaving them for a followup PR.

RANKSHANK commented 4 months ago

102 might be worth looking over - there may be some overlap with what you're doing here.

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.

I prefer verbose naming since it's a lot more understandable to people who aren't familiar with the codebase.

Doneskis

These would be nice to have. Perhaps they could be an alias on top of the baseXX colors, rather than a full swatch, since a swatch of all the same color could result in a broken appearance if it gets used improperly.

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.

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.

So nest them in attrs? eg button.active.hovered instead of buttonActiveHovered? That'll be easier than the current kebab to camel method. Would we also want to nest the swatches dir to match? Currently we've got a flat dir so: ./swatches/button-active-hovered.nix would change into either:

  1. ./swatches/button/active/hovered.nix

or

  1. ./swatches/button/active/hovered/default.nix

depending on what you think works best

Both of these would be useful if implemented, but I would suggest leaving them for a followup PR.

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.

danth commented 4 months ago

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 .hovered.active or .active.hovered, or both, to make this kind of usage as convenient as possible.

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 default.nix, since that avoids the need to parse file names and remove the .nix extension.

RANKSHANK commented 4 months ago

We should consider whether to provide .hovered.active or .active.hovered, or both, to make this kind of usage as convenient as possible.

I'd definitely avoid permuting the states, some element types can balloon eg button.radio.active.checked.hovered. The file based source of truth means we have to enforce conventions on the dev side of things, and with users able to override specific swatches (say they want override button swatches to have pink outlines) they'd need to be aware of the base spec anyways.

We'd probably want something like [ Class > Subtype > App/Container State > Element State > Ui State ] as a base line.

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 default.nix, since that avoids the need to parse file names and remove the .nix extension.

Another thing to consider is when people want to check which swatches are available. It'll probably be easier in a flat directory.

trueNAHO commented 3 months ago

Todos for this PR

  • documentation (lucky last)

Assuming we implement potentially all of VIM's highlight groups

our documentation could simply link to VIM's highlight groups documentation.

(Source: https://github.com/danth/stylix/issues/249#issuecomment-2012933635)

trueNAHO commented 3 months ago

Currently we've got a flat dir so: ./swatches/button-active-hovered.nix would change into either:

  1. ./swatches/button/active/hovered.nix

or

  1. ./swatches/button/active/hovered/default.nix

I believe ./swatches/button/active/hovered/default.nix to be more idiomatic than ./swatches/button/active/hovered.nix because it better integrates with Nix' import function and Nix flakes' imports key.

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 default.nix, since that avoids the need to parse file names and remove the .nix extension.

Another thing to consider is when people want to check which swatches are available. It'll probably be easier in a flat directory.

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.