Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughIntroduces a shareable package OpenGraph card system: new Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
test/unit/a11y-component-coverage.spec.ts (1)
31-34: Avoid long-term a11y skip forPackage/ShareModal.vue.Line 32 skips an interactive modal in a core user flow; that can mask regressions. Consider keeping this as a short-lived skip with a tracked follow-up to add at least one focused modal a11y smoke test.
app/components/Package/ShareModal.vue (2)
68-79: Theasynckeyword is unnecessary here.
downloadCardis declaredasyncbut contains noawaitexpressions. Thetry/finallyblock handles synchronous operations only.♻️ Proposed simplification
-async function downloadCard() { +function downloadCard() { const a = document.createElement('a') a.href = cardUrl.value a.download = `${props.packageName.replace('/', '-')}-card.png` document.body.appendChild(a) try { a.click() } finally { document.body.removeChild(a) } showAlt.value = true }
31-33: Alt text may be redundant for non-latest versions.When
isLatestis false, the tag falls back toresolvedVersion, producing alt text like"nuxt 4.4.2 (4.4.2)"which duplicates the version. Consider omitting the parenthetical for non-latest versions, or showing the actual dist-tag if available.♻️ Proposed improvement
const altText = computed(() => { - const tag = props.isLatest ? 'latest' : props.resolvedVersion - const parts: string[] = [`${props.packageName} ${props.resolvedVersion} (${tag})`] + const versionPart = props.isLatest + ? `${props.resolvedVersion} (latest)` + : props.resolvedVersion + const parts: string[] = [`${props.packageName} ${versionPart}`]
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 56dab79f-1b89-43a9-acfd-ce896de2de51
⛔ Files ignored due to path filters (3)
test/e2e/og-image.spec.ts-snapshots/og-image-for--package-nuxt-v-3-20-2.pngis excluded by!**/*.pngtest/e2e/og-image.spec.ts-snapshots/share-card-nuxt-dark.pngis excluded by!**/*.pngtest/e2e/og-image.spec.ts-snapshots/share-card-nuxt-light.pngis excluded by!**/*.png
📒 Files selected for processing (16)
app/components/OgImage/BlogPost.vueapp/components/OgImage/ShareCard.d.vue.tsapp/components/OgImage/ShareCard.vueapp/components/Package/Header.vueapp/components/Package/ShareModal.vueapp/components/Package/Skeleton.vueapp/pages/package/[[org]]/[name].vueapp/pages/share-card/[[org]]/[name].vueapp/utils/colors.tsapp/utils/formatters.tsapp/utils/string.tsnuxt.config.tsserver/api/card/[...pkg].get.tsshared/utils/constants.tstest/e2e/og-image.spec.tstest/unit/a11y-component-coverage.spec.ts
| aria-label="Share package card" | ||
| @click="shareModal.open()" | ||
| > | ||
| <span class="max-sm:sr-only">share</span> | ||
| </ButtonBase> |
There was a problem hiding this comment.
Localise the new share button label text.
Line 237 and Line 240 use hardcoded English strings; this should use $t(...) so the header stays fully localised.
🌐 Proposed fix
<ButtonBase
classicon="i-lucide:share-2"
- aria-label="Share package card"
+ :aria-label="$t('package.share_card')"
`@click`="shareModal.open()"
>
- <span class="max-sm:sr-only">share</span>
+ <span class="max-sm:sr-only">{{ $t('common.share') }}</span>
</ButtonBase>📝 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.
| aria-label="Share package card" | |
| @click="shareModal.open()" | |
| > | |
| <span class="max-sm:sr-only">share</span> | |
| </ButtonBase> | |
| aria-label="Share package card" | |
| `@click`="shareModal.open()" | |
| > | |
| <span class="max-sm:sr-only">share</span> |
| aria-label="Share package card" | |
| @click="shareModal.open()" | |
| > | |
| <span class="max-sm:sr-only">share</span> | |
| </ButtonBase> | |
| :aria-label="$t('package.share_card')" | |
| `@click`="shareModal.open()" | |
| > | |
| <span class="max-sm:sr-only">{{ $t('common.share') }}</span> |
| export function withAlpha(color: string, alpha: number): string { | ||
| if (color.startsWith('oklch(')) return color.replace(')', ` / ${alpha})`) | ||
| if (color.startsWith('#')) | ||
| return ( | ||
| color + | ||
| Math.round(alpha * 255) | ||
| .toString(16) | ||
| .padStart(2, '0') | ||
| ) | ||
| return color |
There was a problem hiding this comment.
withAlpha can emit invalid colour formats for valid inputs.
Lines 9-16 append alpha blindly, which breaks inputs that already include alpha (oklch(... / a) or #RRGGBBAA) and allows out-of-range alpha values.
🎨 Proposed fix
export function withAlpha(color: string, alpha: number): string {
- if (color.startsWith('oklch(')) return color.replace(')', ` / ${alpha})`)
- if (color.startsWith('#'))
- return (
- color +
- Math.round(alpha * 255)
- .toString(16)
- .padStart(2, '0')
- )
+ const clamped = Math.min(Math.max(alpha, 0), 1)
+ if (color.startsWith('oklch(')) {
+ const withoutAlpha = color.replace(/\s*\/\s*[^)]+(?=\))/i, '')
+ return withoutAlpha.replace(')', ` / ${clamped})`)
+ }
+ if (/^#([a-f\d]{6}|[a-f\d]{8})$/i.test(color)) {
+ const base = color.slice(0, 7)
+ const a = Math.round(clamped * 255).toString(16).padStart(2, '0')
+ return `${base}${a}`
+ }
return color
}| export function formatDate(date: string | undefined): string { | ||
| if (!date) return '' | ||
| try { | ||
| return new Date(date).toLocaleDateString('en-US', { | ||
| year: 'numeric', | ||
| month: 'short', | ||
| day: 'numeric', | ||
| }) | ||
| } catch { | ||
| return date | ||
| } | ||
| } |
There was a problem hiding this comment.
Invalid Date is not caught by try/catch — consider explicit validation.
new Date(invalidString) doesn't throw; it returns an Invalid Date object. Calling toLocaleDateString() on it returns the string "Invalid Date" rather than throwing. The fallback to date won't trigger for malformed inputs.
🛡️ Proposed fix to handle Invalid Date
export function formatDate(date: string | undefined): string {
if (!date) return ''
- try {
- return new Date(date).toLocaleDateString('en-US', {
- year: 'numeric',
- month: 'short',
- day: 'numeric',
- })
- } catch {
- return date
- }
+ const parsed = new Date(date)
+ if (Number.isNaN(parsed.getTime())) return date
+ return parsed.toLocaleDateString('en-US', {
+ year: 'numeric',
+ month: 'short',
+ day: 'numeric',
+ })
}📝 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.
| export function formatDate(date: string | undefined): string { | |
| if (!date) return '' | |
| try { | |
| return new Date(date).toLocaleDateString('en-US', { | |
| year: 'numeric', | |
| month: 'short', | |
| day: 'numeric', | |
| }) | |
| } catch { | |
| return date | |
| } | |
| } | |
| export function formatDate(date: string | undefined): string { | |
| if (!date) return '' | |
| const parsed = new Date(date) | |
| if (Number.isNaN(parsed.getTime())) return date | |
| return parsed.toLocaleDateString('en-US', { | |
| year: 'numeric', | |
| month: 'short', | |
| day: 'numeric', | |
| }) | |
| } |
| export function truncate(s: string, n: number): string { | ||
| return s.length > n ? s.slice(0, n - 1) + '…' : s |
There was a problem hiding this comment.
Handle non-positive n explicitly in truncate.
Line 2 misbehaves for n <= 0 (e.g. negative/zero limits can still return content). Please guard this boundary.
✂️ Proposed fix
export function truncate(s: string, n: number): string {
- return s.length > n ? s.slice(0, n - 1) + '…' : s
+ if (n <= 0) return ''
+ if (s.length <= n) return s
+ return n === 1 ? '…' : s.slice(0, n - 1) + '…'
}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 768e278c-0830-4137-959a-4d5dd790c78a
📒 Files selected for processing (3)
app/components/Package/Header.vuenuxt.config.tstest/unit/a11y-component-coverage.spec.ts
✅ Files skipped from review due to trivial changes (1)
- test/unit/a11y-component-coverage.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- nuxt.config.ts
| <!-- Share modal --> | ||
| <PackageShareModal | ||
| v-if="pkg" | ||
| :package-name="packageName" | ||
| :resolved-version="resolvedVersion ?? ''" | ||
| :is-latest="resolvedVersion === pkg?.['dist-tags']?.latest" | ||
| :license="displayVersion?.license" | ||
| /> |
There was a problem hiding this comment.
Avoid empty resolvedVersion in share modal props.
Line 201 can pass '', which degrades generated alt text in PackageShareModal. Prefer a non-empty fallback version.
Proposed fix
<PackageShareModal
v-if="pkg"
:package-name="packageName"
- :resolved-version="resolvedVersion ?? ''"
- :is-latest="resolvedVersion === pkg?.['dist-tags']?.latest"
+ :resolved-version="resolvedVersion ?? pkg['dist-tags']?.latest ?? ''"
+ :is-latest="(resolvedVersion ?? pkg['dist-tags']?.latest) === pkg['dist-tags']?.latest"
:license="displayVersion?.license"
/>📝 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.
| <!-- Share modal --> | |
| <PackageShareModal | |
| v-if="pkg" | |
| :package-name="packageName" | |
| :resolved-version="resolvedVersion ?? ''" | |
| :is-latest="resolvedVersion === pkg?.['dist-tags']?.latest" | |
| :license="displayVersion?.license" | |
| /> | |
| <!-- Share modal --> | |
| <PackageShareModal | |
| v-if="pkg" | |
| :package-name="packageName" | |
| :resolved-version="resolvedVersion ?? pkg['dist-tags']?.latest ?? ''" | |
| :is-latest="(resolvedVersion ?? pkg['dist-tags']?.latest) === pkg['dist-tags']?.latest" | |
| :license="displayVersion?.license" | |
| /> |
ghostdevv
left a comment
ghostdevv
left a comment
There was a problem hiding this comment.
could you take a look at the code rabbit comments?
| '/__og-image__/**': { | ||
| isr: { | ||
| expiration: 3600, | ||
| passQuery: true, | ||
| allowQuery: ['theme', 'color'], | ||
| }, | ||
| }, |
|
|
||
| // Strip .png extension from the final segment (e.g. /api/card/nuxt.png) | ||
| if (segments.length > 0) { | ||
| const last = segments[segments.length - 1]! |
There was a problem hiding this comment.
You can use segments.at(-1), should probably do the null check
| const color = | ||
| rawColor && (ACCENT_COLOR_IDS as readonly string[]).includes(rawColor) | ||
| ? `&color=${rawColor}` | ||
| : '' | ||
|
|
||
| return sendRedirect( | ||
| event, | ||
| `/__og-image__/image/share-card/${packageName}/og.png?theme=${theme}${color}`, | ||
| 302, | ||
| ) |
There was a problem hiding this comment.
you could use URLSearchParams rather than manually concat-ing strings to be a bit more readable
| async function downloadCard() { | ||
| const a = document.createElement('a') | ||
| a.href = cardUrl.value | ||
| a.download = `${props.packageName.replace('/', '-')}-card.png` | ||
| document.body.appendChild(a) | ||
| try { | ||
| a.click() | ||
| } finally { | ||
| document.body.removeChild(a) | ||
| } | ||
| showAlt.value = true | ||
| } |
There was a problem hiding this comment.
there's a download link util now you can use!
2 participants
🔗 Linked issue
Resolves #2146
🧭 Context
Add a share button to help user generate a well-designed, shareable card and post it on social media or send it to friends.
📚 Description
Some implementation details need to be mentioned here:
nuxt-og-image, inline styles and hard code icon are used to ensure rendering compatibilityACCENT_COLORSis widely used, a temporaryACCENT_COLOR_TOKENSis introduced in this PR — refactoring will be completed once mergedDemo
PixPin_2026-03-29_23-08-00.mp4
ALT of nuxt
Screenshot