Skip to content

Comments

fix: correct package name copy tooltip hiding#838

Merged
danielroe merged 3 commits intonpmx-dev:mainfrom
alexdln:fix/fix-package-name-hiding
Feb 3, 2026
Merged

fix: correct package name copy tooltip hiding#838
danielroe merged 3 commits intonpmx-dev:mainfrom
alexdln:fix/fix-package-name-hiding

Conversation

@alexdln
Copy link
Member

@alexdln alexdln commented Feb 3, 2026

I spent a lot of time debugging to find the cause, but:

  • All classes work correctly
  • All styles and their weights are correct
  • All elements are in place and inserted correctly.

The hover state remains on the element. JS mouseLeave works correctly (both on button and group), everything is checked correctly, everything works as expected, but the element remains visible.

We can disable the transition and this bug will go away, but explicitly set display:none also works

Relates #594 #473

Closes #834

@vercel
Copy link

vercel bot commented Feb 3, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Feb 3, 2026 2:23pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Feb 3, 2026 2:23pm
npmx-lunaria Ignored Ignored Feb 3, 2026 2:23pm

Request Review

@codecov
Copy link

codecov bot commented Feb 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 3, 2026

📝 Walkthrough

Walkthrough

Removed the inline transition-all duration-150 from the copy button in the template. Added a .copy-button CSS block that initially hides the button via clipping, fixed dimensions and opacity, and defines reveal behaviour on :hover/group-hover and :focus-visible with shorter transitions. Added a media query to hide the copy button on devices that do not support hover. The component's public API and exports remain unchanged.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description relates to the changeset, addressing the copy button visibility issue mentioned in the linked issues.
Linked Issues check ✅ Passed The PR successfully addresses issue #834 by fixing the copy button's hidden state through CSS modifications (clip-path, transitions, and media query).
Out of Scope Changes check ✅ Passed All changes in the PR are scoped to fixing the copy button visibility issue; no unrelated modifications are present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
app/pages/package/[...package].vue (1)

1194-1199: Consider progressive enhancement for transition-behaviour: allow-discrete support.

The feature is supported in Chrome/Edge 117+ (Sep 2023), Safari 17.4+ (Mar 2024), and Firefox 129+ (Aug 2024), but remains newly available—not yet widely deployed across production browsers. Without the fallback, users on older browser versions will see the display transition snap rather than animate smoothly.

Guard with @supports to ensure older browsers fall back to the opacity transition:

Progressive enhancement with @supports
 .copy-button {
-  transition:
-    opacity ease-in-out 0.15s,
-    display 0.15s;
-  transition-behavior: allow-discrete;
+  transition: opacity ease-in-out 0.15s;
 }
+
+@supports (transition-behavior: allow-discrete) {
+  .copy-button {
+    transition:
+      opacity ease-in-out 0.15s,
+      display 0.15s;
+    transition-behavior: allow-discrete;
+  }
+}

@alexdln
Copy link
Member Author

alexdln commented Feb 3, 2026

Hmm, some transition styles were deleted after the build. I'll fix that now

@alexdln
Copy link
Member Author

alexdln commented Feb 3, 2026

Everything is working as expected now

Copy link
Member

@knowler knowler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@dominikg
Copy link

dominikg commented Feb 3, 2026

is there even a reason to only show the copy button on hover, remove all the transitions etc and always have it there. less code, more intuitive (use the familiar double rectangle icon for copy with a tooltip maybe)

@danielroe
Copy link
Member

@dominikg we hide it in the interests of not cluttering things visually

it's a nice to have - people can discover the feature naturally if they go to select package name

@danielroe danielroe added this pull request to the merge queue Feb 3, 2026
Merged via the queue into npmx-dev:main with commit ffb7914 Feb 3, 2026
14 checks passed
@dominikg
Copy link

dominikg commented Feb 3, 2026

image

looks pretty clean to me on nuxt.com and if you want to declutter, replacing the three circled box with a less prominent one like this would be more. the flicker when you move your cursor over the text alongside the fact that a keyboard user won't know its there until they tabbed to it make me not like it. At least it has aria infos but even that would be less needed if it was just a regular always visible button, no?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hide package name copy-button

5 participants