[이지현] Sprint7 #204
[이지현] Sprint7 #204
Conversation
추후 토큰 발급하여 테스트할 예정
GANGYIKIM
left a comment
GANGYIKIM
left a comment
There was a problem hiding this comment.
지현님 7번째 미션 작업 고생하셨습니다~
기존 작업하신 것도 리펙토링 하시다니 대단하네요!
전반적으로 잘 작성하신 코드였어요. 다만 시간이 되실때 좀 더 정리하시면 좋을 것 같아요~
다음 미션도 화이팅입니다!
patch, post, delete 는 로그인 회원가입 토근 얻고 하는게 맞을까요??:
스웨거 상에서 각 API 옆에 있는 자물쇠 모양은 해당 API에서 token이 필요할 경우 보여집니다.
코멘트 생성, 삭제, 수정 모두 회원가입 이후 구현이 가능한 로직이니 추후 구현하시면 됩니다~
There was a problem hiding this comment.
❗️ 수정요청
같은 이미지가 왜 다른 확장자로 두개 존재해야하는지 모르겠어요~
There was a problem hiding this comment.
❗️ 수정요청
해당 요소를 굳이 이미지로 사용하신 특별한 이유가 있을까요? CSS로도 동일한 형태를 구현할 수 있어 불필요한 이미지일 것 같아요.
| <button css={gotoListButton} onClick={() => navigate("/items")}> | ||
| 목록으로 돌아가기 | ||
| <img src={backIcon} alt="backIcon" /> | ||
| </button> |
There was a problem hiding this comment.
💊 제안
지금 코드만으로 보면 버튼 태그보다 Link 태그를 쓰는게 더 적절해보여요.
단순 페이지 이동이 아니라 특정 로직을 실행해야한다면 지금처럼 버튼 태그를 사용하셔도 좋지만 아니라면 Link 태그로 변경해주세요~
링크 태그를 사용하시면 마우스 오른쪽 클릭이나 새탭 기능도 제공이 가능해서 UX 측면에서도 좋습니다.
| export const commentAPI = { | ||
| postComment: async (productId, content) => { |
There was a problem hiding this comment.
💊 제안
commentAPI 객체명과 postComment 메소드명이 모두 “comment”를 포함해 불필요하게 중복되는 것 같아요. 이름에 대해 고민해보시면 좋겠어요~
| throw new Error("상품을 불러오는 데 실패했습니다."); | ||
| } | ||
| }, | ||
| // 추후 로그인 회원가입 기능으로 토큰 얻고 기능추가 |
There was a problem hiding this comment.
👍 칭찬
주석한 이유에 대해 적어주신 것 좋습니다 👍
| dayjs.extend(relativeTime); | ||
| dayjs.locale(ko); |
There was a problem hiding this comment.
💊 제안
dayjs는 프로젝트 전반에 걸쳐 사용되는 시간관련 라이브러리임으로 플러그인과 로케일 설정은 더 상위 진입점에서 처리하시는 것이 적절해보입니다~
There was a problem hiding this comment.
👍 칭찬
정확하게 구현하기위해 시간 관련 라이브러리 쓰시는 것 좋아요!
| dayjs.locale(ko); | ||
|
|
||
| export default function CommentEdit({ comment, setIsEditing }) { | ||
| const [editedContent, setEditedContent] = useState(comment.content); |
There was a problem hiding this comment.
💊 제안
지금처럼 textarea 값을 useState로 제어하면 사용자가 입력할 때마다 컴포넌트가 리렌더링됩니다.
지금 코드상에서는 textarea 값을 바꿔주는 것이 아니라 제어 컴포넌트로 관리할 필요가 없을 것 같아요!
이를 비제어 컴포넌트로 바꾸셔서 불필요한 리렌더링을 줄이시는 것을 추천드려요!
|
|
||
| const CommentItem = ({ comment }) => { | ||
| const [isEditing, setIsEditing] = useState(false); | ||
| const [settingStates, setSettingStates] = useState({}); |
There was a problem hiding this comment.
💊 제안
코멘트의 id 별로 메뉴가 열렸는지에 대한 상태를 가지지 않고 selectedId 같이 어떤 코멘트의 메뉴가 열렸는지 알 수 있는 id값만 들고 있어도 될 것 같아요~
그렇게 되면 전반적으로 코드가 깔끔해지고 의도도 명확해질 것 같습니다.
| function CommentList() { | ||
| const [comments, setComments] = useState([]); | ||
| const location = useLocation(); | ||
| const productId = location.pathname.split("/")[2]; |
There was a problem hiding this comment.
❗️ 수정요청
지금 코드에서는 위치기반으로 값을 가지고 오고 있어 유연하지 못합니다.
사용하시는 라이브러리에서 제공하는 useParams 훅을 이용하시는 것을 추천드려요~
| <div css={ProductDetailOwnerName}> | ||
| <p>{product.ownerNickname}</p> | ||
| <div css={ProductDetailOwnerDate}> | ||
| {new Date(product.createdAt).toLocaleDateString()} |
There was a problem hiding this comment.
👍 칭찬
내장 메소드로 간단하게 구현하신 점 좋습니다!
참고로 날짜를 formatting하는 로직들은 일반적으로 재사용되는 경우가 많으니 util로 분리하시면 좋을 것 같아요.
2 participants
Github Repo: https://github.com/two678/15-Sprint-Mission/tree/React-%EC%9D%B4%EC%A7%80%ED%98%84-sprint7
배포사이트: https://pandamaket-jihyun.netlify.app/
요구사항
기본
심화
주요 변경사항
스크린샷
상세페이지_첫번째_시연
상세페이지_두번째_시연
멘토에게