Skip to content

Add documentation for including styles that use the value of a prop #23

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

PasghettiCode
Copy link

The current documentation doesn't clearly call out the use of inline styles in the cases where the value comes from a prop, which can lead to implementations that attempt to use the "style" prop along with "withStyles" and violate the style guide.

The example provided is consistent with the best practices detailed in the style guide: https://github.com/airbnb/javascript/tree/master/css-in-javascript#inline

To: @lencioni

function MyComponent({ bold, padding, styles }) {
const {
backgroundImageUrl,
} = this.props;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a stateless functional component, this.props is not right. This should be destructured in the method signature along with the rest of the props.

@@ -317,6 +317,35 @@ export default withStyles(({ color, unit }) => ({

`className` and `style` props must not be used on the same elements as `css()`.

### Using the value of a prop / high cardinality

To include styles that use the value of a prop (or any other styles that have a high cardinality and shouldn't be in a themed stylesheet), you can use inline styles in conjunction with `withstyles()`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

withstyles() => withStyles()

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

Successfully merging this pull request may close these issues.

3 participants