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

CSS multiple media queries being overidden #803

Closed
cocacrave opened this issue May 19, 2017 · 14 comments
Closed

CSS multiple media queries being overidden #803

cocacrave opened this issue May 19, 2017 · 14 comments
Labels

Comments

@cocacrave
Copy link

cocacrave commented May 19, 2017

v.2.0.0-17

I used this css function many times before to output css like this:

      padding-left: 8px;

      @media only screen and (min-width: 320px) {
        padding-left: calc(8px + 8 * (100vw - 320px) / 704);
      }

      @media only screen and (min-width: 1024px) {
        padding-left: 16px;
      }

      padding-right: 8px;

      @media only screen and (min-width: 320px) {
        padding-right: calc(8px + 8 * (100vw - 320px) / 704);
      }

      @media only screen and (min-width: 1024px) {
        padding-right: 16px;
      }

But with create-react-app, when I do this, the second (padding-right)'s media queries (min-width: 320px and min-width: 1024px) is being overriden by padding-right: 8px; but padding-left works as expected.

In browser it outputs like this:

@media only screen and (min-width: 320px)
.gHTQkl > .frame > .focus > .content {
  padding-left: calc( 320px + 704 * (100vw - 320px) / 704 );
}

.gHTQkl > .frame > .focus > .content {
  padding-left: 8px; // CROSSED OUT
  padding-right: 8px;
}

@media only screen and (min-width: 320px)
.gHTQkl > .frame > .focus > .content {
  padding-right: calc( 320px + 704 * (100vw - 320px) / 704 );  // CROSSED-OUT
}

I made the function to loop 3 times to add padding-left and padding-right to appear next to each other like:

      padding-left: 8px;
      padding-right: 8px;

      @media only screen and (min-width: 320px) {
        padding-left: calc(8px + 8 * (100vw - 320px) / 704);
        padding-right: calc(8px + 8 * (100vw - 320px) / 704);
      }

      @media only screen and (min-width: 1024px) {
        padding-left: 16px;
        padding-right: 16px;
      }

This seems to fix the problem but obviously if I run same function for another css property for example, width, in the same class it won't work.

Any idea why? I'm so confused...

@cocacrave
Copy link
Author

I 'fixed' it by separating out the function calls in same class like this:

  > .frame > .focus > .content {
    ${props => props.theme.mixins.fluid({
      props: ['padding-left', 'padding-right'],
      min: props.theme.scales.gutter.min,
      max: props.theme.scales.gutter.max,
    })}
  }

  > .frame > .focus > .content {
    ${props => props.theme.mixins.fluid({
      props: ['width'],
      min: props.theme.scales.viewport.min,
      max: props.theme.scales.viewport.max,
    })}
  }

@bhough
Copy link
Contributor

bhough commented May 21, 2017

@cocacrave my first guess is that it is an ordering issue, but i'd need to see it in action. Would probably be worth putting together a WebpackBin for us to look at.

@kitten
Copy link
Member

kitten commented May 22, 2017

My first thought is that this seems to be a problem with your CSS specificity and not styled-components or stylis.

It generates the following CSS:

.test {
    padding-left: 8px;
}
@media only screen and (min-width: 320px) {
    .test {
        padding-left: calc(8px + 8 * (100vw - 320px) / 704);
    }
}
@media only screen and (min-width: 1024px) {
    .test {
        padding-left: 16px;
    }
}
.test {
    padding-right: 8px;
}
@media only screen and (min-width: 320px) {
    .test {
        padding-right: calc(8px + 8 * (100vw - 320px) / 704);
    }
}
@media only screen and (min-width: 1024px) {
    .test {
        padding-right: 16px;
    }
}

My guess is that there's something else weird going on in your code, that immediately overwrites these selectors' specificity maybe? I'll keep this open but it doesn't seem to be a bug.

@cocacrave
Copy link
Author

God I'm so confused. Everything works fine now lol. Sorry for the trouble.

@kitten
Copy link
Member

kitten commented May 22, 2017

No worries! Feel free to reply here or ping someone or myself on Gitter if the issue persists :)

@cocacrave
Copy link
Author

@philpl Hey there.

I found out how to cause it. It happens on v2 but it works as expected on v1.
I created a webpackbin
Note: Try changing the version from v1 to v2 and the css breaks.
ie, #box-not-working (redbox) works in v1 but breaks in v2.

What's supposed to happen is redbox should grow width and height as you resize the window. In v2, one of the props's media query (width in this case) gets ignored. You can see this in chrome's styles tab.

Phew.

@cocacrave cocacrave reopened this May 22, 2017
@kitten
Copy link
Member

kitten commented May 22, 2017

@cocacrave Oh awesome job! I was able to figure out the bug. Stylis seems to group the properties since they're in a common namespace, but this destroys the deliberate order. This is why the bug disappears if you duplicate the namespace for each media query in your prop array.

This is the minimal reproduction: https://codesandbox.io/s/Z6om8wA42

This is the generated CSS:

@media only screen and (min-width: 10px) {
    .test > #box-not-working {
        width: calc( 10px + 90px * (100vw - 10px) / 90);
    }
}
@media only screen and (min-width: 90px) {
    .test > #box-not-working {
        width: 90px;
    }
}
.test > #box-not-working {
    background: red;
    padding-left: 8px;
    width: 10px;
    height: 10px;
}
@media only screen and (min-width: 10px) {
    .test > #box-not-working {
        height: calc( 10px + 90px * (100vw - 10px) / 90);
    }
}
@media only screen and (min-width: 90px) {
    .test > #box-not-working {
        height: 90px;
    }
}

@thysultan I'd love it, if you'd have a look :) seems like a weird one this time

@kitten kitten added this to the v2.0 milestone May 22, 2017
@kitten
Copy link
Member

kitten commented May 24, 2017

ping @thysultan :)

@thysultan
Copy link

thysultan commented May 24, 2017

@philpl Do you think s-c v2 can bump the stylis version to V3?

@mxstbr
Copy link
Member

mxstbr commented May 24, 2017

support no semi colons (on by default)

YES, want to submit a PR @thysultan?

@thysultan
Copy link

@mxstbr sure, can you point me to relevant parts where stylis API surface is used.

@k15a
Copy link
Member

k15a commented May 25, 2017

@kitten kitten removed this from the v2.0 milestone May 25, 2017
@kitten
Copy link
Member

kitten commented May 25, 2017

It's not going to go into v2 due to some last minute bugs in stylis v3 :( #829

cc @mxstbr

@mxstbr
Copy link
Member

mxstbr commented Jul 2, 2017

This is fixed now that we've upgraded!

@mxstbr mxstbr closed this as completed Jul 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants