fix(ui): reduce size of link icon#1566
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. 🎉 📝 WalkthroughWalkthroughAdds a single CSS property, 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)
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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/components/Readme.vue (1)
153-162:⚠️ Potential issue | 🟡 MinorMove
font-size: 0.75emto the base::afterrule to prevent layout shift on hover.The
::afterpseudo-element is alreadydisplay: inlinewithms-1margin, meaning it occupies inline space in the document flow even when invisible (opacity-0). Since UnoCSS preset-icons sizes icons viawidth: 1em; height: 1em, applyingfont-sizeonly in the:hoverstate causes the reserved space to change size on hover, producing a visible text reflow/layout shift in the heading.Moving the declaration to the base rule keeps the reserved space constant regardless of hover state.
🐛 Proposed fix
.readme :deep(a[href^='#']::after) { /* I don't know what kind of sorcery this is, but it ensures this icon can't wrap to a new line on its own. */ content: '__'; - `@apply` inline i-lucide:link rtl-flip ms-1 opacity-0; + `@apply` inline i-lucide:link rtl-flip ms-1 opacity-0; + font-size: 0.75em; } .readme :deep(a[href^='#']:hover::after) { - `@apply` opacity-100; - font-size: 0.75em; + `@apply` opacity-100; }
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
app/components/Readme.vue
Outdated
| .readme :deep(a[href^='#']:hover::after) { | ||
| @apply opacity-100; | ||
| font-size: 0.75em; |
There was a problem hiding this comment.
I think this should be set not just on hover, but in general on the ::after pseudo element
also a question: do we need to add some padding to increase tap target?
There was a problem hiding this comment.
Ahh, yes, it should be on the ::after!
Hmm, the whole heading is the tap target, so I don't think it needs padding in that case, unless we want to add functionality to copy to clipboard only when the icon is clicked, then probably it's good to have a bigger tap target specifically for the icon
Fixes: #1563