cssinjs / jss

JSS is an authoring tool for CSS which uses JavaScript as a host language.
https://cssinjs.org
MIT License
7.06k stars 397 forks source link

createUseStyles + @media + Adapting based on props: not updating correctly #1327

Open csantos1113 opened 4 years ago

csantos1113 commented 4 years ago

Expected behavior: When a property is changed, styles that are inside a @media selector are not being re-generated.

Describe the bug: When a property is changed, all styles that are properties-dependent should be re-generated and re-applied to the component.

Steps:

  1. "Swap color" button loads with background red and border/text-shadow blue
  2. Click on the "Swap color" button
  3. Background is changed to "blue" ✅
  4. Text-shadow is changed to "red" ✅
  5. Border is kept "blue" ❌

Codesandbox link: https://codesandbox.io/s/createusestyles-media-vf5zs

Versions (please complete the following information):

trival commented 4 years ago

I want to add, that having multiple instances of a component with different props affecting media queries also seems broken.

Here is a little encanced version of the codesandbox example above: https://codesandbox.io/s/createusestyles-media-g7uvk

RobertAron commented 3 years ago

I think this + https://github.com/cssinjs/jss/issues/1320 + https://github.com/cssinjs/jss/pull/1343 can all be closed. I think the syntax has changed in the newest version so media queries work for all these cases.

https://codesandbox.io/s/admiring-rosalind-ureou?file=/src/App.js

j4mesjung commented 3 years ago

@RobertAron I just tried this case and it doesn't work

const useStyles = createUseStyles({
  test: {
    backgroundColor: (prop) => prop.c
  },
  "@media (min-width: 1024px)": {
    test: {
      backgroundColor: "red"
    }
  }
});

when the window is min width of 1024 I expect it to be red but it's green.

kof commented 3 years ago

@j4mesjung we need to document this, but function rules/values have higher source order specificity, because rendered after static rules

j4mesjung commented 3 years ago

@kof makes sense but how would we structure the css so that the media takes precedence over the function values?

kof commented 3 years ago

Either by also using a function value or by placing it in a separate style sheet that is rendered after.

RobertAron commented 3 years ago

You can also cheat the specificity by doing this

const useStyles = createUseStyles({
  test: {
    backgroundColor: (prop) => prop.c
  },
  "@media (min-width: 1024px)": {
    test: {
      '&&':{
        backgroundColor: "red"
      }
    }
  }
})

The only reason I know that was from looking at other issues to work on and saw this: https://github.com/cssinjs/jss/issues/1045