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

[FEATURE] Add CssInliner::getMatchingUninlinableSelectors #707

Merged
merged 3 commits into from
Sep 13, 2019

Conversation

JakeQZ
Copy link
Contributor

@JakeQZ JakeQZ commented Sep 12, 2019

Part of #380.

The basic functionality of the method is unit-tested. More extensive unit tests
involving all kinds of CSS rules and HTML already exist for testing the full
content of the <style> element containing the uninlinable rules.

Part of #380.

The basic functionality of the method is unit-tested.  More extensive unit tests
involving all kinds of CSS rules and HTML already exist for testing the full
content of the `<style>` element containing the uninlinable rules.
@JakeQZ JakeQZ added this to the 3.0.0 milestone Sep 12, 2019
@JakeQZ JakeQZ self-assigned this Sep 12, 2019
Copy link
Contributor

@oliverklee oliverklee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the approach. I've added my remarks on some possible smaller improvements.

- Throw exception if `inlineCss` has not been called;
- Put `@media` in backticks in PHPDoc;
- Refactor tests to use a data provider (and added one additional test).
@JakeQZ
Copy link
Contributor Author

JakeQZ commented Sep 13, 2019

Made changes based on suggestions and repushed...

Copy link
Contributor

@oliverklee oliverklee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Just one final polish.

@JakeQZ
Copy link
Contributor Author

JakeQZ commented Sep 13, 2019

Repushed...

Copy link
Contributor

@oliverklee oliverklee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@oliverklee oliverklee merged commit a8b1f9e into master Sep 13, 2019
@oliverklee oliverklee deleted the feature/get-uninlinable-selectors branch September 13, 2019 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants