-
Notifications
You must be signed in to change notification settings - Fork 39
[김영현] Sprint7 #216
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 #216
The head ref may contain hidden characters: "React-\uAE40\uC601\uD604-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번째 미션 제출 수고하셨습니다.
다음 미션도 화이팅이에요!
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
| <button className="item-detail__back-button"> | ||
| <span>목록으로 돌아가기</span> | ||
| <img src={ic_back} alt="뒤로가기" className="item-detail__back-icon" onClick={handleGoBack} /> | ||
| </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.
❗️ 수정요청
상호작용이 가능한 button 태그에 onClick 함수를 연결하시는 게 더 적절해보입니다!
| <button className="item-detail__back-button"> | |
| <span>목록으로 돌아가기</span> | |
| <img src={ic_back} alt="뒤로가기" className="item-detail__back-icon" onClick={handleGoBack} /> | |
| </button> | |
| <button className="item-detail__back-button" onClick={handleGoBack}> | |
| <span>목록으로 돌아가기</span> | |
| <img src={ic_back} alt="뒤로가기" className="item-detail__back-icon" /> | |
| </button> |
| const handleGoBack = () => { | ||
| if (window.history.length > 1) { | ||
| navigate(-1); | ||
| } else { | ||
| navigate("/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.
💊 제안
"목록으로 돌아가기" 버튼은 목록으로 가면 되니 아래처럼 해도 될 것 같아요~
버튼으로 처리하거나 history 를 확인할 필요가 없어 보입니다~
<Link className="item-detail__back-button" to='/items'>
<span>목록으로 돌아가기</span>
<img src={ic_back} alt="뒤로가기" className="item-detail__back-icon" />
</Link>추후 단순 페이지 이동이 아니라 특정 로직을 실행해야한다면 지금처럼 버튼 태그를 사용하셔도 좋습니다.
링크 태그를 사용하시면 마우스 오른쪽 클릭이나 새탭 기능도 제공이 가능해서 UX 측면에서도 좋습니다.
| const isBest = variant === "best"; | ||
| return ( | ||
| <div className="product-card"> | ||
| <div className={`product-card ${isBest ? "product-card--best" : "product-card--list"}`} onClick={() => nav(`/items/${item.id}`)}> |
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 태그가 더 시멘틱하고 다른 코멘트에서 언급한 것처럼 UX측면에서도 더 좋을 것 같아요~
| <div className={`product-card ${isBest ? "product-card--best" : "product-card--list"}`} onClick={() => nav(`/items/${item.id}`)}> | |
| <Link className={`product-card ${isBest ? "product-card--best" : "product-card--list"}`} to={`/items/${item.id}`}> |
| onChange={(e) => setText(e.target.value)} | ||
| /> | ||
| <div className="inquiry-input__form"> | ||
| <button className={`inquiry-input__submit ${text.trim() ? "inquiry-input__submit--active" : ""}`}>등록</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.
💊 제안
문의하기가 text가 없을때 비활성화가 될 수 있게 해주세요~
또한 동적 class를 활용하시기보다 css 가상클래스 :disabled를 통해 구현하시는 것이 더 적절할 것 같습니다.
| <button className={`inquiry-input__submit ${text.trim() ? "inquiry-input__submit--active" : ""}`}>등록</button> | |
| <button disabled={!!text.trim()} className='inquiry-input__submit'>등록</button> |
| // 외부 클릭 시 드롭다운 닫기 | ||
| useEffect(() => { | ||
| const handleClickOutside = (event) => { | ||
| if (openDropdownId && !event.target.closest(".comment-list__dropdown-toggle")) { | ||
| setOpenDropdownId(null); | ||
| } | ||
| }; | ||
|
|
||
| document.addEventListener("mousedown", handleClickOutside); | ||
| return () => { | ||
| document.removeEventListener("mousedown", handleClickOutside); | ||
| }; | ||
| }, [openDropdownId]); |
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.
💊 제안
특정 컴포넌트의 외부가 클릭되면 특정 함수가 실행되는 코드로 묶을 수 있으니 custom hook으로 분리하셔서 재사용하시면 더 좋을 것 같아요.
| return ( | ||
| <div className="inquiry-input"> | ||
| <h4 className="inquiry-input__title">문의하기</h4> | ||
| <textarea |
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.
| useEffect(() => { | ||
| const fetchComments = async () => { | ||
| try { | ||
| const data = await getComments(productId, 3); |
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.
❗️ 수정요청
지금은 고정값으로 코멘트를 불러오고 있어요~
모든 코멘트가 보이도록 무한스크롤 방식으로 구현해주시면 좋겠습니다!


요구사항
기본
심화
주요 변경사항
배포 사이트
https://hyunbara7.netlify.app/
멘토에게