Conversation
marlinejohn
requested review from
Antonov548,
Bianca-Dascalescu,
ONDANINE,
SAK001-WB,
awalmubarak,
fabiorodriguesdev,
japint,
ls-ramos and
sonali582000
| <Route path="/explore" element={<ExplorePage />} /> | ||
| <Route path="/card/:id" element={<CardDetailPage />} /> | ||
| <Route path="/detail" element={<ImageDetailPage />} /> | ||
| <Route path="/detail/:id" element={<ImageDetailPage />} /> |
There was a problem hiding this comment.
The ImageDetailPage doesn't seem to handle the id parameter, as in the CardDetailPage component that uses useParams hook to extract the id parameter from the route. My understanding is that if the id is required for rendering the content it might lead to unexpected behavior when the id might be needed for fetching the data or rendering specific details.
My suggestion would be to use the same hook as in the CardDetailsPage to extract the id. If the ImageDetailPage was not intended, might need adjustment.
|
|
||
| function ImageDetailPage() { | ||
| const location = useLocation(); | ||
| const { image } = location.state || {}; |
There was a problem hiding this comment.
It can happen that the location is undefined or the image is missing so we need to check for it.
e.g.
if (!image){ return <div>No image available</div> }
| {galleryImages.map((image, index) => ( | ||
| <div key={index} className={styles.imageContainer}> | ||
| <div className={styles.imageWrapper}> | ||
| <Link to={`/detail/${index}`} key={index} state={{ image: image }} > |
There was a problem hiding this comment.
The key prop isn't needed here as you specified it already in line 74.
3 participants
No description provided.