Skip to content

Comments

fix: add strategy to tooltip and use it in sticky elems#1330

Merged
danielroe merged 1 commit intonpmx-dev:mainfrom
alexdln:fix/add-strategy-to-tooltips
Feb 10, 2026
Merged

fix: add strategy to tooltip and use it in sticky elems#1330
danielroe merged 1 commit intonpmx-dev:mainfrom
alexdln:fix/add-strategy-to-tooltips

Conversation

@alexdln
Copy link
Member

@alexdln alexdln commented Feb 10, 2026

The tooltips moved smoothly, even though their trigger remained in place (sticky package header). This looked odd. Floating UI supports this feature out of the box, so I configured it as prop for Tooltip and used it in the places I found

I didn't add any new tests, as it's a third-party library' logic

Closes #1251

@vercel
Copy link

vercel bot commented Feb 10, 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 10, 2026 0:05am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Feb 10, 2026 0:05am
npmx-lunaria Ignored Ignored Feb 10, 2026 0:05am

Request Review

@codecov
Copy link

codecov bot commented Feb 10, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 10, 2026

📝 Walkthrough

Walkthrough

This pull request introduces configurable tooltip positioning strategy across the application. The Tooltip Base component receives a new optional strategy prop (defaulting to 'absolute') which is passed to the useFloating hook from @floating-ui/vue. The MetricsBadges component and package details page are updated to utilise strategy="fixed" on three and two TooltipApp instances respectively, enabling fixed positioning for these tooltips.

Possibly related PRs

Suggested reviewers

  • knowler
  • danielroe
  • Adebesin-Cell
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The PR description accurately describes the changes: adding strategy prop to Tooltip and using it in sticky elements to fix tooltip movement during scrolling.
Linked Issues check ✅ Passed The changes directly address issue #1251 by adding configurable positioning strategy to tooltips and applying fixed strategy to sticky header tooltips, preventing unwanted movement during scroll.
Out of Scope Changes check ✅ Passed All changes are focused on solving the tooltip movement issue: adding strategy prop, configuring useFloating with it, and applying fixed strategy to relevant tooltips. No unrelated modifications detected.

✏️ 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

No actionable comments were generated in the recent review. 🎉


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

@danielroe danielroe added this pull request to the merge queue Feb 10, 2026
Merged via the queue into npmx-dev:main with commit e45597c Feb 10, 2026
18 checks passed
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.

When scrolling the package details page on the mobile device, the tool tip box keeps shaking.

2 participants