Conversation
|
@Deejarh Some tests are failing. Can you please fix them? |
|
|
womencodingcommunity
left a comment
womencodingcommunity
left a comment
There was a problem hiding this comment.
Thanks for working on the mentor image rendering issue @Deejarh! 🎉
Here are some inline suggestions and feedback.
The main recommendation is to avoid disabling Next.js Image Optimization globally via next.config.mjs (which disables WebP/AVIF generation, compression, and responsive resizing across the entire website) and instead scope unoptimized directly to the <Image unoptimized /> component in MentorProfileCard.tsx alongside a safeguard against infinite fallback error loops.
| return config; | ||
| }, | ||
| images: { | ||
| unoptimized: true, |
There was a problem hiding this comment.
Setting unoptimized: true globally disables Next.js Image Optimization for all images across the whole website (heroes, event cards, blog posts, etc.).
Instead of disabling optimization globally, we can add the unoptimized prop directly to the specific <Image /> component in MentorProfileCard.tsx.
| unoptimized: true, |
| import { Box, Icon, Tab, Tabs, Typography } from '@mui/material'; | ||
| import Image from 'next/image'; | ||
| import React, { useState } from 'react'; | ||
| import React, { useCallback, useState } from 'react'; |
There was a problem hiding this comment.
| import React, { useCallback, useState } from 'react'; | |
| import React, { useCallback, useEffect, useState } from 'react'; |
| const [imgSrc, setImgSrc] = useState( | ||
| mentor.images[0]?.path || FALLBACK_IMAGE, | ||
| ); | ||
| const handleImageError = useCallback(() => { | ||
| setImgSrc(FALLBACK_IMAGE); | ||
| }, []); |
There was a problem hiding this comment.
- Sync
imgSrcwhen thementorprop updates (e.g. during search/filter re-renders). - Guard against infinite error loops if
FALLBACK_IMAGEever fails to load.
| const [imgSrc, setImgSrc] = useState( | |
| mentor.images[0]?.path || FALLBACK_IMAGE, | |
| ); | |
| const handleImageError = useCallback(() => { | |
| setImgSrc(FALLBACK_IMAGE); | |
| }, []); | |
| const [imgSrc, setImgSrc] = useState( | |
| mentor.images[0]?.path || FALLBACK_IMAGE, | |
| ); | |
| useEffect(() => { | |
| setImgSrc(mentor.images[0]?.path || FALLBACK_IMAGE); | |
| }, [mentor.images]); | |
| const handleImageError = useCallback(() => { | |
| setImgSrc((current) => (current !== FALLBACK_IMAGE ? FALLBACK_IMAGE : current)); | |
| }, []); |
| width={120} | ||
| height={120} | ||
| onError={handleImageError} |
There was a problem hiding this comment.
Adding unoptimized here allows external mentor avatars to bypass Next.js optimization/domain limits without affecting site-wide performance:
| width={120} | |
| height={120} | |
| onError={handleImageError} | |
| width={120} | |
| height={120} | |
| unoptimized | |
| onError={handleImageError} |
| async ({ page }) => { | ||
| await expect(page.getByRole('banner')).toHaveScreenshot( | ||
| 'nav-mobile-closed.png', | ||
| { maxDiffPixels: 100 }, |
There was a problem hiding this comment.
Threshold adjustments for navigation visual regression are unrelated to mentor profile images and should be updated via snapshot update or in a separate navigation PR.
| { maxDiffPixels: 100 }, |
|
This branch has not been deployed
4 participants
Description
What Changed
Type
Related Issue
Screenshots
BEFORE
AFTER
Testing
Pull request checklist
Please check if your PR fulfills the following requirements: