-
Notifications
You must be signed in to change notification settings - Fork 32
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
Add optional origin to withUrl #38
base: master
Are you sure you want to change the base?
Conversation
tests/Pest/ManagerTest.php
Outdated
->toContain('<meta property="og:url" content="https://foo.com/testing/5" />') | ||
->toContain('<link rel="canonical" href="https://foo.com/testing/5" />'); |
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.
Can you link MDN docs or something where I can see that both og:url
and canonical
should be the canonical link?
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.
The package is already coupling og:url and canonical together. All my pull request does is give users the option to manually set the origin.
laravel-seo/resources/views/components/meta.blade.php
Lines 31 to 34 in f6fd5f4
@if(seo('url')) | |
<meta property="og:url" content="@seo('url')"> | |
<link rel="canonical" href="@seo('url')"> | |
@endif |
@@ -224,9 +224,13 @@ public function favicon(): static | |||
} | |||
|
|||
/** Append canonical URL tags to the document head. */ | |||
public function withUrl(): static | |||
public function withUrl(?string $origin = null): static |
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.
Is origin the right term here, or would canonical be more accurate?
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.
This RFC says the scheme/host/port combination is an origin. Note that's all we're passing in here. The full canonical url is still derived from the request path.
This pull request adds an optional
$origin
parameter to thewithUrl()
method allowing users to set a fixed origin such ashttps://foo.com
. This useful to prevent duplicate content issues with publicly accessible test environments or underlying serverless functions that alias to your domain.