Skip to content

Conversation

@raveclassic
Copy link
Contributor

@raveclassic raveclassic commented Sep 23, 2016

This PR provides nested theme objects support - some words about it.
Suppose you have a complex component that accepts child component class as props:

const CHILD_THEME_SHAPE = { child: React.PropTypes.string } const Child = ({ theme }) => ( <div className={theme.container}> Hi, I am a basic child! </div> ) Child.propTypes = { theme: React.PropTypes.shape(CHILD_THEME_SHAPE) } const COMPLEX_THEME_SHAPE = { complex: React.PropTypes.string } class Complex extends React.Component { static propTypes = { Child: React.PropTypes.func, theme: React.PropTypes.oneOfType([ React.PropTypes.shape({ //naive implementation ...COMPLEX_THEME_SHAPE, ...CHILD_THEME_SHAPE //possible collision here }), React.PropTypes.shape({ //improve "namespaced" implementation ...COMPLEX_THEME_SHAPE, //we can take Child component's name here Child: React.PropTypes.shape(CHILD_THEME_SHAPE) }) ]) } static defaultProps = { Child } render() { const { Child, theme } = this.props //naive solution is to construct child's theme here on the fly const childTheme = { child: theme.child //keys may be different } return ( <div className={theme.complex}> {/*if you have multiple child component classes yoy end up on constructing theme objects on the fly for each of them*/} <Child theme={childTheme}/> {/*but you could just pass a nested theme to child's component*/} <Child theme={theme.Child}/> </div> ) } } //render const render = () => <Complex Child={SomeCustomChildComponent}/>

The point is to allow nested objects in theme definition.
Also when using themr context a theme can be easily constructed from multiple css-modules:

import complex from './Complex.css' import child from './Child.css' const context = { Complex: { ...Complex, Child: child } } 

Accepting these changes would be extremely nice because it's very tedios to construct dynamic theme in complex components for all nested Child components.

UPDATE: Also PR contains some jsdoc

@raveclassic
Copy link
Contributor Author

I don't get why Travis is failing :(
I'm able to run all tests on local machine:
node v5.8.0
npm 3.7.4
clean npm i

@dchambers
Copy link

I don't know if it's because I did a clean npm install or because I'm also using Linux, but I'm able to replicate the error seen on Travis. I was able to fix it with this change:

 </ThemeProvider> - )).toThrow(/exactly one child/) + )).toThrow(/expected to receive a single React element child/) expect(() => TestUtils.renderIntoDocument( <ThemeProvider theme={theme}> </ThemeProvider> - )).toThrow(/exactly one child/) + )).toThrow(/expected to receive a single React element child/) } finally { ThemeProvider.propTypes = propTypes

but I would assume that that breaks it for you @raveclassic?

@raveclassic
Copy link
Contributor Author

@dchambers Awesome! Seems like React guys have changed React.Children.only error text: react-css-themr/node_modules/react/lib/onlyChild.js:34

@javivelasco
Copy link
Owner

Great, I was thinking to do exactly this!! Thanks!

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

Labels

None yet

3 participants