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

New variants api and type fixes #134

Merged
merged 10 commits into from Aug 29, 2020

Conversation

hadihallak
Copy link
Member

@hadihallak hadihallak commented Aug 26, 2020

What's inside:

const Button = styled("button", {
  // Base styles:
  backgroundColor: "red",
  // Variants are scoped into a variants property:
  variants: {
    variant: {
      red: {},
      black: {},
      blue: {},
      yellow: {},
    },
    size: {
      1: {},
      2: {},
      3: {},
      4: {},
    },
  },
});
const Button = styled('button', {
  variants: {
    isActive: {
      true: {},
      false: {},
    }
  }
})

// now you will be able to use it like so
<Button isActive={true} />
or
<Button isActive="true" />

with both of the options being strictly typed to the actual values

Please hammer this down with testing for both typing and actual functionality as there was a fair bit of types/functionality was re-written and we currently don't have testing setup for the styled package so play around with it and let me know if you find any bugs

@peduarte
Copy link
Contributor

@christianalfoni would you mind having a look as well, pleeeeease? 🙏

@peduarte
Copy link
Contributor

regarding the variant API, thanks for that, im looking at the codesandbox and doing some tests

So to clarify, in order for types to work properly, the variant calls need to be chained.

const Button = styled('button', {})
  .variant('color', {})
  .variant('size', {})

Instead of the original idea, which was to have completely seperate blocks:

const Button = styled('button', {})
Button.variant('color', {})
Button.variant('size', {})

Personally, Im not a fan of chaining, but I could live with it. What worries me the most is the confusion around the fact that types will not work unless you chain. These things become really hard to document, and will drive people nuts. Because from a technical perspective, you’d think that if you can chain, you can also call Button.variant - which you can, but… types wont work

@peduarte
Copy link
Contributor

typing stuff seems to be working very well though

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.

None yet

2 participants