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

critical-selector inefficient with @media rules #12

Closed
jkphl opened this issue May 11, 2017 · 5 comments
Closed

critical-selector inefficient with @media rules #12

jkphl opened this issue May 11, 2017 · 5 comments

Comments

@jkphl
Copy link
Contributor

jkphl commented May 11, 2017

When using critical-selector inside a @media rule, the complete rule content will be seen as critical. Based on your example.css:

@media {
	.foo {
		critical-selector: this;
		display: flex;
		width: calc(100% - 200px);
	}

	.bar {
		border: 10px solid gold;
		height: 100%;
	}

	.baz::before {
		content: 'test';
		position: fixed;
	}
}

I'd expect only the .foo selector to be considered critical, resulting in

@media{.foo{display:flex;width:calc(100% - 200px)}}

However, the whole stylesheet is copied to critical.css.

@zgreen
Copy link
Owner

zgreen commented May 11, 2017

Hmm, yeah. While working on #11 I think I may have found the cause of this; I'll try and roll a fix for this into that work. Thanks for the report.

@jkphl
Copy link
Contributor Author

jkphl commented May 12, 2017

I wish I could help you with fixing this but unfortunately I won't be available for the next 2 weeks. We're about launching a project where we'd need this, so the sooner there's a fix the better. Thank you! :)

@zgreen
Copy link
Owner

zgreen commented May 15, 2017

@jkphl I've got a bit more work to do on it (mostly clean up) but #13 should address this issue (the test passes, currently). I'm going to try and merge it in the next day or two.

@jkphl
Copy link
Contributor Author

jkphl commented May 16, 2017

Sounds great, @zgreen, thanks! Very much looking forward to the next release!

@zgreen
Copy link
Owner

zgreen commented May 19, 2017

Resolved by #13

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