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

[Chat] 전송 실패 메시지 처리 방식 변경 #2938

Merged
merged 5 commits into from
Oct 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions packages/chat/src/chat-bubble/chat-bubble.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ interface ChatBubbleProps {
displayTarget: UserType
postMessageAction?: PostMessageActionType
disableUnreadCount?: boolean
onRetryButtonClick?: () => void
onRetryCancelButtonClick?: () => void
onRetryButtonClick?: (message: MessageInterface) => void
onRetryCancelButtonClick?: (message: MessageInterface) => void
blindedText?: string
bubbleColor?: ChatBubbleColor
}
Expand Down Expand Up @@ -79,15 +79,15 @@ const ChatBubble = ({
!!postMessageAction

const onRetry = () => {
onRetryButtonClick?.()
onRetryButtonClick?.(message)
return postMessageAction?.(
message.payload as TextPayload | ImagePayload,
true,
)
}

const onCancel = () => {
onRetryCancelButtonClick?.()
onRetryCancelButtonClick?.(message)
}

return (
Expand Down
2 changes: 1 addition & 1 deletion packages/chat/src/chat/chat-container.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const ScrollProvider = dynamic(() => import('./scroll-context'), {

export interface ChatContainerProps extends ChatContextValue {
/**
* Chat list와 보내기 Input 창을 감싸는 컨테이너로, 커스텀 스타일 등 적용 가능
* Chat list를 감싸는 컨테이너로, 커스텀 스타일 등 적용 가능
*/
container: ElementType
/**
Expand Down
87 changes: 63 additions & 24 deletions packages/chat/src/chat/chat.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ export const Chat = ({
const [
{
messages,
failedMessages,
hasPrevMessage,
otherUnreadInfo,
lastMessageId,
Expand Down Expand Up @@ -202,7 +203,6 @@ export const Chat = ({
dispatch({
action: ChatActions.POST,
messages: newMessages,
payload,
})

const lastMessage = newMessages[newMessages.length - 1]
Expand All @@ -211,7 +211,7 @@ export const Chat = ({
dispatch({
action: ChatActions.FAILED_TO_POST,
message: {
id: NaN,
id: new Date().getTime(),
roomId: room.id,
senderId: userInfo.me.id,
payload,
Expand Down Expand Up @@ -262,34 +262,73 @@ export const Chat = ({
}
}

function removeFromFailedMessages(message: MessageInterface) {
dispatch({
action: ChatActions.REMOVE_FROM_FAILED,
message,
})
}

function onRetry(message: MessageInterface) {
removeFromFailedMessages(message)
onRetryButtonClick?.()
}

function onRetryCancel(message: MessageInterface) {
removeFromFailedMessages(message)
onRetryButtonClick?.()
}

return (
<>
<IntersectionObserver onChange={onChangeScroll}>
<HiddenElement />
</IntersectionObserver>
<Container ref={chatRoomRef} id={CHAT_CONTAINER_ID} {...props}>
<ul>
{messages.map((message: MessageInterface, index) => (
<li key={index}>
{userInfo ? (
<ChatBubble
displayTarget={displayTarget}
message={message}
userInfo={userInfo}
postMessageAction={
postMessage ? postMessageAction : undefined
}
otherReadInfo={otherUnreadInfo}
onRetryButtonClick={onRetryButtonClick}
onRetryCancelButtonClick={onRetryCancelButtonClick}
disableUnreadCount={disableUnreadCount}
blindedText={blindedText}
bubbleColor={bubbleColor}
/>
) : null}
</li>
))}
</ul>
{userInfo ? (
<>
<ul id="messages_list">
{messages.map((message: MessageInterface) => (
<li key={message.id}>
<ChatBubble
displayTarget={displayTarget}
message={message}
userInfo={userInfo}
postMessageAction={
postMessage ? postMessageAction : undefined
}
otherReadInfo={otherUnreadInfo}
onRetryButtonClick={onRetry}
onRetryCancelButtonClick={onRetryCancel}
disableUnreadCount={disableUnreadCount}
blindedText={blindedText}
bubbleColor={bubbleColor}
/>
</li>
))}
</ul>
<ul id="failed_messages_list">
Copy link
Contributor

Choose a reason for hiding this comment

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

messages와 failedMessages의 렌더링 차이가 없어보여서 [...messages, ...failedMessages]로 합쳐서 렌더링해도 괜찮을 것 같아요!
아니면 ul 태그의 id를 사용하는 경우가 있어 별도로 렌더링을 하는 방법을 택하신 걸까요?👀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

chat-web은 메시지의 아이디가 숫자로 1씩 올라가게 됩니다. 시간을 기반으로 getTime()을 한 숫자와 많은 차이가 있겠지만 혹시 id가 겹칠 염려가 있어 두 개를 분리해두었습니다. 추후 메시지 id를 따라 스크롤이 된다거나 하는 등의 기능이 추가될 때를 대비한 바도 있습니다!

{failedMessages.map((message: MessageInterface) => (
<li key={message.id}>
<ChatBubble
displayTarget={displayTarget}
message={message}
userInfo={userInfo}
postMessageAction={
postMessage ? postMessageAction : undefined
}
otherReadInfo={otherUnreadInfo}
onRetryButtonClick={onRetry}
onRetryCancelButtonClick={onRetryCancel}
disableUnreadCount={disableUnreadCount}
blindedText={blindedText}
bubbleColor={bubbleColor}
/>
</li>
))}
</ul>
</>
) : null}
</Container>
<HiddenElement ref={bottomRef} />
</>
Expand Down
32 changes: 15 additions & 17 deletions packages/chat/src/chat/reducer.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
import {
MessageInterface,
OtherUnreadInterface,
TextPayload,
ImagePayload,
} from '../types'
import { MessageInterface, OtherUnreadInterface } from '../types'

export enum ChatActions {
INIT, // 최초에 메시지
Expand All @@ -12,10 +7,12 @@ export enum ChatActions {
POST, // 메시지 전송
FAILED_TO_POST, // 메시지 전송 실패
UPDATE, // 읽음 표시 업데이트
REMOVE_FROM_FAILED, // 전송 실패 메세지 재전송 또는 삭제
}

export interface ChatState {
messages: MessageInterface[]
failedMessages: MessageInterface[]
hasPrevMessage: boolean
otherUnreadInfo: OtherUnreadInterface[]
firstMessageId: number | null
Expand All @@ -39,7 +36,6 @@ export type ChatAction =
| {
action: ChatActions.POST
messages: MessageInterface[]
payload: TextPayload | ImagePayload
}
| {
action: ChatActions.FAILED_TO_POST
Expand All @@ -49,6 +45,7 @@ export type ChatAction =
action: ChatActions.UPDATE
otherUnreadInfo: OtherUnreadInterface[]
}
| { action: ChatActions.REMOVE_FROM_FAILED; message: MessageInterface }

export const ChatReducer = (
state: ChatState,
Expand Down Expand Up @@ -81,15 +78,7 @@ export const ChatReducer = (
case ChatActions.POST:
return {
...state,
messages: mergeMessages(
state.messages.some((message) => !message.id)
? state.messages.filter(
(message) =>
!message.id && !Object.is(message.payload, action.payload),
)
: state.messages,
action.messages,
),
messages: mergeMessages(state.messages, action.messages),
lastMessageId: Number(action.messages[action.messages.length - 1].id),
}

Expand All @@ -103,7 +92,7 @@ export const ChatReducer = (
case ChatActions.FAILED_TO_POST:
return {
...state,
messages: [...state.messages, action.message],
failedMessages: [...state.failedMessages, action.message],
}

case ChatActions.UPDATE:
Expand All @@ -112,13 +101,22 @@ export const ChatReducer = (
otherUnreadInfo: action.otherUnreadInfo,
}

case ChatActions.REMOVE_FROM_FAILED:
return {
...state,
failedMessages: state.failedMessages.filter(
(message) => message.id !== action.message.id,
),
}

default:
throw new Error('unexpected action')
}
}

export const initialChatState: ChatState = {
messages: [],
failedMessages: [],
hasPrevMessage: true,
otherUnreadInfo: [],
firstMessageId: null,
Expand Down
4 changes: 2 additions & 2 deletions packages/chat/src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export interface RoomListResultWithPagingInterface
export interface RoomMetadata {
name: string
memberCounts: number
faqId?: string
articleId?: string
Copy link
Contributor

Choose a reason for hiding this comment

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

여기 타입이 변경된 히스토리를 알고 싶습니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

#2939
앗 여기 PR에 설명이 있군요 브랜치가 섞인 걸까요??

Copy link
Contributor

Choose a reason for hiding this comment

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

앗 cananry 충돌을 방지하기 위해 #2939 내용을 추가하신 것 같네요..!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

chat-web에서 카나리 테스트를 할 때 타입 오류를 방지하기 위해 해당 브랜치 리베이스를 진행했었습니다...!

}

export interface RoomInterface {
Expand All @@ -56,7 +56,7 @@ export interface RoomInterface {
members: UserInterface[]
isDirect: boolean
createdAt: string
metadata: RoomMetadata
metadata?: RoomMetadata
}

export type DisplayTargetAll = 'all'
Expand Down