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

treewide: swatch proposal #270

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

Conversation

RANKSHANK
Copy link
Contributor

@RANKSHANK RANKSHANK commented Mar 2, 2024

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

  • investigate home-manager/darwin impl
  • user facing option to adjust the % factor used in blending
  • factor based color blending
  • add a lib function to load colors directly from base16 packages
  • swatch implementations
  • more color conversion functions
  • user facing base16scheme -> swatch conversion
  • additional end user facing functionality to help with manipulating swatches
  • feed blending functions into the swatch generators as well
  • expand overrides so that they are applied to 'lower' prio schemes when no base16scheme is supplied
  • documentation (lucky last)

Discussion topics

  1. use this pr's sweeping changes to address Don't configure colors #248, the option's base implementation is trivial, and we could just require they get implemented in the migration PRs
  2. base24 implementation (addresses Feature Request: Base24 support #252)
  3. verbose color handles vs shorthand (eg 'outline' vs 'ol' and 'red' vs 'r')
  4. specialized 'pure' color swatches (eg red blue etc)
  5. additional swatch components (more than fg bg ol)
  6. should colors remain unconverted as hex strings as a base line
  7. potential doc: separate page for each target #210 support, could list which swatches a given module uses
  8. swatch naming conventions

Drawbacks/Concerns

  • Across the board it's going to be encroaching on java's level of verbosity vs the usual with colors; methodology
  • Pretty certain it'll be slightly heavier on processing time and memory footprint. I tried to write it so much of it will be nooped away by nix's laziness but the options system and wrapping has me concerned that wrapping calls won't benefit from the usual memoization between modules. Compared to compiling something that's not in the cache it'll be peanuts but It's worth mentioning/discussing.

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.

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
@RANKSHANK
Copy link
Contributor Author

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
Copy link
Owner

danth commented Mar 4, 2024

#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 #248, the option's base implementation is trivial, and we could just require they get implemented in the migration PRs

potential #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 RANKSHANK changed the title stylix: swatch proposal treewide: swatch proposal Mar 5, 2024
@RANKSHANK
Copy link
Contributor Author

#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
Copy link
Owner

danth commented Mar 5, 2024

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
Copy link
Contributor Author

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.

Fixed missed renames from previous commit
@trueNAHO
Copy link
Collaborator

trueNAHO commented Mar 21, 2024

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: #249 (comment))

@trueNAHO
Copy link
Collaborator

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.

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.

Different themes per application Create aliases for colors Standardize hover colors
3 participants