Boolean + Responsive variants overriding with `false` have unexpected behaviour

[LimeWub]

Bug report

Describe the bug

Hello ^^ Firstly thanks for developing and maintaining stitches, it's a great library for doing CSSinJS and I've enjoyed using it at my work :)

I've spotted a weird bug with using responsive variants in combination with variants which expect boolean values.

In our codebase, we were trying to get a Button to be fullWidth on mobile but revert to its natural width on >sm breakpoints. We were using code like so <Button fullWidth={{'@initial': true, '@sm': false}} /> which should be working to do exactly what we wanted, but what we were actually getting was the button just staying as fullWidth=true and ignoring the @sm false value. We debugged it a bit and noticed that adding an explicit false version for the variant in the <Button /> component was making this work correctly and the button was reverting to non-fullWidth in sm.

I have replicated this behaviour to demonstrate the point in the codesanbox attached below.

To Reproduce

Reduced case code sandbox: https://codesandbox.io/s/jovial-wozniak-msfzhf?file=/src/App.js

1. Go to App.js
2. Resize the preview window
3. Notice that the first <Button /> is not respecting the fullWidth=false override on the @sm breakpoint (<- This is the bug)
4. Notice how overriding the @sm breakpoint with @md has the same problem in the second button (so the problem is with overriding all true values)
5. Notice how overriding @initial in non-boolean properties seems to work as expected in the third <Button />.
6. Go to Button.tsx
7. Uncomment the explicit styling for false
8. Resize the preview window
9. Notice the first button override of @initial now works. (Huh)

Expected behavior

I would expect overriding 'true' @initial to work using the prop syntax alone. I do not expect to specifically have to 'unset' my CSS when I just want a false value of a boolean variant to apply on a breakpoint. I believe there is a bug with overriding smaller breakpoints setting a true value on a boolean variant.

Please, do let me know if you'd like me to clarify on anything or if I've done something wrong in the code ^^

System information

• OS: macOS
• Browser (if applies) Chrome
• Version of Stitches: latest (1.2.8)

[hadihallak]

Hey @LimeWub Thank you so much for the detailed issue and the codesandbox this helps a lot.

Took a quick look at it and i believe this is working as intended and your intuition to fixing it was correct. The initial breakpoint is always visible unless it was overridden by another variant on an another breakpoint so unless you apply styles to the false variant that would revert the changes that you had in the true variant, you are going to get results similar to what you experienced

[LimeWub]

Hey @hadihallak

Thanks for reading this and having a look :)

Sooo, I don't know at which point you checked but I edited this shortly after posting as I noticed the second button which does the overriding without using @initial is actually also broken and not working correctly. ( I hadn't noticed that originally, I thought it was working as expected, which it wasn't - my bad)

So the second button, which doesn't use @initial, is setting @sm to fullWidth: true then @md to fullWidth: false - without the explicit false version of the variant this doesn't work either. If false is defined it works correct.

Which leads me to believe this is a "problem" with overwriting any true variant on a bigger breakpoint.

Could you please take another look?

[LimeWub]

Unless what you're saying here...

The initial breakpoint is always visible unless it was overridden by another variant on an another breakpoint so unless you apply styles to the false variant that would revert the changes that you had in the true variant, you are going to get results similar to what you experienced

...means that this is the case for all overrides of css properties in bigger breakpoints?

The same way that if we used a media query in CSS to set something then we need to unset on a bigger breakpoint; we do so by setting something on the property again on the bigger breakpoint?

(I thought stitches was doing something smart with these breakpoints behind the scenes but if it's just translated to css media queries then this would make sense)

[hadihallak]

yeah we do that to be efficient with how much css we inject so another (un-realistic on purpose) example would be something like this:

const Button = styled('button', {
    variants: {
      variant: {
        red: {
          backgroundColor: 'red',
          color: 'red',
        },
        blue:  {
          backgroundColor: 'blue',
        },,
      }
    }
  }
)

so now if you render the following button

<Button variant={{"@bp1": "red", "@bp2": "blue"} />

it would translate to something like this:

@media(min-width: 520px){
  .random-hash-variant-bp1-red{
    background-color: red;
    color: red;
  }
}

@media(min-width: 900px){
.random-hash-variant-bp2-blue{
    background-color: blue;
  }
}

// this button will have a blue background and a red text
<button class="random-hash-variant-bp1-red random-hash-variant-bp2-blue" />

Hopefully this should better illustrate what happens behind the scenes and why it would be better to have different values of the same variant be setting the same css properties because if the blue variant did set the color css property then it wouldn't have had the red color leaking from the red variant

[LimeWub]

Thanks for explaining this @hadihallak, I hadn't realised this is how it worked behind the scenes. I thought it would be adding/removing the class with the breakpoint + some magic; but it makes sense that perhaps that would be more complex than needed for most cases now that you explain it.

In which case adding false variations to our boolean variants is probably the way to go. Do you think there's any problem that could be caused when adding false styles in to variants that actually don't utilise them to future proof? ie. Do you think there will be lots of unnecessary css generated for the false unused variants? Would you recommend only adding the false when needed?