Skip to content

채팅 컴포넌트 마크업 (after #6 merged)#8

Open
jiyong1 wants to merge 17 commits intodevfrom
feature/#7
Open

채팅 컴포넌트 마크업 (after #6 merged)#8
jiyong1 wants to merge 17 commits intodevfrom
feature/#7

Conversation

@jiyong1
Copy link
Copy Markdown
Member

@jiyong1 jiyong1 commented Dec 27, 2021

개요

  • 채팅 컴포넌트 마크업

이슈 번호

변경사항

  • 채팅 컴포넌트 마크업을 완료하였습니다.

특이사항

  • 이후 채팅 리스트와 상태에 따른 페이지 넘어가기 기능을 추가할 예정입니다.
  • Storybook

@jiyong1 jiyong1 added the feature New feature or request label Dec 27, 2021
@jiyong1 jiyong1 self-assigned this Dec 27, 2021
@jiyong1 jiyong1 changed the title 채팅 컴포넌트 마크업 채팅 컴포넌트 마크업 (after #6 merged) Dec 27, 2021
Copy link
Copy Markdown
Member

@Seogeurim Seogeurim 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 +9 to +24
interface ChatItemProps {
content: string;
fromAdmin: boolean;
adminName?: string;
image?: string;
}

const ChatItem = memo<ChatItemProps>(
({ adminName, content, fromAdmin, image }) => {
const theme = useTheme();

return (
<div css={chatItemWrapperStyle(theme, fromAdmin)}>
{image && <img src={image} alt="채팅 로고 이미지" />}
<div>
{adminName && <div css={chatAdminNameStyle}>{adminName}</div>}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  • fromAdmin이 true일 때만 adminName과 image가 존재하는 것일텐데, 이 부분을 타입으로 강제해줄 수는 없을까요?
  • image -> adminImage로 네이밍을 바꿔도 좋지 않을까요? 또는 admin?: { name: string; image: string; } 이런 식으로 ..?

export const chatFromUser = {
fromAdmin: false,
content: `안녕하세요! 프로젝트 너무 훌륭하군요 !! ㅎㅎㅎ 다만 사용해보니 로그인 기능에 버튼이 제대로 동작하지 않는 버그가 있는 것 같습니다.`,
};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

여기 스토리북 컴포넌트 argument를 fixture로 빼니까 뭔가 스토리북 코드 내에서 컴포넌트의 props 정보를 한눈에 보기 힘들다는 단점이 있는 것 같은데, 어떻게 생각하시나효?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants