[Feat/#48] 채팅 페이지 QA #49
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| onKeyDown={(e) => { | ||
| if (e.key === "Enter" && e.shiftKey) return; | ||
| if (e.key === "Enter" && !isComposing) { | ||
| e.preventDefault(); | ||
| submit(); | ||
| } | ||
| }} |
There was a problem hiding this comment.
p3) 이부분은 handleKeyDown으로 함수화 해서 분리하면 코드 가독성이 더 좋을 것 같아요!
| type AcceptRejectButtonDisplayContextType = { | ||
| isVisible: boolean; | ||
| show: () => void; | ||
| hide: () => void; | ||
| }; | ||
|
|
There was a problem hiding this comment.
p2) provider가 많아지면서 hide를 의미하는 다양한 provider가 생긴것 같아요 이렇게 되면 컴포넌트 내에서 불러올 때 hide의 이름을 재정의 해주어야하고, 그러면 여러 컴포넌트에서 각각이 다른 이름을 사용하게 되어서 혼란을 일으킬 수 있을 것 같아요!
이부분을 Context 의미에 맞는 이름으로 변경하면 좋을 것 같아요!
hide를 각각 다르게 정의하고 사용하는 부분 코드 예시
hide: hideTextField,
focus: focusTextField,
} = useTextFieldInChatDisplayContext();
const { hide: hideAcceptRejectButtons }
|
|
||
| interface Props { | ||
| show: boolean; | ||
| } | ||
| export default function AcceptRejectButtons({ show }: Props) { | ||
| const { addMessage, deleteMessage, editMessage } = useChatMessagesContext(); |
There was a problem hiding this comment.
p3) 이부분은 제가 잘 이해 못한 부분일 수도 있는데 props로 show를 받아와야만 하는 이유가 있나요? 해당 컴포넌트에서 useContext를 사용하면 render 로직에 문제가 있는지 궁금합니다!
There was a problem hiding this comment.
p2) 또한 isVisible로 사용되는 값과 해당 isVisible을 true 변경하는 함수 show 가 있는데 반해
해당 컴포넌트에선 show 로 boolean값을 판단하니 조금은 혼동이 있을 수도 있을 것 같다는 의견 남깁니다!
| <AcceptRejectButtonDisplayProvider> | ||
| <ChatRoom /> | ||
| </AcceptRejectButtonDisplayProvider> |
There was a problem hiding this comment.
p3) 준근님만의 Provider를 도입하는 기준이 궁금해요!
이슈 번호
closes #48
작업한 내용
디스코드 qa냥 채널에 보고된 채팅 페이지 관련 QA
작업 결과
.
비고
시간이 없어서 티켓을 못 나누고 한 번에 작업했어 ㅜ 커밋 메시지로 변경 사항 구분해 놨으니 참고