feat: added feature to copy version number by hovering on the version#2660
feat: added feature to copy version number by hovering on the version#2660PreethiPantangi wants to merge 5 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThe changes add a copy-to-clipboard flow for the resolved package version: UI button integration around the version selector, clipboard logic and state, a command-palette command to copy the version, and new i18n keys/schema for the copy label. ChangesPackage header / version copy
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as PackageHeader (UI)
participant Version as VersionSelector
participant Clipboard as ClipboardService
participant Commands as CommandPalette
participant Announcer as Announcer
User->>UI: Click copy button / invoke "copy version" command
UI->>Version: (reads) resolvedVersionDisplay / isOpen
UI->>Clipboard: copy(resolvedVersionDisplay)
Clipboard-->>UI: copy result
UI->>Announcer: announce success/failure
UI->>Commands: mark command as executed / update copied state
Announcer-->>User: audible/ARIA feedback
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: Turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value). 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
|
Hello! Thank you for opening your first PR to npmx, @PreethiPantangi! 🚀 Here’s what will happen next:
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/Package/Header.vue`:
- Around line 108-118: The command object with id 'package-copy-version'
currently uses props.resolvedVersion directly for keywords and always calls
copyPkgVersion() and announce(), so guard these actions by checking that
props.resolvedVersion is a non-empty string before invoking copyPkgVersion() or
announce(); if absent, no-op (or return) so nothing is copied or announced; also
ensure keywords is strictly string-only by transforming/filtering it to a string
(e.g., use String(props.resolvedVersion) or include only when truthy) so
keywords never contains undefined/null; apply the same guard/keyword fix for the
other similar command block that uses resolvedVersion (the block noted in the
review).
🪄 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: 82e6bf58-7e13-4dac-81e0-768c1f1550c6
📒 Files selected for processing (2)
app/components/Package/Header.vuei18n/locales/en.json
gameroman
left a comment
gameroman
left a comment
There was a problem hiding this comment.
Overall very good, just make sure the CI passes, also left a couple of comments
| import type { RouteLocationRaw } from 'vue-router' | ||
| import type { CommandPaletteContextCommandInput } from '~/types/command-palette' | ||
| import { SCROLL_TO_TOP_THRESHOLD } from '~/composables/useScrollToTop' | ||
| import { useClipboard } from '@vueuse/core' |
There was a problem hiding this comment.
iirc, this isn't necessary since it's auto-imported
| announce($t('command_palette.announcements.copied_to_clipboard')) | ||
| }, | ||
| }, | ||
| ] |
There was a problem hiding this comment.
coderabbit's suggestions makes sense here. I would do something like this here
const commands: CommandPaletteContextCommandInput[] = []
if (packageName.value) {
commands.push({
id: 'package-copy-name',
group: 'package',
label: $t('package.copy_name'),
keywords: [packageName.value],
iconClass: 'i-lucide:copy',
action: () => {
copyPkgName()
announce($t('command_palette.announcements.copied_to_clipboard'))
},
})
}
if (props.resolvedVersion) {
commands.push({
id: 'package-copy-version',
group: 'package',
label: $t('package.versions.copy_version'),
keywords: [props.resolvedVersion],
iconClass: 'i-lucide:copy',
action: () => {
copyPkgVersion()
announce($t('command_palette.announcements.copied_to_clipboard'))
},
})
}
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/components/Package/Header.vue (1)
312-328:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGate the version copy wrapper to avoid an empty focusable control.
On Line 312,
CopyToClipboardButtonalways renders, whileVersionSelectoris gated on Line 320. When version data is missing, this can leave a focusable copy control with no visible anchor and an empty copy value. Apply the samev-ifto the wrapper and renderVersionSelectorunconditionally inside it.Suggested fix
- <CopyToClipboardButton + <CopyToClipboardButton + v-if="resolvedVersion && pkg?.versions && pkg?.['dist-tags']" :copied="copiedPkgVersion" :copy-text="$t('package.versions.copy_version')" class="inline-flex items-center min-w-0" `@click`="copyPkgVersion()" > <!-- Version selector --> <VersionSelector - v-if="resolvedVersion && pkg?.versions && pkg?.['dist-tags']" :package-name="packageName" :current-version="resolvedVersion" :versions="pkg.versions" :dist-tags="pkg['dist-tags']" :url-pattern="versionUrlPattern" position-class="max-md:inset-is-0 md:inset-ie-0" /> </CopyToClipboardButton>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/Package/Header.vue` around lines 312 - 328, The CopyToClipboardButton wrapper is rendered unconditionally while VersionSelector is gated by resolvedVersion and pkg data, causing an empty focusable control; change the v-if condition from the inner VersionSelector to the CopyToClipboardButton so the whole wrapper (the CopyToClipboardButton that uses copiedPkgVersion and copyPkgVersion) only renders when resolvedVersion && pkg?.versions && pkg?.['dist-tags'] are present, and then render VersionSelector unconditionally inside it (keep props: :package-name="packageName", :current-version="resolvedVersion", :versions="pkg.versions", :dist-tags="pkg['dist-tags']", :url-pattern="versionUrlPattern").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/components/Package/Header.vue`:
- Around line 312-328: The CopyToClipboardButton wrapper is rendered
unconditionally while VersionSelector is gated by resolvedVersion and pkg data,
causing an empty focusable control; change the v-if condition from the inner
VersionSelector to the CopyToClipboardButton so the whole wrapper (the
CopyToClipboardButton that uses copiedPkgVersion and copyPkgVersion) only
renders when resolvedVersion && pkg?.versions && pkg?.['dist-tags'] are present,
and then render VersionSelector unconditionally inside it (keep props:
:package-name="packageName", :current-version="resolvedVersion",
:versions="pkg.versions", :dist-tags="pkg['dist-tags']",
:url-pattern="versionUrlPattern").
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 46061c25-5981-4e68-bad1-638fc48efcfb
📒 Files selected for processing (4)
Please[👉app/components/Package/Header.vuei18n/schema.json
✅ Files skipped from review due to trivial changes (1)
- i18n/schema.json
PreethiPantangi
changed the title
PreethiPantangi
changed the title
|
@gameroman I’ve addressed the feedback from the code review. Also, deleted the two junk files created accidentally. |
|
Looks good One thing to change, it probably should hide the copy button when you press the button to change the version 
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/components/VersionSelector.vue (1)
527-530: ⚡ Quick winConsider exposing
isOpenasreadonlyto enforce a read-only API contract.The parent (
Header.vue) only needs to readisOpento decide whether to hide the copy button. Exposing the rawShallowRefgives any consumer the ability to write to it (versionSelectorRef.value.isOpen.value = …), which could silently clobber the component's internal dropdown state.♻️ Proposed refactor
-// Expose isOpen state to parent components -defineExpose({ - isOpen, -}) +// Expose isOpen state to parent components (read-only to prevent external mutation) +defineExpose({ + isOpen: readonly(isOpen), +})
readonlyis a Vue core built-in — no additional import required in<script setup>.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/VersionSelector.vue` around lines 527 - 530, The component currently exposes the mutable ShallowRef isOpen via defineExpose, allowing consumers to mutate the dropdown state; change the exposure to a read-only API by wrapping isOpen with Vue's readonly and exposing that instead (keep the internal isOpen ref unchanged) so consumers like Header.vue can only read the state; use the built-in readonly in the <script setup> and update the defineExpose call to export the readonly wrapper (no extra imports required).app/components/CopyToClipboardButton.vue (1)
15-15: ⚡ Quick win
hideCopyTextis a misleading prop name — it removes the entire button, not just its text.
v-if="!hideCopyText"is placed on the<button>element itself, so when the prop istruethe full button (icon + label) is removed from the DOM. A reader of the prop API will expecthideCopyTextto suppress only the text portion while keeping the icon or the button shell. Consider renaming tohideButtonorhideCopyButtonto accurately reflect what it controls.♻️ Proposed rename
- hideCopyText?: boolean + hideButton?: boolean- v-if="!hideCopyText" + v-if="!hideButton"In
Header.vue, update the bound prop name correspondingly:- :hideCopyText="shouldHideCopyText" + :hideButton="shouldHideCopyText"Also applies to: 36-36
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/CopyToClipboardButton.vue` at line 15, The prop hideCopyText on CopyToClipboardButton.vue is misleading because v-if="!hideCopyText" removes the entire <button>; rename the prop to a clearer name such as hideButton or hideCopyButton, update the prop declaration in CopyToClipboardButton.vue (replace hideCopyText with the new name), change the v-if on the <button> to use the new prop, and update every caller (e.g., Header.vue) to bind the new prop name so behavior and API are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/components/CopyToClipboardButton.vue`:
- Line 15: The prop hideCopyText on CopyToClipboardButton.vue is misleading
because v-if="!hideCopyText" removes the entire <button>; rename the prop to a
clearer name such as hideButton or hideCopyButton, update the prop declaration
in CopyToClipboardButton.vue (replace hideCopyText with the new name), change
the v-if on the <button> to use the new prop, and update every caller (e.g.,
Header.vue) to bind the new prop name so behavior and API are consistent.
In `@app/components/VersionSelector.vue`:
- Around line 527-530: The component currently exposes the mutable ShallowRef
isOpen via defineExpose, allowing consumers to mutate the dropdown state; change
the exposure to a read-only API by wrapping isOpen with Vue's readonly and
exposing that instead (keep the internal isOpen ref unchanged) so consumers like
Header.vue can only read the state; use the built-in readonly in the <script
setup> and update the defineExpose call to export the readonly wrapper (no extra
imports required).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 30c44452-918f-465f-ad6c-8bcde988d98e
📒 Files selected for processing (3)
app/components/CopyToClipboardButton.vueapp/components/Package/Header.vueapp/components/VersionSelector.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- app/components/Package/Header.vue
2 participants
🔗 Linked issue
Resolves #2646
🧭 Context
The feature provides user with the capability to copy the version number of the package by hovering over it. This helps the user to easily grab the data instead of typing it out.
Adds copy-to-clipboard functionality for package version via hover interaction in the header.
📚 Description
Files changed:
- Header.vue
- en.json
Header.vue
- The VersionSelector component is wrapped with CopyToClipboardButton component to ensure the user is able to copy the version number on hover.
en.json
- Added the key value pair "copy_version": "Copy version number" to ensure internationalization
No tests added as this change is UI-only and does not affect business logic. Existing functionality remains unchanged.
No documentation changes required
I've used co-pilot to understand some parts of the code before diving into making the feature changes