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

L2 optimization drops some new css styles #1124

Closed
vladmandic opened this issue Sep 7, 2020 · 7 comments
Closed

L2 optimization drops some new css styles #1124

vladmandic opened this issue Sep 7, 2020 · 7 comments

Comments

@vladmandic
Copy link

Like the subject line says...
Note that content-visibility is a new style included in latest versions of browsers.

Input CSS

.listitem { display: flex; scroll-snap-align: start; scroll-margin-top: 0; content-visibility: auto; }

Actual output CSS

incorrect with 2: { all: true }

.listitem{scroll-snap-align:start}

Expected output CSS

correct with 2: { all: false }

.listitem{display:flex;scroll-snap-align:start;scroll-margin-top:0;content-visibility:auto}
@jakubpawlowicz
Copy link
Collaborator

@vladmandic Can you please include more details, like a bigger CSS snippet? It's rather not possible clean-css removes display, scroll-margin-top and content-visibility, but more likely it removes them because later in a stylesheet .listitem is redefined, but I can't tell just from what you posted above. Thanks!

@vladmandic
Copy link
Author

i cannot see any redefinition of that style.

actual css can is at https://github.com/vladmandic/pigallery/blob/master/client/pigallery.css
and problematic line is:
.listitem { display: flex; scroll-snap-align: start; scroll-margin-top: 0; content-visibility: auto; }

modifying server/build.js to include level: { 1: { all: true }, 2: { all: true } }
results in:

2021-01-15 07:44:18 STATE:  Client CSS rebuild: 3505 ms imports 552789 byes outputs 452788 bytes

and resulting dist/pigallery.css containts:
.listitem{scroll-snap-align:start}

modifying server/build.js to include level: { 1: { all: true }, 2: { all: false } }
results in:

2021-01-15 07:45:50 STATE:  Client CSS rebuild: 1858 ms imports 552789 byes outputs 473508 bytes

and resulting dist/pigallery.css containts:
.listitem{display:flex;scroll-snap-align:start;scroll-margin-top:0;content-visibility:auto}

@jakubpawlowicz
Copy link
Collaborator

Thanks @vladmandic I'll check why it happens.

@jakubpawlowicz
Copy link
Collaborator

So it's a bug in restructuring.

@jakubpawlowicz
Copy link
Collaborator

On the second thought it may not be, as here's the output from both clean-css 4.2 and main branch (soon 5.0). You can see

.divider,
.listitem {
  scroll-snap-align: start
}

at the beginning and then

.listitem {
  display: flex;
  scroll-margin-top: 0;
  content-visibility: auto
}

further down (note: I have formatted output of both).

Could you please check which version of clean-css are you running?

output.tar.gz

@vladmandic
Copy link
Author

@jakubpawlowicz i'm running clean-css 4.2.3

i've tested again and yes, the rule is correct but split into two separate rules. for some reason, it all works now - i know it didn't use to work with older chrome when i opened this issue 4 months ago.

thanks.

@jakubpawlowicz
Copy link
Collaborator

Cool, I'm glad it works fine now. Please reopen if needed, and sorry it took me a while to have a look at it.

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

No branches or pull requests

2 participants