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

Added alpha variants and overlay colors #5

Closed
wants to merge 1 commit into from

Conversation

thewebartisan7
Copy link

@thewebartisan7 thewebartisan7 commented Apr 9, 2023

As discussed here #4 this is the PR for add alpha variants and overlay colors (whiteA and blackA).

For add the alpha variants and overlay colors you need to explicitly add them in config, in this way someone can exclude them if they are not using.

Example config:

// Add 'blueA' for blue alpha and 'whiteA' for overlay white
presetRadix({
  palette: ['blue', 'blueA', 'green', 'amberA', 'whiteA'],
}),

Overlay colors: whiteA and blackA
Alpha variants: blueA, greenA, etc...

The dark variants is added automatically, so for example when you add 'blueA' also the 'blueDarkA' will be added.

I have also exclude dark colors if you set the option darkSelector to falsy value so that if someone are not using dark mode, the CSS for dark colors will not be generated.

I see there is not any tests and not sure how is your workflow for tests code, I did a build and move dist folder to a unocss project for testing. I think would be good to have some tests.

Let me know if you have any feedback about and I will adjust accordingly.

Thanks

@endigma
Copy link
Owner

endigma commented Apr 9, 2023

I've thought about tests but I'm not sure how to implement them, I don't see any documentation on how to clean-room run a unocss preset or inspect output. As far as I know none of the official presets have tests either.

This looks good on GitHub, I'll test properly later and merge if it works how I expect.

@thewebartisan7
Copy link
Author

thewebartisan7 commented Apr 9, 2023

Yes I see there is not anything in docs, but I found here tests https://github.com/unocss/unocss/tree/main/test

And it seem there is for default preset here: https://github.com/unocss/unocss/blob/main/test/preset-uno.test.ts using vitest.

We could make similar.

I can check into if you are interested, so it will help during development to quickly test new features.

I am interested to understand if we can add new color on demand instead of adding in config, so for example using bg-blue10, will add just blue10 var and nothing else. But not sure if this is possible, since I see you are pushing in theme config the colors. So I suspect this could not work on runtime?

@endigma
Copy link
Owner

endigma commented Apr 9, 2023

I'm working on a pretty large upgrade today, it will help test this PR and all future ones too. See #6 for the generation idea, I don't want to start with giga-prs.

@endigma
Copy link
Owner

endigma commented Apr 9, 2023

You should update to current main, it has a vercel demo page thing now and I also changed some stuff you'll have to fix again for your thing, sorry.

@thewebartisan7
Copy link
Author

I tested your new updates without this PR, and I see that the build seem works fine but I got some types error, see screenshot attached. Do you see them?

Screenshot 2023-04-10 at 15 03 27

Screenshot 2023-04-10 at 15 03 41

@endigma
Copy link
Owner

endigma commented Apr 10, 2023

I actually don't, not even when explicitly running tsc, perhaps do you have a stricter tsconfig somewhere? It looks as though your errors stem from the definition of palette as readonly. This is super hard to debug without actually getting the error, can you share what settings/IDE/etc you're using that causes this to happen?

@thewebartisan7
Copy link
Author

I am using JetBrains, and it's configured to use tsc from local node_modules, so it's the same config. I am not using any stricter tsconfig as I am aware of.

Screenshot 2023-04-10 at 17 08 34

Yes I see that is about the palette, I have to investigate a bit more about, but before want to ask you, maybe you already know how to fix this.

@endigma
Copy link
Owner

endigma commented Apr 10, 2023

Ah, I use vscode, it doesn't show this error so I can't really figure whats wrong with it. If you can provide more details from your error or figure out what the problem is I'll happily fix it. Those generator functions are a little "first-draft", could probably do better with more tactful use of the array comprehensions.

I want to port all of it over to something functional/immutable but that would introduce another dependency I think, which isn't so nice for a library.

@thewebartisan7
Copy link
Author

Yes I see with vscode it doesn't complain, with ESLint extension, however I think that it's not so reliable.

I actually don't, not even when explicitly running tsc

I tried run tsc and I get errors, how you make this works?

I notice that with old tsconfig.js everything still works fine like before, I tried with this:

b4e12f0#diff-b55cdbef4907b7045f32cc5360d48d262cca5f94062e353089f189f4460039e0

With the new tsconfig.js I get first of all a deprecation issue with importsNotUsedAsValues:

Screenshot 2023-04-10 at 19 28 06

Then after adding "ignoreDeprecations": "5.0" as suggested by error message, I get 3 new errors:

Screenshot 2023-04-10 at 19 28 31

@endigma
Copy link
Owner

endigma commented Apr 10, 2023

Those errors are in vitest? I should fix the imports... thing in tsconfig, otherwise none of those are related.

@thewebartisan7
Copy link
Author

thewebartisan7 commented Apr 10, 2023

Those errors are in vitest? I should fix the imports... thing in tsconfig, otherwise none of those are related.

Yes it's seem coming from vitest after running tsc

@endigma
Copy link
Owner

endigma commented Apr 10, 2023

Took a shot in the dark on main, try that?

@thewebartisan7
Copy link
Author

Now is much better, I can "tsc" without errors.

About the others two errors that I see in my IDE, the first one inside export function presetRadix I see that the problems is with rules type, which is rules?: Rule<Theme>[];

When try to cast as Rule<Theme>[]; typescript doesn't complain anymore. It's not ideal but just to see which one cause problem. I am still not sure how everything works in unocss and docs doesn't explain too much, even trying with docs example https://unocss.dev/config/rules#full-controlled-rules same issue appear.

66450d74b7a2dee87c7b07741d3e32aa

The error said:

Returned expression type [RegExp, (([, color]: readonly [any, any]) => (string | ""))][] is not assignable to type Rule<Theme>[] 

In case this doesn't help you to understand how to fix, I will try to ask someone from unocss.

The second issue I will check a bit more in next days, this one:

Screenshot 2023-04-10 at 21 18 34

@endigma
Copy link
Owner

endigma commented Apr 22, 2023

@thewebartisan7 Are you planning on completing this?

@thewebartisan7
Copy link
Author

Hi @endigma sorry for delay, I was trying to find another solution for include radix colors in unocss and I did indeed succeed with it. However, I can also complete this PR, please give me a few more days.

@endigma
Copy link
Owner

endigma commented Apr 22, 2023

What caused you to use a different solution? I'd like to improve the library if there's something missing.

@thewebartisan7
Copy link
Author

thewebartisan7 commented Apr 22, 2023

There is nothing wrong with this preset.

The problem is that the project I am working on is with SASS, and all colors is already in a SASS map, from which the CSS variables are added in :root. So I was trying to use unocss with radix colors instead of using bg-[var(--red1)] and by using this preset all CSS variables are again added in :root.

So with a simple new rule I was able to create new color utility like bg-red-1 while the CSS variables are still output from SCSS map.

[/^bg-(.+)-(a)?([1-9]|1[0-2])$/, ([
      selector,
      color,
      alpha,
      scale
    ], ctx) => {
      const colorName = alpha ? `${color}A${scale}` : `${color}${scale}`

      return `
.${selector} {
  background-color: var(--${colorName});
}
`
}],

I am thinking to move all this in unocss config but for now there are too much things to be changed/remove in current SCSS code.

I am also trying to figure out a way to create the CSS variable on demand, I could just output them inside this rule, and I make a rule for this, but the problem is that for each color there is :root selector, while would be better to group all colors in one single :root.

For this other rule I grab colors directly from radix library via:

import {
  blue,
  blueA,
  blueDark,
  blueDarkA
} from '@radix-ui/colors';

In defineConfig:

theme: {
    colors: {
      radix: {
        blue,
        blueA,
        blueDark,
        blueDarkA,
      }
    }
  },

The similar to other rule:

    [/^bg-(.+)-(a)?([1-9]|1[0-2])$/, ([
      selector,
      color,
      alpha,
      scale
   ], ctx) => {
     if (color.includes('Dark')) {
       return
     }

      // Rename red-a => redA
      const palette = alpha ? `${color}A` : color

     // Make dark name from color name, e.g. red => redDark or redA => redDarkA
      const paletteDark = alpha ? `${color}DarkA` : `${color}Dark`

      // Rename b-red-a1 => redA1
      const colorName = alpha ? `${color}A${scale}` : `${color}${scale}`

      if (ctx.theme.colors.radix[palette] && ctx.theme.colors.radix[palette][colorName]) {
        return `
:root {
  --${colorName}: ${ctx.theme.colors.radix[palette][colorName]}
}

${ctx.theme.colors.radix[paletteDark] && ctx.theme.colors.radix[paletteDark][colorName] ?
`.dark .${selector} {
  --${colorName}: ${ctx.theme.colors.radix[paletteDark][colorName]}
}` : ''}

.${selector} {
  background-color: var(--${colorName});
}
`
      }
    }],

But I am still investing into.

In case I found a solution I will share here, so if you are interested we can improve this preset.

@thewebartisan7
Copy link
Author

I end up with a custom preset that suite better my need, check here https://github.com/primus-ui/unocss-preset-radix-palette

In case you are interested in this PR, let me know, and I can try to fix. The problem that I faced when I try to fix is with typescript, most of them mentioned. However, as soon as I try to add my changes, more typescript errors come out.

Considering that I need also others features and not only this one, I made a new one.

About issue with rules typescripts, working on I see that error disappear when you type rule arguments, in this case: ([, color]: string[]).

We can end up with single preset in case you are interested in additional features that I added.

@endigma endigma closed this Apr 25, 2023
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.

2 participants