-
Notifications
You must be signed in to change notification settings - Fork 17
fixed-protocol-image-cropping-on-hover #22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe Card component's CSS overflow behavior has been modified, changing the outer container and inner image wrapper from Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/card.tsx (1)
19-27: Image wrapper overflow-visible compounds layout issues.Changing the image wrapper to
overflow-visibleallows the scaled image (105% on hover) to extend beyond its container. Combined with the outer container's overflow-visible, this creates multiple issues:
- The
rounded-mdclass becomes ineffective since content can overflow the rounded corners.- The visual hierarchy and containment are lost.
- Adjacent elements may be obscured during hover.
The current approach attempts to prevent cropping during the scale animation, but
overflow-visibleis not the optimal solution for interactive scale effects within card layouts.
| <div | ||
| key={index} | ||
| className="group relative flex h-auto flex-col justify-evenly rounded-lg border border-zinc-200 p-6 shadow-xl" | ||
| className="group relative flex h-auto flex-col justify-evenly rounded-lg border border-zinc-200 p-6 shadow-xl overflow-visible" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overflow-visible may cause layout issues and break visual design.
Adding overflow-visible to the card container allows content to extend beyond the border, shadow, and rounded corners. This can cause several problems:
- Visual inconsistency: Overflowing content extends past the card's visual boundaries (border and shadow), breaking the intended design containment.
- Adjacent card overlap: In a grid layout, scaled images will overlap neighboring cards without proper z-index management.
- Lost clickable area: Overflowing portions may not trigger the Link wrapper correctly.
Consider these alternative approaches:
Solution 1 (Recommended): Keep overflow-hidden and adjust the scale effect
- className="group relative flex h-auto flex-col justify-evenly rounded-lg border border-zinc-200 p-6 shadow-xl overflow-visible"
+ className="group relative flex h-auto flex-col justify-evenly rounded-lg border border-zinc-200 p-6 shadow-xl overflow-hidden transition-all duration-500 hover:shadow-2xl hover:z-10"Then scale down the image initially so hover brings it to 100%:
- className="h-64 w-full object-contain transition duration-500 group-hover:scale-105"
+ className="h-64 w-full object-contain transition duration-500 scale-95 group-hover:scale-100"Solution 2: Add z-index management if overflow is necessary
- className="group relative flex h-auto flex-col justify-evenly rounded-lg border border-zinc-200 p-6 shadow-xl overflow-visible"
+ className="group relative flex h-auto flex-col justify-evenly rounded-lg border border-zinc-200 p-6 shadow-xl overflow-visible hover:z-10"However, this still breaks the visual design boundaries and requires careful grid spacing.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In components/card.tsx around line 17, the card container uses overflow-visible
which lets child content bleed past borders/shadows and cause layout/interaction
issues; change overflow-visible to overflow-hidden to contain visuals, then
adjust the image scaling behavior so hover scales from a slightly smaller base
(e.g., set the image to scale-95 normally and scale-100 on hover) so it doesn't
overflow when animating; if overflow must be preserved, instead keep
overflow-visible but add explicit z-index to the hovered element and increase
grid spacing to prevent overlap, though the preferred fix is replacing
overflow-visible with overflow-hidden and tweaking the image scale transition.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.