[WIP] Feat: add sidenavigation chatlist and userinfo (#308) #315
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이 PR은 사이드 네비게이션 컴포넌트의 구조를 대대적으로 재구성하며, 채팅 삭제 기능을 추가합니다. 구체적으로 채팅 삭제 API 유틸리티를 신규 작성하고, 사이드 네비게이션을 헤더, 메뉴 섹션, 채팅 룸 섹션, 하단 영역으로 분리한 새로운 서브컴포넌트 구조를 도입합니다. Popover 컴포넌트는 자식 프롭을 함수 타입으로 변경하고 외부 클릭 핸들러를 추가하도록 리팩토링하며, NavItem 컴포넌트는 children 프롭에서 label 프롭 기반으로 전환됩니다. 이 외에도 모달 저장소에 DELETE_CHAT 타입을 추가하고 기존 SideNavToggle 컴포넌트를 제거합니다. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✨ 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: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/basics/Popover/PopoverProvider.tsx (1)
12-43: 타입 불일치: popoverRef가 context 타입에 누락됨Line 40에서
popoverRef를 context value에 추가했지만,PopoverContextType인터페이스(PopoverContext.tsx의 5-12번째 줄)에는popoverRef필드가 정의되어 있지 않습니다. 이로 인해 타입 안정성 문제가 발생합니다.🔎 PopoverContext.tsx에 popoverRef 타입 추가 필요
src/components/basics/Popover/PopoverContext.tsx파일을 다음과 같이 수정하세요:+import { RefObject } from 'react'; + interface PopoverContextType { anchorEl?: HTMLElement | null; activeKey: string | null; placement: Placement; close: () => void; open: (key: string, anchor: HTMLElement) => void; toggle: (key: string, anchor: HTMLElement) => void; + popoverRef: RefObject<HTMLElement | null>; }
🤖 Fix all issues with AI Agents
In @src/apis/chatApi.ts:
- Around line 4-7: API_URL and API_TOKEN can be undefined at runtime, and
CHAT_ENDPOINT is constructed unguarded; add a validation step (e.g., in module
init or a small helper like validateEnv or getChatEndpoint/getApiToken) that
checks API_URL and API_TOKEN (and prevents constructing CHAT_ENDPOINT when
API_URL is falsy), throw or log a clear error if missing, and use the validated
values to build CHAT_ENDPOINT or return them to callers so no request is made
with an invalid URL; reference the constants API_URL, API_TOKEN and
CHAT_ENDPOINT when making these changes.
- Around line 18-30: The deleteChat response validation is incomplete: update
the validation in deleteChat to ensure body is an object matching
ApiResponseBase<string> by checking success is a boolean, status is a non-empty
string, message is a non-empty string, and data is present (not undefined/null)
and of type string; also if timestamp exists ensure its type is valid. Use the
DeleteChatApiResponse/ApiResponseBase<string> symbols to guide checks on the
returned body and throw an Error with body?.message when validation fails.
In @src/components/basics/Popover/Content.tsx:
- Around line 23-25: The code directly assigns anchorRef.current = anchorEl ??
null during render (using anchorRef and anchorEl), which is an anti-pattern;
move this assignment into a useEffect that runs when anchorEl changes (set
anchorRef.current inside useEffect) or alternatively stop using anchorRef and
pass anchorEl directly into usePopoverPosition and useOutsideClick so both hooks
receive the external element without mutating refs during render.
In
@src/components/layout/SideNavigation/components/Bottom/SideNavigationBottom.tsx:
- Line 6: The SideNavigationBottom component declares an unused prop isOpen;
remove isOpen from the component signature and its type annotation in
SideNavigationBottom, and update any parent/usage sites that pass isOpen to this
component so they no longer supply that prop (or stop forwarding it) to avoid
unused/TS errors.
- Around line 13-23: PopoverContent currently expects a function-as-children
that receives a close callback, but the code passes a Button element; change the
PopoverContent children to be a function (close: () => void) => ReactNode that
returns the Button and, if appropriate, hook the close callback into the
Button's onClick (or call close after performing logout). Update the JSX around
PopoverContent to use a children function and ensure the Button still has the
same props (variant, contextStyle, label, size, icon, radius, className) while
wiring in close.
In
@src/components/layout/SideNavigation/components/ChatRoomSection/ChatItem.tsx:
- Around line 20-22: handleItemClick currently calls router.push with the
literal string 'chat/{id}', so the id value isn't interpolated; change the call
in the ChatItem component to use a template literal or string concatenation to
inject the actual id (e.g., `/chat/${id}`), ensuring you reference the local
id/props variable used in this component and keep the leading slash for an
absolute path when calling router.push from handleItemClick.
In
@src/components/layout/SideNavigation/components/ChatRoomSection/DeleteChatModal.tsx:
- Around line 11-14: handleDelete currently calls
deleteChatMutation.mutate(chatId) then immediately close(), causing the modal to
close before deletion completes; change this to await the async result (use
deleteChatMutation.mutateAsync(chatId) or move close() into the mutation's
onSuccess callback) so the modal only closes after successful deletion and
handle errors (onError or try/catch) to keep the modal open and surface the
error to the user.
In @src/components/layout/SideNavigation/components/MenuSection/MenuSection.tsx:
- Around line 26-43: The MenuSection renders item.item twice and uses a
transition without animate; replace the two motion.div blocks with a single
motion.div (the one currently using initial/animate/style) that controls both
open/closed visuals via animate or variants based on isOpen (e.g., animate={{
opacity: isOpen ? 1 : 0, x: isOpen ? 0 : -4 }} and a transition prop), remove
the separate absolute positioned block and the redundant !isOpen check, and keep
pointerEvents logic on that single motion.div so item.item is rendered once and
the transition actually animates.
In @src/components/layout/SideNavigation/components/NavItem/NavItem.tsx:
- Around line 18-27: The mergeRefs function assigns to ref.current which can be
readonly; update mergeRefs to set refs in a type-safe way by detecting object
refs and casting to React.MutableRefObject<HTMLButtonElement | null> (or using a
helper setRef) before assigning, e.g., treat forwardedRef and blurRef as
function refs or mutable refs and assign via (ref as
React.MutableRefObject<...>).current = el, or replace mergeRefs with a proven
utility like useMergeRefs from @radix-ui/react-compose-refs /
@floating-ui/react; ensure you import React types if you add the
MutableRefObject cast and preserve handling for function refs in mergeRefs.
🧹 Nitpick comments (7)
src/components/layout/SideNavigation/components/Header/SideNavigationHeader.tsx (1)
9-21: 컴포넌트 재사용성 고려가 필요합니다.이 컴포넌트는
SideNavToggle컴포넌트와 거의 동일한 구조입니다. 11번 라인의mb-10클래스가 하드코딩되어 있어 다른 간격이 필요한 경우 재사용이 어렵습니다. 마진을 props로 받거나 부모 컴포넌트에서 제어하는 것을 고려해보세요.🔎 제안하는 리팩토링
interface Props { isOpen: boolean; onClick: (e: MouseEvent<HTMLButtonElement>) => void; + className?: string; } -const SideNavigationHeader = ({ isOpen, onClick }: Props) => { +const SideNavigationHeader = ({ isOpen, onClick, className }: Props) => { return ( - <div className="mb-10"> + <div className={className}> <IconButton icon="IC_SidenavOpen" variant="tertiary_subtle" size="lg" ariaLabel={isOpen ? '좌측 사이드바 메뉴 닫기' : '좌측 사이드바 메뉴 열기'} onClick={onClick} /> </div> ); };src/components/layout/SideNavigation/components/MenuSection/MenuSection.tsx (1)
13-20: MENU_ITEMS 배열을 컴포넌트 외부로 추출하는 것을 고려하세요.
MENU_ITEMS배열이 매 렌더링마다 재생성됩니다. 이 배열을 컴포넌트 외부의 상수로 추출하고,open함수만 props로 전달하면 불필요한 재생성을 방지할 수 있습니다.src/components/layout/SideNavigation/components/ChatRoomSection/ChatRoomSection.tsx (2)
7-14: 불필요한 Fragment를 제거하세요.단일 최상위 요소만 반환하므로 빈 Fragment(
<>)가 필요하지 않습니다.🔎 제안하는 수정
const ChatRoomSection = () => { return ( - <> - <div className="mt-10 flex min-h-0 flex-1 flex-col"> - <Label className="mb-2 shrink-0">채팅</Label> - <div className="custom-scrollbar min-h-0 flex-1 overflow-x-hidden overflow-y-auto"> - <ChatItem id={1} label="어쩌구저쩌궁ㄹㅇㄹ아러ㅣ라ㅓ이ㅏ러나ㅣ러나이런아ㅣ런알" /> - </div> - </div> - </> + <div className="mt-10 flex min-h-0 flex-1 flex-col"> + <Label className="mb-2 shrink-0">채팅</Label> + <div className="custom-scrollbar min-h-0 flex-1 overflow-x-hidden overflow-y-auto"> + <ChatItem id={1} label="어쩌구저쩌궁ㄹㅇㄹ아러ㅣ라ㅓ이ㅏ러나ㅣ러나이런아ㅣ런알" /> + </div> + </div> ); };
11-11: 프로덕션 전에 하드코딩된 테스트 데이터를 실제 데이터로 교체해야 합니다.
id={1}과 임의의 긴 문자열이 하드코딩되어 있습니다. WIP 단계에서는 문제없지만, 머지 전에 다음 중 하나로 변경이 필요합니다:
- Props로 채팅 목록 데이터 받기
- API/store에서 채팅 목록 fetch하기
- 실제 ChatSummary 타입의 데이터 매핑
향후 실제 채팅 목록을 렌더링하는 구현이 필요하면 도움을 드릴 수 있습니다. 새로운 이슈를 생성할까요?
src/components/layout/SideNavigation/components/ChatRoomSection/DeleteChatModal.tsx (1)
26-27: 삭제 진행 중 버튼 비활성화를 고려해보세요.삭제 요청 중 사용자가 버튼을 여러 번 클릭할 수 있습니다.
isPending상태를 활용하여 중복 요청을 방지하면 좋습니다.🔎 isPending 상태 활용 예시
+const { mutate, isPending } = deleteChatMutation; + // ... -<Button variant="secondary" label="취소하기" className="flex-1" onClick={close} /> -<Button variant="primary" label="삭제하기" className="flex-1" onClick={handleDelete} /> +<Button variant="secondary" label="취소하기" className="flex-1" onClick={close} disabled={isPending} /> +<Button variant="primary" label={isPending ? '삭제 중...' : '삭제하기'} className="flex-1" onClick={handleDelete} disabled={isPending} />src/components/layout/SideNavigation/components/ChatRoomSection/hooks/useDeleteChat.ts (1)
10-13: 삭제된 채팅의 개별 쿼리 무효화는 불필요합니다.
['chats', id]쿼리는 삭제된 채팅에 대한 것이므로 무효화할 필요가 없습니다. 삭제 후 해당 데이터는 더 이상 존재하지 않습니다.🔎 불필요한 무효화 제거
onSuccess: (_data, id) => { qc.invalidateQueries({ queryKey: ['chats'] }); - qc.invalidateQueries({ queryKey: ['chats', id] }); + qc.removeQueries({ queryKey: ['chats', id] }); },
removeQueries를 사용하면 캐시에서 해당 쿼리를 완전히 제거하여 더 명확한 의도를 표현할 수 있습니다.src/components/layout/SideNavigation/components/ChatRoomSection/ChatItem.tsx (1)
70-70: 모달을 상위 컴포넌트에서 렌더링하는 것을 고려해보세요.각
ChatItem이DeleteChatModal을 조건부로 렌더링하고 있어, 여러ChatItem이 존재할 경우 잠재적으로 여러 모달 인스턴스가 생성될 수 있습니다.모달을 전역 레벨(예: 레이아웃 또는 앱 루트)에서 렌더링하고,
modalStore의props를 활용하여chatId를 전달하는 패턴이 더 깔끔합니다.🔎 전역 모달 패턴 예시
// modalStore에서 props 활용 open('DELETE_CHAT', { chatId: id }); // 상위 레벨에서 모달 렌더링 const { type, props } = useModalStore(); {type === 'DELETE_CHAT' && <DeleteChatModal chatId={props.chatId} />}
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/components/Icons/svgs/ic_logo.svgis excluded by!**/*.svg
📒 Files selected for processing (22)
src/apis/chatApi.tssrc/components/Icons/icons.tssrc/components/basics/Popover/Content.tsxsrc/components/basics/Popover/PopoverProvider.tsxsrc/components/basics/Popover/index.tsxsrc/components/layout/SideNavigation/SideNavigation.tsxsrc/components/layout/SideNavigation/components/Bottom/SideNavigationBottom.tsxsrc/components/layout/SideNavigation/components/ChatRoomSection/ChatItem.tsxsrc/components/layout/SideNavigation/components/ChatRoomSection/ChatRoomSection.tsxsrc/components/layout/SideNavigation/components/ChatRoomSection/DeleteChatModal.tsxsrc/components/layout/SideNavigation/components/ChatRoomSection/hooks/useDeleteChat.tssrc/components/layout/SideNavigation/components/Header/SideNavigationHeader.tsxsrc/components/layout/SideNavigation/components/MenuSection/AddLinkButton.tsxsrc/components/layout/SideNavigation/components/MenuSection/AddLinkModal.tsxsrc/components/layout/SideNavigation/components/MenuSection/AllLinkButton.tsxsrc/components/layout/SideNavigation/components/MenuSection/MenuSection.tsxsrc/components/layout/SideNavigation/components/MenuSection/NewChatButton.tsxsrc/components/layout/SideNavigation/components/NavItem/LinkNavItem.tsxsrc/components/layout/SideNavigation/components/NavItem/NavItem.tsxsrc/components/layout/SideNavigation/components/SideNavToggle/SideNavToggle.tsxsrc/hooks/util/useOutsideClickHandler.tsxsrc/stores/modalStore.ts
💤 Files with no reviewable changes (1)
- src/components/layout/SideNavigation/components/SideNavToggle/SideNavToggle.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-23T14:52:20.769Z
Learnt from: Bangdayeon
Repo: Team-SoFa/linkiving PR: 102
File: src/components/layout/SideNavigation/components/AddLinkModal/AddLinkModal.tsx:23-23
Timestamp: 2025-11-23T14:52:20.769Z
Learning: In src/components/layout/SideNavigation/components/AddLinkModal/AddLinkModal.tsx, the hardcoded '/file.svg' thumbnail is intentional as a placeholder because the backend API for fetching link metadata/thumbnails is still in preparation. The actual thumbnail fetch logic will be implemented after the backend is ready.
Applied to files:
src/components/layout/SideNavigation/components/Bottom/SideNavigationBottom.tsxsrc/components/layout/SideNavigation/components/Header/SideNavigationHeader.tsxsrc/components/layout/SideNavigation/components/MenuSection/MenuSection.tsxsrc/components/layout/SideNavigation/SideNavigation.tsxsrc/components/layout/SideNavigation/components/ChatRoomSection/ChatItem.tsxsrc/components/layout/SideNavigation/components/ChatRoomSection/DeleteChatModal.tsx
📚 Learning: 2025-11-23T12:03:33.890Z
Learnt from: Bangdayeon
Repo: Team-SoFa/linkiving PR: 97
File: src/components/basics/LinkCard/LinkCard.tsx:12-19
Timestamp: 2025-11-23T12:03:33.890Z
Learning: In src/components/basics/LinkCard/LinkCard.tsx, the summary prop should remain required (string type) because the backend always provides it as a string value. The isHaveSummary flag controls whether to display the summary text or show the AddSummaryButton, not whether the data exists.
Applied to files:
src/components/layout/SideNavigation/components/NavItem/LinkNavItem.tsx
🧬 Code graph analysis (11)
src/components/Icons/icons.ts (2)
src/components/Icons/SVGIcon.tsx (2)
SVGIconProps(5-11)props(19-37)src/components/wrappers/LinkIconButton/LinkIconButton.tsx (1)
props(14-39)
src/components/layout/SideNavigation/components/Header/SideNavigationHeader.tsx (5)
src/components/layout/SideNavigation/components/SideNavToggle/SideNavToggle.tsx (2)
SideNavToggleProps(9-19)SideNavToggleProps(4-7)src/components/layout/SideNavigation/components/AllLink/AllLinkButton.tsx (1)
LinkNavItem(3-7)src/components/basics/Header/Header.tsx (1)
Header(10-74)src/components/layout/SideNavigation/components/AddLink/AddLinkButton.tsx (1)
AddLinkButtonProps(7-11)src/components/basics/Accordion/AccordionButton/AccordionButton.tsx (1)
AccordionButtonProps(12-24)
src/stores/modalStore.ts (4)
src/stories/ReSummaryModal.stories.tsx (1)
useModalStore(17-26)src/stories/Modal.stories.tsx (2)
type(33-48)openType(37-40)src/components/basics/Modal/Modal.tsx (1)
ModalProps(17-21)src/stories/ReportModal.stories.tsx (1)
useModalStore(15-23)
src/components/basics/Popover/PopoverProvider.tsx (2)
src/components/basics/Popover/Trigger.tsx (1)
e(47-50)src/components/basics/Popover/PopoverContext.tsx (1)
PopoverContextType(6-13)
src/components/layout/SideNavigation/components/MenuSection/MenuSection.tsx (4)
src/stores/modalStore.ts (1)
useModalStore(19-24)src/components/layout/SideNavigation/components/AddLinkModal/AddLinkModal.tsx (1)
useModalStore(22-112)src/components/layout/SideNavigation/components/NewChat/NewChatButton.tsx (1)
LinkNavItem(3-5)src/components/layout/SideNavigation/components/AllLink/AllLinkButton.tsx (1)
LinkNavItem(3-7)
src/components/layout/SideNavigation/components/ChatRoomSection/ChatRoomSection.tsx (2)
src/app/(route)/chat/ChatPage.tsx (1)
Chat(5-13)src/types/api/chatApi.ts (1)
ChatSummary(3-6)
src/components/layout/SideNavigation/SideNavigation.tsx (1)
src/stores/sideNavStore.ts (1)
useSideNavStore(9-13)
src/components/layout/SideNavigation/components/ChatRoomSection/hooks/useDeleteChat.ts (2)
src/types/api/chatApi.ts (1)
DeleteChatApiResponse(26-26)src/apis/chatApi.ts (1)
deleteChat(18-30)
src/components/layout/SideNavigation/components/NavItem/NavItem.tsx (4)
src/components/basics/Button/Button.tsx (1)
ButtonProps(11-25)src/components/Icons/icons.ts (1)
IconMapTypes(77-77)src/stores/sideNavStore.ts (1)
useSideNavStore(9-13)src/hooks/util/useBlurOnClick.ts (1)
useBlurOnClick(3-14)
src/apis/chatApi.ts (2)
src/types/api/chatApi.ts (1)
DeleteChatApiResponse(26-26)src/hooks/util/server/safeFetch.ts (1)
safeFetch(37-125)
src/components/layout/SideNavigation/components/ChatRoomSection/DeleteChatModal.tsx (2)
src/stores/modalStore.ts (1)
useModalStore(19-24)src/components/layout/SideNavigation/components/ChatRoomSection/hooks/useDeleteChat.ts (1)
useDeleteChat(5-15)
🪛 GitHub Actions: CI
src/components/layout/SideNavigation/components/Bottom/SideNavigationBottom.tsx
[error] 14-14: TS2322: Type 'Element' is not assignable to type '(close: () => void) => ReactNode'.
🪛 GitHub Check: ci
src/components/layout/SideNavigation/components/Bottom/SideNavigationBottom.tsx
[failure] 14-14:
Type 'Element' is not assignable to type '(close: () => void) => ReactNode'.
🔇 Additional comments (13)
src/components/layout/SideNavigation/components/Header/SideNavigationHeader.tsx (1)
1-7: 인터페이스 정의가 명확합니다.Props 타입 정의가 간결하고 명확합니다.
src/components/layout/SideNavigation/components/MenuSection/MenuSection.tsx (1)
46-46: 모달 렌더링 로직이 적절합니다.모달 스토어의 타입에 따라 조건부 렌더링하는 패턴이 올바르게 구현되었습니다.
src/components/Icons/icons.ts (1)
20-20: IC_Logo 아이콘 추가가 올바르게 구현되었습니다.아이콘 import와 IconMap 추가가 기존 패턴을 따르고 있어 적절합니다.
Also applies to: 74-74
src/hooks/util/useOutsideClickHandler.tsx (1)
30-32: 'click' 이벤트 사용의 의도와 영향을 확인하세요.
useOutsideClick훅이'click'이벤트를 사용하는 것이 의도된 설계인지, 그리고 이것이 적절한지 재검토가 필요합니다:
- 'mousedown'과의 차이: 'click'은 마우스 버튼을 누르고 떼는 전체 사이클이 필요하지만, 'mousedown'은 누르는 즉시 발생합니다.
- Popover 동작 변화: 사용자가 Popover 외부에서 마우스를 누른 후 드래그하여 Popover 내부로 움직인 후 떼는 경우, 'click' 이벤트는 발생하지 않아 Popover가 닫히지 않습니다.
- 응답성 저하: 'mousedown' 대비 'click'은 감지 시점이 늦습니다.
이 설계 선택이 PopoverContent를 포함한 모든 사용처에서 의도된 동작인지, 그리고 관련 테스트가 있는지 확인하세요.
src/components/basics/Popover/PopoverProvider.tsx (1)
1-11: 변경사항 승인외부 클릭 핸들러 제거에 따른 import 변경이 적절합니다. PopoverContent에서 해당 로직을 처리하도록 책임이 이동된 것으로 보입니다.
src/components/basics/Popover/index.tsx (1)
1-5: LGTM!Popover 컴포넌트들의 통합된 public API를 제공하는 표준 barrel export 패턴입니다.
src/stores/modalStore.ts (1)
7-7: LGTM!채팅 삭제 모달 타입이 올바르게 추가되었습니다. 기존 패턴과 일관성이 유지됩니다.
src/apis/chatApi.ts (1)
9-16: LGTM!Authorization 헤더를 추가하는 withAuth 헬퍼 함수의 구현이 올바릅니다. 기존 헤더를 보존하면서 인증 정보를 병합합니다.
src/components/layout/SideNavigation/components/Bottom/SideNavigationBottom.tsx (1)
1-11: LGTM!Popover 구조와 NavItem 구성이 올바르게 설정되어 있습니다. 새로운 IC_Logo 아이콘이 적절히 사용되고 있습니다.
src/components/layout/SideNavigation/SideNavigation.tsx (1)
21-27: 컴포넌트 구조 리팩토링이 잘 되었습니다.사이드 네비게이션을 Header, MenuSection, ChatRoomSection, Bottom으로 분리하여 관심사 분리와 유지보수성이 향상되었습니다.
custom-scrollbar와overflow-y-auto를 통한 스크롤 처리도 적절합니다.src/components/basics/Popover/Content.tsx (1)
11-12: Render prop 패턴으로의 변경이 적절합니다.
children을 함수 타입(close: () => void) => React.ReactNode로 변경하여 팝오버 내부에서close함수에 직접 접근할 수 있게 되었습니다. 이는 유연성과 캡슐화를 향상시킵니다.Also applies to: 45-45
src/components/layout/SideNavigation/components/NavItem/LinkNavItem.tsx (1)
29-38: label prop 기반 렌더링으로의 변경이 일관성 있습니다.NavItem 컴포넌트와 동일한 패턴을 따르고 있어 코드베이스 전체의 일관성이 유지됩니다.
h-10!의 Tailwind important 수식자 사용은 스타일 충돌을 해결하기 위한 것으로 보이지만, 가능하다면 CSS 우선순위 문제의 근본 원인을 해결하는 것이 장기적으로 더 좋습니다.src/components/layout/SideNavigation/components/NavItem/NavItem.tsx (1)
13-56: forwardRef와 label prop 기반 리팩토링이 적절합니다.
forwardRef패턴을 통해 외부에서 ref 접근이 가능해졌고,children대신labelprop을 사용하여LinkNavItem과 일관된 API를 제공합니다.
Comment on lines
+18
to
+30
| export const deleteChat = async (id: number): Promise<DeleteChatApiResponse> => { | ||
| const body = await safeFetch<DeleteChatApiResponse>( | ||
| `${CHAT_ENDPOINT}/${id}`, | ||
| withAuth({ | ||
| method: 'DELETE', | ||
| }) | ||
| ); | ||
| if (!body || typeof body.success !== 'boolean' || !body.status || !body.message) { | ||
| throw new Error(body?.message ?? 'Invalid response'); | ||
| } | ||
|
|
||
| return body; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for DeleteChatApiResponse type definition
echo "=== DeleteChatApiResponse 타입 정의 ==="
rg -A 5 "type DeleteChatApiResponse|interface DeleteChatApiResponse" --type=ts
# Search for ApiResponseBase or similar base response types
echo -e "\n=== API 응답 기본 타입 정의 ==="
rg -B 2 -A 10 "interface ApiResponseBase|type ApiResponseBase|interface.*Response.*Base" --type=ts
# Search for other API response validations to see patterns
echo -e "\n=== 다른 API 응답 검증 패턴 ==="
rg -B 3 -A 3 "typeof.*\.success|\.status.*truthy" --type=ts
# Check the overall structure of chatApi.ts
echo -e "\n=== chatApi.ts 파일 구조 ==="
head -50 src/apis/chatApi.tsRepository: Team-SoFa/linkiving
Length of output: 2345
API 응답 검증 로직이 실제 API 스키마와 불일치
검증 로직이 ApiResponseBase<string> 인터페이스의 모든 필드를 검증하지 않고 있습니다.
ApiResponseBase정의:success(boolean),status(string),message(string),data(required),timestamp?(optional)- 현재 검증:
success(boolean 타입 확인),status(truthy 확인),message(truthy 확인)만 수행 - 누락된 검증:
data필드 존재 여부 확인 불가 (필수 필드) - 문제점:
status는 문자열 타입이므로 빈 문자열("")도 스키마 검증을 통과하게 됨
ApiResponseBase<string>의 모든 필드를 적절하게 검증하도록 수정 필요합니다.
🤖 Prompt for AI Agents
In @src/apis/chatApi.ts around lines 18 - 30, The deleteChat response validation
is incomplete: update the validation in deleteChat to ensure body is an object
matching ApiResponseBase<string> by checking success is a boolean, status is a
non-empty string, message is a non-empty string, and data is present (not
undefined/null) and of type string; also if timestamp exists ensure its type is
valid. Use the DeleteChatApiResponse/ApiResponseBase<string> symbols to guide
checks on the returned body and throw an Error with body?.message when
validation fails.
Comment on lines
+23
to
+25
| const anchorRef = useRef<HTMLElement | null>(null); | ||
|
|
||
| anchorRef.current = anchorEl ?? null; |
There was a problem hiding this comment.
렌더링 중 ref 직접 할당은 React 안티패턴입니다.
anchorRef.current = anchorEl ?? null을 렌더 페이즈에서 직접 할당하면 React의 동시성 기능(Concurrent Mode)에서 예기치 않은 동작이 발생할 수 있습니다.
🔎 useEffect로 ref 할당 이동 제안
const contentRef = useRef<HTMLDivElement | null>(null);
const anchorRef = useRef<HTMLElement | null>(null);
-anchorRef.current = anchorEl ?? null;
+useEffect(() => {
+ anchorRef.current = anchorEl ?? null;
+}, [anchorEl]);또는 anchorEl을 직접 usePopoverPosition과 useOutsideClick에 전달하는 방식도 고려해보세요.
🤖 Prompt for AI Agents
In @src/components/basics/Popover/Content.tsx around lines 23 - 25, The code
directly assigns anchorRef.current = anchorEl ?? null during render (using
anchorRef and anchorEl), which is an anti-pattern; move this assignment into a
useEffect that runs when anchorEl changes (set anchorRef.current inside
useEffect) or alternatively stop using anchorRef and pass anchorEl directly into
usePopoverPosition and useOutsideClick so both hooks receive the external
element without mutating refs during render.
|
|
||
| import NavItem from '../NavItem/NavItem'; | ||
|
|
||
| const SideNavigationBottom = ({ isOpen }: { isOpen: boolean }) => { |
There was a problem hiding this comment.
사용되지 않는 prop 제거
isOpen prop이 선언되었지만 컴포넌트 내부에서 전혀 사용되지 않습니다.
🔎 제안된 수정
-const SideNavigationBottom = ({ isOpen }: { isOpen: boolean }) => {
+const SideNavigationBottom = () => {🤖 Prompt for AI Agents
In
@src/components/layout/SideNavigation/components/Bottom/SideNavigationBottom.tsx
at line 6, The SideNavigationBottom component declares an unused prop isOpen;
remove isOpen from the component signature and its type annotation in
SideNavigationBottom, and update any parent/usage sites that pass isOpen to this
component so they no longer supply that prop (or stop forwarding it) to avoid
unused/TS errors.
Comment on lines
+13
to
+23
| <PopoverContent popoverKey="user"> | ||
| <Button | ||
| variant="tertiary_subtle" | ||
| contextStyle="onPanel" | ||
| label="로그아웃" | ||
| size="sm" | ||
| icon="IC_Logout" | ||
| radius="full" | ||
| className="m-3 pr-13" | ||
| /> | ||
| </PopoverContent> |
There was a problem hiding this comment.
CRITICAL: PopoverContent children 타입 불일치 수정 필요
파이프라인 실패 로그에 따르면, PopoverContent는 (close: () => void) => ReactNode 함수 타입의 children을 기대하지만 현재 Element를 전달하고 있습니다. AI 요약에서도 언급된 것처럼, PopoverContent는 이제 function-as-children 패턴을 사용하며 close 콜백을 children 함수에 전달합니다.
🔎 제안된 수정
- <PopoverContent popoverKey="user">
- <Button
- variant="tertiary_subtle"
- contextStyle="onPanel"
- label="로그아웃"
- size="sm"
- icon="IC_Logout"
- radius="full"
- className="m-3 pr-13"
- />
- </PopoverContent>
+ <PopoverContent popoverKey="user">
+ {(close) => (
+ <Button
+ variant="tertiary_subtle"
+ contextStyle="onPanel"
+ label="로그아웃"
+ size="sm"
+ icon="IC_Logout"
+ radius="full"
+ className="m-3 pr-13"
+ onClick={close}
+ />
+ )}
+ </PopoverContent>📝 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
| <PopoverContent popoverKey="user"> | |
| <Button | |
| variant="tertiary_subtle" | |
| contextStyle="onPanel" | |
| label="로그아웃" | |
| size="sm" | |
| icon="IC_Logout" | |
| radius="full" | |
| className="m-3 pr-13" | |
| /> | |
| </PopoverContent> | |
| <PopoverContent popoverKey="user"> | |
| {(close) => ( | |
| <Button | |
| variant="tertiary_subtle" | |
| contextStyle="onPanel" | |
| label="로그아웃" | |
| size="sm" | |
| icon="IC_Logout" | |
| radius="full" | |
| className="m-3 pr-13" | |
| onClick={close} | |
| /> | |
| )} | |
| </PopoverContent> |
🧰 Tools
🪛 GitHub Actions: CI
[error] 14-14: TS2322: Type 'Element' is not assignable to type '(close: () => void) => ReactNode'.
🪛 GitHub Check: ci
[failure] 14-14:
Type 'Element' is not assignable to type '(close: () => void) => ReactNode'.
🤖 Prompt for AI Agents
In
@src/components/layout/SideNavigation/components/Bottom/SideNavigationBottom.tsx
around lines 13 - 23, PopoverContent currently expects a function-as-children
that receives a close callback, but the code passes a Button element; change the
PopoverContent children to be a function (close: () => void) => ReactNode that
returns the Button and, if appropriate, hook the close callback into the
Button's onClick (or call close after performing logout). Update the JSX around
PopoverContent to use a children function and ensure the Button still has the
same props (variant, contextStyle, label, size, icon, radius, className) while
wiring in close.
Comment on lines
+20
to
+22
| const handleItemClick = () => { | ||
| router.push('chat/{id}'); | ||
| }; |
There was a problem hiding this comment.
심각한 버그: 라우트 경로에 템플릿 리터럴이 아닌 일반 문자열이 사용되었습니다.
'chat/{id}'는 일반 문자열이므로 id 값이 대체되지 않습니다. 모든 채팅 항목 클릭 시 /chat/{id} 문자 그대로 이동하게 됩니다.
🔎 템플릿 리터럴로 수정
const handleItemClick = () => {
- router.push('chat/{id}');
+ router.push(`/chat/${id}`);
};참고: 경로 앞에 /도 추가하여 절대 경로로 설정하는 것이 권장됩니다.
📝 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 handleItemClick = () => { | |
| router.push('chat/{id}'); | |
| }; | |
| const handleItemClick = () => { | |
| router.push(`/chat/${id}`); | |
| }; |
🤖 Prompt for AI Agents
In @src/components/layout/SideNavigation/components/ChatRoomSection/ChatItem.tsx
around lines 20 - 22, handleItemClick currently calls router.push with the
literal string 'chat/{id}', so the id value isn't interpolated; change the call
in the ChatItem component to use a template literal or string concatenation to
inject the actual id (e.g., `/chat/${id}`), ensuring you reference the local
id/props variable used in this component and keep the leading slash for an
absolute path when calling router.push from handleItemClick.
Comment on lines
+11
to
+14
| const handleDelete = () => { | ||
| deleteChatMutation.mutate(chatId); | ||
| close(); | ||
| }; |
There was a problem hiding this comment.
삭제 실패 시 모달이 먼저 닫히는 문제가 있습니다.
mutate()는 비동기이지만 close()가 즉시 호출되어, 삭제 실패 여부와 관계없이 모달이 닫힙니다. 사용자가 에러를 확인할 수 없습니다.
🔎 mutateAsync 또는 onSuccess 콜백 사용 제안
const handleDelete = () => {
- deleteChatMutation.mutate(chatId);
- close();
+ deleteChatMutation.mutate(chatId, {
+ onSuccess: () => close(),
+ onError: (error) => {
+ // 에러 처리 로직 (예: toast 알림)
+ console.error('채팅 삭제 실패:', error);
+ },
+ });
};📝 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); | |
| close(); | |
| }; | |
| const handleDelete = () => { | |
| deleteChatMutation.mutate(chatId, { | |
| onSuccess: () => close(), | |
| onError: (error) => { | |
| // 에러 처리 로직 (예: toast 알림) | |
| console.error('채팅 삭제 실패:', error); | |
| }, | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In
@src/components/layout/SideNavigation/components/ChatRoomSection/DeleteChatModal.tsx
around lines 11 - 14, handleDelete currently calls
deleteChatMutation.mutate(chatId) then immediately close(), causing the modal to
close before deletion completes; change this to await the async result (use
deleteChatMutation.mutateAsync(chatId) or move close() into the mutation's
onSuccess callback) so the modal only closes after successful deletion and
handle errors (onError or try/catch) to keep the modal open and surface the
error to the user.
Comment on lines
+26
to
+43
| <div key={item.id} className="relative h-10"> | ||
| {/* 열렸을 때 */} | ||
| <motion.div | ||
| initial={false} | ||
| animate={{ opacity: isOpen ? 1 : 0 }} | ||
| transition={{ duration: 0.2 }} | ||
| style={{ pointerEvents: isOpen ? 'auto' : 'none' }} | ||
| > | ||
| {item.item} | ||
| </motion.div> | ||
|
|
||
| {/* 닫혔을 때 */} | ||
| {!isOpen && ( | ||
| <motion.div transition={{ duration: 1 }} className="absolute inset-0"> | ||
| {!isOpen && item.item} | ||
| </motion.div> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
애니메이션 로직에 중복과 비효율이 있습니다.
현재 구현에는 다음 문제들이 있습니다:
-
중복 렌더링:
isOpen이 false일 때 동일한item.item이 두 번 렌더링됩니다 (30번 라인의 opacity 0인 요소와 38-42번 라인의 absolute positioned 요소). -
불필요한 조건문: 40번 라인의
!isOpen && item.item에서!isOpen체크는 이미 38번 라인에서 확인되었으므로 중복입니다. -
효과 없는 transition: 39번 라인의
transition={{ duration: 1 }}은animate속성이 없어 실제로 아무 효과가 없습니다.
🔎 제안하는 수정
{MENU_ITEMS.map(item => (
- <div key={item.id} className="relative h-10">
- {/* 열렸을 때 */}
- <motion.div
- initial={false}
- animate={{ opacity: isOpen ? 1 : 0 }}
- transition={{ duration: 0.2 }}
- style={{ pointerEvents: isOpen ? 'auto' : 'none' }}
- >
- {item.item}
- </motion.div>
-
- {/* 닫혔을 때 */}
- {!isOpen && (
- <motion.div transition={{ duration: 1 }} className="absolute inset-0">
- {!isOpen && item.item}
- </motion.div>
- )}
- </div>
+ <motion.div
+ key={item.id}
+ initial={false}
+ animate={{ opacity: isOpen ? 1 : 0 }}
+ transition={{ duration: 0.2 }}
+ style={{ pointerEvents: isOpen ? 'auto' : 'none' }}
+ className="h-10"
+ >
+ {item.item}
+ </motion.div>
))}📝 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 key={item.id} className="relative h-10"> | |
| {/* 열렸을 때 */} | |
| <motion.div | |
| initial={false} | |
| animate={{ opacity: isOpen ? 1 : 0 }} | |
| transition={{ duration: 0.2 }} | |
| style={{ pointerEvents: isOpen ? 'auto' : 'none' }} | |
| > | |
| {item.item} | |
| </motion.div> | |
| {/* 닫혔을 때 */} | |
| {!isOpen && ( | |
| <motion.div transition={{ duration: 1 }} className="absolute inset-0"> | |
| {!isOpen && item.item} | |
| </motion.div> | |
| )} | |
| </div> | |
| <motion.div | |
| key={item.id} | |
| initial={false} | |
| animate={{ opacity: isOpen ? 1 : 0 }} | |
| transition={{ duration: 0.2 }} | |
| style={{ pointerEvents: isOpen ? 'auto' : 'none' }} | |
| className="h-10" | |
| > | |
| {item.item} | |
| </motion.div> |
🤖 Prompt for AI Agents
In @src/components/layout/SideNavigation/components/MenuSection/MenuSection.tsx
around lines 26 - 43, The MenuSection renders item.item twice and uses a
transition without animate; replace the two motion.div blocks with a single
motion.div (the one currently using initial/animate/style) that controls both
open/closed visuals via animate or variants based on isOpen (e.g., animate={{
opacity: isOpen ? 1 : 0, x: isOpen ? 0 : -4 }} and a transition prop), remove
the separate absolute positioned block and the redundant !isOpen check, and keep
pointerEvents logic on that single motion.div so item.item is rendered once and
the transition actually animates.
Comment on lines
+18
to
+27
| // ref 병합 | ||
| const mergeRefs = (el: HTMLButtonElement | null) => { | ||
| [forwardedRef, blurRef].forEach(ref => { | ||
| if (typeof ref === 'function') { | ||
| ref(el); | ||
| } else if (ref) { | ||
| ref.current = el; | ||
| } | ||
| }); | ||
| }; |
There was a problem hiding this comment.
ref.current 할당 시 TypeScript 오류가 발생할 수 있습니다.
RefObject의 current 프로퍼티는 readonly이므로 Line 24의 ref.current = el 할당이 TypeScript에서 오류를 발생시킬 수 있습니다.
🔎 타입 안전한 mergeRefs 구현
const mergeRefs = (el: HTMLButtonElement | null) => {
[forwardedRef, blurRef].forEach(ref => {
if (typeof ref === 'function') {
ref(el);
- } else if (ref) {
- ref.current = el;
+ } else if (ref && 'current' in ref) {
+ (ref as React.MutableRefObject<HTMLButtonElement | null>).current = el;
}
});
};또는 @radix-ui/react-compose-refs나 @floating-ui/react의 useMergeRefs 같은 유틸리티 사용을 고려해보세요.
🤖 Prompt for AI Agents
In @src/components/layout/SideNavigation/components/NavItem/NavItem.tsx around
lines 18 - 27, The mergeRefs function assigns to ref.current which can be
readonly; update mergeRefs to set refs in a type-safe way by detecting object
refs and casting to React.MutableRefObject<HTMLButtonElement | null> (or using a
helper setRef) before assigning, e.g., treat forwardedRef and blurRef as
function refs or mutable refs and assign via (ref as
React.MutableRefObject<...>).current = el, or replace mergeRefs with a proven
utility like useMergeRefs from @radix-ui/react-compose-refs /
@floating-ui/react; ensure you import React types if you add the
MutableRefObject cast and preserve handling for function refs in mergeRefs.
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 설명