added propMerge() in react package

[otbe]

Like discussed in glamor gitter chatroom this PR adds a better way for composing react components with glamor styles.

const cardButton = style({
  height: '20px',
  color: 'green'
});

const defaultButtonStyle = style({
  color: 'red',
  height: '50px',
  border: '1px solid black'
});

let Button: SFC<HTMLProps<HTMLButtonElement>> = (props) => 
<button {...propMerge(defaultButtonStyle, props)}/>;

let CardButton: SFC<HTMLProps<HTMLButtonElement>> = (props) => 
<Button {...propMerge(cardButton, props)}/>;

let AnotherButton: SFC<HTMLProps<HTMLButtonElement>> = (props) => 
<button {...defaultButtonStyle} {...props}/>;

let AnotherCardButton: SFC<HTMLProps<HTMLButtonElement>> = (props) => 
<AnotherButton {...cardButton} {...props}/>;

Will result in

[rszewczyk]

this is great! I'm wondering though what you think about having a second style parameter that would be merged after the variant styles so that we could more easily handle theming. So it would do something like assign(baseStyles, variantStyles, themeStyles)?

Are you using the themes from glamor/react? How are you handling them now?

[rszewczyk]

or, maybe even better? Have the mergeProps function handle theme props. I have a first hack at something like that working:

import { style, merge, isLikeRule } from 'glamor'

export default function mergeProps(props, baseStyle) {
  const restProps = {}
  const variantStyles = []
  const themeStyles = []

  for (let k in props) {
    if (!!/data\-css\-([a-zA-Z0-9]+)/.exec(k)) {
      variantStyles.push({ [k]: props[k] })
    } else if (!!/data\-glamor\-theme\-([a-zA-Z0-9]+)/.exec(k)) {
      themeStyles.push(props[k])
    } else if (props.hasOwnProperty(k)) {
      restProps[k] = props[k]
    }
  }

  if (variantStyles.length === 0) {
    return Object.assign(restProps, merge(baseStyle, ...themeStyles))
  }

  if (variantStyles.length !== 1) {
    Object.assign(restProps, isLikeRule(baseStyle) ? style(baseStyle) : baseStyle)
    variantStyles.forEach(s => Object.assign(restProps, s))
    return Object.assign(restProps, merge(...themeStyles))
  }

  return Object.assign(restProps, merge(baseStyle, variantStyles[0], ...themeStyles))
}

I haven't completely thought the through the implications of multiple themes, particularly on the non happy path where there are multiple variant styles. I'm not sure if there's anything else to do but merge them.

[otbe]

@rszewczyk we do not use theming in our SPAs, so I can't help you a lot :/
Maybe @threepointone can? :)

[rszewczyk]

Yep - we discussed it briefly in another issue. After his feedback, and some of my own experimentation, it doesn't seem like such a great idea after all. At least I couldn't figure out a way to make it work without making assumptions about how the user was going to use theming. As @threepointone pointed out in #95 - it's a little too much magic (I guess that'd be magic on top of magic). Thanks for indulging me @otbe.

[otbe]

Any thoughts @threepointone ? :)

[threepointone]

Will merge this week, chill :)

[otbe]

Great! Thanks!

[threepointone]

this is live in 2.17.15, give it a spin and lemme know how it goes. thanks @otbe!