Fix: allow protocol card images to overflow on hover instead of being… #17
Conversation
WalkthroughUpdated Changes
Sequence Diagram(s)Not applicable — changes are presentational (UI/hover) only. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
components/card.tsx (2)
18-18: Let the zoomed image reliably paint above neighbors; make overflow intent explicit.Without
overflow-hidden, the scaled image can be occluded by adjacent cards in certain stacking contexts. Recommend: explicitly setoverflow-visibleon the wrapper and elevate the card on hover.- <div - key={index} - className="group relative flex h-auto flex-col justify-evenly rounded-lg border border-zinc-200 p-6 shadow-xl" - > + <div + key={index} + className="group relative flex h-auto flex-col justify-evenly rounded-lg border border-zinc-200 p-6 shadow-xl z-0 hover:z-10 isolate" + > @@ - <div className="relative h-fit rounded-md flex justify-center items-center"> + <div className="relative h-fit rounded-md flex justify-center items-center overflow-visible">Quick check: hover a middle card next to others and confirm the zoom isn’t clipped or painted under neighbors.
25-25: Smoother transform, fewer accidental intercepts.Tighten transition to transforms, hint the compositor, and avoid the image intercepting clicks when it overflows.
- className="h-64 w-full object-contain transition duration-500 group-hover:scale-110" + className="h-64 w-full object-contain transition-transform duration-500 ease-out will-change-transform transform-gpu group-hover:scale-110 pointer-events-none"Optional: mirror the effect for keyboard users with
group-focus-within:scale-110, and consider a more descriptivealt(e.g.,alt={title}) for a11y.
There was a problem hiding this comment.
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)
27-59: Remove misused key prop.Line 29 uses
key={index}on a div that is not inside a.map()iteration. Thekeyprop should only be used when rendering lists of elements. Since thisCardcomponent is likely called within a.map()in the parent component, the key should be applied there, not within the component itself.Apply this diff to remove the unnecessary key prop:
<Link href={link}> <div - key={index} className="group relative flex h-auto flex-col justify-evenly rounded-lg border border-zinc-200 p-6 shadow-xl" >
- Added `hover:z-10 focus-visible:z-10` to card wrapper for proper stacking - Maintains smooth hover zoom overflow and accessibility - Fixes image clipping and layering in protocol card grid
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/card.tsx (1)
4-12: Remove unusedindexprop from interface.The
indexprop is defined in thePropsinterface but is not destructured or used anywhere in the component. This creates inconsistency and should be removed.Apply this diff to remove the unused prop:
interface Props { - index: number image: string title: string description: string link: string }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/card.tsx(1 hunks)
🔇 Additional comments (5)
components/card.tsx (5)
14-14: Good accessibility improvement!Adding
aria-label={title}to the Link improves screen reader navigation by providing clear context for the link destination.
16-16: LGTM! Z-index stacking issue resolved.The addition of
hover:z-10andfocus-visible:z-10addresses the previous review concern about scaled images being occluded by neighboring cards. The transition andhover:shadow-2xlclasses add nice polish to the hover effect.Based on previous review comments.
18-28: Core fix implemented correctly!The removal of
overflow-hiddenfrom the image container (line 19) successfully addresses the PR objective of allowing protocol card images to overflow on hover instead of being clipped. Additional improvements:
- The
flex justify-center items-centerclasses properly center the image- Converting
widthandheightfrom strings to numbers (lines 24-25) is correct—Next.js Image component expects numeric values- Increasing hover scale to 110 (line 26) makes the zoom effect more pronounced
31-31: Minor cleanup noted.Removed extraneous space in className string—good attention to detail.
34-41: Cleaner description rendering.Refactoring from explicit
returnto inline map expression improves readability. Using index as thekeyprop is acceptable here since description lines are static text that won't be dynamically reordered.
1 participant
When hovering over protocol images, they were getting clipped by card margins.
Changes made:
overflow-hiddenfrom image containerFixes #16
Summary by CodeRabbit
New Features
Accessibility
Style