-
Notifications
You must be signed in to change notification settings - Fork 0
SideNavBottom 섹션 추가 #324
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: main
Are you sure you want to change the base?
SideNavBottom 섹션 추가 #324
Conversation
Walkthrough이 PR은 SideNavigation 레이아웃에 새로 작성된 SideNavBottom 컴포넌트를 추가하고 이를 SideNavigation에 렌더링하도록 변경합니다. SideNavBottom은 사용자 트리거를 가진 Popover를 사용해 로그아웃 버튼을 제공하는 하단 네비게이션을 구현합니다. 또한 아이콘 매핑에 IC_Logo SVG를 추가하고 IconMap에 IC_Account와 IC_Logo 항목 순서를 조정했습니다. Popover Trigger와 NavItem 컴포넌트들은 트리거/레프 병합 및 forwardRef 적용을 위해 타입과 레프 처리 로직이 확장되었습니다. Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/layout/SideNavigation/SideNavigation.tsx (1)
30-60: SideNavBottom의 레이아웃 구조를 수정해야 합니다.SideNavBottom 컴포넌트가
mt-auto클래스를 사용하여 하단에 배치되도록 의도했지만, 부모 div(30번 라인)에 flex 관련 클래스가 없어 제대로 작동하지 않습니다.mt-auto는 부모가flex flex-col구조일 때만 동작합니다.🔎 레이아웃 구조 수정 제안
- <div> + <div className="flex flex-col h-full"> {/* 사이드 메뉴 헤더 */} <div className="mb-10"> <SideNavHeaderIconButton isOpen={isOpen} onClick={toggle} /> </div> {/* 메뉴 아이템*/} <nav className="flex flex-col gap-4"> {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> ))} </nav> <SideNavBottom /> </div>
🤖 Fix all issues with AI Agents
In
@src/components/layout/SideNavigation/components/SideNavBottom/SideNavBottom.tsx:
- Around line 1-28: The SideNavigationBottom component currently renders an
empty div because the user/profile UI is commented out; uncomment the block in
SideNavigationBottom and restore the imports for Button, Popover
(PopoverContent/PopoverTrigger) and NavItem, then fix the NavItem prop mismatch
by updating NavItem usage or types: add a `label` prop to the NavItemProps
interface (or adjust the JSX to use the supported props like `icon` and
`ariaLabel` and render the user name elsewhere) so that NavItem accepts/display
the user name without TypeScript errors; also ensure the logout Button props
(variant/contextStyle/label/size/icon/radius/className) match the Button
component's prop types and wire up the actual logout handler if required.
🧹 Nitpick comments (1)
src/components/Icons/icons.ts (1)
74-74: IconMap 항목의 알파벳 순서를 맞춰주세요.IconMap에 IC_Logo가 마지막에 추가되었지만, 다른 항목들은 알파벳 순서로 정렬되어 있습니다. IC_Logo는 IC_LinkOpen과 IC_Logout 사이(58번 라인)에 위치하는 것이 일관성 있는 코드 구조입니다.
🔎 알파벳 순서 정렬 제안
IC_LinkOpen, + IC_Logo, IC_Logout, IC_MoreHori, IC_MoreVert, IC_Regenerate, IC_SendFilled, IC_SendOutline, IC_SidenavOpen, IC_Stop, IC_SumGenerate, IC_ThumbDownFilled, IC_ThumbDownOutline, IC_ThumbUpFilled, IC_ThumbUpOutline, IC_Undo, IC_Up, IC_Warning, - IC_Logo, } as const;
📜 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 (3)
src/components/Icons/icons.tssrc/components/layout/SideNavigation/SideNavigation.tsxsrc/components/layout/SideNavigation/components/SideNavBottom/SideNavBottom.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 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/Icons/icons.tssrc/components/layout/SideNavigation/components/SideNavBottom/SideNavBottom.tsxsrc/components/layout/SideNavigation/SideNavigation.tsx
🧬 Code graph analysis (1)
src/components/Icons/icons.ts (2)
src/components/Icons/SVGIcon.tsx (2)
SVGIconProps(5-11)props(19-37)src/components/layout/SideNavigation/components/NavItem/NavItem.tsx (1)
NavItemProps(7-10)
🔇 Additional comments (1)
src/components/Icons/icons.ts (1)
20-20: 아이콘 import가 올바르게 추가되었습니다.IC_Logo 아이콘이 알파벳 순서에 맞게 정확히 추가되었습니다.
src/components/layout/SideNavigation/components/SideNavBottom/SideNavBottom.tsx
Outdated
Show resolved
Hide resolved
3c6f10e to
0f9c28f
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/components/Icons/icons.ts (1)
21-21: IC_Logo 위치가 알파벳 순서와 맞지 않습니다.
IC_Logoimport는 알파벳 순서(line 21)로 추가되었지만,IconMap에서는 맨 끝(line 76)에 추가되어 일관성이 없습니다.IC_Logout앞에 배치하는 것이 좋겠습니다.♻️ 제안된 수정
IC_LinkAdd, IC_LinkOpen, + IC_Logo, IC_Logout, IC_MoreHori, ... IC_Up, IC_Warning, - IC_Logo, } as const;Also applies to: 76-76
src/components/basics/Popover/Trigger.tsx (1)
39-51:mergeRefs유틸리티 함수 중복 발견.
NavItem.tsx에서도 동일한 ref 병합 로직이 구현되어 있습니다. 공통 유틸리티 함수로 추출하면 중복을 줄이고 일관성을 유지할 수 있습니다.// hooks/util/mergeRefs.ts 또는 유사한 위치에 추출 가능 export const mergeRefs = <T>( ...refs: Array<React.Ref<T> | undefined> ): React.RefCallback<T> => { return element => { refs.forEach(ref => { if (typeof ref === 'function') { ref(element); } else if (ref) { (ref as React.MutableRefObject<T | null>).current = element; } }); }; };src/components/layout/SideNavigation/components/NavItem/NavItem.tsx (2)
56-56: 불필요한 non-null assertion.
ariaLabel은NavItemProps에서 필수 prop으로 정의되어 있어!연산자가 필요하지 않습니다.♻️ 제안된 수정
- ariaLabel={ariaLabel!} + ariaLabel={ariaLabel}
18-33: ref 병합 로직이Trigger.tsx와 중복됩니다.앞서
Trigger.tsx에서 언급한 대로, 이mergedRef로직을 공통 유틸리티로 추출하면 코드 중복을 줄일 수 있습니다. 예:hooks/util/mergeRefs.ts
📜 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 (5)
src/components/Icons/icons.tssrc/components/basics/Popover/Trigger.tsxsrc/components/layout/SideNavigation/SideNavigation.tsxsrc/components/layout/SideNavigation/components/NavItem/NavItem.tsxsrc/components/layout/SideNavigation/components/SideNavBottom/SideNavBottom.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/layout/SideNavigation/components/SideNavBottom/SideNavBottom.tsx
- src/components/layout/SideNavigation/SideNavigation.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
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(79-79)src/stores/sideNavStore.ts (1)
useSideNavStore(9-13)src/hooks/util/useBlurOnClick.ts (1)
useBlurOnClick(3-14)
🔇 Additional comments (2)
src/components/basics/Popover/Trigger.tsx (1)
53-62: LGTM!cloneElement 구현이 잘 되어 있습니다:
- ref 병합으로 내부/외부 ref 모두 지원
- 자식의 onClick을 먼저 호출하여 기존 동작 보존
- ARIA 속성으로 접근성 확보
src/components/layout/SideNavigation/components/NavItem/NavItem.tsx (1)
13-14: LGTM!
forwardRef패턴으로의 전환이 잘 되어 있습니다.PopoverTrigger와의 통합을 위해 외부 ref를 지원하면서, 내부useBlurOnClickref도 함께 동작합니다.displayName설정도 적절합니다.Also applies to: 42-60


관련 이슈
PR 설명