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 HtmlPruner::removeRedundantClassesAfterCssInlined #724

Merged
merged 2 commits into from
Sep 28, 2019

Conversation

JakeQZ
Copy link
Contributor

@JakeQZ JakeQZ commented Sep 27, 2019

Also added section for HtmlPruner to the README.

Closes #380.

@JakeQZ JakeQZ added this to the 3.0.0 milestone Sep 27, 2019
@JakeQZ JakeQZ self-assigned this Sep 27, 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.

Looks very good. Just some suggestions to help static code analysis and to help reduce the cognitive load when reading the tests.

tests/Unit/Emogrifier/HtmlProcessor/HtmlPrunerTest.php Outdated Show resolved Hide resolved
tests/Unit/Emogrifier/HtmlProcessor/HtmlPrunerTest.php Outdated Show resolved Hide resolved
@JakeQZ
Copy link
Contributor Author

JakeQZ commented Sep 28, 2019

Have made suggested changes 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! Could you please rebase to resolve the conflict?

Also added section for `HtmlPruner` to the README.

Closes #380.
- Return numeric array and use `list` to collate the return value, in the case
  of the ‘multiple return values problem’ (rather than `compact`/`extract`);
- Added helper assertion methods: `assertContainsNone`, `assertContainsAll`.
@JakeQZ JakeQZ force-pushed the feature/remove-redundant-classes-after-css-inlined branch from d2c599c to 4441dc0 Compare September 28, 2019 12:24
@JakeQZ
Copy link
Contributor Author

JakeQZ commented Sep 28, 2019

Looks good! Could you please rebase to resolve the conflict?

Have done. (Now forewarned and forearmed, git rebase master worked seamlessly, with forced push.)

@oliverklee oliverklee merged commit a358450 into master Sep 28, 2019
@oliverklee oliverklee deleted the feature/remove-redundant-classes-after-css-inlined branch September 28, 2019 13:42
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.

remove the class="xxx" from html after converting to inline-css
2 participants