fix: blog share pill click target - #692
Conversation
✅ Deploy Preview for incredible-eclair-ada155 ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The share button size seems to be reduced after the change, let's avoid that Before 
After 
|
| <BlogShareDialog | ||
| blogURL={url} | ||
| size={20} | ||
| triggerClassName='px-[10px] py-[6px] bg-foreground-light/80 dark:bg-foreground-dark/80 backdrop-blur-sm rounded-full shadow-sm ring-1 ring-border-light dark:ring-border-dark hover:bg-foreground-light dark:hover:bg-foreground-dark hover:ring-brand-orange/50 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-brand-orange' |
There was a problem hiding this comment.
Do we really need this many tailwind properties here?
There was a problem hiding this comment.
Most of these classes are not required for the click functionality. They are needed only to preserve the existing pill appearance and add clearer hover/focus feedback.
So, the full list is not strictly required, but the padding, background, rounded shape, and hover/focus styles should remain to keep the intended design and UX.
SO what should i do with them lmk.
I feel it looks fine tho.
| DialogTrigger, | ||
| } from '@the-monkeys/ui/atoms/dialog'; | ||
| import { toast } from '@the-monkeys/ui/hooks/use-toast'; | ||
| import { twMerge } from 'tailwind-merge'; |
There was a problem hiding this comment.
why did we import twmerge here?
There was a problem hiding this comment.
the twMerge is used so the default button classes and the caller-provided triggerClassName can safely be combined. It resolves conflicting Tailwind utilities such as:
'p-1' and: 'px-[10px] py-[6px]'
Without twmerge, both classes may remain in the final class string, and the resulting styling can depend on Tailwind’s generated CSS order. twMerge ensures the custom trigger styles correctly override the defaults.
|
Please rebase your pr and address all the comments |
4 participants
📃 Why Merge This PR?
Summary
triggerClassNameprop toBlogShareDialogfor caller-specific trigger styling.See the implemented fix in action:
Before (current behaviour):
currentSharePill.mp4
After (Expected Behaviour):
expectedSharePill.mp4
🛠️ Issue Fixed
#691
🔍 PR Type