perf(ui): optimize avatar rendering by replacing <img> with next/image in testimonials#347
perf(ui): optimize avatar rendering by replacing <img> with next/image in testimonials#347SadhanaShree25 wants to merge 12 commits intokeploy:mainfrom
Conversation
Signed-off-by: SadhanaShree25 <180205437+SadhanaShree25@users.noreply.github.com> Signed-off-by: frontend <edstartupteam@gmail.com>
Signed-off-by: SadhanaShree25 <180205437+SadhanaShree25@users.noreply.github.com>
|
Hey @SadhanaShree25 👋 — thanks for putting this PR together, we appreciate the effort! We've gone ahead and requested a Copilot review on this. Here's some context from the reviewer:
Once you've had a chance to go through the comments, please address the feedback and resolve the threads — and we'll get this across the line. Feel free to ask if anything's unclear. Happy coding! 💙 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the testimonials ReviewCard avatar rendering to use Next.js next/image, and adjusts surrounding markup to improve rendering behavior and styling consistency.
Changes:
- Replaced
<img>with<Image>for avatar rendering, using numericwidth/height. - Added
useRouter()+ URL logic to proxy external avatar URLs through an internal API route. - Minor className/whitespace tweaks and added gradient overlay divs to the marquee container.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Sadhana Shree <180205437+SadhanaShree25@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
.github/workflows/lighthouse_comment.yml:24
- This workflow is now triggered on
pull_request, but it attempts to download thelighthouse-commentartifact without specifying arun-id.actions/download-artifactwill only download artifacts from the current workflow run by default, so this job will not find the artifact produced by the separateLighthouse – Runworkflow. Consider reverting to aworkflow_runtrigger (and using the triggering run’s id), or restructure so the comment generation and posting happen in the same workflow/run.
on:
pull_request:
branches:
- main
permissions:
issues: write
pull-requests: write
jobs:
comment:
runs-on: ubuntu-latest
permissions:
contents: read
pull-requests: write
steps:
- name: Download Lighthouse comment artifact
uses: actions/download-artifact@v4
with:
name: lighthouse-comment
path: .
lint_utf8.txt:23
lint_utf8.txtlooks like capturednext lintoutput (including a UTF-8 BOM on line 1) and contains transient warnings. Committing this kind of build/lint output into the repo will create noise and go stale quickly; it’s better to keep lint output in CI logs/artifacts instead of version control.
> lint
> next lint
./components/more-stories.tsx
65:6 Warning: React Hook useEffect has a missing dependency: 'localSearchTerm'. Either include it or remove the dependency array. react-hooks/exhaustive-deps
151:6 Warning: React Hook useEffect has missing dependencies: 'buffer.length', 'initialPageInfo?.hasNextPage', and 'loadMoreInBackground'. Either include them or remove the dependency array. react-hooks/exhaustive-deps
./components/navbar/FloatingNavbarClient.tsx
202:6 Warning: React Hook useEffect has missing dependencies: 'communityState.length', 'router.basePath', and 'techState.length'. Either include them or remove the dependency array. react-hooks/exhaustive-deps
764:6 Warning: React Hook useEffect has a missing dependency: 'router?.basePath'. Either include it or remove the dependency array. react-hooks/exhaustive-deps
784:6 Warning: React Hook useEffect has missing dependencies: 'communityLatest' and 'techLatest'. Either include them or remove the dependency array. react-hooks/exhaustive-deps
./components/post-body.tsx
152:6 Warning: React Hook useEffect has a missing dependency: 'isUserEnteredURL'. Either include it or remove the dependency array. react-hooks/exhaustive-deps
221:6 Warning: React Hook useEffect has a missing dependency: 'handleHeadingCopyClick'. Either include it or remove the dependency array. react-hooks/exhaustive-deps
./components/testimonials.tsx
31:11 Warning: Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element @next/next/no-img-element
info - Need to disable some ESLint rules? Learn more here: https://nextjs.org/docs/basic-features/eslint#disabling-rules
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hi @SadhanaShree25 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
.github/workflows/lighthouse_comment.yml:23
- This workflow is triggered on
pull_requestbut it downloads an artifact namedlighthouse-commentwithout any prior step/job in the same workflow run uploading that artifact. As written,actions/download-artifact@v4will fail because the artifact won’t exist. Either switch back toworkflow_runfor the "Lighthouse – Run" workflow and download from that run-id, or generate/upload thelighthouse-comment.mdartifact in this workflow before attempting to download it.
on:
pull_request:
branches:
- main
permissions:
issues: write
jobs:
comment:
runs-on: ubuntu-latest
permissions:
contents: read
pull-requests: write
steps:
- name: Download Lighthouse comment artifact
uses: actions/download-artifact@v4
with:
name: lighthouse-comment
path: .
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | ||
| > lint | ||
| > next lint | ||
|
|
||
|
|
||
| ./components/more-stories.tsx | ||
| 65:6 Warning: React Hook useEffect has a missing dependency: 'localSearchTerm'. Either include it or remove the dependency array. react-hooks/exhaustive-deps | ||
| 151:6 Warning: React Hook useEffect has missing dependencies: 'buffer.length', 'initialPageInfo?.hasNextPage', and 'loadMoreInBackground'. Either include them or remove the dependency array. react-hooks/exhaustive-deps | ||
|
|
||
| ./components/navbar/FloatingNavbarClient.tsx | ||
| 202:6 Warning: React Hook useEffect has missing dependencies: 'communityState.length', 'router.basePath', and 'techState.length'. Either include them or remove the dependency array. react-hooks/exhaustive-deps | ||
| 764:6 Warning: React Hook useEffect has a missing dependency: 'router?.basePath'. Either include it or remove the dependency array. react-hooks/exhaustive-deps | ||
| 784:6 Warning: React Hook useEffect has missing dependencies: 'communityLatest' and 'techLatest'. Either include them or remove the dependency array. react-hooks/exhaustive-deps | ||
|
|
||
| ./components/post-body.tsx | ||
| 152:6 Warning: React Hook useEffect has a missing dependency: 'isUserEnteredURL'. Either include it or remove the dependency array. react-hooks/exhaustive-deps | ||
| 221:6 Warning: React Hook useEffect has a missing dependency: 'handleHeadingCopyClick'. Either include it or remove the dependency array. react-hooks/exhaustive-deps | ||
|
|
||
| ./components/testimonials.tsx | ||
| 31:11 Warning: Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element @next/next/no-img-element | ||
|
|
||
| info - Need to disable some ESLint rules? Learn more here: https://nextjs.org/docs/basic-features/eslint#disabling-rules |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Sadhana Shree <180205437+SadhanaShree25@users.noreply.github.com>
|
Thank you for the time and effort you put into improving avatar rendering with next/image, Sadhana! We genuinely appreciate contributors who take the initiative to address performance and correctness issues like this. We are closing this PR at this time because the review feedback we shared - removing the accidentally committed lint_output.txt, restoring the avatar fallback handling that was removed, and resolving the conflict with the lighthouse workflow files - has been open for over 17 days with no updates from the branch. Please do not let this discourage you at all. There are plenty of other open issues in the repository that would be a great fit for your skills, and we would love to see more contributions from you! If you would like guidance on what to work on next or have any questions, please reach out to us on Slack at https://keploy.slack.com/join/shared_invite/zt-357qqm9b5-PbZRVu3Yt2rJIa6ofrwWNg |
Description of the Change
This PR updates the
ReviewCardcomponent incomponents/testimonials.tsxto replace the standard<img>tag with the Next.js<Image />component for optimized avatar rendering.Why is this needed?
Performance & Core Web Vitals
The native
<img>tag does not automatically optimize images or serve modern formats (like WebP). Usingnext/imageimproves performance, especially Largest Contentful Paint (LCP).Layout Shift Prevention
Standard
<img>tags can cause Cumulative Layout Shift (CLS) if dimensions are not strictly enforced. The<Image />component ensures proper sizing and improves visual stability.Consistency
Other components (e.g.,
components/tweets.tsx) already usenext/image. This change alignstestimonials.tsxwith the existing pattern and resolves the@next/next/no-img-elementESLint warning.Additional Changes
.github/workflows/lighthouse_comment.ymlto ensure the Lighthouse workflow triggers correctly on pull requests.lint_utf8.txtfile generated during linting.Changes Proposed
next/imageintocomponents/testimonials.tsx<img />with<Image />in theReviewCardcomponentwidthandheightprops from string values ("32") to numeric values ({32})Checklist