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

[Refactor] ChatListItem 리팩토링 #140

Open
7 tasks done
kyubumjang opened this issue Sep 20, 2022 · 0 comments · May be fixed by #152
Open
7 tasks done

[Refactor] ChatListItem 리팩토링 #140

kyubumjang opened this issue Sep 20, 2022 · 0 comments · May be fixed by #152
Assignees

Comments

@kyubumjang
Copy link
Member

kyubumjang commented Sep 20, 2022

✏️ 작업 내용

기존 Avatar와 Badge를 사용한 컴포넌트로 채팅 리스트(매치, 전체)에서만 사용하는 컴포넌트입니다.

이걸 어떤 식으로 컴포넌트화 할 지 고민을 하고 있습니다.

만약 컴포넌트에서 도메인을 분리하게 된다면(Avatar + ListItem + Badge)와 같은 형태로 될 텐데 이런 형식이 다른 곳에서도 사용하는지 먼저 파악 후 컴포넌트답게 변경해야할 것 같습니다.

  • 코드 파악
  • Avatar 적용
  • Badge 적용 고려 => 기존 구조 유지
  • 더 좋은 코드 구조
  • 불필요한 CSS 코드 삭제
  • 도메인 분리 => 도메인 분리가 어려움

  • Text 컴포넌트를 만들어서 글자와 관련된 CSS를 전부 삭제하는 방향 고려

⚠️ 주의사항

새로운 아바타와 뱃지 적용할 때 주의가 필요합니다.

⏰ 예상 시간

2.5H

🔎 추가 정보

개인 프로필이나 팀 프로필과 합칠 수 있지 않을까 생각하고 있습니다.

의견

2022-11-18(금) 추가

코드 파악, Avatar 적용, Badge 적용

코드는 전부 파악했습니다. 그리 오랜 시간이 걸리지는 않았고 Avatar는 문제없이 코드 상으로는 다르지만 UI는 같게 적용했습니다. OldBadge(매치 상태에 따라 backgroundColor를 변경해주는 뱃지)를 Tag로 변경하려다 보니 matchStatus 에 따라 색상 변화를 만드는 것이 어렵다고 판단했습니다. 이 부분에 있어서는 도메인이 포함된 Badge를 새로 만들어야 하는가 고민 중에 있습니다.
=> 이 부분에 대한 의견이 필요합니다.

더 좋은 코드 구조

더 좋은 구조와 관련해서는 고민을 해봤습니다. 구조 상으로는 최소화하여 이전에도 구현하였다 생각했습니다.(단순 구간 분리, flex-direction 처리 용도로만 사용)

하지만 도메인이나 컴포넌트로 역할로 생각을 조금 해보면 다른 곳에서는 이용하기가 어려운 컴포넌트이고 Avatar, Text, Tag를 활용한 같은 형식의 무언가가 없다면 다른 곳에서 쓰게 하기 위해서는 props로 Type을 받아서 조건부 렌더링 처리를 해야할 것 같은데 이 부분이 추가된다면 더욱 복잡한 컴포넌트가 되지 않을까 생각이 들었습니다.

혹은 단순 Text의 상태만 본다고 하더라도 B1, GrayB3 등 줄글을 쓸 때도 match, laschat, nickname 등의 정보를 사용하여 디자인이 다 다르게 나오도록 만들어야하는데 코드 복잡도를 높이게 되니까 단순 템플릿 형태의 코드이지만 지금이 최선이지 않을까 하는 생각도 들었습니다.
=> 이부분에 대해서도 의견이 필요합니다.

결론적으로 도메인 분리에 관해서는 코드가 더 늘어날 것 같고 시간 상으로나, 이 컴포넌트 명(ChatListItem)에 따라 그대로 유지하는게 좋을 것 같다는 생각이 들었습니다. Atomic 디자인 패턴을 적용하게 되면 채팅과 관련된 아바타와 뱃지, Text 글을 합친 분자 구조 정도가 된다고 생각이 들었습니다.

현재 리스트와 관련된 아이템들

  • 채팅리스트

ChatListItem

  • 개인 프로필, 팀 프로필

profile

  • 매치 리스트

MatchListItem

  • 알림 리스트

NotificationItem

불필요한 CSS 코드 삭제

스타일과 관련된 명칭 문제, 위치 문제를 해결하는게 더 올바른 판단이라 생각하여 common에 있는 스타일 Wrapper들을 분리하는 작업을 진행하고 있습니다.

InnerWrapper를 단순하게 레이아웃을 더 편리하게 잡는 하나의 스타일 용도로 사용하고 있는데 코드를 보다보니 한눈에 안보이고 이게 어디 말하는거지? 라는 생각이 들어 style 파일로 가지고 올까하고 있습니다.

사실 이 방법이 옳은 방법인가 봤을 때는 아닐 수도 있지만 가독성 측면에서 훨씬 좋은 방안인 것 같습니다.
=> 여기에 대해서도 의견 부탁드립니다.

전에 Text 컴포넌트를 만들어야겠다 생각했는데 이 부분은 CSS 정리 후 개발 예정입니다. 그러면서 GrayB3와 같은 코드를 전부 날릴 생각입니다.

@kyubumjang kyubumjang self-assigned this Sep 20, 2022
@kyubumjang kyubumjang linked a pull request Nov 21, 2022 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant