-
Notifications
You must be signed in to change notification settings - Fork 506
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
Preload links & Custom Tags #300
base: master
Are you sure you want to change the base?
Conversation
src/SEOTools/SEOMeta.php
Outdated
// if $href is empty jump to next | ||
if (empty($href)) { | ||
continue; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this check is unnecessary: since the addPreload
function already requires both href
& as
to be present.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, you are right. removing it.
src/SEOTools/Facades/SEOMeta.php
Outdated
@@ -18,6 +18,8 @@ | |||
* @method static \Artesaos\SEOTools\Contracts\MetaTags addKeyword(string $keyword) | |||
* @method static \Artesaos\SEOTools\Contracts\MetaTags removeMeta(string $key) | |||
* @method static \Artesaos\SEOTools\Contracts\MetaTags addMeta(array|string $meta, string|null $value = null, string $name = 'name') | |||
* @method static \Artesaos\SEOTools\Contracts\MetaTags addPreload(string $href, string $as) | |||
* @method static \Artesaos\SEOTools\Contracts\MetaTags addCustom(array|string $meta) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* @method static \Artesaos\SEOTools\Contracts\MetaTags addCustom(array|string $meta) | |
* @method static \Artesaos\SEOTools\Contracts\MetaTags addCustom(string $meta) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for very much for your contribution! I have reviewed your submit an found a few minor things that do need to be addressed. Please review then and adjust them where needed.
updated the files as per your comments. Thanks! |
Very nice @sp4r74cus |
If he doesn't make any tests; I will make them in a different commit; since I think this would be an great feature to add to make this package more universal. |
There is a conflict @J-Brk Also, shold be good do the test with it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix conflicts
Dear @vinicius73 & @J-Brk, |
|
Added support for preload links:
https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/rel/preload
Also, to have the ability to add any other meta tag that is not yet supported i added a custom array that takes a raw html tag and adds it in the header.