[박나겸] Sprint 7#312
Hidden character warning
Conversation
|
|
There was a problem hiding this comment.
스프린트 미션하느라 수고 많으셨습니다~👏🏻
폴더구조 다 변경했는데 이런식으로 폴더를 나누는게 낫겠죠,,?
components > OOOPage > components 가 있는 구조가 어색한 느낌이 있어요.
여러 페이지에서 공유하게 되는 컴포넌트는 위계를 다르게 하면 좋을 것 같아요.
components > ui > 데이터에 대한 내용이 없고 순수한 ui 컴포넌트
components > container > 데이터를 가져오고, ui 컴포넌트를 조립해서 만들어주는 페이지 컴포넌트
같은 방식으로 폴더 구조를 나눠보다가 개발해야 할 페이지가 많아지고 도메인이 많아지면
domain > ui, domain > container, domain > hooks, domain > utils, domain > api ... 같이 도메인 단위로 폴더를 묶어도 좋구요.
css, css modules, inline style 혼용해서 사용하고 있는데,
inline style은 동적인 값을 넣어야 해서 꼭 필요한 경우가 아니면 css 우선순위보다 높아서 스타일 수정이나 디버그 할 때 혼란이 발생할 수 있어서 사용을 지양하는게 좋아요.
css modules를 사용하고 있는데, 꼭 필요해서 전역으로 css를 적용하는게 아니라면 css modules를 적극적으로 사용하길 권장해요.
그리고 확인이 끝난 console.log 는 지워주시는 습관 가지시는게 좋아요.
| @@ -0,0 +1,3 @@ | |||
| export default function convertDateToLocalString(dateString = "") { | |||
| return new Date(dateString).toLocaleDateString(); | |||
| } | |||
There was a problem hiding this comment.
간단하지만 하나의 책임을 가지는 유틸성 함수 좋아요~👍
| height={56} | ||
| placeholder={"태그을 입력해주세요"} | ||
| placeholder={"태그를 입력해주세요"} | ||
| type="string" |
There was a problem hiding this comment.
"text"를 넣거나 "text"가 기본값이니까 생략해야 하는데 잘못 넣으신 거죠?
| const [inputValue, setInputValue] = useState(""); // 초기값으로 value 사용 | ||
| // 초기값 빈 배열 | ||
|
|
||
| const handleAddTag = (e) => { |
There was a problem hiding this comment.
아래 handleAddTag를 사용하는 곳에서 event를 넘겨주지 않아서 e 파라미터가 사실상 의미가 없어 보이네요.
|
|
||
| useEffect(() => { | ||
| console.log("tags", tags); | ||
| }, [tags]); |
There was a problem hiding this comment.
개발하면서 확인하기 위한 코드로 보이는데, 추후에 제거하면 좋을 것 같아요.
| useEffect(() => { | ||
| window.addEventListener("resize", (event) => { | ||
| setWindowWidth(event.target.innerWidth); | ||
| }); |
There was a problem hiding this comment.
useEffect 에서 addEventListener 이후에는
unmount할 때 추가한 이벤트 리스너를 제거하도록 return () => window.removeEventListener(~~) 도 추가해 주세요. (참고)
| }} | ||
| /> | ||
| </div> | ||
| <div style={{ display: "flex", flexDirection: "column", gap: "6px" }}> |
There was a problem hiding this comment.
wdith와 height 같은 동적인 값을 넣기 위한 것이 아닐때는 css와 inline style을 혼합해서 사용하는 걸 지양하는게 좋아요.
inline style이 css보다 우선해서 어떤 경우 css가 적용되고 어떤 경우 inline style이 적용되고 나누어지면 스타일 오류 해결할 때 복잡해져요.
| const hoursDiff = differenceInHours(updatedAt, createdAt); | ||
| timeDiff = `${hoursDiff}시간 전`; | ||
| } | ||
| } |
There was a problem hiding this comment.
timeDiff만 구하는 함수를 따로 분리해주면 가독성에 좋을 것 같아요.
2 participants
요구사항
기본
상품 상세
=> favoriteCount : 하트 개수
=> images : 상품 이미지
=> tags : 상품태그
=> name : 상품 이름
=> description : 상품 설명
상품 문의 댓글
=> image : 작성자 이미지
=> nickname : 작성자 닉네임
=> content : 작성자가 남긴 문구
=> description : 상품 설명
=> updatedAt : 문의글 마지막 업데이트 시간
심화
주요 변경사항
스크린샷
멘토에게