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

Migrate CSS to styled components #81

Closed
cleverbeagle opened this issue Oct 6, 2017 · 22 comments
Closed

Migrate CSS to styled components #81

cleverbeagle opened this issue Oct 6, 2017 · 22 comments
Assignees
Labels
Milestone

Comments

@cleverbeagle
Copy link
Owner

There's little method to our madness right now and it'd be good to follow a standard that folks can easily reference.

@tomscholz
Copy link

What about styled components or cssinjs?

@cleverbeagle
Copy link
Owner Author

@tomscholz maybe down the road. Styled components are cool but would like to keep the CSS decoupled for now for simplicity sake.

@cleverbeagle
Copy link
Owner Author

@cleverbeagle cleverbeagle changed the title Discuss usage of a CSS methodology like BEM Migrate CSS to styled components Jan 26, 2018
@cleverbeagle
Copy link
Owner Author

Thinking long-term about where we're headed, although this adds some cognitive weight, it would allow us to keep CSS consistent across all CB code. There's not a ton of custom CSS in Pup and making this move would also aid in getting SSR working by default without a ton of hoop jumping.

@benhsampson
Copy link

Any idea on how the file structure would change given we integrate styled-components? We could possibly have a separate styles directory in the ui folder.

@cleverbeagle
Copy link
Owner Author

@benhsampson Not too much from what he have now, I'd imagine. Ideally we keep the stylesheet as close to the components as possible. Global styles, though, would do well in a folder like you describe, though. Right now /ui/stylesheets serves this purpose.

@toinevk
Copy link
Contributor

toinevk commented Mar 5, 2018

I had some spare time on the weekend to look into this a little more. To migrate to styled components is not as straight forward as it looks. Particularly the lighten and darken helpers, don't exist without a helper library such color. The alternative would be to manually define the colours (i.e. cb-blue-lighten-10) but that seems like alot of additional code. What would your preference be?

@benhsampson
Copy link

I would probably do it this way.

const theme = {
  colors: [
    grey: {
      lighter: '#FFFFFF',
      light: '#EEEEEE',
      default: '#AAAAAA,
      dark: '#555555',
      darker: '#222222',
    },
  ],
};

It's not ideal but at least you can have a predefined color palette increased specificity and consistency.

It is quite a lot of code but it shouldn't take too much time to implement.

@toinevk
Copy link
Contributor

toinevk commented Mar 5, 2018

Yes - I tried something similar.

const Theme = {
  colors: {
    cbBlue: {
      cbBlue: '#4285F4',
      cbBlueLight10: '#4285F4',
      cbBlueLight20: '#75B8FF',
      cbBlueLight30: '#8FD2FF',
      cbBlueLight40: '#A8EBFF',
      cbBlueLight50: '#C1FFFF',
      cbBlueDark10: '#296CDB',
      cbBlueDark20: '#0F52C1',
      cbBlueDark30: '#0039A8',
      cbBlueDark40: '#001F8E',
      cbBlueDark50: '#000675',
    },
  }
};

I think this with a link to a hex color tool would be a sensible approach. It would also take any "magic" out of how it works.

@cleverbeagle
Copy link
Owner Author

I commend the effort, folks :) But this would be overkill. As far as lightening and darkening, it'd probably be best to just write some small JS functions that we can execute inside of our styled-components definitions. E.g.:

const StyledThing = styled.div`
  background: colors.lighten(/* Some color */) // Open to using CSS variables (limits SC coupling)
`

Here, colors.lighten() would just be a function we import from somewhere like /imports/modules/colors.js. I haven't looked but I'd imagine someone has thought through that problem before and has a script for doing the conversion available.

@merlinstardust
Copy link
Contributor

merlinstardust commented Mar 5, 2018

What do folks think about using react-with-styles by AirBnB and Aphrodite by Khan Academy?

I suggest it for two reasons. The first is that we're using AirBnB's style guide so this would remain consistent with that and use their guides for how to work with CSS in JS.

The second is that I find it a lot easier to use JavaScript in it because it already is JS. Using styled-components, you're going back to strings and it's a bit more difficult/annoying to do JS inside of them.

@cleverbeagle
Copy link
Owner Author

@merlinpatt I think alongside React the syntax does make more sense, but not a big fan of losing the ability to write real CSS as opposed to an abstraction.

Another one to have on our radar is the level of support for Aphrodite vs. others. Being created in a big company is a huge plus, but I wonder if it's something widely-deployed at Khan or just a skunk works type of thing.

@merlinstardust
Copy link
Contributor

@cleverbeagle what real CSS are you worried about losing?

@cleverbeagle
Copy link
Owner Author

@merlinpatt the selectors themselves. For example, here's a styled component I was playing with the other day:

const Brand = styled(Link)`
  font-size: 16px;
  color: #aaa;
  text-decoration: none;
  position: relative;
  top: -11px;

  @media (min-width: 768px) {
    font-size: 20px;
  }

  @media (min-width: 992px) {
    font-size: 24px;
  }
`;

In something like Aphrodite, we'd have to convert that into the abstracted object pattern which means learning a different syntax that isn't CSS. In other words, I like styled components because it acts as a wrapper around good ol' fashioned CSS as opposed to introducing a totally different way of authoring styles. Subtle, but thinking long-term, it's likely that this stuff will change again and the closer we are to pure CSS, the less tumult in shifting to whatever is next.

@benhsampson
Copy link

Polished JS is a great toolset for this kind of thing.

@cleverbeagle
Copy link
Owner Author

@benhsampson exactly what I had in mind, good find. Cool to see it's made by the guy doing styled-components, too.

@merlinstardust
Copy link
Contributor

Aphrodite allows you those same selectors as well. It would look like this.

export default withStyles(({breakpoints, colors, units}) => ({
  link: {
    fontSize: units(16),
    color: colors.grey,
    textDecoration: 'none',
    position: 'relative',
    top: units(-11),

    [breakpoints.medium]: {
      fontSize: units(20),
    },

    [breakpoints.large]: {
      fontSize: units(24),
    },
  },
}))(Link);

@cleverbeagle cleverbeagle added this to the 2.0.0 milestone Mar 7, 2018
@cleverbeagle
Copy link
Owner Author

@merlinpatt does getting access to this syntax require us to use the withStyles() container?

@merlinstardust
Copy link
Contributor

No, you could use it without the container. I only prefer the container because it makes it easy to pass in a theme, which you could import. Also, having the container will show up in React DevTools, so it's easy to see what's styled

@cleverbeagle
Copy link
Owner Author

Ahh okay. What's it look like without the container @merlinpatt?

@merlinstardust
Copy link
Contributor

merlinstardust commented Mar 12, 2018

The breakpoints, colors, and units would instead be imported from some file

const styles = StyleSheet.create({
  link: {
    fontSize: units(16),
    color: colors.grey,
    textDecoration: 'none',
    position: 'relative',
    top: units(-11),

    [breakpoints.medium]: {
      fontSize: units(20),
    },

    [breakpoints.large]: {
      fontSize: units(24),
    },
  },
})

@henrysipp
Copy link

I've personally had a very good time using design-system-utils for styled-components
https:/mrmartineau/design-system-utils

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants