cssinjs / styled-jss

Styled Components on top of JSS.
http://cssinjs.org/styled-jss
MIT License
217 stars 25 forks source link

Media queries don't work when using theming function #66

Open aloker opened 6 years ago

aloker commented 6 years ago

When I provide a literal style object, media queries are considered.

const ResponsiveOk = styled("div")({
  color: "green",
  "@media screen and (min-width: 400px)": {
    color: "red"
  }
});

If I use a theme callback instead, media queries do not work anymore:

const ResponsiveNotOk = styled("div")(({ theme }) => ({
  color: "green",
  "@media screen and (min-width: 400px)": {
    color: "red"
  }
}));

Demo: https://codesandbox.io/s/m9v69xvo3x Resize the output window above/below 400 pixel to see the difference: the top row switches between red and green, the bottom row doesn't.

Am I missing something conceptually here or is this supposed to work?

kof commented 6 years ago

Looks like a bug

lttb commented 6 years ago

Hi! Thank you for the issue, it looks like a bug with @media and function rules in jss, described here: https://github.com/cssinjs/jss/issues/706

vbersch commented 6 years ago

The same problem exists for '&:hover', '&:before' and '&:after'. Those all do work in jss in combination with theming.

MarcusNotheis commented 6 years ago

Example snippet for @vbersch's issue: this works:

const Link = styled('a')({
  color: '#24292e',
  '&:before': {
    content: '"*"'
  }
})

this doesn't:

const Link = styled('a')((props) => ({
  color: '#24292e',
  '&:before': {
    content: '"*"'
  }
}))

Both patterns are working fine in react-jss

jmadson commented 5 years ago

I love this JSS plugin! Unfortunately we're experiencing the same issue. Our team has been able to work around it by providing an object which contains functions when the props/theme are needed, but I feel it would be more efficient if we could provide a function which returns a single object instead.

For example (modifying the above for illustration purposes), this works:

const Link = styled('a')({
  color: ({ theme }) => theme.palette.primary.main
  textDecoration: "none",
  '&:hover': {
    textDecoration: "underline"
  }
})

But this does not:

const Link = styled('a')(({ theme }) => ({
  color: theme.palette.primary.main
  textDecoration: "none",
  '&:hover': {
    textDecoration: "underline"
  }
}))
likethemammal commented 5 years ago

Still experiencing the bug with '&:hover' as @MarcusNotheis described

HenriBeck commented 5 years ago

We are currently working on porting this implementation to v10 of jss and also moving it into react-jss in cssinjs/jss#1094

HenriBeck commented 5 years ago

There will be no more work done in this repo. I will add a test case for this bug for the new styled API to see if it's fixed there.

likethemammal commented 5 years ago

@HenriBeck Ah okay. Amazingly quick response, thanks!