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

Replace Pelago/Emogrifier to InlineStyle #14905

Merged
merged 1 commit into from
Dec 18, 2019
Merged

Replace Pelago/Emogrifier to InlineStyle #14905

merged 1 commit into from
Dec 18, 2019

Conversation

bezumkin
Copy link
Contributor

What does it do?

Replaces Pelago/Emogrifier to InlineStyle

Why is it needed?

While Emogrifier block us from use PHP 7.4 we need to replace it

Related issue(s)/PR(s)

#14899

@JoshuaLuckers
Copy link
Contributor

Is it wise to introduce a new dependency that looks unmaintained?

@bezumkin
Copy link
Contributor Author

@JoshuaLuckers I don't think that this dependency should be maintained actively.
I mean, what could be new for parsing CSS from one tag and putting it inline to another tags?

Anyway, we have other option - just to remove this function from core.
Screenshot 2019-12-16 at 22 42 26

@alroniks
Copy link
Collaborator

@JoshuaLuckers I think there is an example of an advantage using a composer when another one can easily replace one tool.

@Jako
Copy link
Collaborator

Jako commented Dec 17, 2019

Switching to a probably unmaintained but working repository is IMHO a straightforward solution. If the code changes to switch to that dependency are that minimal, I am fine with the solution.

Using an active repository of maintainers, that don't have understand the version constraint rules of composer, should be avoided. All the badges in the MyIntervals/emogrifier repository don't hide this strange composer rules.

@opengeek opengeek merged commit 959270c into modxcms:3.x Dec 18, 2019
@bezumkin bezumkin deleted the inline-style branch June 1, 2020 12:31
@opengeek opengeek added this to the v3.0.0-alpha3 milestone Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants