Skip to content

Conversation

@mimizae
Copy link
Collaborator

@mimizae mimizae commented Dec 26, 2024

주요 작업 내용

  • 제가 맡은 공통 컴포넌트 TopBar, UserProfile, PostItem를 리팩토링 했습니다.
  • 각 공통 컴포넌트의 변수명명 규칙을 점검해 수정하고, 경로를 수정하고, 리팩토링 회의 문서 기반으로 코드 순서를 재배치 했습니다. 컴포넌트화 된 svg 아이콘도 컴포넌트를 import 하는 것으로 수정했습니다.

기타 작업 내용

  • 공통 컴포넌트의 props를 수정함에 따라 이 컴포넌트들을 사용하는 파일로 이동해서 전부 수정했습니다.

코드 리뷰 포인트

  • 경로를 따라서 거의 수정을 다 하긴 했는데 혹시나 오류 발생하면 해당 공통 컴포넌트 확인 부탁드립니다!!

작업 화면

  • 없음

@mimizae mimizae added the refactor Improve code structure and readability label Dec 26, 2024
@mimizae mimizae requested a review from gustn99 December 26, 2024 14:40
@mimizae mimizae self-assigned this Dec 26, 2024
const PostItem: React.FC<PostItemProps> = ({ post, isMyPost = true }) => {
const navigate = useNavigate();
const imageUrl = post.imageUrl;
const PostImageUrl = post.imageUrl;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostImageUrl을 의도하신 게 아니라면 postImageUrl로 수정해야 할 것 같습니다~

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수정 완료 했습니답 😅😅

Comment on lines 16 to 22
const handlePostClick = () => {
const path = isMyPost ? `/my-post/${post.id}` : `/post/${post.id}`;
navigate(path);
};

return (
<PostItemContainer onClick={handleClick}>
<PostItemLayout onClick={handlePostClick}>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요 이벤트 핸들러 이름으로 handlePostItemClick는 어떠신가요? PostItem 컴포넌트이기도 하고, 비록 서로 다른 파일로 분리되어 있기는 하지만 handlePostClick는 Post 컴포넌트 클릭으로 읽힐 여지가 있는 것 같아서 제안드립니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋은 제안이십니다 저도 고민했다가 분리돼 있으니 괜찮겠지...? 했는데 명확하게 할 필요가 있을 것 같아용 수정했습니다!!!!!!!!!!!!!!


import theme from '@/styles/theme';
import PinIcon from '@/assets/default/pin.svg';
import Heart from '@components/Icons/Heart';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제 기억에 Heart는 홈/피드에서 사용하는 좋아요 버튼입니다! 여기서 사용되는 컴포넌트는 Like일 것 같아용

Comment on lines 5 to 6
onLeftButtonClick?: () => void;
onRightButtonClick?: () => void;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

변수명명규칙에 따르면 onClickLeftButton/onClickRightButton이 더 적합할 것 같습니다! 조금 헷갈릴 수 있지만,

컴포넌트 내에서 정의되는 이벤트 핸들러는 handle[대상][이벤트]이고,
ex. handleButtonClick -> 버튼 클릭을 처리하는 함수

props로 받아오는 이벤트 핸들러는 on[이벤트][대상]입니다!
ex. onClickButton -> 버튼에 대한 onClick 이벤트 리스너

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아오아오아오아오 죄송함니다 ㅠㅠ 좀 더 꼼꼼하게 살필게요

Comment on lines 4 to 5
import theme from '@/styles/theme';
import PinIcon from '@/assets/default/pin.svg';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

절대경로 사용하실 때 @Styles, @assets를 사용해 주시면 더 좋을 것 같습니다! 린트에서 정렬할 때 @assets 형식의 경로에 대해서 설정된 규칙이 있고, 기본적으로 사전식 배열이기 때문에 @/와 @를 혼용해 사용하면 정렬 일관성이 깨질 수 있을 것 같아용 다른 부분에서도 보이는 문제라 전반적으로 검토해 주시면 감사하겠습니닷!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저 theme을 import 하면 기본적으로 @/styles로 import 되던데 styles와 assets만 @/이 아니라 @으로 직접 수정하면 될까용?? 우선 제 담당 컴포넌트의 경로들 수정해뒀습니다!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

다른것들도다@로수정햇습니다.

@gustn99 gustn99 merged commit 9bccb42 into dev Dec 27, 2024
1 check passed
@gustn99 gustn99 deleted the feat/OD-165 branch December 27, 2024 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Improve code structure and readability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants