[feat] 사용자 채팅방 이름 변경 및 채팅방 나가기 기능 추가#258
Hidden character warning
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough채팅방 이름 변경 및 삭제 API가 추가되었습니다: Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/pages/Chat/index.tsx (1)
13-14: 상대경로 import를@/*alias로 통일해 주세요.해당 파일의 신규 import는 프로젝트 alias 규칙과 맞추는 것이 좋습니다.
As per coding guidelines
**/*.{ts,tsx}: "Use@/*alias for import paths instead of relative paths".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/Chat/index.tsx` around lines 13 - 14, The imports in this file use relative paths; update them to the project alias style by replacing './components/ChatRoomContextMenu' and './hooks/useChat' with the '@/pages/Chat/components/ChatRoomContextMenu' and '@/pages/Chat/hooks/useChat' (or the correct alias-targeted paths for these modules) so that ChatRoomContextMenu and useChat are imported via the `@/`* alias consistent with the repo rule for .ts/.tsx imports.src/apis/chat/index.ts (1)
53-66: 신규 API 응답 타입을 명시해주세요.
apiClient.patch/apiClient.delete의 응답 타입이 빠져 있어 호출부 타입 안정성이 약해집니다.src/apis/chat/entity.ts에 도메인 응답 타입을 정의하고 제네릭으로 연결하는 게 좋습니다.As per coding guidelines
src/apis/**: "요청/응답 타입이 명시되어 있는지" andsrc/apis/**/*.ts: "Define API response types in the domain-specificentity.tsfile".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/apis/chat/index.ts` around lines 53 - 66, patchChatRoomName and deleteChatRoom call apiClient.patch/delete without specifying response generics, reducing type safety; update these functions to import the appropriate response types from src/apis/chat/entity.ts (e.g., ChatRoomNamePatchResponse, ChatRoomDeleteResponse) and pass them as generics to apiClient.patch and apiClient.delete respectively so the returned Promise is strongly typed (refer to functions patchChatRoomName and deleteChatRoom and the domain types in entity.ts).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/Chat/components/ChatRoomContextMenu.tsx`:
- Around line 23-34: The calculated menuWidth/menuHeight (and menuItemHeight)
used for adjustedX/adjustedY do not match the hardcoded class sizes w-[161px]
h-[159px], causing incorrect positioning; either make the numeric constants
equal to the CSS sizes or (better) measure the rendered element using menuRef
(e.g., in a useLayoutEffect) and compute menuWidth/menuHeight from
menuRef.current.getBoundingClientRect() before computing adjustedX/adjustedY so
the positioning uses the actual rendered width/height; update references to
menuWidth, menuHeight, and menuItemHeight (or remove menuItemHeight if measuring
full height) and ensure the position math uses the measured values.
In `@src/pages/Chat/index.tsx`:
- Around line 319-320: The menu entry with label '알림 끄기' is a no-op; either wire
it to the actual mute toggling function or remove/hide it. Locate the menu items
array in Chat/index.tsx where { label: '알림 끄기', onClick: () => {} } is defined
and change onClick to call the existing toggleMute(...) helper (passing the
current chat id or chat object and then closing the menu), or conditionally
filter out this item when toggleMute is not available/ready so the inert action
never appears to users.
- Around line 295-299: When applying the room name change in the onClick handler
for changeRoomName, normalize newRoomName by trimming whitespace and send an
explicit empty string (or the server's default signal) if the trimmed result is
empty; i.e., compute a normalizedName = newRoomName.trim() and pass
normalizedName === '' ? '' : normalizedName to updateRoomName({ chatRoomId:
roomId, name: normalizedName }) while still calling setChangeRoomName(null) as
before, referencing the changeRoomName, setChangeRoomName, updateRoomName and
newRoomName symbols.
- Around line 267-271: The click handler currently calls
deleteChatRoom(leaveRoom.roomId) without awaiting or handling the Promise (using
setLeaveRoom and leaveRoom); update the handler to guard for leaveRoom, then
call the async deleteChatRoom via await inside an async function or use
.then/.catch to handle success and failure, and ensure you reset
setLeaveRoom(null) only on success or always as desired while logging or
displaying errors on rejection (reference functions/variables: setLeaveRoom,
leaveRoom, deleteChatRoom).
---
Nitpick comments:
In `@src/apis/chat/index.ts`:
- Around line 53-66: patchChatRoomName and deleteChatRoom call
apiClient.patch/delete without specifying response generics, reducing type
safety; update these functions to import the appropriate response types from
src/apis/chat/entity.ts (e.g., ChatRoomNamePatchResponse,
ChatRoomDeleteResponse) and pass them as generics to apiClient.patch and
apiClient.delete respectively so the returned Promise is strongly typed (refer
to functions patchChatRoomName and deleteChatRoom and the domain types in
entity.ts).
In `@src/pages/Chat/index.tsx`:
- Around line 13-14: The imports in this file use relative paths; update them to
the project alias style by replacing './components/ChatRoomContextMenu' and
'./hooks/useChat' with the '@/pages/Chat/components/ChatRoomContextMenu' and
'@/pages/Chat/hooks/useChat' (or the correct alias-targeted paths for these
modules) so that ChatRoomContextMenu and useChat are imported via the `@/`* alias
consistent with the repo rule for .ts/.tsx imports.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 05358e60-3c01-40b7-bcba-07f0adef8238
📒 Files selected for processing (8)
src/apis/chat/index.tssrc/apis/chat/mutations.tssrc/pages/Chat/components/ChatRoomContextMenu.tsxsrc/pages/Chat/hooks/useChat.tssrc/pages/Chat/hooks/useChatMutations.tssrc/pages/Chat/index.tsxsrc/utils/hooks/useLongPress.tssrc/utils/hooks/useOutsideTapDismiss.ts
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/pages/Chat/index.tsx (1)
324-324:⚠️ Potential issue | 🟡 Minor'알림 끄기' 메뉴가 동작하지 않습니다.
빈
onClick핸들러가 있어 사용자에게 오작동처럼 보입니다. 실제 기능에 연결하거나 메뉴에서 숨겨 주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/Chat/index.tsx` at line 324, The menu entry with label '알림 끄기' currently has an empty onClick (() => {}) which makes it nonfunctional; replace the empty handler with the real notification toggle or disable action (e.g., call the existing toggleNotifications(), setNotificationsEnabled(false), or props.onToggleNotifications() used elsewhere in Chat/index.tsx), or if no such handler exists, remove or conditionally hide the '알림 끄기' menu item so it is not shown to users.
🧹 Nitpick comments (1)
src/pages/Chat/index.tsx (1)
252-254: 타이포그래피 토큰 사용을 권장합니다.
text-[16px],text-[14px]대신text-sub1,text-body2등 시맨틱 타이포그래피 유틸리티를 사용하면 일관성을 유지할 수 있습니다. As per coding guidelines, "Use semantic typography utilities (text-h1,text-body1, etc.) before custom styles".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/Chat/index.tsx` around lines 252 - 254, The paragraph elements in the Chat component use hardcoded size utilities (`text-[16px]` and `text-[14px]`) which violates the typography token guideline; update the className on the first <p> (currently using `text-[16px] font-bold`) to use the semantic token `text-sub1` (and keep `font-bold` if needed) and update the second <p> (currently `text-[14px]`) to use `text-body2` (or your project's equivalent semantic token) so the component (leaveRoom modal JSX) uses semantic typography utilities instead of custom pixel classes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/Chat/index.tsx`:
- Around line 87-89: The useLongPress hook currently reads e.clientX/e.clientY
inside the setTimeout callback which can be invalidated; instead use the stored
coordinates in startPointRef.current when invoking onLongPress. Modify the timer
callback code in useLongPress (the block that sets timerRef.current and
didLongPressRef.current) to check startPointRef.current and call
onLongPress(startPointRef.current.x, startPointRef.current.y), then call
clearTimer(); ensure you keep references to timerRef, didLongPressRef,
startPointRef, onLongPress and clearTimer unchanged elsewhere.
- Around line 267-275: The click handler clears leaveRoom before checking it, so
deleteChatRoom never runs; capture the current room into a local variable (e.g.,
const room = leaveRoom) or check leaveRoom before calling setLeaveRoom(null),
then if room exists call await deleteChatRoom(room.roomId) inside the try/catch
and only after successful (or attempted) deletion call setLeaveRoom(null);
update the onClick handler that references setLeaveRoom, leaveRoom, and
deleteChatRoom accordingly.
---
Duplicate comments:
In `@src/pages/Chat/index.tsx`:
- Line 324: The menu entry with label '알림 끄기' currently has an empty onClick (()
=> {}) which makes it nonfunctional; replace the empty handler with the real
notification toggle or disable action (e.g., call the existing
toggleNotifications(), setNotificationsEnabled(false), or
props.onToggleNotifications() used elsewhere in Chat/index.tsx), or if no such
handler exists, remove or conditionally hide the '알림 끄기' menu item so it is not
shown to users.
---
Nitpick comments:
In `@src/pages/Chat/index.tsx`:
- Around line 252-254: The paragraph elements in the Chat component use
hardcoded size utilities (`text-[16px]` and `text-[14px]`) which violates the
typography token guideline; update the className on the first <p> (currently
using `text-[16px] font-bold`) to use the semantic token `text-sub1` (and keep
`font-bold` if needed) and update the second <p> (currently `text-[14px]`) to
use `text-body2` (or your project's equivalent semantic token) so the component
(leaveRoom modal JSX) uses semantic typography utilities instead of custom pixel
classes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aa8166e1-9dc8-426d-9dbd-65189df0f771
📒 Files selected for processing (2)
src/pages/Chat/components/ChatRoomContextMenu.tsxsrc/pages/Chat/index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/Chat/components/ChatRoomContextMenu.tsx
| const longPress = useLongPress({ | ||
| onLongPress: (x: number, y: number) => onLongPress(x, y, room), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# useLongPress 훅의 setTimeout 콜백 확인
cat src/utils/hooks/useLongPress.tsRepository: BCSDLab/KONECT_FRONT_END
Length of output: 2291
useLongPress 훅의 이벤트 객체 참조 버그
setTimeout 콜백에서 e.clientX/e.clientY를 직접 참조하는 것은 위험합니다. 이벤트 핸들러 반환 후 React의 이벤트 객체가 무효화되므로 좌표가 NaN이 됩니다. 이미 저장된 startPointRef.current.x/y를 사용하도록 수정하세요:
timerRef.current = setTimeout(() => {
didLongPressRef.current = true;
if (startPointRef.current) {
onLongPress(startPointRef.current.x, startPointRef.current.y);
}
clearTimer();
}, delay);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/Chat/index.tsx` around lines 87 - 89, The useLongPress hook
currently reads e.clientX/e.clientY inside the setTimeout callback which can be
invalidated; instead use the stored coordinates in startPointRef.current when
invoking onLongPress. Modify the timer callback code in useLongPress (the block
that sets timerRef.current and didLongPressRef.current) to check
startPointRef.current and call onLongPress(startPointRef.current.x,
startPointRef.current.y), then call clearTimer(); ensure you keep references to
timerRef, didLongPressRef, startPointRef, onLongPress and clearTimer unchanged
elsewhere.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/pages/Chat/index.tsx (1)
13-14: 상대 경로 import를 새로 늘리지 않는 편이 좋겠습니다.Line 13의 새 import는 이 레포 컨벤션과 다르게 상대 경로를 사용합니다.
@/pages/Chat/components/ChatRoomContextMenu처럼 alias로 맞춰주세요.As per coding guidelines, 'Use
@/*alias for import paths instead of relative paths'.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/Chat/index.tsx` around lines 13 - 14, The import for ChatRoomContextMenu uses a relative path which violates the repo's alias convention; replace the relative import "import ChatRoomContextMenu from './components/ChatRoomContextMenu';" with the aliased path "import ChatRoomContextMenu from '@/pages/Chat/components/ChatRoomContextMenu';" (and similarly audit other imports in this file such as useChat to ensure they use the '@/...' alias if required), then run a quick typecheck/build to confirm the aliased path resolves.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/layout/Header/components/ChatHeader.tsx`:
- Around line 58-64: The click handler currently uses `void
toggleMute(numericRoomId)` which doesn't prevent unhandled rejections; update
the `onClick` in ChatHeader.tsx to properly handle the Promise from `toggleMute`
by either making the handler async and using `await` inside a try/catch (e.g.,
`onClick={async () => { try { await toggleMute(numericRoomId) } catch (e) { /*
log/handle */ } }}`) or call `toggleMute(numericRoomId).catch(e => { /*
log/handle */ })` so any API failure is caught; ensure you still respect
`isTogglingMute`/disabled state and do not leak the raw Promise to the event
system.
In `@src/pages/Chat/index.tsx`:
- Around line 224-230: The changeName handler closes the UI
(setChangeRoomName(null)) before the async updateRoomName completes, which can
lose input on failure; change it to await updateRoomName inside a try/catch and
only call setChangeRoomName(null) on success, and show a user-visible error
(toast/snackbar) on failure; apply the same pattern to the other room-action
handlers (the other rename/topic/update functions) that currently
fire-and-forget updateRoomName/updateRoomTopic calls so they await the Promise,
handle errors, and close modals/menus only after success.
- Around line 78-96: The room management menu is currently only reachable via
pointer long-press in ChatRoomListItem using useLongPress/onLongPress, which
blocks keyboard and assistive-device users; add an explicit menu trigger that is
keyboard-accessible and screen-reader friendly (e.g., a small button element
rendered inside ChatRoomListItem with aria-haspopup="menu", aria-label (e.g.,
"Open room menu"), role="button" if not a native button, and tabIndex=0), wire
its onClick to the same handler that opens the menu (call the existing
onLongPress callback or an extracted openMenu(room, coords?) helper), and add an
onKeyDown handler that activates the same action on Enter and Space; apply the
same change where ChatRoomListItem is reused (the other occurrence referenced)
so both pointer and keyboard/assistive users can open the name-change/leave
menu.
- Around line 92-96: The Link element's className uses the non-standard Tailwind
utility "user-select-none" which has no effect; update the class string on the
Link component (the JSX Link with props {...longPress} and
to={`${room.roomId}`}) to replace "user-select-none" with Tailwind v4's correct
"select-none" utility, and scan for other occurrences of "user-select-none" to
replace them as well.
---
Nitpick comments:
In `@src/pages/Chat/index.tsx`:
- Around line 13-14: The import for ChatRoomContextMenu uses a relative path
which violates the repo's alias convention; replace the relative import "import
ChatRoomContextMenu from './components/ChatRoomContextMenu';" with the aliased
path "import ChatRoomContextMenu from
'@/pages/Chat/components/ChatRoomContextMenu';" (and similarly audit other
imports in this file such as useChat to ensure they use the '@/...' alias if
required), then run a quick typecheck/build to confirm the aliased path
resolves.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 174a74c9-2d92-4026-9e65-f45f7df75c04
📒 Files selected for processing (7)
src/apis/chat/mutations.tssrc/components/layout/Header/components/ChatHeader.tsxsrc/pages/Chat/components/ChatRoomContextMenu.tsxsrc/pages/Chat/hooks/useChat.tssrc/pages/Chat/hooks/useChatMutations.tssrc/pages/Chat/index.tsxsrc/utils/hooks/useLongPress.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/pages/Chat/hooks/useChatMutations.ts
- src/pages/Chat/components/ChatRoomContextMenu.tsx
- src/utils/hooks/useLongPress.ts
- src/apis/chat/mutations.ts
| <button | ||
| type="button" | ||
| disabled={isTogglingMute} | ||
| onClick={() => void toggleMute()} | ||
| onClick={() => void toggleMute(numericRoomId)} | ||
| className={`relative inline-flex h-6 w-11 items-center rounded-full transition-colors ${ | ||
| isMuted ? 'bg-gray-300' : 'bg-primary' | ||
| } disabled:cursor-not-allowed disabled:opacity-60`} |
There was a problem hiding this comment.
헤더의 mute 토글 Promise를 처리해주세요.
Line 61의 void toggleMute(numericRoomId)는 rejection을 막지 못합니다. API 실패 시 unhandled rejection이 남을 수 있으니 await/catch로 감싸거나, fire-and-forget이 목적이면 Promise를 밖으로 노출하지 않는 형태로 바꾸는 편이 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/layout/Header/components/ChatHeader.tsx` around lines 58 - 64,
The click handler currently uses `void toggleMute(numericRoomId)` which doesn't
prevent unhandled rejections; update the `onClick` in ChatHeader.tsx to properly
handle the Promise from `toggleMute` by either making the handler async and
using `await` inside a try/catch (e.g., `onClick={async () => { try { await
toggleMute(numericRoomId) } catch (e) { /* log/handle */ } }}`) or call
`toggleMute(numericRoomId).catch(e => { /* log/handle */ })` so any API failure
is caught; ensure you still respect `isTogglingMute`/disabled state and do not
leak the raw Promise to the event system.
| interface ChatRoomListItemProps { | ||
| room: Room; | ||
| onLongPress: (x: number, y: number, room: Room) => void; | ||
| } | ||
|
|
||
| function ChatRoomListItem({ room, onLongPress }: ChatRoomListItemProps) { | ||
| const isGroup = room.chatType === 'GROUP'; | ||
| const hasUnreadMessage = room.unreadCount > 0; | ||
| const previewMessage = room.lastMessage?.trim() || DEFAULT_LAST_MESSAGE; | ||
| const longPress = useLongPress({ | ||
| onLongPress: (x: number, y: number) => onLongPress(x, y, room), | ||
| }); | ||
|
|
||
| return ( | ||
| <Link | ||
| {...longPress} | ||
| to={`${room.roomId}`} | ||
| className="active:bg-indigo-5 flex items-center gap-3 bg-white px-5 py-3 transition-colors" | ||
| className="active:bg-indigo-5 user-select-none flex touch-pan-y items-center gap-3 bg-white px-5 py-3 transition-colors" | ||
| > |
There was a problem hiding this comment.
롱프레스만으로는 방 관리 메뉴에 접근할 수 없습니다.
현재 이름 변경/나가기 진입점이 포인터 롱프레스뿐이라 키보드·보조기기 사용자는 기능을 실행할 방법이 없습니다. 같은 메뉴를 여는 별도 버튼을 두거나, 최소한 Enter/Space로 열 수 있는 대체 경로를 추가해주세요.
As per coding guidelines, '접근성(aria-*, role, 키보드 탐색)이 적절히 처리되는지'.
Also applies to: 282-282
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/Chat/index.tsx` around lines 78 - 96, The room management menu is
currently only reachable via pointer long-press in ChatRoomListItem using
useLongPress/onLongPress, which blocks keyboard and assistive-device users; add
an explicit menu trigger that is keyboard-accessible and screen-reader friendly
(e.g., a small button element rendered inside ChatRoomListItem with
aria-haspopup="menu", aria-label (e.g., "Open room menu"), role="button" if not
a native button, and tabIndex=0), wire its onClick to the same handler that
opens the menu (call the existing onLongPress callback or an extracted
openMenu(room, coords?) helper), and add an onKeyDown handler that activates the
same action on Enter and Space; apply the same change where ChatRoomListItem is
reused (the other occurrence referenced) so both pointer and keyboard/assistive
users can open the name-change/leave menu.
✨ 요약
😎 해결한 이슈
close #255
✅ 작업 내용
🧪 테스트
pnpm lintSummary by CodeRabbit