Skip to content

Hide implementation details from styles API #7636

Description

@Izhaki

So yesterday I was looking into the css in js section in the v1 docs, and equally delved into withRoot, and other examples. Amazing job.

But I suspect, having done so myself many times in the past, that you guys have put so much effort and thought into this, that you fell a step short from perfecting the API.

That said from a client perspective.

Current API

Currently, the examples involves:

import { withStyles, createStyleSheet } from 'material-ui/styles';

const styleSheet = createStyleSheet(theme => ({
  body: {
    margin: 0,
  },
}));

Followed by:

AppWrapper = withStyles(styleSheet)(AppWrapper);

Possible improvements

Now consider you achieve the same thing with:

import { applyStyleSheet } from 'material-ui/styles';

then (just an object)

const styleSheet = {
  body: {
    margin: 0,
  },
}

or (a function in case theme is needed)

const styleSheet = theme => ({
  body: {
    margin: 0,
  },
})

or (a tuple in case the sytlesheet name is important - the second parameter may be a function or an object)

const styleSheet = [ 'MyStyleName', theme => ({
  body: {
    margin: 0,
  },
})]

Followed by (given applyStyleSheet is curried):

return applyStyleSheet( styleSheet, MyComponent )

Basically, applyStyleSheet hides away withStyles and createStyleSheet, and has some switch logic to see if the style sheet provided is an object, function, or a tuple.

props.classes?

Lastly, another part of the API, as seen in your own doc's AppContent involves:

function AppContent(props) {
  const { className, classes, children } = props

  return (
    <div className={ classNames( classes.content, className ) }>
      {children}
    </div>
  )
}

withStyles injects a classes property on props.

It took a bit of mental mapping to understand that classes.content is actually the name of the class that is used in the stylesheet provided to withStyles.

It is nobody's fault ES6 has classes and CSS has classes.

I just wonder if using styleSheetClasses instead of classes, albeit more verbose, would be clearer? As in:

function AppContent(props) {
  const { className, styleSheetClasses, children } = props

  return (
    <div className={ classNames( styleSheetClasses.content, className ) }>
      {children}
    </div>
  )
}

Metadata

Metadata

Labels

discussiongood first issueGreat for first contributions. Enable to learn the contribution process.

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions