-
Notifications
You must be signed in to change notification settings - Fork 5
[FE] feat: 채팅 기능 연동 및 전반적인 구조 수정 #86
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
Conversation
f482dd1 to
a1d6fa1
Compare
zelkovaria
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
작업량이,, 구조도 개선해주시고 구현하시느라 정말 고생많으셨어요!👍🏻 몇가지 질문사항이 있어서 코멘트 남겼어요 :) 추가로 아래와 같은 내용도 궁금해요!
- 기존의 카테고리 리스트가
CategorySection이 되고, guildId 값의 유무에 따라GuildCategory,DirectMessageCategory로 나뉘어지는게 맞을까요? - 유저 정보는 하단의 설정을 위한 메뉴 말고도 채팅과 관련한 컴포넌트에도 필요해서 전역 상태로 관리하시나요?
CategorySection에서UserProfile로 넘어가는 부분이 이해가 잘 안되는 것 같아요 ,,! 친구리스트와 관련한 부분이 맞을까요?
| }; | ||
| }, []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 로직들에서 channelId를 사용하고 있는데 dependency에 추가를 안해놔도 괜찮을까요?-?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이걸 빼먹다니.... 반영하겠습니다!
| // 연결 성공 시 subscribe | ||
| client.subscribe(`/topic/direct-message/${channelId}`, (message) => { | ||
| const parsedMessage: ChatMessage = JSON.parse(message.body); | ||
| // setMessages((prev) => [...prev, parsedMessage]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 부분은 주석 처리가 되어있어서 실제로 메시지를 상태에 추가할 수 없을 것 같은데 테스트 때문에 해두신걸까요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useChat에서도 subscribe하는 로직이 있는데, onConnect에서 바로 구독할지 useChat에서 구독할지 아직 결정하지 못했어요. 이 부분은 조금 더 테스트가 필요할 것 같아요 :)
| const file = e.target.files?.[0]; | ||
| if (!file) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분은 저도 길드 생성시 누락되어있는 부분이긴한데 프론트에서 파일 유효성 검사는 없어도 될까요? 저도 반영해야하는 부분이라 철민님의 의견이 궁금해요 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
파일 유효성 검사라면 단순히 file이 선택되었는지 여부일까요? 혹은 전송 가능한 확장자인지, 전송 가능한 용량인지까지를 포함하신 걸까요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
당장 생각나는 부분은 확장자, 용량이었습니당!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
용량의 경우에 사용자의 데이터 사용량과도 직접적인 관련이 있어서 검사해야 할 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 부분은 우선 순위가 더 높은 기능들 붙이고 추가로 이야기 해봐요 ㅎㅅㅎ
| const currentView: keyof typeof component = isGuildChattingDisplayed | ||
| ? 'guildChat' | ||
| : isFriendChattingDisplayed | ||
| ? 'directChat' | ||
| : 'friendsManagement'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const currentView: keyof typeof component = isGuildChattingDisplayed | |
| ? 'guildChat' | |
| : isFriendChattingDisplayed | |
| ? 'directChat' | |
| : 'friendsManagement'; | |
| const getCurrentView = (): 'guildChat' | 'directChat' | 'friendsManagement' => { | |
| if (guildId) return 'guildChat' | |
| if (selectedDMChannel) return ~ | |
| } | |
| const currentView = getCurrentView() | |
| return component[currentView] |
요런 스타일은 어떠세요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
가독성이 훨 좋은 것 같네요!
헤더 + 카테고리 목록이 들어있던 기존의 CategorySection은 크게 헤더+콘텐츠 영역과 UserProfile로 나눴어요. 헤더+콘텐츠 영역에 들어가는 게 guildId 값의 유무에 따라 GuildCategory, DirectMessageCategory로 나누어집니다!
맞아요! 유저 정보가 곳곳에서 필요해요. 좀 더 자세하게는, tanStack Query와 Zustand persist를 혼합해서 관리할 것 같아요. query에서는 요청 성공하면 zustand에 값을 설정하게 하고, 사용하는 쪽에서는 zustand store만 호출해서 사용하게끔요.
UserProfile은 위치상 CategorySection의 바닥에 붙어있어서 컴포넌트 구조를 그렇게 두었어요. 위의 사진을 참고하시면 좋을 것 같아요! 물론 UserProfile이 아직 유저 정보를 전역 상태로 가져오지 않고 있어서 prop으로 받고 있는 것들이 모두 사라질 예정이에요. 친구 리스트는 ChattingSection에 뜨는 거라 별개입니다! 혹시 설명이 더 필요하다면 말씀해주세요 :) |
zelkovaria
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확인했어요!! 수고많으셨습니당 달려봅시닷,,! 🚀

이슈번호
요약(개요)
▲ 컴포넌트 구조도
크게 길드 리스트, 카테고리 들어가는 영역, 채팅방 들어가는 영역으로 구분했습니다.
영역 자체를 나타내는 CategorySection, ChattingSection은
pages/FriendsPage하위에 두었습니다. 그밖에 영역에 들어가는 세부 컴포넌트들은src/components하위에 두었습니다. 채팅 관련 컴포넌트는 아직 구조가 완전히 잡히지 않은 상태라 우선은 ChattingSection 내부에 components로 두었습니다.채팅 기능을 붙이다 보니 먼저 구현해야 하는 컴포넌트가 굉장히 많았기 때문에 우선 PR을 먼저 올렸습니다. 전반적인 구조를 통일하고 계속 작업하는 것이 나중에 덜 힘들 것 같아요.
또한, 컴포넌트 구조도 상에서 현재 선택된 채널 정보나 유저 정보 등은 필요로 하는 컴포넌트들이 많고, 곳곳에 흩어져 있기 때문에 상태 끌어올리기 및 props로 전달하기보다는 zustand persist 기반의 전역 상태로 관리하는 것이 적절하다 판단했습니다.
<현재 관리 중인 전역 상태>
작업 내용
집중해서 리뷰해야 하는 부분
기타 전달 사항 및 참고 자료(선택)
▲ 길드 채널에 접속한 상황
▲ DM으로 들어갔을 때
▲ 메시지 수정 UI (미완성)