[FE-18] assets 절대경로 추가, 뒤로가기 버튼 생성 아이콘 저장 #5
Conversation
-assets 절대경로 누락된거 tsconfig에 추가
-뒤로가기 버튼 추가, 해당 svg 파일 저장
endmoseung
requested review from
Seongtaek-H and
sookyeonghwang
as code owners
sookyeonghwang
left a comment
sookyeonghwang
left a comment
There was a problem hiding this comment.
모승 수고하셨습니다!
우선 제 스타일대로 리뷰를 남겼습니다!
앞으로도 너무 리뷰가 빡세다고 생각하시면 댓글 부탁드려요!
느슨하게 하자고 했는데 어느정도로 해야 하는지 감이 안와서 남깁니다!
감사합니다!
src/components/BackButton.tsx
Outdated
|
|
||
| function BackButton() { | ||
| const navigate = useNavigate() | ||
| const locateBack = () => { |
There was a problem hiding this comment.
함수명으로 handleLocateBack은 어떠신가요?
저는 보통 컴포넌트에 props로 데이터를 넘길 떄는 on 접두사, 아닌 경우엔 모두 handle이라는 접두사를 사용합니다!
on vs handle <- 이벤트 핸들러 네이밍에 대한 링크입니다.
There was a problem hiding this comment.
handle붙이는게 의미상 더 맞는것 같아요 정말 좋은거 배워갑니당:)
src/components/BackButton.tsx
Outdated
|
|
||
| return ( | ||
| <div onClick={locateBack}> | ||
| <Back></Back> |
There was a problem hiding this comment.
저는 children이 없는 경우는 <Back />으로 한번에 쓰는 것을 선호합니다!
There was a problem hiding this comment.
항상 습관적으로 어떤걸 만들떄 외부에 div를 삽입했었는데 굳이 그럴필욛가 없겠네요 감사합니다! 이건 저희룰로 추가해요 그러면
Seongtaek-H
left a comment
Seongtaek-H
left a comment
There was a problem hiding this comment.
워터 의견대로 수정하시고 다시 올려주시면 될 것 같습니닷
-불필요한 div수정 -함수명을 더 명확하게
수정했습니다 반영부탁드려요! |
src/components/BackButton.tsx
Outdated
| navigate(-1) | ||
| } | ||
|
|
||
| return <Back onClick={handleLocateBack}></Back> |
There was a problem hiding this comment.
워터 리뷰는 밖에 frag components 를 없애라는게 아니라 <Back onClick={handleLocateBack} />
이런식으로 닫혀있는 단일 노드로 만들자는 거 아닌가요?
There was a problem hiding this comment.
어 다시 읽어보니까 그게 맞는거 같아요 이거 린트로 설정 가능하지 않나요 ?
There was a problem hiding this comment.
저것도 린트로 설정이 가능한가요? 할수있으면 하시죵
-children이 없는 tag는 바로 닫기도록
작업 내용
assets 절대경로가 추가되지 않아서 이를 tsconfig에서 추가했고, 뒤로가기 버튼을 만들었는데 이때 svg파일을 figma에서 가져와서 저장
참고 이미지(선택)
어떤 점을 리뷰 받고 싶으신가요?
뒤로가기 라우팅을 즉시실행함수로 안하고 handle~로 했는데 즉시실행함수로 하는게 더 가독성이 좋을지 ?