Merge sitewide SEO tags instead of replacing them - #155
Merged
Conversation
The seo partial was an @if/@else, so any page filling the `seo` slot dropped every sitewide tag. Post pages therefore rendered no meta description at all, and newsletter archive pages also lost og:url, og:site_name and og:locale. The partial now always renders the defaults and appends the slot. Pages can override the description through a new `description` prop on x-app-layout. Tags the defaults provide were removed from the post and newsletter slots so the merge doesn't duplicate them. Also: - Point the favicon at /favicon.ico so it stops resolving to /posts/favicon.ico on nested urls. - Drop the hardcoded twitter:card from the newsletter archive, which conflicted with the summary_large_image that RenderOgImageMiddleware appends unconditionally. - Use url()->current() for og:url so it matches the canonical and no longer emits query strings. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The seo partial was an
@if/@else, so any page filling theseoslot dropped every sitewide tag: post pages rendered no meta description at all, and newsletter archive pages also lostog:url,og:site_nameandog:locale. It now always renders the defaults and appends the slot, with pages overriding the description through a newdescriptionprop onx-app-layout, and the tags the defaults provide were removed from the post and newsletter slots so the merge does not duplicate them.Also points the favicon at
/favicon.ico(it resolved to/posts/favicon.icoon nested urls), drops the newsletter archivetwitter:cardthat conflicted with thesummary_large_imageRenderOgImageMiddleware appends unconditionally, and switchesog:urltourl()->current()so it matches the canonical. Despite the branch name, this adds no dependency:laravel/headwould fix this class of bug with its field-by-field merge, but it is at v0.1.0, so these fixes stand on their own for now.