Replace img tags with Next.js Image component for optimized image loading #34
Conversation
WalkthroughReplaced native Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User Agent / Browser
participant C as Component (Header/Product/Footer)
participant I as next/Image
participant S as CDN/Server
rect rgb(240,248,255)
C->>I: Render <Image src=... sizes=... placeholder="blur" blurDataURL=... priority? />
end
rect rgb(245,255,235)
I->>U: Show blur placeholder (low-res)
U-->>I: Request full-res (on viewport/priority)
I->>S: Fetch optimized asset (WebP/size)
S-->>I: Return optimized image
I->>U: Display optimized image
end
alt image load error
U->>C: image load failed
C->>C: set imgSrc -> fallback
C->>I: re-render <Image> with fallback src
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 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: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/footer-pill.tsx(2 hunks)src/components/header.tsx(2 hunks)src/components/product/ProductCard.tsx(2 hunks)
|
@Vaibhav91one Could you rebase with main to resolve the merge conflicts? |
…ding and performance
Vaibhav91one
force-pushed
the
feature/image-optimization
branch
from
5e71000 to
23c36c3
Compare
|
Done @Amanfromearth. |
|
@Vaibhav91one lingo.dev is translating the
and then use:
instead of passing |
|
Hi @Amanfromearth, I have made the changes and also I have made changes that coderabbit requested. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/footer-pill.tsx (1)
37-45: Consider using standard<img>for small SVG logos.The Next.js
Imagecomponent is now correctly implemented with a proper base64blurDataURL(past issue resolved). However, for small static SVG logos like this 20×20px brand asset, Next.jsImageoptimization provides minimal benefit since:
- SVGs are already optimized vector graphics
- No format conversion (e.g., WebP) applies to SVGs
- The optimization overhead may outweigh benefits for such small assets
If you prefer a simpler approach for the SVG logo:
-<Image - src="/images/brand-assets/logo/logo.svg" - alt="logo" - width={20} - height={20} - className="object-cover object-center" - placeholder={IMAGE_PLACEHOLDER} - blurDataURL={BLUR_DATA_URL} -/> +<img + src="/images/brand-assets/logo/logo.svg" + alt="logo" + width={20} + height={20} + className="object-cover object-center" +/>Note: Given the PR objective explicitly states replacing ALL
<img>tags, the current implementation may be intentional. This suggestion is optional.src/components/header.tsx (1)
32-40: Consider standard<img>for SVG logo (same as footer).The logo
Imagecomponent now correctly uses a base64blurDataURL(past issue resolved). However, similar to the footer logo, using Next.jsImagefor a 72×72px SVG logo provides minimal optimization benefit since SVGs are already optimized vector graphics.If you prefer a simpler approach:
-<Image - src={business.logo || "/images/brand-assets/logo/logo.svg"} - alt="Business Logo" - width={72} - height={72} - className="object-cover object-center" - placeholder={IMAGE_PLACEHOLDER} - blurDataURL={BLUR_DATA_URL} -/> +<img + src={business.logo || "/images/brand-assets/logo/logo.svg"} + alt="Business Logo" + width={72} + height={72} + className="object-cover object-center" +/>Note: This is optional since the current implementation is technically correct and aligns with the PR objective to replace ALL
<img>tags.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/lingo/meta.json(3 hunks)src/components/footer-pill.tsx(2 hunks)src/components/header.tsx(2 hunks)src/components/product/ProductCard.tsx(3 hunks)
🔇 Additional comments (8)
src/components/footer-pill.tsx (1)
7-11: LGTM! Constants approach successfully avoids lingo.dev translation issues.The implementation correctly follows the recommendation from PR comments to define
IMAGE_PLACEHOLDERas a constant rather than passing the string literal"blur"directly, preventing lingo.dev from attempting to translate it. TheBLUR_DATA_URLis also properly formatted as a base64-encoded data URL.Based on PR comments.
src/components/product/ProductCard.tsx (3)
1-13: LGTM! Proper imports and constants defined.The implementation correctly imports
useEffectfor prop synchronization and defines appropriate constants for image handling. The constants approach matches the pattern used across other components and prevents lingo.dev translation issues.
57-67: Excellent error handling implementation!The error handling logic is well-implemented:
- State management properly tracks the current image source
useEffectsynchronizesimgSrcwhen theimageprop changeshandleImageErrorusesuseCallbackfor performance and prevents redundant state updates by checking if already on fallback- Prevents infinite error loops
This addresses the past review concern about missing error handling.
113-122: LGTM! Image component properly configured.The
Imagecomponent implementation correctly addresses all past review concerns:
- Uses state-managed
imgSrcfor dynamic fallback handling- Proper
filland responsivesizesattributes- Valid base64
blurDataURL(past issue resolved)onErrorhandler connected (past issue resolved)- No redundant
priority={false}prop (past issue resolved)The implementation meets all acceptance criteria for image optimization with error handling.
src/components/header.tsx (3)
10-12: LGTM! Constants properly defined.The constants follow the same pattern as other components and prevent lingo.dev translation issues as recommended in PR comments.
18-27: LGTM! Banner image properly optimized.The banner
Imagecomponent is correctly configured:
- Proper base64
blurDataURL(past issue resolved)priorityprop correctly set since the banner is above the foldsizes="100vw"appropriate for full-width bannerfillfor responsive container-bound sizing
1-8: Business interface expansion is compatible with backend changes and requires no additional updates.The verification confirms all required fields (
business_id,logo,name) are already provided by the backendBusinessResponsetype. The single consumer of theBusinessinterface is the Header component insrc/app/[slug]/page.tsx, which receives the complete data object fromgetBusiness(). The interface expansion aligns with the backend response structure—no additional changes needed.src/app/lingo/meta.json (1)
1-465: Translation metadata properly updated.The lingo metadata file has been updated with new entries corresponding to the Image components' alt text and UI strings added across footer, header, and product components. The JSON structure is valid and changes align with the component updates.
2 participants
Summary
This PR replaces standard
<img>tags with Next.js's<Image>component across the application to leverage automatic image optimization and improve performance.Changes Made
src/components/footer-pill.tsx- Converted Dodo logo to Next.js Imagesrc/components/header.tsx- Converted business banner and logo to Next.js Imagesrc/components/product/ProductCard.tsx- Converted product images to Next.js ImageTechnical Details
fillprop for container-bound imagespriorityfor above-the-fold banner imagesplaceholder="blur"with appropriateblurDataURLsizesattribute for responsive behaviorCloses #30
Summary by CodeRabbit
Chores
New Features