refactor(ui): move package external links to component#2077
refactor(ui): move package external links to component#2077graphieros merged 2 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds a new Vue 3 component app/components/Package/ExternalLinks.vue (script setup, TypeScript) that centralises rendering of package external links. It accepts props Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 36158c33-aab6-49a4-b89d-6e80d591ab92
📒 Files selected for processing (3)
app/components/Package/ExternalLinks.vueapp/pages/package/[[org]]/[name].vuetest/nuxt/a11y.spec.ts
| <LinkBase | ||
| :to="`https://www.npmjs.com/package/${pkg.name}`" | ||
| :title="$t('common.view_on.npm')" | ||
| classicon="i-simple-icons:npm" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="app/components/Link/Base.vue"
echo "== $file =="
sed -n '1,220p' "$file"
echo
echo "== title-related lines =="
rg -n -C2 '\btitle\b|v-bind="props"|<NuxtLink|<span' "$file"Repository: npmx-dev/npmx.dev
Length of output: 5693
🏁 Script executed:
# Verify ExternalLinks.vue uses title attribute in the mentioned lines
sed -n '85,100p' app/components/Package/ExternalLinks.vue | cat -nRepository: npmx-dev/npmx.dev
Length of output: 632
Add title prop to LinkBase and forward it to both template branches.
The title attribute passed on lines 89 and 96 is not declared as a prop in app/components/Link/Base.vue and will be silently dropped at runtime. The component uses a v-if/v-else fragment with a disabled <span> and <NuxtLink> branch; Vue 3 does not automatically apply fallthrough attributes in this pattern, and v-bind="props" only forwards declared props. Add title to defineProps and bind :title="title" on both the <span> and <NuxtLink> branches to ensure the hover text appears.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
🧭 Context
Smaller components are easier to work with and help prevent unrelated merge conflicts.
📚 Description
This PR extracts code from the Package into an individual PackageExternalLinks component to decrease the component's size.