-
Notifications
You must be signed in to change notification settings - Fork 39
[윤진우] sprint7 #198
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 #198
The head ref may contain hidden characters: "React-\uC724\uC9C4\uC6B0-sprint7"
Conversation
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번째 미션 작업 고생하셨습니다~
이번에 작업 시간이 넉넉하지 않으신걸 알아서 피드백 반영을 못하셨을거라고 생각했는데 벌써 일부 반영하셨다니 대단하네요. 말씀해주신 것처럼 이를 참고해 피드백 드렸으니 추후 수정해보세요!
다음 미션도 화이팅이에요!
- 배포 사이트에서 확인시 문의하기 인풋의 폰트가 이상하게 보이네요. 한번 확인해보세요~
| useEffect(() => { | ||
| const handleKeyDown = (e) => { | ||
| if (e.key === "Escape") { | ||
| setShowDropdown(false); | ||
| } | ||
| }; | ||
|
|
||
| if (showDropdown) { | ||
| document.addEventListener("keydown", handleKeyDown); | ||
| } | ||
|
|
||
| return () => { | ||
| document.removeEventListener("keydown", handleKeyDown); | ||
| }; | ||
| }, [showDropdown]); |
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.
💊 제안
ESC 키가 입력되면 특정 메소드가 실행되는 코드로 묶을 수 있으니 custom hook으로 분리하셔서 재사용하시면 더 좋을 것 같아요.
| <button onClick={onClickButton} className="goback-button"> | ||
| 목록으로 돌아가기 | ||
| <img src={goback} alt="돌아가기" /> | ||
| </button> |
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.
💊 제안
지금 코드만으로 보면 버튼 태그보다 Link 태그를 쓰는게 더 적절해보여요.
단순 페이지 이동이 아니라 특정 로직을 실행해야한다면 지금처럼 버튼 태그를 사용하셔도 좋지만 아니라면 Link 태그로 변경해주세요~
링크 태그를 사용하시면 마우스 오른쪽 클릭이나 새탭 기능도 제공이 가능해서 UX 측면에서도 좋습니다.
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.
💊 제안
가독성을 위해 내부 로직들과 컴포넌트를 분리하시면 좋겠습니다~
크게 상품 정보 영역, 코멘트 작성 영역, 코멘트 리스트 정도로 나누면 좋겠네요~
| function timeAgo(dateInput) { | ||
| const date = new Date(dateInput); | ||
| const now = new Date(); | ||
| const seconds = Math.floor((now - date) / 1000); | ||
|
|
||
| if (seconds < 60) return `${seconds}초 전`; | ||
| const minutes = Math.floor(seconds / 60); | ||
| if (minutes < 60) return `${minutes}분 전`; | ||
| const hours = Math.floor(minutes / 60); | ||
| if (hours < 24) return `${hours}시간 전`; | ||
| const days = Math.floor(hours / 24); | ||
| if (days < 7) return `${days}일 전`; | ||
| const weeks = Math.floor(days / 7); | ||
| if (weeks < 5) return `${weeks}주 전`; | ||
| const months = Math.floor(days / 30); | ||
| if (months < 12) return `${months}개월 전`; | ||
| const years = Math.floor(days / 365); | ||
| return `${years}년 전`; | ||
| } |
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.
💊 제안
timeAgo 함수를 직접 구현하신 것 좋습니다.
더 정확하게 구현하기 위해서는 브라우저 제공 API인 Intl.RelativeTimeFormat을 활용하실 수 있습니다.
내장 메소드이기에 더 정확하며 로직도 간결해질 수 있습니다~
https://velog.io/@real-bird/JavaScript-Intl.RelativeTimeFormat
| return ( | ||
| <> | ||
| <div className="comment-section"> | ||
| <div className="make-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.
💊 제안
input 과 button 있을 경우 form을 사용하시는 것이 좋습니다~
| onChange={(e) => setContent(e.target.value)} | ||
| value={content} | ||
| ></textarea> | ||
| <button className="register-button" disabled={content.trim() === ""}> |
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.
💊 제안
문의를 남길 수 있으면 더 좋을 것 같아요~ 시간되실 때 구현해보세요!
| <div className="user-text"> | ||
| <div className="user-name">{item.ownerNickname}</div> | ||
| <div className="date"> | ||
| {item.createdAt.split("T")[0].replaceAll("-", ". ")} |
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.
💊 제안
날짜를 formatting하는 로직들은 일반적으로 재사용되는 경우가 많으니 util로 분리하시면 좋을 것 같아요.
참고로 Date 객체를 이용해서 구현하시면 간단하고 더 정확하게 구현이 가능합니다.
| {item.createdAt.split("T")[0].replaceAll("-", ". ")} | |
| {new Date(item.createdAt).toLocaleDateString("ko-KR")} |
요구사항
https://sprint-panda-yun.netlify.app/
기본
상품 상세
상품 문의 댓글
심화
주요 변경사항
스크린샷
멘토에게