-
Notifications
You must be signed in to change notification settings - Fork 4
fix: metadata #289
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
fix: metadata #289
Conversation
WalkthroughUpdates app icons/favicons across multiple platforms to SVGs or new filenames, adds db/.gitignore to ignore data files, replaces the TwitterIcon SVG implementation, renames many UI strings from “Tweet” to “Blab/Blabsy”, and changes registry DB error logging to a structured object. Changes
Sequence Diagram(s)(omitted — changes do not introduce new control flow or runtime interactions that warrant a sequence diagram) Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 4
🧹 Nitpick comments (3)
db/.gitignore (1)
1-1
: Keep the directory tracked while ignoring contentsIf you want to keep db/data in the repo (e.g., via a placeholder), add an exception for .gitkeep; otherwise the folder will disappear when empty.
data/* +!data/.gitkeep
Optionally add an empty db/data/.gitkeep file.
platforms/blabsy/src/components/common/app-head.tsx (1)
8-8
: Add type/sizes to SVG favicon and consider a legacy fallbackModern browsers benefit from type="image/svg+xml" and sizes="any". Consider keeping a .ico fallback for older/edge cases.
- <link rel='icon' href='/logo.svg' /> + <link rel='icon' href='/logo.svg' type='image/svg+xml' sizes='any' /> + {/* Optional fallback for legacy browsers */} + <link rel='icon' href='/favicon.ico' />platforms/registry/src/index.ts (1)
101-105
: Filter out undefined platform URLs to avoid nulls in the responseEnvironment variables may be unset; returning [null, ...] isn’t ideal.
server.get("/platforms", async (request, reply) => { - const platforms = [ process.env.PUBLIC_PICTIQUE_BASE_URL, process.env.PUBLIC_BLABSY_BASE_URL, process.env.PUBLIC_GROUP_CHARTER_BASE_URL, process.env.PUBLIC_CERBERUS_BASE_URL ] + const platforms = [ + process.env.PUBLIC_PICTIQUE_BASE_URL, + process.env.PUBLIC_BLABSY_BASE_URL, + process.env.PUBLIC_GROUP_CHARTER_BASE_URL, + process.env.PUBLIC_CERBERUS_BASE_URL + ].filter((v): v is string => Boolean(v)); return platforms });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
platforms/blabsy/public/logo.svg
is excluded by!**/*.svg
platforms/group-charter-manager/public/logo.png
is excluded by!**/*.png
platforms/pictique/static/Logo.svg
is excluded by!**/*.svg
📒 Files selected for processing (8)
db/.gitignore
(1 hunks)platforms/blabsy/public/site.webmanifest
(1 hunks)platforms/blabsy/src/components/common/app-head.tsx
(1 hunks)platforms/blabsy/src/components/ui/custom-icon.tsx
(1 hunks)platforms/eVoting/src/app/layout.tsx
(1 hunks)platforms/group-charter-manager/src/app/layout.tsx
(1 hunks)platforms/pictique/src/app.html
(1 hunks)platforms/registry/src/index.ts
(2 hunks)
🔇 Additional comments (2)
platforms/group-charter-manager/src/app/layout.tsx (1)
13-15
: Verified logo.png asset presence; consider adopting SVG for consistencyWe confirmed that
/public/logo.png
exists inplatforms/group-charter-manager
and there is nologo.svg
. If you want to align with the SVG usage elsewhere, consider adding alogo.svg
and updating the icon reference inlayout.tsx
(with a PNG fallback if needed).• File: platforms/group-charter-manager/src/app/layout.tsx (lines 13–15)
• Current snippet:icons: { icon: "/logo.png" }platforms/eVoting/src/app/layout.tsx (1)
7-9
: Asset path verified –Logo.png
exists as expected
- File checked: platforms/eVoting/src/app/layout.tsx
•icons.icon
set to"/Logo.png"
- Resolved at: platforms/eVoting/public/Logo.png (case matches)
Optional suggestion: consider migrating to an SVG (
Logo.svg
) with a PNG fallback for consistency across apps.
"src": "/logo.svg", | ||
"type": "image/svg+xml", | ||
"sizes": "any" |
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.
🛠️ Refactor suggestion
Good move to SVG; add maskable purpose and consider PNG fallbacks for iOS
SVG with sizes "any" is fine for many platforms. Add purpose "any maskable" for better Android adaptive icons, and consider providing PNG fallbacks (192x192, 512x512) plus an apple-touch-icon for iOS (iOS ignores Web Manifest icons).
{
"src": "/logo.svg",
"type": "image/svg+xml",
- "sizes": "any"
+ "sizes": "any",
+ "purpose": "any maskable"
}
Optional: add additional PNG entries and an <link rel="apple-touch-icon" ...> in the HTML head.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
"src": "/logo.svg", | |
"type": "image/svg+xml", | |
"sizes": "any" | |
{ | |
"src": "/logo.svg", | |
"type": "image/svg+xml", | |
"sizes": "any", | |
"purpose": "any maskable" | |
} |
🤖 Prompt for AI Agents
In platforms/blabsy/public/site.webmanifest around lines 12 to 14, the SVG icon
entry only declares "type" and "sizes" and should be expanded for better
platform support: add "purpose": "any maskable" to the SVG entry, and add PNG
fallback entries (e.g., 192x192 and 512x512 with "type": "image/png" and
corresponding "sizes") to the manifest; additionally, add an apple-touch-icon
link tag in the HTML head pointing to a PNG (e.g., 180x180) because iOS ignores
webmanifest icons.
<head> | ||
<meta charset="utf-8" /> | ||
<link rel="icon" href="%sveltekit.assets%/favicon.png" /> | ||
<link rel="icon" href="%sveltekit.assets%/Logo.svg" /> |
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.
💡 Verification agent
🧩 Analysis chain
Normalize asset case and add type/sizes for SVG
The path uses an uppercase L (Logo.svg) whereas other apps use lowercase (logo.svg). On case-sensitive filesystems this can 404 if the filename differs. Also add type/sizes.
- <link rel="icon" href="%sveltekit.assets%/Logo.svg" />
+ <link rel="icon" href="%sveltekit.assets%/logo.svg" type="image/svg+xml" sizes="any" />
Run to verify asset existence and references:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "References in pictique app.html:"
rg -n 'Logo\.svg|logo\.svg' platforms/pictique/src/app.html || true
echo ""
echo "Matching files in repo (case-insensitive):"
fd -a -i 'logo.svg' || true
fd -a -i 'Logo.svg' || true
Length of output: 821
Add SVG favicon type and sizes (retain existing filename casing)
Verified that the pictique app’s static assets use Logo.svg
(uppercase “L”), so switching to logo.svg
would break the link unless you also rename the files. Instead, update the <link>
tag to include type
and sizes
, keeping the existing filename:
– platforms/pictique/src/app.html (line 5)
- <link rel="icon" href="%sveltekit.assets%/Logo.svg" />
+ <link rel="icon" href="%sveltekit.assets%/Logo.svg" type="image/svg+xml" sizes="any" />
If you’d rather normalize to lowercase across all apps, you’ll need to:
- Rename
platforms/pictique/static/Logo.svg
→logo.svg
- Rename
platforms/pictique/static/images/Logo.svg
→logo.svg
- Then update this link to use
/logo.svg
instead.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<link rel="icon" href="%sveltekit.assets%/Logo.svg" /> | |
<link rel="icon" href="%sveltekit.assets%/Logo.svg" type="image/svg+xml" sizes="any" /> |
🤖 Prompt for AI Agents
platforms/pictique/src/app.html around line 5: the <link> tag for the favicon
currently only references Logo.svg (uppercase L) and lacks type/sizes
attributes; update this tag to keep the existing filename casing (Logo.svg) and
add type="image/svg+xml" and sizes="any" (or an appropriate sizes value) so the
favicon is correctly discovered and identified without renaming assets.
server.log.info("Database connection initialized"); | ||
} catch (error) { | ||
server.log.error("Error during database initialization:", error); | ||
server.log.error({message: "Error during database initialization", detail: error}); |
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.
🛠️ Refactor suggestion
Use Fastify/Pino error semantics to preserve stack traces
Passing the Error as the first arg ensures proper err field and stack in logs. Current object logging may drop structured error fields.
- server.log.error({message: "Error during database initialization", detail: error});
+ server.log.error(error as any, "Error during database initialization");
+ // Or:
+ // server.log.error({ err: error }, "Error during database initialization");
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
server.log.error({message: "Error during database initialization", detail: error}); | |
server.log.error(error as any, "Error during database initialization"); | |
// Or: | |
// server.log.error({ err: error }, "Error during database initialization"); |
🤖 Prompt for AI Agents
In platforms/registry/src/index.ts around line 27, the current log call passes
the Error in an object which can drop structured error fields and stack; change
the call to pass the Error as the first argument to server.log.error and provide
the message/context as the second argument (e.g., server.log.error(error, {
message: "Error during database initialization", ... })) so Fastify/Pino
preserves the err field and stack trace while still logging your contextual
fields.
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
♻️ Duplicate comments (1)
platforms/blabsy/public/site.webmanifest (1)
12-15
: Add purpose "any maskable" to SVG iconThis improves Android adaptive icon rendering.
{ "src": "/logo.svg", "type": "image/svg+xml", - "sizes": "any" + "sizes": "any", + "purpose": "any maskable" }
🧹 Nitpick comments (11)
platforms/blabsy/src/components/common/app-head.tsx (1)
8-9
: Add apple-touch-icon (iOS) and consider Safari pinned tabMany iOS devices ignore web manifest icons. Add an apple-touch-icon, and optionally a mask-icon for Safari pinned tabs.
Apply something like:
<Head> <title>Blabsy</title> <meta name='og:title' content='Blabsy' /> <link rel='icon' href='/logo.png' type='image/png' /> <link rel='icon' href='/logo.svg' type='image/svg+xml' /> + <link rel='apple-touch-icon' sizes='180x180' href='/apple-touch-icon.png' /> + <!-- Optional for Safari pinned tabs --> + <link rel='mask-icon' href='/safari-pinned-tab.svg' color='#1DA1F2' /> <link rel='manifest' href='/site.webmanifest' key='site-manifest' /> <meta name='twitter:site' content='@ccrsxx' /> <meta name='twitter:card' content='summary_large_image' /> </Head>Note: Ensure the referenced files exist in public/.
platforms/blabsy/src/components/common/placeholder.tsx (1)
8-8
: Confirm OG/Twitter image dimensions or adjust card typeUsing /logo.png for a summary_large_image card may not meet recommended aspect ratio (typically 1200x630). Consider using a wide hero image or switch to summary if you intend a square logo.
Would you like me to propose a SEO image strategy (assets + Head tags) aligned with summary_large_image best practices?
Also applies to: 10-10
platforms/blabsy/src/components/user/user-header.tsx (1)
86-89
: Simplify string interpolation and verify pluralization helperMinor cleanup and a quick check:
- The nested template literal is redundant.
- Ensure isPlural(totalTweets) returns '' or 's' (as used elsewhere).
Suggested tweak:
- ? `${totalTweets} ${`Blab${isPlural( - totalTweets - )}`}` + ? `${totalTweets} Blab${isPlural(totalTweets)}`platforms/blabsy/src/components/view/view-parent-tweet.tsx (1)
35-35
: Confirm external help link matches new brandingMessage rebrand is good. The “Learn more” link still points to help.twitter.com. If that’s intentional, fine; otherwise, replace with your own help page or a neutral doc.
Optionally tweak copy to be brand-neutral:
- This Blab was deleted by the Blab author.{'\u0020'} + This Blab was deleted by its author.{'\u0020'}platforms/blabsy/src/components/tweet/tweet-share.tsx (3)
36-49
: Capitalize “Bookmarks” consistently in toast messagesThe add case uses “Bookmarks” while the remove case uses “bookmarks”. Align for consistency.
- : 'Blab removed from your bookmarks' + : 'Blab removed from your Bookmarks'
28-35
: Harden bookmark action with error handlingIf manageBookmark throws, the menu closes without user feedback. Add try/catch and an error toast.
- closeMenu(); - await manageBookmark(...args); - - toast.success( + closeMenu(); + try { + await manageBookmark(...args); + toast.success( type === 'bookmark' ? (): JSX.Element => ( <span className='flex gap-2'> Blab added to your Bookmarks <Link href='/bookmarks'> <a className='custom-underline font-bold'> View </a> </Link> </span> ) - : 'Blab removed from your bookmarks' - ); + : 'Blab removed from your Bookmarks' + ); + } catch (err) { + toast.error('Failed to update Bookmarks. Please try again.'); + }Also applies to: 33-49
52-56
: Add clipboard error handlingClipboard can fail due to permissions or unsupported environment; provide feedback on failure.
- const handleCopy = (closeMenu: () => void) => async (): Promise<void> => { - closeMenu(); - await navigator.clipboard.writeText(`${siteURL}/tweet/${tweetId}`); - toast.success('Copied to clipboard'); - }; + const handleCopy = (closeMenu: () => void) => async (): Promise<void> => { + closeMenu(); + try { + await navigator.clipboard.writeText(`${siteURL}/tweet/${tweetId}`); + toast.success('Copied to clipboard'); + } catch { + toast.error('Failed to copy link. Please try again.'); + } + };platforms/blabsy/src/components/tweet/tweet-actions.tsx (3)
52-58
: Fix typo: “to from” → “to your”Grammatical error in the pin modal title.
- title: 'Pin Blab to from profile?', + title: 'Pin Blab to your profile?',
119-124
: Capitalize “Blab” for consistency in toastsElsewhere “Blab” is capitalized as the post type. Align deletion/pin toasts.
- toast.success( - `${isInAdminControl ? `@${username}'s` : 'Your'} blab was deleted` - ); + toast.success( + `${isInAdminControl ? `@${username}'s` : 'Your'} Blab was deleted` + );- toast.success( - `Your blab was ${ - tweetIsPinned ? 'unpinned' : 'pinned' - } to your profile` - ); + toast.success( + `Your Blab was ${tweetIsPinned ? 'unpinned' : 'pinned'} to your profile` + );Also applies to: 133-136
52-65
: Consider centralizing UI copy for consistency and i18nPin modal strings are inline. Centralizing them (e.g., constants/locale files) avoids drift across components and supports future localization.
platforms/blabsy/src/pages/user/[id]/media.tsx (1)
44-46
: Improve empty-state phrasing“hasn't Blabbed Media” is awkward. Suggest clearer copy.
- title={`@${username as string} hasn't Blabbed Media`} - description='Once they do, those Blabs will show up here.' + title={`@${username as string} hasn't posted any media Blabs`} + description='Once they do, those media Blabs will show up here.'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
platforms/blabsy/public/assets/twitter-avatar.jpg
is excluded by!**/*.jpg
platforms/blabsy/public/favicon.ico
is excluded by!**/*.ico
platforms/blabsy/public/logo.png
is excluded by!**/*.png
📒 Files selected for processing (16)
platforms/blabsy/public/site.webmanifest
(1 hunks)platforms/blabsy/src/components/common/app-head.tsx
(1 hunks)platforms/blabsy/src/components/common/placeholder.tsx
(1 hunks)platforms/blabsy/src/components/layout/user-home-layout.tsx
(1 hunks)platforms/blabsy/src/components/modal/display-modal.tsx
(1 hunks)platforms/blabsy/src/components/modal/mobile-sidebar-modal.tsx
(1 hunks)platforms/blabsy/src/components/tweet/tweet-actions.tsx
(2 hunks)platforms/blabsy/src/components/tweet/tweet-share.tsx
(2 hunks)platforms/blabsy/src/components/ui/custom-icon.tsx
(1 hunks)platforms/blabsy/src/components/user/user-header.tsx
(2 hunks)platforms/blabsy/src/components/view/view-parent-tweet.tsx
(1 hunks)platforms/blabsy/src/pages/bookmarks.tsx
(1 hunks)platforms/blabsy/src/pages/tweet/[id].tsx
(1 hunks)platforms/blabsy/src/pages/user/[id]/index.tsx
(1 hunks)platforms/blabsy/src/pages/user/[id]/media.tsx
(1 hunks)platforms/eVoting/src/app/layout.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- platforms/blabsy/src/components/layout/user-home-layout.tsx
- platforms/blabsy/src/pages/user/[id]/index.tsx
- platforms/blabsy/src/pages/bookmarks.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- platforms/eVoting/src/app/layout.tsx
- platforms/blabsy/src/components/ui/custom-icon.tsx
🔇 Additional comments (7)
platforms/blabsy/src/components/modal/mobile-sidebar-modal.tsx (1)
39-39
: Label update looks goodBranding change is consistent with the rest of the PR. No functional impact.
platforms/blabsy/src/pages/tweet/[id].tsx (1)
58-58
: LGTM: Title rebrandTitle switch to 'Blab' for non-thread case is consistent and safe.
platforms/blabsy/src/components/user/user-header.tsx (1)
52-53
: No functional changeClassName reflow only; behavior and styles unchanged.
platforms/blabsy/src/components/tweet/tweet-share.tsx (2)
96-99
: LGTM on label“Copy link to Blab” aligns with the rebranding.
52-56
: Confirm dynamic route path for shared linksThe copy handler currently uses
/tweet/${tweetId}
, but I didn’t find any/blab/
routes in the repo. Please verify where your individual-post route lives (for example inplatforms/blabsy/src/pages/tweet/[tweetId].tsx
orsrc/app/tweet/[tweetId]/page.tsx
):
- If your dynamic segment is still under /tweet, no change is needed.
- If you’ve renamed the folder to /blab, update the copy URL:
- await navigator.clipboard.writeText(`${siteURL}/tweet/${tweetId}`); + await navigator.clipboard.writeText(`${siteURL}/blab/${tweetId}`);platforms/blabsy/src/components/tweet/tweet-actions.tsx (1)
170-177
: LGTM: “Blabsy search results”Updated platform name in deletion description looks correct.
platforms/blabsy/src/pages/user/[id]/media.tsx (1)
35-39
: LGTM: SEO title rebranding“Media Blabs by … / Blabsy” is consistent with the new terminology.
"src": "/logo.png", | ||
"type": "image/png", | ||
"sizes": "512x512" | ||
"sizes": "any" | ||
} |
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.
PNG icon should declare explicit sizes; avoid "any"
For raster icons, set concrete sizes (e.g., 192x192, 512x512). Keep SVG with sizes:any, but add PNG fallbacks with explicit sizes.
Apply a revision like:
"icons": [
{
"src": "/logo.svg",
"type": "image/svg+xml",
"sizes": "any",
"purpose": "any maskable"
},
- {
- "src": "/logo.png",
- "type": "image/png",
- "sizes": "any"
- }
+ {
+ "src": "/logo-192.png",
+ "type": "image/png",
+ "sizes": "192x192"
+ },
+ {
+ "src": "/logo-512.png",
+ "type": "image/png",
+ "sizes": "512x512"
+ }
]
Ensure the new PNGs exist in public/, and consider adding an apple-touch-icon in HTML head for iOS (iOS ignores manifest icons).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
"src": "/logo.png", | |
"type": "image/png", | |
"sizes": "512x512" | |
"sizes": "any" | |
} | |
"icons": [ | |
{ | |
"src": "/logo.svg", | |
"type": "image/svg+xml", | |
"sizes": "any", | |
"purpose": "any maskable" | |
}, | |
{ | |
"src": "/logo-192.png", | |
"type": "image/png", | |
"sizes": "192x192" | |
}, | |
{ | |
"src": "/logo-512.png", | |
"type": "image/png", | |
"sizes": "512x512" | |
} | |
] |
🤖 Prompt for AI Agents
In platforms/blabsy/public/site.webmanifest around lines 17 to 20, the PNG icon
currently declares "sizes":"any" which is inappropriate for raster images;
replace that entry with one or more PNG entries that declare concrete sizes (for
example "sizes":"192x192" and/or "512x512"), add separate manifest objects for
each PNG size pointing to the correct files (ensure those PNG files exist in
public/), keep SVG entries as "sizes":"any" if present, and also add an
apple-touch-icon link tag in the site's HTML head pointing to a suitable PNG to
support iOS.
> | ||
<div className='grid grid-cols-[auto,1fr] gap-3'> | ||
<UserAvatar src='/assets/twitter-avatar.jpg' alt='Blabsy' /> | ||
<UserAvatar src='logo.png' alt='Blabsy' /> |
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.
Broken asset path: use absolute public path
'logo.png' is a relative URL and will likely 404 on nested routes. Public assets should be referenced with a leading slash.
Apply this diff:
- <UserAvatar src='logo.png' alt='Blabsy' />
+ <UserAvatar src='/logo.png' alt='Blabsy' />
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<UserAvatar src='logo.png' alt='Blabsy' /> | |
<UserAvatar src='/logo.png' alt='Blabsy' /> |
🤖 Prompt for AI Agents
In platforms/blabsy/src/components/modal/display-modal.tsx around line 42, the
UserAvatar src uses a relative path 'logo.png' which will 404 on nested routes;
change it to use the absolute public path by replacing the value with
'/logo.png' so the asset is resolved from the site root.
Description of change
fixes
Issue Number
n/a
Type of change
How the change has been tested
Manual
Change checklist
Summary by CodeRabbit
Style
Chores