[김수민] Sprint7 #193
[김수민] Sprint7 #193
Conversation
GANGYIKIM
left a comment
GANGYIKIM
left a comment
There was a problem hiding this comment.
수민님 7주차 미션 제출 고생하셨습니다~
다음 미션도 화이팅이에요!
- 변수명, 함수명, props 등에서 중복되는 내용들이 있네요. 명명하실 때 신경써주시면 더 깔끔한 코드가 될 것 같아요.
- 그전 주차 리뷰에서 드린 코멘트와 중복되는 내용은 최대한 지양했습니다~ 참고해주세요.
코멘트 부분 수정하기 및 삭제하기 기능이 디자인에 있는 것 같은데 기본, 심화 요구사항에 따로 없어 추후 구현해보겠습니다!:
너무 좋은 생각이신 것 같아요~ 해당 기능 구현을 위한 API 가 존재하니 시간이 되실때 추가해보세요 👍
There was a problem hiding this comment.
💊 제안
현재 위치한 page를 저장해 UX를 향상하려고 시도하신 점 좋습니다.
다만 지금의 경우 전체 목록중 page만을 고려하셔서 동작시 매끄럽지 않은 부분들이 있습니다.
page param이 있는 경우 정렬기준이 defaultValue가 아닐때와 검색어가 입력되는 경우들도 고려하셔서 수정하시면 더 좋을 것 같습니다.
| <button className={styles.arrow} onClick={() => currentPage < totalPage && setCurrentPage(currentPage + 1)}> | ||
| <button | ||
| className={`${styles.arrow} ${currentPage === totalPage ? styles.disabled : ''}`} | ||
| onClick={() => currentPage < totalPage && handlePageChange(currentPage + 1)} |
There was a problem hiding this comment.
💊 제안
특정 조건에서 해당 버튼을 disabled 처리하셨으니 currentPage < totalPage 조건 검사는 불필요합니다~
| onClick={() => currentPage < totalPage && handlePageChange(currentPage + 1)} | |
| onClick={() => handlePageChange(currentPage + 1)} |
There was a problem hiding this comment.
👍 칭찬
css 파일을 각각 import 하시는 대신 배럴 파일을 만드셔서 가독성을 높이신 점 좋습니다~
There was a problem hiding this comment.
💊 제안
relativeTime 함수를 직접 구현하신 것 좋습니다.
더 정확하게 구현하기 위해서는 브라우저 제공 API인 Intl.RelativeTimeFormat을 활용하실 수 있습니다.
내장 메소드이기에 더 정확하며 로직도 간결해질 수 있습니다~
https://velog.io/@real-bird/JavaScript-Intl.RelativeTimeFormat
| <div id='container' className='itemsPage'> | ||
| <div className='inner04'> | ||
| <ItemDetailTop productId={productId} /> | ||
| <DetailInquiry label='문의하기' /> |
There was a problem hiding this comment.
💊 제안
문의하기 인풋이 "문의하기" 말고 다른 라벨을 받는 경우가 있나요? 없다면 그냥 label을 props으로 받지 않는게 더 좋을 것 같아요.
| const navigate = useNavigate(); | ||
| const location = useLocation(); | ||
| const searchParams = new URLSearchParams(location.search); |
There was a problem hiding this comment.
💊 제안
search 값만 필요하니 아래처럼 하시는 것이 더 좋을 것 같아요.
또한 선언만 되고 사용되지 않는 searchParams 변수는 제거하시는 것이 좋습니다~
| const navigate = useNavigate(); | |
| const location = useLocation(); | |
| const searchParams = new URLSearchParams(location.search); | |
| const navigate = useNavigate(); | |
| const { search } = useLocation(); |
| import { useLocation } from 'react-router-dom'; | ||
| import styles from './styles/DetailBackToListButton.module.css'; | ||
| import { useNavigate } from 'react-router-dom'; |
There was a problem hiding this comment.
💊 제안
| import { useLocation } from 'react-router-dom'; | |
| import styles from './styles/DetailBackToListButton.module.css'; | |
| import { useNavigate } from 'react-router-dom'; | |
| import { useNavigate, useLocation } from 'react-router-dom'; | |
| import styles from './styles/DetailBackToListButton.module.css'; |
| const [loading, setLoading] = useState(true); | ||
| const [error, setError] = useState(null); |
There was a problem hiding this comment.
💊 제안
API 호출하는 부분에서 loading, error도 그렇고 반복되는 로직들이 보여요~ 이를 공통화하셔도 좋을 것 같아요.
물론 추후 배우실 서버상태관리 라이브러리들을 사용하시면 라이브러리에서 처리해줄테니 참고만 하셔도 됩니다!
There was a problem hiding this comment.
💬 여담
디자인대로 작업하시고 focus된 것을 알 수 있게 해주신 점 좋아요!
다만 CSS의 :focus 가상 선택자는 마우스로 클릭할 때도 outline을 표시하기 때문에, 마우스 탐색 시에는 불필요하게 보일 수 있어요. 키보드 네비게이션에서만 포커스 스타일이 적용되길 원하신다면 :focus-visible을 사용해 보세요!
There was a problem hiding this comment.
💊 제안
메뉴 외의 영역을 눌렀을때도 닫히게 해주시면 더 사용성이 좋아질 것 같아요.
2 participants
https://dduminipandamarket.netlify.app/items/819
요구사항
기본
상품 상세
상품 문의 댓글
심화
주요 변경사항
스크린샷
PC
M
멘토에게