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

[5.4] Check Htmlable contract instead of instanceof HtmlString in Mailer #18459

Merged
merged 2 commits into from
Mar 23, 2017
Merged

[5.4] Check Htmlable contract instead of instanceof HtmlString in Mailer #18459

merged 2 commits into from
Mar 23, 2017

Conversation

jbraud
Copy link
Contributor

@jbraud jbraud commented Mar 23, 2017

Check the Htmlable contract instead of the concrete HtmlString class when rendering the view in the Mailer class.

Reason being, it then allows packages to hook into the Mailer code without needing to inherit from the class, which is an issue I've come across.

@jbraud jbraud changed the title Check Htmlable contract instead of instanceof HtmlString in Mailer [5.4] Check Htmlable contract instead of instanceof HtmlString in Mailer Mar 23, 2017
use Illuminate\Contracts\View\Factory;
use Illuminate\Contracts\Events\Dispatcher;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Contracts\Mail\Mailer as MailerContract;
use Illuminate\Contracts\Queue\Factory as QueueContract;
use Illuminate\Contracts\Mail\Mailable as MailableContract;
use Illuminate\Contracts\Mail\MailQueue as MailQueueContract;
use Illuminate\Contracts\Support\Htmlable as HtmlableContract;
Copy link
Member

@JosephSilber JosephSilber Mar 23, 2017

Choose a reason for hiding this comment

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

There's no need to alias this (the others have been aliased due to conflicts or ambiguity).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair, spot of auto-pilot there. I'll sort that.

@taylorotwell taylorotwell merged commit c462ee9 into laravel:5.4 Mar 23, 2017
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.

3 participants