-
Notifications
You must be signed in to change notification settings - Fork 39
[김서연] Sprint7 #209
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
[김서연] Sprint7 #209
The head ref may contain hidden characters: "React-\uAE40\uC11C\uC5F0-sprint7"
Conversation
…nd/15-Sprint-Mission into React-김서연-sprint6
GANGYIKIM
left a comment
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.
서연님 7번째 미션 작업 고생하셨습니다.
이제 12개의 미션중 절반 이상을 하셨네요!
지금처럼 남은 미션들도 화이팅이에요~
| @@ -1,12 +1,9 @@ | |||
| import AddItemContent from "../components/AddItem/AddItem"; | |||
| import Banner from "../components/Item/Banner"; | |||
| import Header from "../components/Item/Header"; | |||
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.
❗️ 수정요청
사용하지 않는 컴포넌트면 지워주세요~
| import Header from "../components/Item/Header"; |
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.
💊 제안
페이지들을 구조화하시려고 시도하신 점 좋습니다.
다만 layout 개념에 대해 다시 생각해보시면 좋겠습니다.
지금 item과 item Detail 의 페이지를 Item이라는 컴포넌트로 감싸고 있고 이 Item을 레이아웃 컴포넌트라고 얘기할 수 있습니다.
해당 레이아웃에는 자식 페이지들에서 공통으로 사용되는 요소들만 들어가는 것이 적절합니다. 이에 BestItem, AllItem은 속하지 않습니다~
또한 지금까지 작업한 페이지들을 기반으로 Header를 분리하면 아래와 같을 것 같아요!
- header X, footer X 페이지: 로그인, 회원가입
- header O(menuX), footer O: 메인
- header O(menuO), footer X: 중고마켓, 아이템 상세 페이지
이를 기반으로 레이아웃을 어떻게 분리해야할 지 다시 고민해보시고 작업하시면 좋겠습니다~
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.
💊 제안
이렇게 쪼갤때의 장점에 대해 잘 모르겠습니다.
제가 저번에 드린 피드백으로 이러한 시도를 해보신 것 같아요. 다만 제가 말씀드리고 싶었던 방향은 지금처럼 react router dom의 Declarative 모드가 아닌 Data와 같은 다른 모드를 이용해서 객체로 관리하시라는 말씀이었어요 🥹
By moving route configuration outside of React rendering, Data Mode adds data loading, actions, pending states and more with APIs like loader, action, and useFetcher
Data 모드는 문서에서 말하는 것처럼 리엑트 컴포넌트와 라우트를 분리해서 작성할 수 있는 모드입니다.
물론 다른 특징도 있고 Declarative 모드보다 더 다양한 기능을 제공하니 이로 변경해보시라는 의미였어요~
https://reactrouter.com/start/modes
https://api.reactrouter.com/v7/types/react_router.RouteObject.html
| comments.map((comment, index) => ( | ||
| <Review key={index} comment={comment} /> | ||
| ))} |
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.
❗️ 수정요청
key는 렌더링 최적화를 위한 값으로 key로 사용할 수 있는 다른 유니크한 값이 있다면 해당 값을 이용해주세요~
저라면 comment id 값을 이용할 것 같아요!
| comments.map((comment, index) => ( | |
| <Review key={index} comment={comment} /> | |
| ))} | |
| comments.map((comment) => ( | |
| <Review key={comment.id} comment={comment} /> | |
| ))} |
https://panda-market-api.vercel.app/docs/#/Comment/ListProductComments
| return <p>상품 정보를 불러오는 중입니다...</p>; | ||
| } | ||
|
|
||
| console.log(comments); |
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.
❗️ 수정요청
| console.log(comments); |
| fetchData(); | ||
| }, [productId]); | ||
|
|
||
| if (loading || !items) { |
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.
💊 제안
로딩중이거나 아이템이 없을 때 대체 UI를 보여주시는 점 좋습니다~
다만 로딩중일 때와 결과값이 없을 때는 다른 경우이니 다른 UI면 더 좋을 것 같아요~
| export function getTimeAgo(dateString) { | ||
| const createdDate = new Date(dateString); | ||
| const now = new Date(); | ||
|
|
||
| const diffMs = now - createdDate; | ||
| const diffSec = Math.floor(diffMs / 1000); | ||
| const diffMin = Math.floor(diffSec / 60); | ||
| const diffHour = Math.floor(diffMin / 60); | ||
| const diffDay = Math.floor(diffHour / 24); | ||
|
|
||
| if (diffDay >= 1) { | ||
| return `${diffDay}일 전`; | ||
| } else if (diffHour >= 1) { | ||
| return `${diffHour}시간 전`; | ||
| } else if (diffMin >= 1) { | ||
| return `${diffMin}분 전`; | ||
| } else { | ||
| return `방금 전`; | ||
| } | ||
| } |
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.
💊 제안
relativeTime 함수를 직접 구현하신 것 좋습니다.
더 간편하게 구현하시기 위해 브라우저 제공 API인 Intl.RelativeTimeFormat을 활용하실 수 있습니다.
내장 메소드이기에 더 정확하며 로직도 간결해질 수 있습니다~
https://velog.io/@real-bird/JavaScript-Intl.RelativeTimeFormat
| } | ||
|
|
||
| export default function Review({ comment }) { | ||
| const name = comment.writer.nickname; |
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.
💊 제안
해당 값만 객체에서 따로 변수에 담아줘야할 이유가 있을까요?
JSX에서 바로 comment.writer.nickname로 접근하셔도 될 것 같아요~
| <div className={styles.kebabMenu} ref={menuRef}> | ||
| <button | ||
| onClick={toggleMenu} | ||
| style={{ background: "none", border: "none" }} |
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.
💊 제안
인라인 스타일링보다 클래스를 이용해주세요~
| import styles from "./styles/KebabMenu.module.css"; | ||
|
|
||
| export default function KebabMenu({ onEdit, onDelete }) { | ||
| const [open, setOpen] = useState(false); |
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.
💊 제안
bool 타입을 알 수 있는 이름은 isOpen 같은 이름을 추천드려요. 일반적으로 리액트에서는 함수가 동사로 시작합니다~
요구사항
배포사이트
https://sprint-mission7-pandamerket.netlify.app/item/199
기본
심화
주요 변경사항
스크린샷
멘토에게