Skip to content

Add theme to childContext #22

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

Closed
wtgtybhertgeghgtwtg opened this issue Sep 7, 2016 · 5 comments
Closed

Add theme to childContext #22

wtgtybhertgeghgtwtg opened this issue Sep 7, 2016 · 5 comments

Comments

@wtgtybhertgeghgtwtg
Copy link

Let's say I have some two components, ChildComponent

@themr('ChildComponent')
class ChildComponent extends Component {
  render() {
    const { theme } = this.props;
    return (
      <div className={theme.child}></div>
    )
  }
}

and ParentComponent

@themr('ParentComponent')
class ParentComponent extends Component {
  render() {
    const { theme } = this.props;
    return (
      <div className={theme.parent}>
        <ChildComponent theme={theme} />
      </div>
    )
  }
}

For the theme prop of ParentComponent to be applied to ChildComponent, it has to be explicitly passed down. It would be a breaking change, but could Themed define getChildContext so that the theme of ParentComponent is automatically part of the theme of ChildComponent?

@javivelasco
Copy link
Owner

I guess there is something we could do to handle these cases explicitly. If a child component can be considered part of the parent component, they both can define (and must define) the same key. For example themr('Radio', style).

If you provide a theme via context it's going to work nicely because they use the same key but I guess that if you pass down a style to a parent of an specific type, it makes sense if this component writes in the context the updated theme. Therefore, the child would read from context the theme you passed to the parent via props.

This update is not breaking and has no downsides in my opinion apart from the cost of writing in the context when a theme is received

@raveclassic
Copy link
Contributor

raveclassic commented Sep 23, 2016

@wtgtybhertgeghgtwtg
You can easily set the same theme for different keys when constructing context:

import css from './shared.css';
const context = {
 ChildComponent: css,
 ParentComponent: css
};

On the other hand, if you accept exactly the same theme in different components why not to use the same keys (names) for them?

UPDATE: If your case is about including a theme passed to props, then I believe the best way here is to explicitly extract child's theme from props and to pass it down. I've opened #24 to help with such cases.

UPDATE: The reason of avoiding context is that it can't hold dynamic values when using pure-rendering: facebook/react#2517

@javivelasco
Copy link
Owner

Yeah, you're right, that's what we are doing with react-toolbox. Just using the same key for both child and parent if the belong to the same complex component

@raveclassic
Copy link
Contributor

@wtgtybhertgeghgtwtg What you are suggesting is a good old cascade when Parent component affects all possible Children on all levels deeper. Such behavior is unsafe and was meant to be eliminated by introduction of css-modules. However, ThemeProviders could still be nested to make such behavior explicit and #19 tracks it.
@javivelasco I believe this issue can be closed.

@javivelasco
Copy link
Owner

Cool, thanks @raveclassic !

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

No branches or pull requests

3 participants