Skip to content

support nested theme objects #24

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

Merged

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants