weaveworks / ui-components

A collection of UI components that are shared across Weaveworks projects
http://weaveworks-ui-components.s3-website-us-west-2.amazonaws.com/
Apache License 2.0
30 stars 26 forks source link

[RFC] - add selectors for grabbing theme values #403

Closed guyfedwards closed 5 years ago

guyfedwards commented 5 years ago

Composable selectors for grabbing values from props and specifically theme in styled components.

Changes this: background-color: ${props => props.theme.colors.orange}; into background-color: ${color('orange')}

After implementing this I discovered the fromAtoms util and generally the Atoms concept. @fbarl could you give me a breakdown about the thought behind that?

foot commented 5 years ago

This is pretty slick :). Can you type it 😈 ?

I like the terseness, the selector fn itself takes a second to read. I wonder what alternate syntaxes look like that have a simpler base.

  1. font-size: ${fontSize('small')} - proposed here.
  2. font-size: ${themeGet('fontSize.small')} - a get() wrapper
  3. font-size: ${fontSize.small} <- w/ a proxy / getter / setter? this is probably not simpler to understand..
guyfedwards commented 5 years ago

Flow still isn't setup in here ( ͡↑ ͜ʖ ͡↑), its something I would like to get done but lost momentum after the difficulty with webpack proxying to ui-components for local dev. Should really pick it up again regardless...

Yea the idea with the selector is to be allow any composed functions to be created and not just limit it to theme ¯\(ツ)/¯

fbarl commented 5 years ago

After implementing this I discovered the fromAtoms util and generally the Atoms concept. @fbarl could you give me a breakdown about the thought behind that?

I've personally never used it, but I remember @jpellizzari put it in place for one of our first components, not sure what we want to do with it - I think the concept is valuable for some cases, but not quite sure we want to keep it global (vs. spread it between component files).

I wonder what alternate syntaxes look like that have a simpler base...

All the suggestions seem like an improvement but I think my vote still goes for the first suggestion :)