-
Notifications
You must be signed in to change notification settings - Fork 4
Detail Page Navigation #177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
1a35775
027f69b
1dc5e17
5796b11
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,9 @@ | ||
| import React from "react"; | ||
| import { useParams } from "react-router-dom"; | ||
| import Breadcrumb from "./Breadcrumb"; | ||
| import styles from "./CardDetailPage.module.css"; | ||
| import { cards } from "./cardDetails"; | ||
| import { Link } from "react-router-dom"; | ||
|
|
||
| const CardDetailPage = () => { | ||
| const { id } = useParams(); | ||
|
|
@@ -72,17 +73,16 @@ | |
| {galleryImages.map((image, index) => ( | ||
| <div key={index} className={styles.imageContainer}> | ||
| <div className={styles.imageWrapper}> | ||
| <img | ||
| src={image.src} | ||
| alt={`Gallery ${index + 1}`} | ||
| className={styles.image} | ||
| /> | ||
|
|
||
|
Check failure on line 76 in frontend/src/components/ExplorePageSection1/CardDetailPage.jsx
|
||
| <Link to={`/detail/${index}`} key={index} state={{ image: image }} > | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The key prop isn't needed here as you specified it already in line 74. |
||
| <img src={image.src} alt={`Gallery ${index + 1}`} className={styles.image} /> | ||
|
Check failure on line 78 in frontend/src/components/ExplorePageSection1/CardDetailPage.jsx
|
||
|
|
||
| <div className={styles.overlay}> | ||
| <span className={styles.overlayText}>Open</span> | ||
| <div className={styles.overlayButtons}> | ||
| <img | ||
| src="/images/share-icon.svg" | ||
| alt="Share" | ||
| className={styles.shareIcon} | ||
| /> | ||
| <img | ||
|
|
@@ -92,6 +92,7 @@ | |
| /> | ||
| </div> | ||
| </div> | ||
| </Link> | ||
| </div> | ||
| <div className={styles.tags}> | ||
| {image.tags.map((tag, tagIndex) => ( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,12 @@ | ||
| import React from "react"; | ||
| import { Link } from "react-router-dom"; | ||
| import { Link, useLocation } from "react-router-dom"; | ||
| import styles from "./Detail.module.css"; | ||
| import Breadcrumb from "./ImageDetailPage/Breadcrumb"; | ||
| import ShopItem from "./ImageDetailPage/ShopItem"; | ||
|
|
||
| function ImageDetailPage() { | ||
| const location = useLocation(); | ||
| const { image } = location.state || {}; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It can happen that the location is undefined or the image is missing so we need to check for it. |
||
| return ( | ||
| <div className={styles.pageWrapper}> | ||
| <Breadcrumb /> | ||
|
|
@@ -16,7 +18,7 @@ function ImageDetailPage() { | |
| </Link> | ||
|
|
||
| <div className={styles.centered}> | ||
| <ShopItem /> | ||
| <ShopItem image={image} /> | ||
| </div> | ||
| </div> | ||
| ); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
ImageDetailPagedoesn't seem to handle theidparameter, as in theCardDetailPagecomponent that usesuseParamshook 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
CardDetailsPageto extract the id. If the ImageDetailPage was not intended, might need adjustment.