-
Notifications
You must be signed in to change notification settings - Fork 3
채팅 기능 구현 #49
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
base: dev
Are you sure you want to change the base?
채팅 기능 구현 #49
Conversation
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.
@xo-yeon
소연님 수고하셨습니다.
전반적으로 조금 이해가 안 가는 코드들과 더불어서 더 좋은 방향성에 대해 코멘트 남겨드렸으니, 확인해 보시고 저도 정답은 아니니 모르거나 의문 있으신 거 언제든 문의 주세요. 디코로 메시지 보내셔서 음성채널에서 뵈어도 되구요.
@@ -137,4 +145,30 @@ const CreatePage: NextPage = () => { | |||
); | |||
}; | |||
|
|||
export default CreatePage; | |||
const CreatePageProvider: NextPage<CreatePageProviderProps> = ({ loginMessage }) => { | |||
useEffect(() => { |
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.
@xo-yeon
일단, 궁금한 게, 왜 굳이 ssr에서 처리 후에 다시 또 클라 단에서 체크하고 routing 시키는 거죠?
그냥 ssr 쓰실 거면, ssr 단에서 처리해도 될 거 같은데요?
그리고 추가로, ssr을 처리하시는 기준이 뭔가요?
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.
ssr에서 alert창을 띄우지 못해 해당 컴포넌트로 가져왔습니다.
그리고 provider를 만든 이유는 alert창이 뜰 때 컨텐츠가 보이지 않도록 하기 위해 래핑을 했습니다.
provider라는 접미사는 저도 고민을 하다가 저렇게 됐는데 wrapper가 나을 것 같네요
const ChatListPage: NextPage = () => ( | ||
<Wrapper> | ||
<DefaultHeader centerArea={<DefaultText text="채팅방" size={17} weight={700} />} /> | ||
<QuerySuspenseErrorBoundary |
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.
서버 단에서 전부 응답이 400이나 500이나 참여중인 방이 없으면 똑같은 건가요?
@@ -0,0 +1,27 @@ | |||
export type InfinitePaginationChatDataType<K extends string, T> = { |
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.
이건 왜 서버에서 기존에 맞춰둔 pagination과 타입이 다른가요? 다르다면, 같이 맞추는 게 좋아 보이는데요.
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.
chat에서는 pagination parameter가 page로 되어 있어서 좀 다릅니다. 이후에 이 값들도 통일 시키면 좋을 것 같습니다.
No description provided.