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

[TASK] Deprecate the removal of invisible nodes #473

Merged
merged 1 commit into from
Jan 10, 2018
Merged

Conversation

oliverklee
Copy link
Contributor

Relates to #252

@oliverklee oliverklee added this to the 2.1.0 milestone Jan 7, 2018
@oliverklee oliverklee self-assigned this Jan 7, 2018
@oliverklee
Copy link
Contributor Author

@jjriv @zoliszabo I'd like to have feedback from both of your before this gets merged.

Note: I'm thinking about creating a new class HtmlMinifier that then will provide methods for removing invisible nodes and removing class attributes (both with separate methods).

Copy link
Contributor

@jjriv jjriv left a comment

Choose a reason for hiding this comment

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

I think Emogrifier should preserve the original contents of the email while also making it email friendly. This includes keeping HTML elements with display equal to none.

I do like the idea of adding a minifier class. This would keep the purposes for each class seperate. And minification is a nice feature. But also see my comments on the original issue because I don't want it to become something that is a pain to support. The benefits need to outweigh the efforts required to support it.

@oliverklee
Copy link
Contributor Author

Now that we agree on the roadmap, a review and merge would be nice. 😄

@jjriv jjriv merged commit 1ce47dc into master Jan 10, 2018
@jjriv jjriv deleted the task/remove-invisible branch January 10, 2018 21:04
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 this pull request may close these issues.

2 participants