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

Make sure styled-system is supported #974

Open
1 of 4 tasks
kof opened this issue Jan 7, 2019 · 19 comments
Open
1 of 4 tasks

Make sure styled-system is supported #974

kof opened this issue Jan 7, 2019 · 19 comments
Labels
complexity:moderate We talked about it, you can do it! feature request This will safe many lifes! important The thing you do when you wake up!

Comments

@kof
Copy link
Member

kof commented Jan 7, 2019

Styled-System by @jxnblk https://github.com/jxnblk/styled-system seems like a very solid approach for building design systems. I think we should look into it and see how it can be supported.

Todo

  • See how it can be supported
  • Add it to documentation with links
  • Make codesandbox examples
  • Send a PR to styled-system and link the examples
@kof kof added feature request This will safe many lifes! complexity:moderate We talked about it, you can do it! labels Jan 7, 2019
@jxnblk
Copy link

jxnblk commented Jan 7, 2019

Haven't really tested it recently, but styled-system should "just work" with styled-jss. Definitely let me know if you run into any bugs though

@kof
Copy link
Member Author

kof commented Jan 7, 2019

I wonder if it also can work with react-jss and the future version of it based on hooks (prototype https://github.com/cssinjs/react-jss-hook)

@HenriBeck
Copy link
Member

It should already work with react-jss

@eddort
Copy link

eddort commented Feb 24, 2019

this only works with react-jss @ alpha-10
but if you want to use something like <Box p = {[1, 1/2, 1/4]}> </ Box> you will notice a bug #1044

@kof
Copy link
Member Author

kof commented Feb 24, 2019

Putting high prio on that bug.

@HenriBeck
Copy link
Member

HenriBeck commented Feb 24, 2019

I think the issue is that styled-system is accessing the theme via props and not from our themed styles function.

EDIT: The reason why this is not a problem for styled-components is that they generate an entirely new stylesheet when the props change and not just update the existing rules.

@kevinSuttle
Copy link

Would love to see this also.

@kof kof added the important The thing you do when you wake up! label Apr 18, 2019
@kevinSuttle
Copy link

What is the relationship between https://material-ui.com/system/basics/ and JSS here?

@kof
Copy link
Member Author

kof commented Apr 23, 2019

MUI style system is an independent effort by @oliviertassinari inspired by styled-system. It is a separate package https://github.com/mui-org/material-ui/tree/next/packages/material-ui-system and seems to have no direct dependency on JSS, but can be used together.

I am also porting (right now) an SC-like, similar to styled-jss into the react-jss package to support styled-system and SC-pattern users.

@kevinSuttle
Copy link

@kof
Copy link
Member Author

kof commented Apr 23, 2019

@kevinSuttle what specific use case from primitives-core are you interested in?

@kevinSuttle
Copy link

kevinSuttle commented Apr 23, 2019

Nothing in particular. It's just another approach—one that isn't included in the Prior Art section here: https://material-ui.com/system/basics/#prior-art

@cssinjs cssinjs deleted a comment from kevinSuttle Apr 23, 2019
@kof
Copy link
Member Author

kof commented Apr 25, 2019

Turns out styled system doesn't depend on SC pattern at all, those are just functions. I was confused by the fact they expose propTypes, so I thought they must be rendered as components, but actually those propTypes need to be merged into user component manually if you want the warnings.

The only thing that is actually special about styled system is the data structure which is used to return the styles:

[
  [
    {
      "paddingTop": "32px"
    },
    {
      "paddingBottom": "32px"
    },
    [
      {
        "paddingLeft": "16px"
      },
      {
        "@media screen and (min-width: 40em)": {
          "paddingLeft": "32px"
        }
      }
    ],
    [
      {
        "paddingRight": "16px"
      },
      {
        "@media screen and (min-width: 40em)": {
          "paddingRight": "32px"
        }
      }
    ]
  ],
  []
]
space({theme, px: [3,4], py: 4})

Also there is a hard dependency on props.theme.
And an optional one on fn.propTypes.

@kof
Copy link
Member Author

kof commented Apr 30, 2019

Turns out v3 of styled-system just works without any extra code, the only thing one needs to do is to set injectTheme to true:

    const styles = {
      root: compose(
        space,
        color,
        fontSize,
        width,
        fontWeight,
        lineHeight
      )
    }

    const MyComponent = ({classes}) => <div className={classes.root} />

    const MyStyledComponent = withStyles(styles, {injectTheme: true})(MyComponent)

    const renderer = TestRenderer.create(
      <ThemeProvider theme={theme}>
        <MyStyledComponent
          px={[3, 4]}
          py={[1, 2]}
          color="white"
          bg="blue"
          fontSize={[4, 5, 6]}
          fontWeight="bold"
        />
      </ThemeProvider>
    )

@jxnblk
Copy link

jxnblk commented Apr 30, 2019

@kof Heads up: I've started a PR here: styled-system/styled-system#473 and it's now published as styled-system@4.2.0-0 if you'd like to test it out

@kevinSuttle
Copy link

kevinSuttle commented Apr 30, 2019

How does that affect performance, especially with regards to https://styled-system.com/babel-plugin?

@kof
Copy link
Member Author

kof commented Apr 30, 2019

@jxnblk nice, gonna try it out in a bit

@jxnblk
Copy link

jxnblk commented Apr 30, 2019

@kevinSuttle the babel plugin package is experimental and does not use the same code base as Styled System core

@kof
Copy link
Member Author

kof commented May 2, 2019

@jxnblk I confirm styled-system@4.2.0-0 works with my tests

@kof kof mentioned this issue Jun 12, 2019
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity:moderate We talked about it, you can do it! feature request This will safe many lifes! important The thing you do when you wake up!
Projects
None yet
Development

No branches or pull requests

5 participants