facebook / stylex

StyleX is the styling system for ambitious user interfaces.
https://stylexjs.com
MIT License
8.22k stars 303 forks source link

Media query specificity order. Certian properties broken (`gridColumn`) #517

Open predaytor opened 2 months ago

predaytor commented 2 months ago

Describe the issue

There seems to be a bug related to gridColumn specifically when defining <grid-line> values when mixing <integer> and span values for different media conditions, instead of using just one version of the syntax.

The error also occurs when using the value 1 / -1, not sure why. (1st test case in playground).

(it's hard to debug why this error occurs or if it might be related to other properties like gridArea).

Bug: '@media (max-width: 1024px)': '1 / 3' has higher specificity order then '@media (max-width: 768px)': '1 / -1'.

This happens only between intermediate conditions (those after the default case).

const styles = stylex.create({
  // ...

  x: {
    backgroundColor: 'pink',

    gridColumn: {
      default: '1 / 2',
      '@media (max-width: 1024px)': '1 / 3',
      '@media (max-width: 768px)': '1 / -1',
    },
  },
});

Expected behavior

'@media (max-width: 768px)': '1 / -1' should override previous condition '@media (max-width: 1024px)': '1 / 3'

Steps to reproduce

Inspect the order specificity on gridColumn property between media conditions.

Test case

https://stackblitz.com/edit/remix-run-remix-qzzvuu?file=app%2Froutes%2F_index.tsx

Additional comments

Also tested on the official Stylex + Next sandbox, this issue directly affects StyleX.

nmn commented 2 months ago

This is a known issue. As of right now, you're responsible for ensuring that your media queries do not conflict.

const styles = stylex.create({
  // ...

  x: {
    backgroundColor: 'pink',

    gridColumn: {
      default: '1 / 2',
      '@media (min-width: 768px) and (max-width: 1024px)': '1 / 3',
      '@media (max-width: 768px)': '1 / -1',
    },
  },
});
predaytor commented 2 months ago

@nmn, thx for the solution, but I think to avoid a 1px window where none of the conditions are met, we should add 1px to the lowest boundary, like @media (min-width: 769px) and (max-width: 1024px). Also curious, did this bug affect any other properties? So far I've only noticed this for gridColumn.

nmn commented 2 months ago

Don't use @media (min-width: 769px) and (max-width: 1024px). It won't account for hi-dpi displays and you'll get a 1px dead zone. If you really want to avoid the overlap, use a .1 instead. @media (min-width: 768.1px) and (max-width: 1024px)

Also curious, did this bug affect any other properties? So far I've only noticed this for gridColumn.

The media queries are resolved the same way for all properties.