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.

Sign up for GitHub

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

Jump to bottom

CSS inlining doesn't work when including image on Laravel 10 and Laravel 9 #311

Closed
bbprojectnet opened this issue Jun 22, 2023 · 11 comments
Closed

CSS inlining doesn't work when including image on Laravel 10 and Laravel 9 #311

bbprojectnet opened this issue Jun 22, 2023 · 11 comments
Labels
Feedback needed

Comments

@bbprojectnet
Copy link

Hi,

In Laravel 10 inliner don't work at all.

After a short look at the code, I came up with what to improve to make it work with Laravel 10:

CssInlinerPlugin.php, line 99:

} elseif ($body instanceof AlternativePart || $body instanceof MixedPart) {

change to:

} elseif ($body instanceof AbstractMultipartPart) {

I haven't tested it on a larger scale (e.g. with Laravel 9), I'm posting it as a suggestion.
@DannyvdSluijs
Copy link
Collaborator

Thanks for reporting the issue @bbprojectnet since you already found a solution do you think you can make a pull request for this?

@danhanly danhanly mentioned this issue Aug 23, 2023
Merged
@danhanly
Copy link
Contributor

@bbprojectnet I've created the PR, and included your suggested changes.

When you were working on this, did you get this to work with Laravel 10 by making your suggested change?

@bbprojectnet
Copy link
Author

When you were working on this, did you get this to work with Laravel 10 by making your suggested change?

Yes.

I've created the PR, and included your suggested changes.

Thanks a lot! :)

@kirkbushell
Copy link

Just to reiterate here that I have it working on Laravel 10 without this change.

@bbprojectnet
Copy link
Author

What do you mean by working? Yes, there is no error, but in my case inline css was not added.

@kirkbushell
Copy link

What do you mean by working? Yes, there is no error, but in my case inline css was not added.

As in, this change isn't necessary for the package to do its thing. I had no issues.

@DannyvdSluijs
Copy link
Collaborator

Hi @bbprojectnet

Since time has passed I've learned more about the issues and more often it seems to be a local issue. I've created a small Laravel10MailCssInliner to show the correct wokring of this package in combination with Laravel 10.

Using the SendEmail command (php artisan app:send-email) you can check the logs (storage/logs) where you can see the inlined version of the css which in the view is linked using the link tag.

I recommend you take another look into the problems you're facing and see what the differences are compared to the playground. You can always share you problems here and people could volenteer to help you out.

@bbprojectnet
Copy link
Author

Of course, there are some differences between the example you provided and my code, for example that my email contains both HTML and plain text. I will try to re-examine the matter and point out any differences.

@bbprojectnet
Copy link
Author

bbprojectnet commented Sep 18, 2023 •
edited
Loading

Ok, i figured it out, please add something like this to your test-email.blade.php:

<html>
<head>
    <link rel="stylesheet" type="text/css" href="{{ asset('css/main.css') }}">
</head>
<body>
<h1>Hey you</h1>
<img src="{{ $message->embed(resource_path('logo.png')) }}" />
</body>
</html>

result:

obraz

No inline styles.

Now, apply "patch" from first comment, and works again ;)

@DannyvdSluijs
Copy link
Collaborator

DannyvdSluijs commented Sep 18, 2023 •
edited
Loading

Brilliant this seems to take a different turn in code indeed.
@bbprojectnet thanks for the quick and helpful feedback.

In the Playground example the $body is a Symfony\Component\Mime\Part\TextPart whereas with your addition it changes to a Symfony\Component\Mime\Part\Multipart\RelatedPart which is extending the Symfony\Component\Mime\Part\Multipart\AbstractMultipartPart which is indeed the reason why the proposed fix is working.

I can see the TextPart is also extending the AbstractPart which makes that I would like to take some time and look into the class hierarchy and see if we can improve the suggested changes. And also take Laravel 9 into account.

@DannyvdSluijs DannyvdSluijs changed the title Laravel 10 issue CSS inlining doesn't work when including image on Laravel 10 and Laravel 9 Sep 18, 2023
@DannyvdSluijs
Copy link
Collaborator

This has been fixed in #313

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feedback needed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@kirkbushell @DannyvdSluijs @bbprojectnet @danhanly