[TASK-16465] feat: Add merchant map cta#1408
[TASK-16465] feat: Add merchant map cta#1408Zishan-7 wants to merge 15 commits intopeanut-walletfrom
Conversation
Zishan-7
commented
Nov 6, 2025
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedFailed to post review comments WalkthroughAdded support for an optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/hooks/useHomeCarouselCTAs.tsx (2)
45-45: Consider hosting the flag icon locally to avoid external dependency.The secondary icon uses an external CDN URL (flagcdn.com) which introduces a dependency on third-party availability. If the CDN fails or the URL changes, the icon won't load and there's no error handling or fallback.
Consider either:
- Hosting the Brazilian flag icon locally as a static asset alongside the PIX logo
- Adding error handling/fallback for the external image
For a local asset approach:
- secondaryIcon: 'https://flagcdn.com/w320/br.png', + secondaryIcon: BrazilFlag, // Import from @/assets
42-42: Icon choice doesn't match the CTA purpose.The 'shield' icon is typically associated with security or verification, but this CTA is about merchant maps and PIX payments. Consider using a more semantically appropriate icon like 'map', 'location', or 'payment'.
- icon: 'shield', + icon: 'map', // or 'location' if availablesrc/components/Home/HomeCarouselCTA/CarouselCTA.tsx (2)
109-109: Consider reducing quality setting for small icon.The
quality={100}setting may be excessive for a 16px icon. The default quality of 75 is typically sufficient for small images and would reduce bandwidth without perceptible visual difference.- quality={100} + quality={75}Or simply omit the quality prop to use the default:
src={typeof secondaryIcon === 'string' ? secondaryIcon : secondaryIcon.src} alt="secondary icon" height={16} width={16} - quality={100} className="absolute -right-1 bottom-0 z-50 size-4 rounded-full object-cover"
106-106: Consider more descriptive alt text.The generic "secondary icon" alt text doesn't convey meaningful information to screen reader users. Consider a more descriptive alternative based on the actual content.
- alt="secondary icon" + alt="country flag"Or make it dynamic based on the CTA context if different CTAs use different secondary icons for different purposes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/Home/HomeCarouselCTA/CarouselCTA.tsx(3 hunks)src/components/Home/HomeCarouselCTA/index.tsx(1 hunks)src/hooks/useHomeCarouselCTAs.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/Home/HomeCarouselCTA/CarouselCTA.tsx (1)
src/components/Global/Icons/Icon.tsx (1)
Icon(209-218)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (6)
src/hooks/useHomeCarouselCTAs.tsx (2)
10-10: LGTM!The PIX asset import is properly used as the logo for the new CTA.
23-23: LGTM!The type definition appropriately supports both static image imports and URL strings, providing flexibility for different icon sources.
src/components/Home/HomeCarouselCTA/index.tsx (1)
34-34: LGTM!The secondaryIcon prop is correctly passed through from the CTA data to the CarouselCTA component.
src/components/Home/HomeCarouselCTA/CarouselCTA.tsx (3)
22-22: LGTM!The secondaryIcon prop is properly typed and destructured, maintaining consistency with the type definition in the hook.
Also applies to: 34-34
92-92: LGTM!The coordinated size increases (container, icon, and logo) are proportional and make room for the secondary icon badge. The sizing remains visually balanced.
Also applies to: 98-98, 100-101
105-105: LGTM!The type checking correctly handles both StaticImageData (accessing
.src) and string URLs, matching the type union defined in the props.
|
Closing in favor of #1410 |