Theming about fela HOT 4 CLOSED

@MicheleBertoli @johanneslumpe Got a new idea on my mind. The proposed API design seems quite fine. I guess its no real difference if the changeTheme/updateTheme is returned by theming() or if it is a method of the renderer itself, though being returned by theming seems more transparent.

To do style rehydration I thought about actually providing another enhancer which additionally tracks all rule(id)-props combinations. This would decouple both features which can be useful each by their own.
This would look sth. like:

const renderer = createRenderer({
  enhancers: [ rehydrate() ]
})

let someVar = 'red'

const rule = props => ({
  color: props.color,
  backgroundColor: someVar
})

renderer.renderRule(rule, { fontSize: '15px' }) // => 'c0 c0-foo'
// .c0 { background-color: red }

someVar = 'blue'

renderer.rehydrate()
// .c0 { background-color: blue }

See #71

from fela.

Well done, I love the solution.

Did you already think about a function to change the theme?

from fela.

@rofrischmann @MicheleBertoli
What do you think about this:

Instead of returning an enhancer directly, theming(theme) could return an object with a shape like { enhancer, changeTheme } . changeTheme would be a function which takes a new theme as a parameter to allow on the fly theme changes. That function could then easily be passed down via context in react for example (or via any other way in other frameworks).

The only downside I see here (in a react context) is that this change wouldn't cause a re-render because it's outside of react. This could partially be mitigated by keeping the theme object in a redux store for example, but it still might not force a complete re-render.

from fela.

I thought about adding renderer.updateTheme(theme) to the enhancer to trigger theme updates, but I am yet not sure how to rehydrate all styles. One idea would be to just clear all styles and rerender everything (e.g. using forceUpdate on the <Provider />.
Right now the rendered & cached CSS is compressed so I can not imply the used theme afterwards.

I will have to test this in a real world use case though.

Update: Seems like @johanneslumpe was faster :P But still we have identical ideas I guess :P

from fela.