Skip to content
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

Render time styles overrides over props #6137

Closed
kof opened this issue Feb 14, 2017 · 2 comments
Closed

Render time styles overrides over props #6137

kof opened this issue Feb 14, 2017 · 2 comments

Comments

@kof
Copy link
Contributor

kof commented Feb 14, 2017

Based on the conversation in #6130 to a achieve an ethereal level of override possibilities, we need a way to override any css rule over the props. Its different from overrides and variants in the theme, because those need to be defined in advance and are considered as a predefined standard cases.

This issue is about an override target on just one specific element for the case it needs to be very unique and you don't want/can't predefine its styles in advance.

Example

  const theme = {
    root: X,
    label: Y
  }
  <Button theme={theme}>My Special button</Button>

Now what is X and Y? As of right now, we can't render dynamic styles efficiently, so right now we can go for just a classes map:

  const theme = {
    root: 'your-root-override-class',
    label: 'your-label-override-class'
  }
  <Button theme={theme}>My Special button</Button>

Another option would be available once dynamic sheets lands in JSS, so you can pass styles objects, like the theme:

  const theme = {
    root: {
      borderRadius: 0
    },
    label: {
      color: 'red'
    }
  }
  <Button theme={theme}>My Special button</Button>

And finally we still have inline styles option, which is of course not to recommend, but yet available, it could look basically the same like the previous example, just attached to the node using style instead of className.

@oliviertassinari
Copy link
Member

I have given more thought on that issue, I'm having a hard time finding a situation where we might need it. We can achieve something very similar with the classes property. I believe that the classes solution offers more flexibility.
I'm removing it from the v1.0.0-beta milestone. I'm even tempted to close it.

@oliviertassinari oliviertassinari removed this from the v1.0.0-beta milestone Jul 6, 2017
@oliviertassinari
Copy link
Member

Now what is X and Y? As of right now, we can't render dynamic styles efficiently, so right now we can go for just a classes map:

I have missed that part, that's the solution we have picked up. So we are good to close 😄 .

@oliviertassinari oliviertassinari modified the milestones: v1.0.0-prerelease, v1.0.0-beta Jul 6, 2017
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

2 participants