[WIP] ChatRoomSection 구현 #329
Open
+103
−9
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Walkthrough사이드바가 열려 있을 때만 렌더되는 ChatRoomSection 컴포넌트를 SideNavigation에 추가합니다. 새로운 ChatRoomSection은 "채팅" 라벨과 스크롤 가능한 ChatItem 목록을 렌더하며, ChatItem은 id·label을 받아 클릭 시 Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@src/components/layout/SideNavigation/components/ChatRoomSection/ChatItem.tsx:
- Around line 20-22: handleItemClick uses a wrong template literal string so
router.push always navigates to "/chat/{id}"; change the push argument to use
JavaScript template interpolation (e.g., `/chat/${id}`) in the handleItemClick
function and ensure the id variable referenced is in scope (prop or state)
before calling router.push.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/layout/SideNavigation/SideNavigation.tsxsrc/components/layout/SideNavigation/components/ChatRoomSection/ChatItem.tsxsrc/components/layout/SideNavigation/components/ChatRoomSection/ChatRoomSection.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/layout/SideNavigation/components/ChatRoomSection/ChatItem.tsx (1)
src/stores/modalStore.ts (1)
useModalStore(18-23)
🔇 Additional comments (8)
src/components/layout/SideNavigation/SideNavigation.tsx (2)
10-10: LGTM!import 경로가 올바르게 설정되었습니다.
61-61: LGTM!ChatRoomSection이 사이드 네비게이션 레이아웃에 올바르게 통합되었습니다.
src/components/layout/SideNavigation/components/ChatRoomSection/ChatRoomSection.tsx (2)
1-3: LGTM!필요한 컴포넌트들이 올바르게 import되었습니다.
5-16: LGTM!컴포넌트 구조가 적절하게 구현되었습니다. 스크롤 가능한 채팅 목록을 위한 flex 레이아웃과 overflow 처리가 올바릅니다. WIP 단계에서 하드코딩된 데이터는 예상된 사항입니다.
src/components/layout/SideNavigation/components/ChatRoomSection/ChatItem.tsx (4)
1-9: LGTM!필요한 imports가 올바르게 설정되었습니다. 주석 처리된 imports는 WIP 단계에서 예상된 사항입니다.
11-14: LGTM!Props 인터페이스가 명확하게 정의되었습니다.
16-18: LGTM!훅들이 올바르게 초기화되었습니다. 현재 사용되지 않는
type변수는 WIP 단계에서 향후 기능 구현을 위한 것으로 판단됩니다.
28-72: LGTM!JSX 구조가 적절하게 구현되었습니다. hover 효과와 텍스트 truncate 처리가 올바르게 설정되었습니다. 주석 처리된 Popover와 모달 관련 코드는 WIP 단계에서 예상된 사항입니다.
src/components/layout/SideNavigation/components/ChatRoomSection/ChatItem.tsx
Show resolved
Hide resolved
Bangdayeon
force-pushed
the
feat/#319-chat-room-section
branch
from
4501e6a to
74c598b
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
@src/components/layout/SideNavigation/components/ChatRoomSection/ChatItem.tsx:
- Around line 30-36: The span inside the ChatItem component changes width on
hover via the class "group-hover:w-46", causing layout shift; remove that
dynamic width and give the element a fixed width (or apply the fixed width to
the parent container) and replace hover-driven width changes with
non-layout-affecting visual tweaks (e.g., change text color, opacity, or
translate on hover) so clicking behavior (handleItemClick) and label rendering
remain stable; update the span's class list to drop "group-hover:w-46" and
ensure a consistent "w-XX" is present on either the parent div or the span,
using hover-only classes for color/opacity instead.
- Line 70: The DeleteChatModal is being rendered by every ChatItem because the
modal store only exposes a type; update the modal store usage so it carries the
target chatId (useModalStore -> { type, props }) and change the render condition
to only show DeleteChatModal when type==='DELETE_CHAT' AND props?.chatId === id
(inside ChatItem), or alternatively remove the modal from ChatItem and render a
single instance in ChatRoomSection that checks type==='DELETE_CHAT' &&
props?.chatId before rendering DeleteChatModal with props.chatId.
In
@src/components/layout/SideNavigation/components/ChatRoomSection/DeleteChatModal.tsx:
- Around line 10-18: handleDelete currently calls close() immediately after
starting deleteChatMutation.mutate which closes the modal before the async
operation completes; change handleDelete to call
deleteChatMutation.mutate(chatId, { onSuccess: () => { close(); /* additional
success handling if needed */ }, onError: err => { /* surface error to user,
keep modal open and stop loading */ } }) and remove the immediate close(); also
add a loading state derived from deleteChatMutation.isLoading to disable
controls and show a spinner while the mutation is in progress so the modal stays
open until onSuccess or onError runs.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/layout/SideNavigation/SideNavigation.tsxsrc/components/layout/SideNavigation/components/ChatRoomSection/ChatItem.tsxsrc/components/layout/SideNavigation/components/ChatRoomSection/ChatRoomSection.tsxsrc/components/layout/SideNavigation/components/ChatRoomSection/DeleteChatModal.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/layout/SideNavigation/components/ChatRoomSection/ChatRoomSection.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
src/components/layout/SideNavigation/components/ChatRoomSection/DeleteChatModal.tsx (2)
src/stores/modalStore.ts (1)
useModalStore(19-24)src/hooks/server/Chats/useDeleteChat.ts (1)
useDeleteChat(5-14)
src/components/layout/SideNavigation/components/ChatRoomSection/ChatItem.tsx (2)
src/stores/modalStore.ts (1)
useModalStore(19-24)src/components/basics/Popover/index.tsx (3)
Popover(5-5)PopoverTrigger(5-5)PopoverContent(5-5)
🔇 Additional comments (3)
src/components/layout/SideNavigation/components/ChatRoomSection/ChatItem.tsx (2)
37-67: Popover와 이벤트 처리 구현 우수Popover 구현과 이벤트 전파 제어가 올바르게 처리되었습니다:
handleButtonClick으로 아이콘 버튼 클릭 시 부모 요소로의 이벤트 전파 차단- 삭제 버튼 클릭 시
stopPropagation(),close(),open()순서가 적절함- PopoverContent의 close 콜백을 올바르게 활용
20-22: 네비게이션 로직 정상Next.js의
useRouter를 활용한 클라이언트 사이드 네비게이션 구현이 올바릅니다.src/components/layout/SideNavigation/SideNavigation.tsx (1)
61-61: ChatRoomSection의 스크롤 처리가 올바르게 구현되어 있습니다ChatRoomSection은 이미
overflow-y-auto로 스크롤을 처리하고 있으며,flex-1과min-h-0을 조합하여 flex 컨테이너 내에서 콘텐츠가 증가해도 레이아웃이 올바르게 작동합니다. 부모의justify-between설정과 함께 상단(헤더/네비), 중앙(ChatRoomSection), 하단(MypageButton)의 공간 분배가 적절히 이루어지므로 추가 수정은 불필요합니다.Likely an incorrect or invalid review comment.
Comment on lines
+30
to
+36
| <div | ||
| className="group bg-btn-tertiary-subtle-onpanel flex h-9 w-50 cursor-pointer items-center justify-between rounded-full pr-3 pl-3 transition-colors hover:pr-1" | ||
| onClick={handleItemClick} | ||
| > | ||
| <span className="font-label-md text-gray500 group-hover:text-gray700 truncate group-hover:w-46"> | ||
| {label} | ||
| </span> |
There was a problem hiding this comment.
호버 시 width 변경으로 인한 레이아웃 시프트
34번 라인의 group-hover:w-46이 호버 시 너비를 동적으로 변경하여 시각적으로 불안정한 레이아웃 시프트를 유발합니다. 이는 사용자 경험을 저해할 수 있습니다.
♻️ 개선 제안
너비를 고정하고 내부 요소의 가시성과 패딩만 조정하는 방식을 권장합니다:
<div
- className="group bg-btn-tertiary-subtle-onpanel flex h-9 w-50 cursor-pointer items-center justify-between rounded-full pr-3 pl-3 transition-colors hover:pr-1"
+ className="group bg-btn-tertiary-subtle-onpanel flex h-9 w-50 cursor-pointer items-center justify-between rounded-full pr-3 pl-3 transition-colors"
onClick={handleItemClick}
>
- <span className="font-label-md text-gray500 group-hover:text-gray700 truncate group-hover:w-46">
+ <span className="font-label-md text-gray500 group-hover:text-gray700 truncate flex-1 min-w-0">
{label}
</span>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div | |
| className="group bg-btn-tertiary-subtle-onpanel flex h-9 w-50 cursor-pointer items-center justify-between rounded-full pr-3 pl-3 transition-colors hover:pr-1" | |
| onClick={handleItemClick} | |
| > | |
| <span className="font-label-md text-gray500 group-hover:text-gray700 truncate group-hover:w-46"> | |
| {label} | |
| </span> | |
| <div | |
| className="group bg-btn-tertiary-subtle-onpanel flex h-9 w-50 cursor-pointer items-center justify-between rounded-full pr-3 pl-3 transition-colors" | |
| onClick={handleItemClick} | |
| > | |
| <span className="font-label-md text-gray500 group-hover:text-gray700 truncate flex-1 min-w-0"> | |
| {label} | |
| </span> |
🤖 Prompt for AI Agents
In @src/components/layout/SideNavigation/components/ChatRoomSection/ChatItem.tsx
around lines 30 - 36, The span inside the ChatItem component changes width on
hover via the class "group-hover:w-46", causing layout shift; remove that
dynamic width and give the element a fixed width (or apply the fixed width to
the parent container) and replace hover-driven width changes with
non-layout-affecting visual tweaks (e.g., change text color, opacity, or
translate on hover) so clicking behavior (handleItemClick) and label rendering
remain stable; update the span's class list to drop "group-hover:w-46" and
ensure a consistent "w-XX" is present on either the parent div or the span,
using hover-only classes for color/opacity instead.
| </Popover> | ||
| </div> | ||
|
|
||
| {type === 'DELETE_CHAT' && <DeleteChatModal chatId={id} />} |
There was a problem hiding this comment.
모달이 중복으로 렌더링되는 중대한 오류
현재 구현에서는 모든 ChatItem 컴포넌트가 type === 'DELETE_CHAT' 조건을 개별적으로 확인하기 때문에, 하나의 채팅 아이템에서 삭제 모달을 열면 모든 ChatItem이 각자의 DeleteChatModal을 렌더링하게 됩니다. 전역 modal store가 어떤 채팅의 모달인지 추적하지 않기 때문입니다.
🔧 수정 방안
방안 1: Modal store의 props를 활용하여 chatId 저장 (권장)
const ChatItem = ({ id, label }: Props) => {
const router = useRouter();
- const { type, open } = useModalStore();
+ const { type, props, open } = useModalStore();
// ... 기존 코드 ...
onClick={e => {
e.stopPropagation();
close();
- open('DELETE_CHAT');
+ open('DELETE_CHAT', { chatId: id });
}}
// ... 기존 코드 ...
- {type === 'DELETE_CHAT' && <DeleteChatModal chatId={id} />}
+ {type === 'DELETE_CHAT' && props?.chatId === id && <DeleteChatModal chatId={id} />}방안 2: 모달을 ChatRoomSection으로 이동하여 중앙 관리
ChatItem에서 모달을 제거하고 ChatRoomSection에서 한 번만 렌더링:
// ChatRoomSection.tsx
const ChatRoomSection = () => {
const { type, props } = useModalStore();
return (
<>
{/* chat items */}
{type === 'DELETE_CHAT' && props?.chatId && (
<DeleteChatModal chatId={props.chatId} />
)}
</>
);
};
// ChatItem.tsx에서는 모달 렌더링 부분 제거
- {type === 'DELETE_CHAT' && <DeleteChatModal chatId={id} />}🤖 Prompt for AI Agents
In @src/components/layout/SideNavigation/components/ChatRoomSection/ChatItem.tsx
at line 70, The DeleteChatModal is being rendered by every ChatItem because the
modal store only exposes a type; update the modal store usage so it carries the
target chatId (useModalStore -> { type, props }) and change the render condition
to only show DeleteChatModal when type==='DELETE_CHAT' AND props?.chatId === id
(inside ChatItem), or alternatively remove the modal from ChatItem and render a
single instance in ChatRoomSection that checks type==='DELETE_CHAT' &&
props?.chatId before rendering DeleteChatModal with props.chatId.
Comment on lines
10
to
18
| const handleDelete = () => { | ||
| // deleteChatMutation.mutate(chatId, { | ||
| // onSuccess: () => close(), | ||
| // onError: (error) => { | ||
| // TODO: 에러 처리 | ||
| // }, | ||
| // }); | ||
| deleteChatMutation.mutate(chatId, { | ||
| onSuccess: () => close(), | ||
| onError: err => { | ||
| // TODO: 에러 처리 | ||
| }, | ||
| }); | ||
| close(); | ||
| }; |
There was a problem hiding this comment.
비동기 작업 완료 전 모달이 닫히는 중대한 오류
17번 라인의 close()가 비동기 mutation 시작 직후 즉시 실행되어 다음 문제가 발생합니다:
- 삭제 작업이 완료되기 전에 모달이 닫힘
- 로딩 상태를 사용자에게 표시할 수 없음
- 12번 라인의
onSuccess콜백이 무의미해짐 - 에러 발생 시 모달이 이미 닫혀있어 사용자에게 에러를 알릴 수 없음
🔧 수정 제안
const handleDelete = () => {
deleteChatMutation.mutate(chatId, {
onSuccess: () => close(),
onError: err => {
// TODO: 에러 처리
},
});
- close();
};추가로, 삭제 진행 중 로딩 상태를 표시하는 것을 권장합니다:
const handleDelete = () => {
deleteChatMutation.mutate(chatId, {
onSuccess: () => close(),
onError: err => {
// TODO: 에러 처리
},
});
};
return (
<Modal type="DELETE_CHAT" className="m-10 max-w-130 min-w-100">
<div className="m-4 flex flex-col gap-2">
<span className="font-title-md">채팅 삭제</span>
<span className="font-body-md">
채팅 삭제 시, 채팅에서 나눈 프롬프트, 대답, 기록들 전체가 영구 삭제됩니다.
<br />
채팅을 삭제하시겠습니까?
</span>
<div className="mt-30 flex w-full gap-2">
<Button variant="secondary" label="취소하기" className="flex-1" onClick={close} />
- <Button variant="primary" label="삭제하기" className="flex-1" onClick={handleDelete} />
+ <Button
+ variant="primary"
+ label="삭제하기"
+ className="flex-1"
+ onClick={handleDelete}
+ disabled={deleteChatMutation.isPending}
+ />
</div>
</div>
</Modal>
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const handleDelete = () => { | |
| // deleteChatMutation.mutate(chatId, { | |
| // onSuccess: () => close(), | |
| // onError: (error) => { | |
| // TODO: 에러 처리 | |
| // }, | |
| // }); | |
| deleteChatMutation.mutate(chatId, { | |
| onSuccess: () => close(), | |
| onError: err => { | |
| // TODO: 에러 처리 | |
| }, | |
| }); | |
| close(); | |
| }; | |
| const handleDelete = () => { | |
| deleteChatMutation.mutate(chatId, { | |
| onSuccess: () => close(), | |
| onError: err => { | |
| // TODO: 에러 처리 | |
| }, | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In
@src/components/layout/SideNavigation/components/ChatRoomSection/DeleteChatModal.tsx
around lines 10 - 18, handleDelete currently calls close() immediately after
starting deleteChatMutation.mutate which closes the modal before the async
operation completes; change handleDelete to call
deleteChatMutation.mutate(chatId, { onSuccess: () => { close(); /* additional
success handling if needed */ }, onError: err => { /* surface error to user,
keep modal open and stop loading */ } }) and remove the immediate close(); also
add a loading state derived from deleteChatMutation.isLoading to disable
controls and show a spinner while the mutation is in progress so the modal stays
open until onSuccess or onError runs.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
관련 이슈
PR 설명
ChatRoomSection.tsxChatItem리스트를 보임ChatItem.tsx