cssinjs / styled-jss

Styled Components on top of JSS.
http://cssinjs.org/styled-jss
MIT License
217 stars 25 forks source link

Separate HTML attributes from styling props #7

Closed lttb closed 6 years ago

lttb commented 7 years ago

Online example

As a result of discussions in https://github.com/styled-components/styled-components/issues/439 and https://github.com/styled-components/styled-components/pull/731 let's consider this interface:

<Button style={{color, padding: color.length}} />

As for me it's very useful and clear. And we can use such things:

type Styled<T> = (props: {style?: T}) => React$Element<*>

const Button: Styled<{
  margin: number, 
  color?: 'red' | 'green'
}> = styled('button', {
  margin: ({margin = 0}) => margin,
  color: ({color = 'red'}) => color
})

// ok
<Button style={{margin: 10}} />

// flow type error
<Button style={{margin: 'ten'}} />

// flow type error too
<Button style={{margin: 10, color: 'blue'}} />

image

kof commented 7 years ago

Using style property is a bit opinionated because users still probably have a possibility to use inline styles. Though another way to use inline styles would be to just access a class name and build a styled primitive manually.

lttb commented 7 years ago

The look property instead of style is the best solution for me now

iamstarkov commented 7 years ago

i have the same concern as @kof and dont understand look thingy. @lttb can you help?

iamstarkov commented 7 years ago

though, i love flowtype typings

kof commented 7 years ago

On the other hand if one really needs inline styles which is a bad idea most of the time, one can use base styles and combine classes with inline styles as always. In case of styled primitives, we already expect a primitive to be styled. If style wouldn't mean inline styles for the rest of the world, it would be a quite good fit.

lttb commented 7 years ago

@iamstarkov about style prop: u can look at this from such side: with styled-jss u should not to use inline styles, this abstraction just fully replace it.

but I suggested another way: use 'look' prop instead

<Button look={{color: 'red'}} />
kof commented 7 years ago

This is a bad example:

<Button look={{color: 'red'}} />

This is better:

<Button look={{primary: true}} />

And yet, does it feel natural to you compared to this?

<Button primary />
lttb commented 7 years ago

agree, but it seems that this is a common usage. @kof your example I see this way:

<Button look="primary" />

composition:

<Button look={{primary, secondary}} />

about naming: style has ideological backend. as I said, we force to avoid inline styles. and I like it. it also feels natural.

but look is good as trade off

kof commented 7 years ago

I think they all suck compared to just using normal props, like right now. We should try find another solution for the root of the problem in react: avoiding the dom attributes whitelist. Somehow a styled primitive should be able to filter the props it knows and not forward them.

lttb commented 6 years ago

Close in case of https://github.com/lttb/is-react-prop/pull/3