Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feat/#26 ] 댓글 컴포넌트 구현 #32

Merged
merged 17 commits into from
Jan 14, 2025
Merged

[Feat/#26 ] 댓글 컴포넌트 구현 #32

merged 17 commits into from
Jan 14, 2025

Conversation

Leeyoonji23
Copy link
Collaborator

@Leeyoonji23 Leeyoonji23 commented Jan 13, 2025

🔥 Related Issues

✅ 작업 리스트

  • 댓글 컴포넌트 구현

🔧 작업 내용

import Comment from "@common/component/Comment/Comment";

const Main = () => {
  return <Comment />;
};

export default Main;

📣 리뷰어에게 어떠신가요?

아이콘.. 이... 아무 속성값을 주지 않았는데도 묘하게 작아보이는 느낌이...

📸 스크린샷 / GIF / Link

image

@Leeyoonji23 Leeyoonji23 added ✨ feat 기능 구현 💄 style 기능에 영향을 주지 않는 커밋, 코드 순서, css등의 포맷에 관한 커밋 labels Jan 13, 2025
@Leeyoonji23 Leeyoonji23 self-assigned this Jan 13, 2025
Copy link
Collaborator

Choose a reason for hiding this comment

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

p3) svg 네이밍 변경 필요해 보여용 !!!!( ic_000.svg)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

완료했습니다!

fontWeight: "500",
lineHeight: "2.24rem",
letterSpacing: "-0.16rem",
padding: "0px 11px 0px 40px",
Copy link
Collaborator

Choose a reason for hiding this comment

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

p3) 패딩은 rem 아닌 px 로 한 이유가 있을까요??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

엇 그것은 그냥 저의 무지... padding 값과 borderRadius 값이랑 헷갈렸나봐요 ㅠㅠ 수정했습니다!

{
display: "flex",
alignItems: "center",
padding: "0px 11px 0px 40px",
Copy link
Collaborator

Choose a reason for hiding this comment

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

p3) 여기도 px 확인 해주세용

Copy link
Collaborator

Choose a reason for hiding this comment

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

p2) 이거 mock comment 목 데이터 임시로 만드신 거 같은데, 파일 위치 여기 위치하는 게 맞을지 얘기해보면 좋을 것 같아요

Copy link
Collaborator

@ocahs9 ocahs9 left a comment

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 13
export const commentContainer = style([
{
position: "relative",
display: "flex",
flexDirection: "column",
width: "33.5rem",
padding: "8px 0px",
gap: "0.4rem",
},
]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

p3) 혹시 padding : 8px, 0px 은 어디에 있던 값이었나요?
image
피그마상에서는, padding-top : 16px 로 되어 있던데 확실히 디자인이 달라보이더라구요. 다시 한번 확인 부탁드립니다 !

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 layout에 해당 padding 값이 있어서 적용했습니다..!

image

</span>
</div>
</div>
<p className={styles.text}>{comment.content}</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

p3)
image
이거 보면 content 내용이 길어질경우 피그마와는 다르게 오른쪽으로 조금 삐져나오는데, 이거 조정이 필요할 것 같아요!

Copy link
Collaborator

Choose a reason for hiding this comment

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

저 content를 감싸는 p 태그의 width(혹은 max-width)값을 정해져서 모양이 일관되면 좋을 것 같아요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

수정했습니다~!

<div key={comment.id} className={styles.commentItem}>
<div className={styles.contentContainer}>
<div className={styles.header}>
<Ellipse57 />
Copy link
Collaborator

Choose a reason for hiding this comment

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

p3) 이거 버튼에도 함수를 연결할 수 있도록 만들어야 할 것 같아요!

Comment on lines 5 to 6
const Comment = () => {
const { comments } = mockComments;
Copy link
Collaborator

Choose a reason for hiding this comment

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

p1) mock data를 사용해서 코멘트의 예시 모습을 렌더링하는거 너무 좋은데,
나중에는 Comment 컴포넌트에 prop으로 직접 넘겨줘야하니까 prop들, interface를 전부 구성해둡시다!
(지금 형태는 단순하게 퍼블리싱 된 형태입니다!)

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 1 to 14
const mockComments = {
comments: [
{
id: 1,
nickname: "리트리버사랑해",
breed: "골든리트리버",
petAge: 12,
content: "호흡기 문제일 수 있는데 다른 증상은 없나요?",
createdAt: "1시간전",
},
],
};

export default mockComments;
Copy link
Collaborator

Choose a reason for hiding this comment

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

p1) 그리고 mockData 위에는 반드시 주석 남겨줍시다 "api 연결 후 삭제할 예정입니다"
그리고 common/component 에 파일 만들어서 저장해두는 건 폴더 구조를 위반하는거니까 꼭 수정합시다!
(그냥 해당 예시 데이터를 사용하는 곳에서 직접 정의해두거나, constant 폴더에서 파일 만들고 export 하거나)

Comment on lines +4 to +18
interface CommentData {
id: number;
nickname: string;
breed: string;
petAge: number;
createdAt: string;
content: string;
profileImage: string;
}

interface CommentProps {
comments: CommentData[];
}

const Comment = ({ comments }: CommentProps) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

p5) 코리 반영하신거 깔꼼하네요!

Comment on lines 88 to 107
export const replyContainer = style([
{
display: "flex",
alignItems: "center",
padding: "0rem 1.1rem 0rem 4rem",
},
]);

export const replyText = style([
font.label01,
{
color: semanticColor.text.assistiveLight,
},
]);

export const icon = style([
{
gap: "0.4rem",
},
]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

p3) replyText와 icon이 항상 함께 사용되는데, 스타일을 따로 분리해서 작성하신 이유가 있을실까요?? 다음과 같이 한 번에 사용하지 않고 따로 사용하신 이유가 있는지 궁금합니다!
스타일이 달라지는 것이 아니라면, 한 번에 사용하는 것이 가독성에 더 좋지 않을까 싶어 남겨봅니다..!

export const replyTextWithIcon = style([
  font.label01,
  {
    display: "flex",
    alignItems: "center",
    gap: "0.4rem",
    color: semanticColor.text.assistiveLight,
    padding: "0rem 1.1rem 0rem 4rem",
  },
]);

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오호 처음엔 피그마를 기반으로 replyContainer 하나, 그 안에 답글 이모티콘과 답글쓰기 태그를 따로 보고 구현했습니다!
그런데 예림님 말씀 듣고 보니 저렇게 하는게 훨씬 더 가독성이 좋네요 👏❤️ 관련사항 반영하여 수정했습니다 ! 감사합니다❤️

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

반영해서 다른 것들도 수정했씁니다 ❤️❤️

Copy link
Collaborator

@yarimu yarimu left a comment

Choose a reason for hiding this comment

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

코리 반영하신 것 잘 확인했습니다! 전반적으로 대부분의 요소에 스타일을 주신 것 같은데, 아래의 코멘트 한 번 확인 부탁드려욤~

@Leeyoonji23 Leeyoonji23 requested a review from yarimu January 14, 2025 07:58
@Leeyoonji23 Leeyoonji23 changed the title [Feat] 댓글 컴포넌트 구현 [Feat /#26 ] 댓글 컴포넌트 구현 Jan 14, 2025
@Leeyoonji23 Leeyoonji23 changed the title [Feat /#26 ] 댓글 컴포넌트 구현 [Feat/#26 ] 댓글 컴포넌트 구현 Jan 14, 2025
@ocahs9
Copy link
Collaborator

ocahs9 commented Jan 14, 2025

반영한거 전부 확인했어요~

@ocahs9 ocahs9 merged commit 3c69f40 into develop Jan 14, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ feat 기능 구현 💄 style 기능에 영향을 주지 않는 커밋, 코드 순서, css등의 포맷에 관한 커밋
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ Feat ] 댓글 컴포넌트 구현
4 participants