Replace the wordmark favicon with Milo - #9
Conversation
The old icon was a green "Greg" wordmark and the site's only icon. Milo is the mark everywhere else on the site, so he takes the tab too. public/favicon.svg is his rest pose, traced from src/components/milo/markup.ts, which renders exactly this before its cursor tracker starts. The standalone copy drops the data-milo hooks, the per-instance id namespacing, and the empty groups that existed only for the tracker to grab. A still image needs none of them and an XML parser rejects the first. Verified render-identical to the component output at 16, 32, 180 and 512px. Adds the icon files the site never had: a 16/32/48 ICO for Safari, which still does not read SVG favicons, a 180px apple-touch-icon on an opaque plate, 192 and 512 PNGs, and a manifest. BaseHead also gains a social card fallback. It previously emitted og:image only when a page passed one, so every page except blog posts and the 3D tool shared as bare text, including /milo. Pages that bring a usable image of their own are unchanged. A post with no hero, or with an SVG hero that no social platform can render, now falls through to the Milo card. Co-Authored-By: Claude <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (7)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe site now provides fallback social-card metadata, expanded favicon and theme-color declarations, and a web app manifest describing site identity, icons, navigation, display mode, and colors. ChangesSite metadata
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai full review Context for the review, so you can skip the things that are deliberate and spend effort where it helps. What this is. The site's favicon was a green "Greg" wordmark. It is now Milo, the cat mascot already used in the header and on
Known duplication, accepted on purpose:
Decisions that look wrong but are researched. Please do not "correct" these without a source that postdates the ones cited:
Where scrutiny is genuinely welcome. Verified against built output for: jpg hero (keeps its own), svg hero (falls back, since no platform renders SVG cards), no hero (falls back), and pages that pass nothing. Repo conventions live in Local gates all pass: |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/BaseHead.astro`:
- Around line 33-35: Update readCardImage so SVG detection removes or ignores
URL query strings and fragments before checking the .svg extension. Preserve the
existing fallback behavior for undefined values and SVG paths, while allowing
non-SVG image URLs to continue through unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: da05925e-0d23-41ec-8cc8-9b77cbaaead2
⛔ Files ignored due to path filters (6)
public/apple-touch-icon.pngis excluded by!**/*.pngpublic/favicon.icois excluded by!**/*.icopublic/favicon.svgis excluded by!**/*.svgpublic/icon-192.pngis excluded by!**/*.pngpublic/icon-512.pngis excluded by!**/*.pngpublic/og/default.pngis excluded by!**/*.png
📒 Files selected for processing (2)
public/site.webmanifestsrc/components/BaseHead.astro
A hero carrying a cache-busting query, `/blog/x.svg?v=2`, sailed past the endsWith check and shipped an SVG card that no social platform renders. Co-Authored-By: Claude <noreply@anthropic.com>
Without a maskable variant Android shrinks Milo onto a white plate and letterboxes him. The unpadded icons stay purpose any, since the spec warns against one file serving both, and the new padded file sizes him to 66% of the tile. His ear tips sit at radius 0.604 x scale from centre, and the safe zone is a circle of radius 40%, so that is the largest scale that survives every launcher mask shape. Shortcuts put Blog, Tools and Milo on a long press of the installed icon. Also declares og:image:width and height on the fallback card, which lets Slack and Facebook lay it out before they have fetched the image. Only the fallback can claim a size, since page heroes are arbitrary files. Co-Authored-By: Claude <noreply@anthropic.com>
|
@coderabbitai full review Two commits landed after your review at 06:30, so they have not been looked at. Automatic reviews are off on this repo, hence the explicit re-request. The earlier context comment still applies, in particular the list of deliberate decisions not to "correct". Delta since your pass:
Also worth a look: the manifest grew a nested All CI green: fmt, lint, typecheck, test, fallow. |
|
✅ Action performedFull review finished. |
Replaces the green "Greg" wordmark favicon with Milo, and fills in the icon and social metadata that was missing around it.
The art
public/favicon.svgis Milo's rest pose. The live component insrc/components/milo/markup.tsalready renders exactly this pose before its cursor tracker starts, so the art is traced from there rather than redrawn. The standalone copy drops three things the live rig needs and a still image does not: thedata-milo*hooks (valueless attributes, which are legal in HTML but invalid in an SVG file, since that is parsed as XML), the per-instance id namespacing, and the attribute-less<g>wrappers that existed only for the tracker to grab. The cleanup was verified render-identical to the raw component output at 16, 32, 180 and 512px.The
viewBoxis cropped from the component's 220 units to 144. Milo's ears top out at y=-84 and his chin at y=56, so a 144-unit square centered on the head is the tightest crop that keeps him whole, and it happens to still contain all but ~5 units of whisker tip on each side. The component's own viewBox would have spent most of a 16px favicon on empty margin.Files
favicon.svgfavicon.icoapple-touch-icon.png#f7f4efplate with a 10% inset so iOS's corner mask does not clip the earsicon-192.png,icon-512.pngpurpose: anyicon-maskable-512.pngpurpose: maskable, so Android's launcher mask crops background instead of Milosite.webmanifestog/default.pngMetadata
BaseHead.astropreviously emitted a single<link rel="icon">. It now emits the modern set, plus light and darktheme-colorsourced from the existingpageBg/pageBgDarktokens rather than hardcoded.Two details worth flagging, both from research rather than habit:
sizes="32x32"and the SVG carries none. The widely-copiedsizes="any"advice is superseded and has the opposite effect, making Chrome download both files.purpose, and the padded one is a separate file. Declaringmaskableon unpadded art gets it clipped by Android's adaptive mask, and the spec warns against one file serving both purposes because it ends up wrong on one of them.The maskable icon sizes Milo to 66% of the tile. That number is derived, not eyeballed: his ear tips sit at radius 0.604 x scale from the center, and the W3C safe zone is a circle of radius 40% of the icon, so 0.66 is the largest scale where nothing of him falls outside the guarantee on any launcher mask shape.
Social cards
BaseHeadhad an explicit "no fallback image" rule, so only blog posts (their hero) and the 3D-to-SVG tool shared with a picture. Everything else, including/miloitself, shared as a bare text blob. There is now a fallback to the Milo card. Since an image is always present,twitter:cardis unconditionallysummary_large_imageand the six conditional tag expressions collapse to plain ones.Two cases are worth calling out, both surfaced by the review rather than by me:
blog/[...slug].astroalways passesimageAlt={post.data.title}butheroImageis optional in the schema. My first cut keyed the alt offimageAltalone, so a post with no hero would have shipped the Milo card labelled with the article title. The alt is now derived from the image that actually won.copyparty-tunnel.mduses/blog/copyparty.svg, and no major social platform renders SVGog:image. That post was sharing imageless before this PR and would have kept doing so, which is the exact thing this change exists to fix, so an SVG hero now falls through to the Milo card.Verified against the built output, including a throwaway post with no hero:
og:imageog:image:alt/blog/vilos92-com/(jpg hero)/tools/3d-to-svg//blog/copyparty-tunnel/(svg hero)/,/milo/, the restNotes
id, which is inert without an install flow, and usesdisplay: minimal-uirather thanstandaloneso an installed copy keeps its back button on what is a multi-page content site. It also omitsscreenshots, which would unlock Chrome's richer install dialog but needs real captures and only pays off at install time.og:image:widthandog:image:height. Those numbers are hand-written and would drift if the card were ever regenerated at a different size. The tradeoff is that Slack and Facebook can lay the card out on the first crawl rather than after fetching the image.tokens.ts. A static JSON file cannot import them, and generating the manifest from an endpoint seemed like more machinery than two colors justify.Checks
vp check(format, lint, typecheck),tsc --noEmit, 19 tests, andfallowall pass. Fallow reports 0 dead files and 0 dead exports.Summary by CodeRabbit
New Features
Bug Fixes