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

Use CSS Vars to allow direct passthrough of tailwind variables #40

Closed
The-Code-Monkey opened this issue Apr 3, 2024 · 12 comments
Closed

Comments

@The-Code-Monkey
Copy link
Contributor

This is a suggestion on how to handle #36 as it sounds pretty interesting.

The way i was thinking of it was to enable the use of something like

div:scope {
  --padding: 0;
  padding: var(--padding);

 &[data-padding="--spacing-"] {}

which the parser would see this as

type BazProps = {
  children?: ReactNode,
  padding?: string,
} & JSX.IntrinsicElements['p']

export function Baz({ children, padding, ...props }: BazProps) {
  return (
    <p {...props} className="baz" style={{ "--padding": `var(--spacing-${padding})` }>
      {children}
    </p>
  )
}

I know its not the prettiest solution but it could work in terms of how simple it is, or maybe we could even remove the need for the &[data-padding] somehow and resolve this value from the --padding var directly e.g.

div:scope {
  --padding: var(--spacing-0); <-  default value
  padding: var(--padding);

this way we could infer that for padding prop to grab the first half of the assigned ts variable and then concat the prop onto the end.

@The-Code-Monkey
Copy link
Contributor Author

I would be happy to do this solution if this is something you want to implement, either now or in the future.

@alamenai
Copy link

alamenai commented Apr 3, 2024

Hi @The-Code-Monkey ,

Thank you for the suggestions , but I guess it will add more complexity to the syntax.

@The-Code-Monkey
Copy link
Contributor Author

The-Code-Monkey commented Apr 3, 2024

It depends on which option you look at the second one is not too much syntax and follows a very similar format to how tailwind themselves do stuff.

the first one option is a little more complex but again not too complex its less complex than what im currently doing in my component lib thats using mist.

https://github.com/The-Code-Monkey/MistKit/blob/master/src/primals/flex/index.mist.css

it would make something like this so much cleaner and extendable for custom tailwind values.

as all i would have would be

  @scope (.flex) {
    div:scope {
      --direction: var(--flex-column);
      --justify: var(--justify-start);
      --items: var(--items-start);
      --content: var(--content-start);
      --gap: var(--gap-0);

     display: flex;
     gap: var(--gap);
     flex-direction: var(--direction);
     justify-content: var(--justify);
     align-items: var(--items);
     align-content: var(--content);
  }
}

This is much cleaner than current solution and more dynamic.

@alamenai
Copy link

alamenai commented Apr 3, 2024

Wow, Yep I liked it.

@The-Code-Monkey
Copy link
Contributor Author

Wow, Yep I liked it.

Just have to get @typicode on side now ha ha

@typicode
Copy link
Owner

typicode commented Apr 3, 2024

I've been thinking about CSS variables as well :)
For values that aren't enums (number, percent, color, ...)

So yes, MistCSS should support them. I'd like to a simple way to ignore computed CSS var or distinguish between "public"/"private".

For example:

input[type="checkbox"] {
  width: 80px;
  height: 80px;
  --i: 0;
  --hue: calc(var(--i) * 50 + 100);
  accent-color: hsl(var(--hue), 50%, 50%);
}
<div style="text-align: center">
  <input type="checkbox" checked style="--i: 0"/>
  <input type="checkbox" checked style="--i: 1"/>
  <input type="checkbox" checked style="--i: 2"/>
  <input type="checkbox" checked style="--i: 3"/>
</div>

I'd like the generated component to expose i as a prop but not hue.

@The-Code-Monkey
Copy link
Contributor Author

Yeah I like that idea of how certain values are exposed and others aren't.

I think exposed values should be strings, numbers, CSS vars like my example above but yeah things like calc wouldn't be exposed

@The-Code-Monkey
Copy link
Contributor Author

I'm currently working on a basic implementation for variables currently it will be finished off tomorrow morning and I'll submit it for review

@The-Code-Monkey
Copy link
Contributor Author

I have an implementation written @alamenai typicode is working on refactoring so in the meantime i published my own so i can continue dev, don't recommend using it though as it will be removed once typicode is done with their refactor. And it is also will be buggy as i will test stuff in it.

https://github.com/The-Code-Monkey/mistcss/tree/v-andy

you can see it in action here

https://github.com/The-Code-Monkey/MistKit

@typicode
Copy link
Owner

typicode commented Apr 25, 2024

An alternative to this, that can work with the current CSS variable implementation, could be to use the Tailwind variables in JS.

For example:

<Button color={colors.500} padding={spacing[8]}/>

https://github.com/tailwindlabs/tailwindcss/blob/master/stubs/config.full.js

This would have the benefit of having TS check that the keys used are valid.

// Error from TS
<Button color={colors.54321} padding={spacing.xl}/>

I haven't done research into exactly how this this can imported.

Edit:

Getting Tailwind variables is possible this way:

const defaultTheme = require('tailwindcss/defaultTheme')

@The-Code-Monkey
Copy link
Contributor Author

Yeah this is what I would like to support maybe via a specific tailwind renderer. E.g. react tailwind.

But I would personally say for things like padding should be just 8 not spacing.8 as we should be able to infer the tailwind schema it belongs to. I'll have a look at supporting something next week

@typicode
Copy link
Owner

I see. I understand it could be easier but I think it could be possible with the existing code.

If you wrap Mist components in a function that would convert padding={8} to padding={theme.spaces.8} (and other properties) before passing it to Mist component, that could work.

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 a pull request may close this issue.

3 participants