fix: make social cards clickable and remove z-index trickery#1535
fix: make social cards clickable and remove z-index trickery#1535pauliecodes wants to merge 2 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThe pull request updates the CallToAction component to make the entire card surface trigger navigation. It adds a new Possibly related PRs
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)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
knowler
left a comment
There was a problem hiding this comment.
LGTM! I have an optional suggestion. I’ll defer to others with regard to reviewing the Vue-ness of the code (revueing? 😅).
If all is well, we’ll hold off on merging until vacation is over.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/components/CallToAction.vue (1)
29-38: Consider guarding against non-primary-button clicks.There is no
event.button !== 0check. When a user middle-clicks the card body (outside the<a>element),handleCardClickfires and callslink.click(), which dispatches a synthetic left-click (button0). Because the anchor hastarget="_blank", this opens the link in a new foreground tab, whereas a native middle-click on the anchor itself would open it in a background tab — the standard browser default. Adding the guard avoids this subtle discrepancy:♻️ Proposed fix
function handleCardClick(event: MouseEvent) { + if (event.button !== 0) return if ((event.target as HTMLElement).closest(':any-link')) return if (event.ctrlKey || event.metaKey || event.shiftKey || event.altKey) return
Description
This PR addresses the usability issue where clicking on text within social cards did not trigger navigation.
Previously, the text elements were positioned above the link overlay (using
z-index) to allow for text selection, but this created "dead zones" where clicks were not registered.Changes
absolute inset-0link overlay andz-1classes, simplifying the DOM structure as suggested by @knowler.handleCardClickfunction that programmatically clicks the CTA link when the card is clicked..closest('a'))window.getSelection())Demo
Fixes #1517