-
Notifications
You must be signed in to change notification settings - Fork 0
hotfix: Work on headers #54
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
Conversation
WalkthroughCentralizes SEO URL/image handling by adding Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Page as Page Component
participant SEO as src/support/seo.ts (useSeo / useSeoFromPost)
participant Util as siteUrlFor
participant DOM as Meta/JSON-LD
User->>Page: navigate to route
Page->>SEO: useSeo({ title, description, image: ABOUT_IMAGE, url: siteUrlFor(path), jsonLd })
SEO->>Util: siteUrlFor(path)
Util-->>SEO: absolute URL (SITE_URL + path)
SEO->>DOM: apply meta tags & JSON-LD
sequenceDiagram
autonumber
participant Client as Browser
participant Caddy as Caddy (Static SPA)
participant FS as Filesystem
Client->>Caddy: GET /some/path
Caddy->>FS: try {path}
alt found
FS-->>Caddy: 200 {path}
Caddy-->>Client: serve {path}
else not found
Caddy->>FS: try {path}/index.html
alt found
FS-->>Caddy: 200 {path}/index.html
Caddy-->>Client: serve {path}/index.html
else not found
Caddy->>FS: try /index.html
FS-->>Caddy: 200 /index.html
Caddy-->>Client: serve /index.html
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/support/seo.ts (1)
191-209: Make JSON-LD fields absolute and complete for Articles.Ensure image is absolute (like OG tags) and include url within the JSON-LD for consistency.
Apply this diff:
return { title: value.title, description: value.excerpt, - image: value.cover_image_url, + image: new URL(value.cover_image_url, SITE_URL).toString(), type: 'article', url: siteUrlFor(`/post/${value.slug}`), jsonLd: { '@context': 'https://schema.org', '@type': 'Article', headline: value.title, description: value.excerpt, - image: value.cover_image_url, + image: new URL(value.cover_image_url, SITE_URL).toString(), + url: siteUrlFor(`/post/${value.slug}`), datePublished: value.published_at, author: { '@type': 'Person', name: SITE_NAME, }, }, } satisfies SeoOptions;src/pages/AboutPage.vue (1)
100-103: Minor: computed without reactive deps.aboutPicture is a constant; computed adds overhead. Inline the import or use a const.
Apply this diff:
-const aboutPicture = computed<string>(() => { - return AboutPicture; -}); +const aboutPicture = AboutPicture as string;src/pages/PostPage.vue (2)
59-59: Encode share query params.Unencoded title/URL can break with spaces or unicode. Encode both for X and LinkedIn.
Apply this diff:
-:href="`https://www.linkedin.com/sharing/share-offsite/?url=${fullURLFor(post)}`" +:href="`https://www.linkedin.com/sharing/share-offsite/?url=${encodeURIComponent(fullURLFor(post))}`"-const xURLFor = (post: PostResponse) => { - return `https://x.com/intent/tweet?url=${fullURLFor(post)}&text=${post.title}`; -}; +const xURLFor = (post: PostResponse) => + `https://x.com/intent/tweet?url=${encodeURIComponent(fullURLFor(post))}&text=${encodeURIComponent(post.title)}`;Also applies to: 154-156, 158-158
160-170: Optional: feature-detect Web Share API.Guard navigator.share to avoid noisy errors on unsupported browsers; fallback to opening the LinkedIn share URL.
async function sharePost(post: PostResponse) { const shareData = { title: post.title, text: post.excerpt, url: fullURLFor(post), }; try { - await navigator.share(shareData); + if (navigator.share) { + await navigator.share(shareData); + } else { + window.open(`https://www.linkedin.com/sharing/share-offsite/?url=${encodeURIComponent(shareData.url)}`, '_blank', 'noopener,noreferrer'); + } } catch (err) { console.error("Couldn't share the post:", err); } }src/pages/SubscribePage.vue (1)
172-172: Fix grammar in the descriptionChange “to updates of articles” → “for updates on articles”.
Apply this diff:
- description: `Subscribe to ${SITE_NAME}'s newsletter to updates of articles and cool things he is working on.`, + description: `Subscribe to ${SITE_NAME}'s newsletter for updates on articles and cool things he is working on.`,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
caddy/WebCaddyfile.internal(1 hunks)index.html(1 hunks)src/pages/AboutPage.vue(2 hunks)src/pages/HomePage.vue(1 hunks)src/pages/PostPage.vue(2 hunks)src/pages/ProjectsPage.vue(2 hunks)src/pages/ResumePage.vue(2 hunks)src/pages/SubscribePage.vue(1 hunks)src/support/seo.ts(4 hunks)
🔇 Additional comments (11)
index.html (1)
27-35: Good alignment: give JSON-LD a stable DOM handle.Adding id="seo-jsonld" matches the runtime updater in Seo.setJsonLd. No issues spotted.
caddy/WebCaddyfile.internal (1)
31-31: SPA routing fallback improved.Adding {path}/index.html helps nested routes resolve without 404s. Looks good.
src/support/seo.ts (1)
214-216: Utility export is clear and reusable.siteUrlFor centralizes absolute-URL generation. Good addition.
src/pages/AboutPage.vue (1)
90-90: SEO centralization: good move.Using ABOUT_IMAGE and siteUrlFor('/about') keeps metadata consistent. Looks good.
Also applies to: 110-115
src/pages/PostPage.vue (2)
117-118: URL generation aligned with siteUrlFor.Import and usage look correct; canonical/share URLs now derive from the centralized helper.
Also applies to: 158-159
1-112: Verify router path switch to /post/:slug — unable to confirm automatically.Sandbox search returned "No files were searched"; run these locally and confirm there are no stale "/posts" usages and the router uses "/post/:slug":
- Check: src/pages/PostPage.vue (lines 1–112).
- Commands to run locally:
- rg -n "/posts\b" || git grep -n "/posts"
- rg -nP "path\s*:\s*['"]/post\b" || git grep -nE "path\s*:\s*['"]/post"
- rg -n 'to="/post' || rg -n "to='/post"
- rg -n "xURLFor|fullURLFor"
- If any "/posts" occurrences are found: update routes/links to "/post/:slug" and add a redirect from "/posts" → "/post/:slug" for backward compatibility.
src/pages/HomePage.vue (1)
56-66: Consistent SEO config for Home.ABOUT_IMAGE and siteUrlFor('/') wired correctly. All good.
src/pages/ProjectsPage.vue (1)
68-80: Consistent SEO config for Projects.ABOUT_IMAGE and siteUrlFor('/projects') look correct.
src/pages/ResumePage.vue (1)
58-72: Consistent SEO config for Resume.ABOUT_IMAGE and siteUrlFor('/resume') look correct.
src/pages/SubscribePage.vue (2)
166-166: Centralized SEO imports look goodAdopting
siteUrlForandABOUT_IMAGEkeeps SEO config consistent across pages.
170-171: Absolute URLs are produced; confirm single canonical/og:url and normalize trailing‑slash
- Evidence: src/support/seo.ts defines ABOUT_IMAGE = '/images/profile/about.jpg' and siteUrlFor(path) => new URL(path, SITE_URL).toString(); useSeo resolves options.image with new URL(...) and calls setLink('canonical', url) and setMetaByProperty('og:url', url). So image: ABOUT_IMAGE and url: siteUrlFor('/subscribe') become absolute URLs.
- Actionable checks: continue passing explicit siteUrlFor(...) (or normalize window.location.pathname) to enforce a single trailing‑slash policy; run a repo‑wide search in layouts/SSR/templates for any other emissions of rel="canonical" or og:url to ensure only one canonical and one og:url are emitted.
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.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/pages/HomePage.vue (2)
63-65: Canonical URL generation: validate absolute URL + slash normalization.Ensure
siteUrlFor('/')yields a single, HTTPS, absolute URL and thatsiteUrlFornormalizes leading/trailing slashes to avoid//. If not, normalize inside the helper so callers can pass either/pathorpath.Would you like a tiny unit test for
siteUrlForpath edge cases?
66-69: JSON‑LD: consider enriching minimal WebPage data.If
useSeodoesn’t auto‑inject@context/url/image, add them for richer snippets.Apply this diff within the JSON‑LD block:
- jsonLd: { - '@type': 'WebPage', - name: 'Home', - }, + jsonLd: { + '@type': 'WebPage', + '@id': siteUrlFor('/#webpage'), + url: siteUrlFor('/'), + name: 'Home', + image: ABOUT_IMAGE, + },If
useSeoalready merges these, ignore.src/pages/ResumePage.vue (2)
69-71: Canonical URL for Resume: double‑check normalization and absolute form.Same note as Home: ensure
siteUrlFor('/resume')is absolute and normalized; keep a consistent trailing‑slash policy across pages.
72-75: ProfilePage JSON‑LD: add Person as main entity.Boost semantics by linking the page to the Person entity (name, sameAs, image).
Apply this diff to the JSON‑LD block:
- jsonLd: { - '@type': 'ProfilePage', - name: 'Resume', - }, + jsonLd: { + '@type': 'ProfilePage', + '@id': siteUrlFor('/resume#profile'), + url: siteUrlFor('/resume'), + name: 'Resume', + mainEntity: { + '@type': 'Person', + name: SITE_NAME, + image: ABOUT_IMAGE, + }, + },If
useSeoalready composes a Person graph, skip.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/images/profile/about.jpgis excluded by!**/*.jpg
📒 Files selected for processing (5)
src/pages/AboutPage.vue(2 hunks)src/pages/HomePage.vue(1 hunks)src/pages/ProjectsPage.vue(2 hunks)src/pages/ResumePage.vue(2 hunks)src/pages/SubscribePage.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/pages/ProjectsPage.vue
- src/pages/AboutPage.vue
- src/pages/SubscribePage.vue
🔇 Additional comments (3)
src/pages/HomePage.vue (2)
56-56: SEO helper import centralization looks good.Nice consolidation to
ABOUT_IMAGEandsiteUrlFor.Confirm the
@alias andseo.tsexports are resolved in both dev and build (Vite/TS config).
63-65: Route rename follow‑ups: ensure legacy links and redirects.
- Caddy check: found /home/jailuser/git/caddy/WebCaddyfile.internal — it serves the SPA via try_files; no 301/redirect from /posts/ → /post/ detected. Add an edge redirect in that Caddyfile or your hosting edge.
- Search incomplete: ripgrep failed with "unrecognized file type: vue" so I could not confirm absence of hardcoded "/posts/" links. Run a repo-wide search without --type filters, e.g. rg -nP -C2 '/posts/' -S --hidden and update any matches to /post/ (or rely on the edge redirect).
src/pages/ResumePage.vue (1)
58-58: SEO helper import update looks good.
Summary by CodeRabbit
New Features
Refactor
Chores