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

Multiple resolutions causes order of css to change when @media rules are used #415

Closed
wildhart opened this issue Nov 6, 2019 · 1 comment · Fixed by #428
Closed

Multiple resolutions causes order of css to change when @media rules are used #415

wildhart opened this issue Nov 6, 2019 · 1 comment · Fixed by #428
Assignees
Labels

Comments

@wildhart
Copy link

wildhart commented Nov 6, 2019

I'm generating the critical CSS for my landing page for multiple screen dimensions. If the input CSS contains @media screen size selectors, then the order of the rules in the critical css generated output can change depending on the order of the screen dimensions list.

This is causing my landing page to render incorrectly on certain screen sizes.

This an excerpt from my input CSS (generated from SCSS):

#sales-index-top p {
  font-size: 2.7vw;
  ...other css rules  }
  @media (max-width: 518px) {
    #sales-index-top p {
      font-size: 14px; } }
  @media (min-width: 800px) {
    #sales-index-top p {
      font-size: 21.6px; } }

The expected result is that the text size scales automatically with screen size (using vw units), but with a minimum and maximum size using @media queries.

Here's my dimensions list:

const dimensions = '1440x900,960x600,600x960,412x732,320x568'.split(',').map(dims => {
  const [width, height] = dims.split('x');
  return {width, height}
});

This results in the following (unminified) critical CSS, (note the line numbers):

115 @media (min-width: 800px) {
116   #sales-index-top p {
117     font-size: 21.6px;
118   }
119 }
...
236 #sales-index-top p {
237   font-size: 2.7vw;
         ... other css rules...
240 }
241 @media (max-width: 518px) {
242   #sales-index-top p {
243     font-size: 14px;
244   }
245 }

On large screens the generic rule is overriding the @media (min-width: 800px) rule because it comes later in the file, so my font is too big.

If I reverse the order of my dimenions list then the resulting css is as expected (but there could be other order changes lurking in there).

The expected result is that the ultimate order of css rules remains unchanged regardless of the order that the different screen dimensions are rendered in.

I am using critical@next (2.0.0-23)

@wildhart wildhart changed the title Multiple resolutions causes order of css to change Multiple resolutions causes order of css to change when @media rules are used Nov 6, 2019
@bezoerb bezoerb added the bug label Nov 6, 2019
@bezoerb bezoerb self-assigned this Nov 6, 2019
@bezoerb
Copy link
Collaborator

bezoerb commented Nov 12, 2019

Good catch @wildhart. I've made some tests and it seems the order of the dimensions needs to be from small to wide. I'm enforcing this now by sorting the array so the order in which it's passed to critical doesn't matter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants