Conversation
Walkthrough레이아웃에 Sidebar를 통합하고 사이드바 관련 컴포넌트·훅·포털·유틸을 대거 추가했으며 디자인 시스템 아이콘 목록과 클라이언트 의존성, Popup 루트 클래스 일부를 변경했다. (약간의 import 경로 조정 포함) Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as 사용자
participant L as Layout
participant S as Sidebar
participant NS as useSidebarNav
participant AM as useAnchoredMenu
participant CP as useCategoryPopups
participant OP as OptionsMenuPortal
participant PP as PopupPortal
participant R as Router
U->>L: 페이지 방문
L->>S: Sidebar 렌더
S->>NS: 탭/카테고리 상태 조회
U->>S: 항목 클릭
S->>NS: goRemind/goBookmarks/selectCategory/goLevel
NS->>R: navigate(path)
U->>S: 카테고리 옵션(⋯) 클릭
S->>AM: open(categoryId, anchorEl)
AM-->>OP: {open, style, containerRef}
OP-->>U: 옵션 메뉴 표시
U->>OP: 편집/삭제 선택
OP->>CP: openEdit/openDelete
CP-->>PP: popup 상태
PP-->>U: 팝업 표시
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks (2 passed, 3 warnings)❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
Poem
✨ Finishing Touches
🧪 Generate unit tests
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 |
|
✅ Storybook chromatic 배포 확인: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (10)
apps/client/package.json (1)
13-13:apps/client/package.json에서class-variance-authority삭제
apps/client 코드에서 직접 사용되지 않으며, design-system 패키지가 이미 의존하고 있으므로 불필요합니다. 필요 시 직접 사용하는 시점에 devDependencies로 추가하세요.apps/client/src/layout/Layout.tsx (1)
9-10: 레이아웃 래핑 제안: 사이드바/콘텐츠를 flex로 분리현재는 수직 스택으로 렌더링되어 콘텐츠가 아래로 밀릴 수 있습니다. 다음처럼
aside/main구조로 감싸면 레이아웃이 명확합니다.- <> - {/* TODO: 필요시 레이아웃 추가 */} - {/* TODO: 사이드바 추가 */} - <Sidebar /> - <Outlet /> - </> + <div className="flex"> + <aside aria-label="사이드바 내비게이션"> + <Sidebar /> + </aside> + <main className="flex-1"> + <Outlet /> + </main> + </div>apps/client/src/shared/components/sidebar/Sidebar.tsx (3)
7-7: nav에 접근성 레이블 추가 권장여러 내비게이션 영역이 있을 수 있으므로
aria-label을 추가해 스크린 리더 구분을 돕는 것이 좋습니다.- <nav className="w-[22.4rem] space-y-2"> + <nav className="w-[22.4rem] space-y-2" aria-label="사이드바 내비게이션">
22-51: 색 토큰 일관성 제안
ul은bg-white-bg인데 항목 hover는hover:bg-white로 지정되어 있습니다. 토큰을 통일하면 테마 교체 시 안정적입니다. (예:hover:bg-white-bg로 통일)- <li className="h-[3.6rem] rounded px-3 py-2 hover:bg-white"> + <li className="h-[3.6rem] rounded px-3 py-2 hover:bg-white-bg">
22-56: 하드코딩된 항목을 데이터 드리븐 렌더링으로 변경 제안향후 카테고리 동적 로딩을 고려하면 배열
.map으로 렌더링하는 편이 유지보수에 유리합니다.예시:
const categories = ['카테고리 1', '카테고리 2', '카테고리 3']; <ul className="bg-white-bg space-y-1"> {categories.map((c) => ( <li key={c} className="h-[3.6rem] rounded px-3 py-2 hover:bg-white-bg"> {c} </li> ))} <li> <button className="text-main500 rounded px-3 py-2 hover:bg-white"> + 카테고리 추가 </button> </li> </ul>apps/client/src/shared/components/sidebar/AccordionItem.tsx (2)
34-38: 토글 로직의 안전성 개선 (제어/비제어 모두 고려)현재
isOpen캡처 값에 의존합니다. 제어 모드에서도 항상 최신open값 기준으로 토글되도록 정교화하면 안전합니다.- const toggle = () => { - const next = !isOpen; - onOpenChange?.(next); - if (open === undefined) setInternalOpen(next); - }; + const toggle = () => { + const basis = open ?? internalOpen; + const next = !basis; + onOpenChange?.(next); + if (open === undefined) setInternalOpen(next); + };
41-49: SideItem API로 토글 클릭만 제공 — 헤더 전체 클릭 토글 옵션 검토현재 토글은 트레일링 버튼에 한정됩니다. 사이드바 UX에 따라 헤더 전체 클릭으로 열림/닫힘을 허용하는 옵션(
toggleOnHeaderClick)을 두는 것도 고려해 볼 만합니다.apps/client/src/shared/components/sidebar/SideItem.tsx (3)
1-1: React 네임스페이스 타입 의존 최소화 (사소한 개선)전역 React 네임스페이스에 의존하지 않도록 MouseEvent를 타입 임포트해 사용하세요.
+import type { MouseEvent } from 'react'; ... - onTrailingClick?: (e: React.MouseEvent<HTMLButtonElement>) => void; + onTrailingClick?: (e: MouseEvent<HTMLButtonElement>) => void;Also applies to: 18-18
33-41: active 시 내비게이션 문맥의 접근성 힌트 제공 고려내비게이션 항목으로 사용된다면 현재 위치를 스크린 리더에 알리기 위해 aria-current를 고려해 주세요. (링크로 감싸는 상위 요소가 있다면 해당 요소에 적용 권장)
- <div + <div + aria-current={active ? 'page' : undefined} className={cn(이 컴포넌트가 언제/어디서 링크로 감싸지는지 확인 부탁드립니다. 링크로 감싸진다면 aria-current는 링크 요소로 이동하는 것이 더 적절합니다.
4-9: satisfies 키워드로 IconToken/ICON_MAP 타입 안전성 강화 권장프로젝트의 TypeScript 버전(5.9.2)에서
satisfies사용이 가능하므로, ICON_MAP을const ICON_MAP = { clock: { on: 'ic_clock_active', off: 'ic_clock_disable' }, bookmark: { on: 'ic_bookmark_active', off: 'ic_bookmark_disable' }, } as const satisfies Record<string, { on: IconName; off: IconName }>; export type IconToken = keyof typeof ICON_MAP;패턴으로 선언하여 중복된 리터럴 타입 선언을 제거하고 타입 호환성을 강화하세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
apps/client/public/vite.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
apps/client/package.json(1 hunks)apps/client/src/layout/Layout.tsx(1 hunks)apps/client/src/shared/components/sidebar/AccordionItem.tsx(1 hunks)apps/client/src/shared/components/sidebar/SideItem.tsx(1 hunks)apps/client/src/shared/components/sidebar/Sidebar.tsx(1 hunks)packages/design-system/src/icons/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
apps/client/src/shared/components/sidebar/AccordionItem.tsx (2)
apps/client/src/shared/components/sidebar/SideItem.tsx (2)
IconToken(4-4)SideItem(21-66)packages/design-system/src/lib/utils.ts (1)
cn(108-110)
apps/client/src/layout/Layout.tsx (1)
apps/client/src/shared/components/sidebar/Sidebar.tsx (1)
Sidebar(5-62)
apps/client/src/shared/components/sidebar/Sidebar.tsx (2)
apps/client/src/shared/components/sidebar/SideItem.tsx (1)
SideItem(21-66)apps/client/src/shared/components/sidebar/AccordionItem.tsx (1)
AccordionItem(18-64)
apps/client/src/shared/components/sidebar/SideItem.tsx (1)
packages/design-system/src/lib/utils.ts (1)
cn(108-110)
🔇 Additional comments (2)
packages/design-system/src/icons/index.ts (2)
2-2: 타입 재노출 LGTM
export type를 사용해 번들 영향 없이 타입만 노출하는 방식이 적절합니다.
2-2: 검증 완료:./iconNames모듈이IconName타입을 정상적으로 export합니다
| <div | ||
| id={panelId} | ||
| className={cn( | ||
| 'grid overflow-hidden transition-[grid-template-rows]', | ||
| isOpen ? 'grid-rows-[1fr]' : 'grid-rows-[0fr]' | ||
| )} | ||
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
접근성: 패널에 role/aria 속성 부여 (키보드 포커스/리더 배려)
접힘 상태에서도 포커스 가능 요소가 존재할 수 있으므로 aria-hidden으로 힌트를 주고, role="region"/aria-label을 부여해 의미를 명확히 합니다. 추후 트리거 버튼에 aria-controls/aria-expanded 연동을 추가하면 더 좋습니다.
<div
id={panelId}
- className={cn(
+ role="region"
+ aria-hidden={!isOpen}
+ aria-label={`${label} 패널`}
+ className={cn(
'grid overflow-hidden transition-[grid-template-rows]',
isOpen ? 'grid-rows-[1fr]' : 'grid-rows-[0fr]'
)}
>추가 제안: SideItem의 트레일링 버튼에 접근성 속성을 전달할 수 있도록 trailingButtonProps?: React.ButtonHTMLAttributes<HTMLButtonElement>를 수용하고, 여기서 aria-controls={panelId}, aria-expanded={isOpen}를 전달하면 완성됩니다. 원하시면 패치 제안 드리겠습니다.
📝 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.
| <div | |
| id={panelId} | |
| className={cn( | |
| 'grid overflow-hidden transition-[grid-template-rows]', | |
| isOpen ? 'grid-rows-[1fr]' : 'grid-rows-[0fr]' | |
| )} | |
| > | |
| <div | |
| id={panelId} | |
| role="region" | |
| aria-hidden={!isOpen} | |
| aria-label={`${label} 패널`} | |
| className={cn( | |
| 'grid overflow-hidden transition-[grid-template-rows]', | |
| isOpen ? 'grid-rows-[1fr]' : 'grid-rows-[0fr]' | |
| )} | |
| > |
🤖 Prompt for AI Agents
In apps/client/src/shared/components/sidebar/AccordionItem.tsx around lines 51
to 57, the panel markup lacks accessibility semantics and state linkage: add
role="region", a descriptive aria-label (or aria-labelledby if a title element
exists), and toggle aria-hidden based on isOpen so screenreaders know when
content is hidden; additionally update the SideItem API to accept
trailingButtonProps?: React.ButtonHTMLAttributes<HTMLButtonElement> and pass
aria-controls={panelId} and aria-expanded={isOpen} from the trigger button to
link control and panel (ensure panelId is stable and used on the panel element).
| {trailing && ( | ||
| <button | ||
| type="button" | ||
| className="rounded-[0.4rem] p-1 focus-visible:ring-2" | ||
| onClick={(e) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| onTrailingClick?.(e); | ||
| }} | ||
| > | ||
| <Icon | ||
| name="ic_arrow_down_active" | ||
| aria-hidden | ||
| className={cn( | ||
| 'h-[2rem] w-[2rem] transition-transform', | ||
| !open && 'rotate-180' | ||
| )} | ||
| /> | ||
| </button> | ||
| )} |
There was a problem hiding this comment.
접근성: 토글 버튼에 접근 가능한 이름과 상태 표기가 없습니다.
자식 Icon에 aria-hidden이 있어 버튼의 접근 가능한 이름이 비어 있습니다. 또한 확장/축소 상태를 나타내는 aria-expanded가 없습니다. 스크린 리더 사용성을 위해 추가해 주세요.
다음 패치를 적용하십시오:
<button
type="button"
+ aria-label={open ? '섹션 접기' : '섹션 펼치기'}
+ aria-expanded={open ?? undefined}
className="rounded-[0.4rem] p-1 focus-visible:ring-2"📝 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.
| {trailing && ( | |
| <button | |
| type="button" | |
| className="rounded-[0.4rem] p-1 focus-visible:ring-2" | |
| onClick={(e) => { | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| onTrailingClick?.(e); | |
| }} | |
| > | |
| <Icon | |
| name="ic_arrow_down_active" | |
| aria-hidden | |
| className={cn( | |
| 'h-[2rem] w-[2rem] transition-transform', | |
| !open && 'rotate-180' | |
| )} | |
| /> | |
| </button> | |
| )} | |
| {trailing && ( | |
| <button | |
| type="button" | |
| aria-label={open ? '섹션 접기' : '섹션 펼치기'} | |
| aria-expanded={open ?? undefined} | |
| className="rounded-[0.4rem] p-1 focus-visible:ring-2" | |
| onClick={(e) => { | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| onTrailingClick?.(e); | |
| }} | |
| > | |
| <Icon | |
| name="ic_arrow_down_active" | |
| aria-hidden | |
| className={cn( | |
| 'h-[2rem] w-[2rem] transition-transform', | |
| !open && 'rotate-180' | |
| )} | |
| /> | |
| </button> | |
| )} |
🤖 Prompt for AI Agents
In apps/client/src/shared/components/sidebar/SideItem.tsx around lines 44 to 63,
the trailing toggle button lacks an accessible name and state—its child Icon is
aria-hidden leaving the button without a label and there's no aria-expanded. Add
an accessible name to the button (e.g., aria-label or aria-labelledby) that
reflects the action and/or target (preferably dynamic like `${open ? 'Collapse'
: 'Expand'} sidebar item`), set aria-expanded={open} on the button, and keep the
Icon aria-hidden so screen readers read the button label; preserve existing
event handling and class logic.
jjangminii
changed the title
jllee000
left a comment
jllee000
left a comment
There was a problem hiding this comment.
고생하셨습니다! 페이지 랜딩 지점도 고민해서 ui 짜신게 느껴지네요! 카테고리 리스트 하드코딩 부분만 맵핑 미리해두면 좋을 것 같습니당! 아이콘 export 관련한 부분도 한번 더 좋은 방향 고민해보겠습니다!
| className?: string; | ||
| trailing?: boolean; | ||
| open?: boolean; | ||
| onTrailingClick?: (e: React.MouseEvent<HTMLButtonElement>) => void; |
There was a problem hiding this comment.
just 저의 이해력 이슈로 질문,,
trailing 해당 요소는 어떤 기능으로 쓰이는 걸까료?
There was a problem hiding this comment.
trailing은 사이드 아이템에서는 필요없지만 아코디언에서 드롭다운 아이콘 유/무를 위해서 해뒀습니다-!
| > | ||
| <ul className="bg-white-bg space-y-1"> | ||
| <li className="h-[3.6rem]rounded h-[3.6rem] px-3 py-2 hover:bg-white"> |
There was a problem hiding this comment.
요 하드코딩 사항은 api 붙이기전 미리 맵핑으로 정리해두면 좋을 것 같아용!
(카테고리 idx 1,2,3 임의로)
There was a problem hiding this comment.
이 부분은 생각하지 못했는데 미리 해두면 좋을것같네요-! 좋은 의견 감사합니다
| "dependencies": { | ||
| "class-variance-authority": "^0.7.1", | ||
| "react": "^19.1.1", |
There was a problem hiding this comment.
cva client에서 의존성 설치가 안되어있어서 추가했습니다-! 이부분 혹시 따로 디자인시스템에서 가져와야한는 것일까요?
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
apps/client/src/shared/components/sidebar/CreateItem.tsx (2)
1-3: onClick 타입 일반화 및 확장성: ButtonHTMLAttributes 기반으로 변경 권장소비자가 aria, disabled, onKeyDown 등 표준 button 속성을 자연스럽게 전달할 수 있도록 props를 React의 ButtonHTMLAttributes로 확장하세요. className 병합을 위해 시그니처도 조정합니다.
import { Icon } from '@pinback/design-system/icons'; import { cn } from '@pinback/design-system/utils'; +import type { ButtonHTMLAttributes } from 'react'; -interface CreateItemProps { - onClick?: () => void; -} +interface CreateItemProps extends ButtonHTMLAttributes<HTMLButtonElement> {} -export default function CreateItem({ onClick }: CreateItemProps) { +export default function CreateItem({ className, ...props }: CreateItemProps) {참고: 위 버튼 전환 diff 적용 시, 버튼 엘리먼트에 다음과 같이 반영하세요.
- className={cn('...')} - onClick={onClick} - disabled={!onClick} + {...props} + className={cn('...', className)} + disabled={props.disabled ?? !props.onClick}Also applies to: 4-6, 9-9
7-7: 불필요한 TODO 정리이미 onClick이 연결되어 있으므로 혼란을 주는 TODO는 제거하세요.
-//TODO: onClick 이벤트 추가apps/client/src/shared/components/sidebar/CategoryItem.tsx (2)
4-8: Props 정합성: active는 선택적으로, onClick 노출현재 active가 필수지만 기본값을 제공하고 있습니다. 선택적으로 두고 루트 버튼 onClick을 외부로 전달하세요.
@@ -interface CategoryItemProps { - label: string; - active: boolean; - className?: string; -} +export interface CategoryItemProps { + label: string; + active?: boolean; + className?: string; + onClick?: () => void; + onIconClick?: () => void; // 아이콘이 별도 액션일 때만 사용 +} @@ -export default function CategoryItem({ - label, - active = false, - className, -}: CategoryItemProps) { +export default function CategoryItem({ + label, + active = false, + className, + onClick, + onIconClick, +}: CategoryItemProps) {Also applies to: 12-16
36-39: 아이콘 이름 타입 안정성 강화(사소)디자인 시스템이 IconName을 재노출한다면, 아이콘 이름을 IconName으로 한 번 단언해 두면 오타를 컴파일타임에 차단할 수 있습니다.
예:
import type { IconName } from '@pinback/design-system/icons'; // ... const iconName: IconName = active ? 'ic_details_category' : 'ic_details_disable'; <Icon name={iconName} aria-hidden className={cn('h-[1.8rem] w-[1.8rem]')} />
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/client/src/shared/components/sidebar/AccordionItem.tsx(1 hunks)apps/client/src/shared/components/sidebar/CategoryItem.tsx(1 hunks)apps/client/src/shared/components/sidebar/CreateItem.tsx(1 hunks)apps/client/src/shared/components/sidebar/Sidebar.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/client/src/shared/components/sidebar/AccordionItem.tsx
- apps/client/src/shared/components/sidebar/Sidebar.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-03T08:57:48.602Z
Learnt from: jjangminii
PR: Pinback-Team/pinback-client#52
File: apps/client/src/shared/components/sidebar/SideItem.tsx:57-60
Timestamp: 2025-09-03T08:57:48.602Z
Learning: SideItem 컴포넌트에서 아이콘 디자인이 일반적인 기대와 반대여서, 회전 로직도 반대로 구현되었습니다. 이를 명확히 하기 위해 prop 이름을 open에서 close로 변경하여 실제 동작과 일치시켰습니다.
Applied to files:
apps/client/src/shared/components/sidebar/CategoryItem.tsx
🧬 Code graph analysis (2)
apps/client/src/shared/components/sidebar/CategoryItem.tsx (1)
packages/design-system/src/lib/utils.ts (1)
cn(108-110)
apps/client/src/shared/components/sidebar/CreateItem.tsx (1)
packages/design-system/src/lib/utils.ts (1)
cn(108-110)
🔇 Additional comments (3)
apps/client/src/shared/components/sidebar/CreateItem.tsx (1)
20-24: 아이콘 접근성 설정 LGTM장식적 아이콘에
aria-hidden을 부여한 점 좋습니다.apps/client/src/shared/components/sidebar/CategoryItem.tsx (2)
21-22: aria-pressed 사용 목적 확인 필요이 항목이 “토글 버튼”이 아니라 “현재 위치/선택 표시”라면 aria-pressed 대신 아래가 더 적합합니다:
- 내비게이션 링크: aria-current="page"
- 선택 리스트: aria-selected
확인 후 목적에 맞게 교체해 주세요.
23-27: LGTM: 상태별 스타일 처리 깔끔합니다cn과 data-state/transition-colors 조합이 명확합니다.
| <div | ||
| role="button" | ||
| className={cn( | ||
| 'bg-white-bg flex h-[3.6rem] w-full gap-[0.8rem] rounded-[0.4rem] p-[0.8rem]', | ||
| 'items-center transition-colors' | ||
| )} | ||
| onClick={onClick} | ||
| > | ||
| <button type="button"> | ||
| <Icon | ||
| name={'ic_plus'} | ||
| aria-hidden | ||
| className={cn('h-[1.6rem] w-[1.6rem]')} | ||
| /> | ||
| </button> | ||
| <p className={'body4-r text-main500'}>카테고리 추가</p> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
상호작용 요소 중첩 + 키보드 접근성 문제: div[role=button] → 단일 button으로 전환하세요
- 현재 전체 행은 마우스로는 클릭 가능하나, 키보드 포커스는 내부 button에만 잡혀 UX가 불일치합니다.
- div[role="button"]에는 tabIndex 및 Space/Enter 처리(onKeyDown)가 없어 a11y 결함입니다.
- 상호작용 요소(내부 button)를 또 다른 상호작용 컨테이너(div[role="button"] ) 안에 중첩하는 것은 안티패턴입니다.
아래처럼 외곽 컨테이너를 button으로 바꾸고 내부 button을 제거해 전체 영역을 일관된 클릭/포커스 타겟으로 만드세요. onClick이 없을 땐 disabled 처리도 함께 권장합니다.
- <div
- role="button"
- className={cn(
- 'bg-white-bg flex h-[3.6rem] w-full gap-[0.8rem] rounded-[0.4rem] p-[0.8rem]',
- 'items-center transition-colors'
- )}
- onClick={onClick}
- >
- <button type="button">
- <Icon
- name={'ic_plus'}
- aria-hidden
- className={cn('h-[1.6rem] w-[1.6rem]')}
- />
- </button>
- <p className={'body4-r text-main500'}>카테고리 추가</p>
- </div>
+ <button
+ type="button"
+ className={cn(
+ 'bg-white-bg flex h-[3.6rem] w-full gap-[0.8rem] rounded-[0.4rem] p-[0.8rem]',
+ 'items-center transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-main400',
+ 'disabled:cursor-not-allowed disabled:opacity-50'
+ )}
+ onClick={onClick}
+ disabled={!onClick}
+ >
+ <Icon
+ name={'ic_plus'}
+ aria-hidden
+ className={cn('h-[1.6rem] w-[1.6rem]')}
+ />
+ <p className={'body4-r text-main500'}>카테고리 추가</p>
+ </button>📝 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.
| <div | |
| role="button" | |
| className={cn( | |
| 'bg-white-bg flex h-[3.6rem] w-full gap-[0.8rem] rounded-[0.4rem] p-[0.8rem]', | |
| 'items-center transition-colors' | |
| )} | |
| onClick={onClick} | |
| > | |
| <button type="button"> | |
| <Icon | |
| name={'ic_plus'} | |
| aria-hidden | |
| className={cn('h-[1.6rem] w-[1.6rem]')} | |
| /> | |
| </button> | |
| <p className={'body4-r text-main500'}>카테고리 추가</p> | |
| </div> | |
| <button | |
| type="button" | |
| className={cn( | |
| 'bg-white-bg flex h-[3.6rem] w-full gap-[0.8rem] rounded-[0.4rem] p-[0.8rem]', | |
| 'items-center transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-main400', | |
| 'disabled:cursor-not-allowed disabled:opacity-50' | |
| )} | |
| onClick={onClick} | |
| disabled={!onClick} | |
| > | |
| <Icon | |
| name={'ic_plus'} | |
| aria-hidden | |
| className={cn('h-[1.6rem] w-[1.6rem]')} | |
| /> | |
| <p className={'body4-r text-main500'}>카테고리 추가</p> | |
| </button> |
🤖 Prompt for AI Agents
In apps/client/src/shared/components/sidebar/CreateItem.tsx around lines 11 to
27, replace the outer div[role="button"] and the inner nested button with a
single semantic button element that holds the existing className, onClick,
type="button", and children (Icon and text); remove the inner button wrapper,
ensure the button supports keyboard focus by default, and if onClick is
undefined pass disabled to the button and adjust styling accordingly; keep Icon
aria-hidden and existing classNames and preserve the p text node (or convert it
to a span) inside the button for a consistent, accessible clickable area.
There was a problem hiding this comment.
의미상으로도 최상위가 button태그인 것이 맞을 것 같아요!
There was a problem hiding this comment.
@constantly-dev 맞습니다! 시멘틱 HTML 관점에서도 상호작용 가능한 요소는 적절한 HTML 요소(<button>)를 사용하는 것이 가장 좋은 접근 방식이죠. 접근성과 의미론적 정확성을 모두 만족하는 해결책에 동의해 주셔서 감사합니다! 🎯
🧠 Learnings used
Learnt from: constantly-dev
PR: Pinback-Team/pinback-client#2
File: pnpm-workspace.yaml:3-3
Timestamp: 2025-08-18T13:48:59.065Z
Learning: constantly-dev는 docs 디렉터리를 컨벤션 문서 추가용으로 사용할 예정이라고 명시했습니다.
into feat/#48/SidebarRow-component
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/client/src/shared/components/sidebar/CreateItem.tsx (1)
11-27: 상호작용 요소 중첩 및 키보드 접근성 결함: div[role=button] 대신 단일 button으로 정리하세요현재 div[role="button"] 안에 button이 중첩되어 있고, 키보드 포커스/Space/Enter 동작이 불완전합니다. 행 전체를 하나의 button으로 바꾸면 포커스, 활성화, 스크린리더 동작이 일관됩니다.
- <div - role="button" - className={cn( - 'bg-white-bg flex h-[3.6rem] w-full gap-[0.8rem] rounded-[0.4rem] p-[0.8rem]', - 'items-center transition-colors' - )} - onClick={onClick} - > - <button type="button"> - <Icon - name={'ic_plus'} - aria-hidden - className={cn('h-[1.6rem] w-[1.6rem]')} - /> - </button> - <p className={'body4-r text-main500'}>카테고리 추가</p> - </div> + <button + type="button" + className={cn( + 'bg-white-bg flex h-[3.6rem] w-full gap-[0.8rem] rounded-[0.4rem] p-[0.8rem]', + 'items-center transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-main400' + )} + onClick={onClick} + > + <Icon + name={'ic_plus'} + aria-hidden + className={cn('h-[1.6rem] w-[1.6rem]')} + /> + <p className={'body4-r text-main500'}>카테고리 추가</p> + </button>apps/client/src/shared/components/sidebar/CategoryItem.tsx (1)
21-26: A11Y: focusable div + nested button → 단일 의미 있는 으로 통합하세요현재 루트에 role="button"/tabIndex만 있고 클릭·키보드 핸들러는 내부 버튼에만 있어 탭 정지점이 2개로 늘고 키보드 조작이 깨집니다. 루트를 실제
으로 바꾸고, 우측 아이콘은 장식 요소로 처리하세요. 또한 내부에는 문단() 대신 문장()을 사용해야 합니다.
아래 diff로 한번에 정리 가능합니다.
- <div - role="button" - tabIndex={0} - aria-pressed={active} - data-state={active ? 'active' : 'default'} - className={cn( + <button + type="button" + aria-pressed={active} + data-state={active ? 'active' : 'default'} + onClick={onClick} + className={cn( 'flex h-[3.6rem] w-full rounded-[0.4rem] p-[0.8rem]', 'items-center justify-between transition-colors', active ? 'bg-main0' : 'bg-white-bg', className )} - > - <p - className={cn('body4-r', active ? 'text-main600' : 'text-font-gray-2')} - > - {label} - </p> - <button type="button" onClick={onClick}> - <Icon - name={active ? 'ic_details_category' : 'ic_details_disable'} - aria-hidden - className={cn('h-[1.8rem] w-[1.8rem]')} - /> - </button> - </div> + > + <span + className={cn('body4-r', active ? 'text-main600' : 'text-font-gray-2')} + > + {label} + </span> + <span aria-hidden> + <Icon + name={active ? 'ic_details_category' : 'ic_details_disable'} + aria-hidden + className={cn('h-[1.8rem] w-[1.8rem] pointer-events-none')} + /> + </span> + </button>Also applies to: 33-37, 38-45, 45-45
🧹 Nitpick comments (8)
apps/client/src/shared/components/sidebar/CreateItem.tsx (2)
7-7: 불필요한 TODO 제거이미 onClick이 연결되어 있으므로 혼동을 줄이기 위해 주석을 삭제하세요.
-//TODO: onClick 이벤트 추가
21-23: 하드코딩"ic_plus"대신 디자인 시스템 토큰/매핑 사용
디자인 시스템의IconToken또는ICON_MAP을 이용해 아이콘 식별자를 일관되게 관리하고 오타·런타임 오류를 예방하세요.apps/client/src/shared/components/sidebar/MyLevelItem.tsx (3)
4-6: active prop 미사용 — 최소 활용으로 TS/ESLint 정리
active는 선언만 되고 사용되지 않습니다. 최소한 data-attribute/ARIA로 소비해 경고를 없애고, 이후 스타일 훅으로 재사용하기 좋게 해두세요.적용 diff:
export default function MyLevelItem({ active }: MyLevelItemProps) { return ( <div + data-active={active ? 'true' : undefined} + aria-current={active ? 'page' : undefined} className={cn( 'bg-white-bg flex h-[6.2rem] w-full rounded-[0.4rem] p-[0.8rem]', 'transition-colors' )} ></div> ); }Also applies to: 9-9, 11-16
11-16: (선택) active 변형은 cva로 관리 + className 병합 지원본 PR에서 cva가 의존성에 추가되었으니, variant 패턴으로 active 스타일을 관리하면 확장성이 좋아집니다. 또한
className/기타 div 속성 패스스루를 열어 조합성을 높이세요.적용 예시 diff:
-import { cn } from '@pinback/design-system/utils'; +import { cn } from '@pinback/design-system/utils'; +import { cva } from 'class-variance-authority'; interface MyLevelItemProps { active: boolean; } //TODO: 프로필카드 만들기 -export default function MyLevelItem({ active }: MyLevelItemProps) { +const myLevelItemClass = cva( + 'bg-white-bg flex h-[6.2rem] w-full rounded-[0.4rem] p-[0.8rem] transition-colors', + { + variants: { + active: { + true: 'ring-1 ring-gray-200', // 필요시 디자인 토큰으로 교체 + false: '' + } + } + } +); + +export default function MyLevelItem({ + active, + ...divProps +}: MyLevelItemProps & React.HTMLAttributes<HTMLDivElement>) { return ( <div - data-active={active ? 'true' : undefined} - aria-current={active ? 'page' : undefined} - className={cn( - 'bg-white-bg flex h-[6.2rem] w-full rounded-[0.4rem] p-[0.8rem]', - 'transition-colors' - )} - ></div> + data-active={active ? 'true' : undefined} + aria-current={active ? 'page' : undefined} + className={cn(myLevelItemClass({ active }))} + {...divProps} + /> ); }
7-7: TODO를 이슈로 승격프로필 카드 TODO를 트래킹 이슈로 등록하고, 본 컴포넌트의 기대 API(아이콘/아바타, 레벨/진척도, 클릭 동작 등)를 간단히 명세해 주세요.
원하시면 체크리스트 포함한 이슈 템플릿/초안을 드리겠습니다.
apps/client/src/shared/components/sidebar/CategoryItem.tsx (3)
4-9: Props 정합성 및 타입 구체화active에 기본값을 주는 만큼 선택적 prop으로 두는 편이 자연스럽고, onClick은 실제 버튼 핸들러 시그니처로 타이핑하면 안전합니다.
+import type { MouseEventHandler } from 'react'; import { Icon } from '@pinback/design-system/icons'; import { cn } from '@pinback/design-system/utils'; interface CategoryItemProps { label: string; - active: boolean; + active?: boolean; className?: string; - onClick: () => void; + onClick?: MouseEventHandler<HTMLButtonElement>; } export default function CategoryItem({ label, active = false, className, onClick, }: CategoryItemProps) {Also applies to: 14-19, 1-3
39-41: 아이콘 하드코딩 → 토큰/매핑 일원화 검토PR 요약에 소개된 IconToken/ICON_MAP 패턴을 이 컴포넌트에도 적용하면 문자열 오타를 컴파일 타임에 방지할 수 있습니다. SideItem과 동일한 매핑 소스 재사용을 권장합니다.
10-13: Stale TODO 정리
- onClick은 이미 연결됨.
- “활성화 로직”은 상위에서 active를 제어하는 현재 API와 충돌합니다.
- “인덱스 번호”는 요구사항이 불명확합니다.
주석 제거하거나 이슈로 전환해 추적하세요. 필요하면 TODO에 이슈 번호를 명시하세요.
원하시면 TODO 정리 커밋/이슈 초안 만들어 드립니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
packages/design-system/src/icons/source/chippi_profile.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
apps/client/src/layout/Layout.tsx(2 hunks)apps/client/src/shared/components/sidebar/CategoryItem.tsx(1 hunks)apps/client/src/shared/components/sidebar/CreateItem.tsx(1 hunks)apps/client/src/shared/components/sidebar/MyLevelItem.tsx(1 hunks)apps/client/src/shared/components/sidebar/Sidebar.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/client/src/layout/Layout.tsx
- apps/client/src/shared/components/sidebar/Sidebar.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-03T08:57:48.602Z
Learnt from: jjangminii
PR: Pinback-Team/pinback-client#52
File: apps/client/src/shared/components/sidebar/SideItem.tsx:57-60
Timestamp: 2025-09-03T08:57:48.602Z
Learning: SideItem 컴포넌트에서 아이콘 디자인이 일반적인 기대와 반대여서, 회전 로직도 반대로 구현되었습니다. 이를 명확히 하기 위해 prop 이름을 open에서 close로 변경하여 실제 동작과 일치시켰습니다.
Applied to files:
apps/client/src/shared/components/sidebar/CategoryItem.tsx
🧬 Code graph analysis (3)
apps/client/src/shared/components/sidebar/MyLevelItem.tsx (1)
packages/design-system/src/lib/utils.ts (1)
cn(108-110)
apps/client/src/shared/components/sidebar/CategoryItem.tsx (1)
packages/design-system/src/lib/utils.ts (1)
cn(108-110)
apps/client/src/shared/components/sidebar/CreateItem.tsx (1)
packages/design-system/src/lib/utils.ts (1)
cn(108-110)
🪛 GitHub Check: lint
apps/client/src/shared/components/sidebar/MyLevelItem.tsx
[warning] 9-9:
'active' is defined but never used
[warning] 1-1:
'Icon' is defined but never used
🪛 GitHub Actions: ci
apps/client/src/shared/components/sidebar/MyLevelItem.tsx
[error] 1-1: Client build failed during 'tsc -b && vite build': TS6133: 'Icon' is declared but its value is never read.
🔇 Additional comments (5)
apps/client/src/shared/components/sidebar/CreateItem.tsx (1)
1-3: 디자인시스템 유틸/아이콘 사용 적절
@pinback/design-system의Icon,cn조합 사용은 일관되고 적절합니다.apps/client/src/shared/components/sidebar/MyLevelItem.tsx (2)
11-16: (선택) 상호작용 의도 확인 및 시맨틱 역할 부여이 요소가 내비게이션/액션을 수행한다면 a/button 사용 또는 role/탭 순서/키보드 핸들링을 고려해 주세요. 현재는 div로 렌더링됩니다.
컴포넌트 용도가 결정되면 알려 주세요. 필요한 접근성 보일러플레이트를 바로 제안하겠습니다.
1-16: 타입 체크 통과 확인 — 클라이언트 앱에서 오류 없이 빌드되었습니다.apps/client/src/shared/components/sidebar/CategoryItem.tsx (2)
24-25: ARIA 의미 확인: pressed vs current/selectedactive가 “토글 버튼의 상태”가 아니라 “라우팅 하이라이트(현재 위치)”라면 aria-pressed 대신 aria-current="page"가 맞습니다. 탭/리스트 선택 패턴이면 aria-selected를 고려하세요. 의도 확인 부탁드립니다.
예시(내비게이션인 경우):
- aria-pressed={active} + aria-current={active ? 'page' : undefined}
26-31: 스타일링/상태 표현은 적절합니다data-state와 transition-colors 조합, className 병합 순서가 적절해 오버라이드 여지가 확보되어 있습니다.
| interface CreateItemProps { | ||
| onClick: () => void; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
onClick 타입을 React MouseEventHandler로 교정하세요
현재 () => void를 DOM onClick에 그대로 전달하면 TS에서 타입 불일치가 날 수 있습니다. button으로 전환 시 핸들러 타입을 명시적으로 맞춰주세요.
+import type { MouseEventHandler } from 'react';
interface CreateItemProps {
- onClick: () => void;
+ onClick: MouseEventHandler<HTMLButtonElement>;
}
export default function CreateItem({ onClick }: CreateItemProps) {Also applies to: 9-9
🤖 Prompt for AI Agents
In apps/client/src/shared/components/sidebar/CreateItem.tsx around lines 4-6
(and also line 9), the onClick prop is typed as () => void which can conflict
with DOM button onClick types; change the prop type to
React.MouseEventHandler<HTMLButtonElement> (or import type { MouseEventHandler }
from 'react' and use MouseEventHandler<HTMLButtonElement>) so the prop matches a
button's event handler signature, and update any usages to accept the event
parameter if needed.
into feat/#48/SidebarRow-component
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/client/src/shared/components/sidebar/MyLevelItem.tsx (1)
1-1: 이전 CI 실패 코멘트는 현재 기준 해소되었습니다.
Icon이 실제로 사용되고 있어 TS6133는 더 이상 발생하지 않습니다.apps/client/src/shared/components/sidebar/CategoryItem.tsx (1)
1-1: 이전 접근성 지적사항 해결 확인이전 PR 코멘트(역할/중첩 버튼 문제)가 본 구현에서 해소되었습니다.
🧹 Nitpick comments (4)
apps/client/src/pages/level/components/LevelInfoCard.tsx (1)
4-7: LevelInfoCard.tsx에서 @shared 별칭으로 import 경로 통일
apps/client/src/pages/level/components/LevelInfoCard.tsx의 TreeLevel import를 상대경로에서 @shared 별칭으로 변경해 주세요.-import { - TREE_LEVEL_TABLE, - type TreeLevel, -} from '../../../shared/utils/treeLevel'; +import { + TREE_LEVEL_TABLE, + type TreeLevel, +} from '@shared/utils/treeLevel';apps/client/src/shared/utils/anchorPosition.ts (1)
2-4: 오버플로/정렬 보정 옵션 추가 제안드롭다운 높이/우측 여백 이슈가 생길 수 있으므로 Y 오프셋 같은 가벼운 훅을 두면 재사용성이 좋아집니다.
아래처럼 offsetY를 허용하면 행간 조정이 쉬워집니다(BC 유지).
-export function rightOf(anchor: HTMLElement, gap = 8) { +export function rightOf(anchor: HTMLElement, gap = 8, offsetY = 0) { const r = anchor.getBoundingClientRect(); - return { top: r.top, left: r.right + gap }; + return { top: r.top + offsetY, left: r.right + gap }; }apps/client/src/shared/hooks/useAnchoredMenu.ts (1)
50-69: 모바일/펜 입력 대응 및 ESC 닫힘 개선바깥 클릭 닫힘을
mousedown에서pointerdown으로 바꾸고, ESC 키로 닫힘을 지원하면 UX가 안정적입니다.아래처럼 교체/추가를 제안합니다.
- const onDocMouseDown = (e: MouseEvent) => { + const onDocPointerDown = (e: PointerEvent) => { const t = e.target as Node; if ( state.anchorEl && !state.anchorEl.contains(t) && !containerRef.current?.contains(t) ) { close(); } }; + const onDocKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Escape') close(); + }; @@ - document.addEventListener('mousedown', onDocMouseDown); + document.addEventListener('pointerdown', onDocPointerDown); + document.addEventListener('keydown', onDocKeyDown); @@ - document.removeEventListener('mousedown', onDocMouseDown); + document.removeEventListener('pointerdown', onDocPointerDown); + document.removeEventListener('keydown', onDocKeyDown);apps/client/src/shared/components/sidebar/Sidebar.tsx (1)
45-51: nav에 접근 가능한 이름 추가스크린 리더용 네비게이션 레이블을 부여하세요.
- <nav className="bg-white-bg sticky top-0 h-screen w-[22.4rem] overflow-y-auto border-r border-gray-300 px-[0.8rem]"> + <nav + aria-label="사이드바 내비게이션" + className="bg-white-bg sticky top-0 h-screen w-[22.4rem] overflow-y-auto border-r border-gray-300 px-[0.8rem]" + >
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
packages/design-system/src/icons/source/chippi_profile.svgis excluded by!**/*.svgpackages/design-system/src/icons/source/logo.svgis excluded by!**/*.svg
📒 Files selected for processing (11)
apps/client/src/layout/Layout.tsx(1 hunks)apps/client/src/pages/level/components/LevelInfoCard.tsx(1 hunks)apps/client/src/pages/level/components/TreeStatusCard.tsx(1 hunks)apps/client/src/shared/components/sidebar/AccordionItem.tsx(1 hunks)apps/client/src/shared/components/sidebar/CategoryItem.tsx(1 hunks)apps/client/src/shared/components/sidebar/MyLevelItem.tsx(1 hunks)apps/client/src/shared/components/sidebar/SideItem.tsx(1 hunks)apps/client/src/shared/components/sidebar/Sidebar.tsx(1 hunks)apps/client/src/shared/hooks/useAnchoredMenu.ts(1 hunks)apps/client/src/shared/utils/anchorPosition.ts(1 hunks)packages/design-system/src/icons/iconNames.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/design-system/src/icons/iconNames.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/client/src/shared/components/sidebar/AccordionItem.tsx
- apps/client/src/layout/Layout.tsx
- apps/client/src/shared/components/sidebar/SideItem.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-04T08:38:00.583Z
Learnt from: jjangminii
PR: Pinback-Team/pinback-client#54
File: apps/client/src/pages/level/components/TreeStatusCard.tsx:12-12
Timestamp: 2025-09-04T08:38:00.583Z
Learning: In TreeStatusCard component, the progress bar calculation `info.level * 20` is intentional design - level 1 starts at 20%, level 2 at 40%, etc., up to level 5 at 100%. This ensures no level shows 0% progress for UX purposes.
Applied to files:
apps/client/src/pages/level/components/TreeStatusCard.tsxapps/client/src/shared/components/sidebar/MyLevelItem.tsx
🧬 Code graph analysis (3)
apps/client/src/shared/components/sidebar/Sidebar.tsx (4)
apps/client/src/shared/hooks/useAnchoredMenu.ts (1)
useAnchoredMenu(14-89)apps/client/src/shared/utils/anchorPosition.ts (1)
rightOf(2-5)apps/client/src/shared/components/sidebar/CategoryItem.tsx (1)
CategoryItem(13-61)apps/client/src/shared/components/optionsMenuButton/OptionsMenuButton.tsx (1)
OptionsMenuButton(16-41)
apps/client/src/shared/components/sidebar/MyLevelItem.tsx (2)
apps/client/src/shared/utils/treeLevel.ts (1)
getTreeLevel(47-50)packages/design-system/src/lib/utils.ts (1)
cn(108-110)
apps/client/src/shared/components/sidebar/CategoryItem.tsx (1)
packages/design-system/src/lib/utils.ts (1)
cn(108-110)
🔇 Additional comments (9)
apps/client/src/pages/level/components/TreeStatusCard.tsx (2)
3-3: shared 유틸 경로를 별칭으로 변경한 부분, 좋습니다모듈 경로 일관성과 이동 내성 측면에서 👍
12-12: 진행률 20% 단위 계산, 의도대로 유지 OK이전 학습 메모대로 레벨 1→20%…5→100% UX 의도가 반영되어 있습니다. 추가 변경 불필요합니다.
apps/client/src/shared/utils/anchorPosition.ts (1)
2-5: 기본 포지셔닝은 적절합니다.viewport 기준(top/right) + fixed 포털 전략과 잘 맞습니다.
apps/client/src/shared/components/sidebar/MyLevelItem.tsx (1)
20-21: Progress 계산 로직 확인(Learnings 반영 ok).레벨당 20% 단계형 진행도는 과거 합의대로 의도된 UX입니다. 그대로 유지 적합합니다.
apps/client/src/shared/hooks/useAnchoredMenu.ts (1)
61-67: 스크롤 리스너 캡처 사용 적절캡처링으로 내부 스크롤까지 포착하는 접근은 적합합니다.
apps/client/src/shared/components/sidebar/Sidebar.tsx (2)
114-123: MyLevelItem 위치 지정이 적용되지 않습니다(하위 컴포넌트 버그 영향).
className="fixed bottom-[2.8rem]"을 전달하지만, MyLevelItem이className을 반영하지 않아 고정 하단 배치가 먹지 않습니다. MyLevelItem 수정 후(별도 코멘트 참조) 다시 확인 바랍니다.
96-112: 포털 메뉴 닫힘 시나리오 OK앵커/컨테이너 외부 포인터 다운 시 닫힘, 스크롤/리사이즈 재포지셔닝 모두 적절합니다.
apps/client/src/shared/components/sidebar/CategoryItem.tsx (2)
22-30: 루트 컨테이너 역할 분리 적절클릭은 라벨 버튼, 옵션은 우측 버튼으로 분리되어 접근성/포커스 흐름이 명확합니다.
42-58: 옵션 버튼 접근성 처리 양호
aria-haspopup="menu"와 명시적 라벨 부여가 적절합니다. 아이콘은aria-hidden으로 장식 처리되어 있습니다.
| export default function MyLevelItem({ | ||
| acorns, | ||
| onClick, | ||
| active, | ||
| }: MyLevelItemProps) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
className 프롭이 무시됩니다(레이아웃 깨짐).
Sidebar에서 넘기는 className="fixed bottom-[2.8rem]"이 적용되지 않습니다. 프롭 구조 분해에 className을 추가하세요.
아래 패치에 루트 엘리먼트를 button으로 바꿔 접근성도 함께 개선했습니다(키보드 포커스/Space/Enter 지원).
-export default function MyLevelItem({
- acorns,
- onClick,
- active,
-}: MyLevelItemProps) {
+export default function MyLevelItem({
+ acorns,
+ onClick,
+ active,
+ className,
+}: MyLevelItemProps) {
@@
- return (
- <div
- onClick={onClick}
- className={cn(
+ return (
+ <button
+ type="button"
+ onClick={onClick}
+ className={cn(
'h-[6.2rem] w-full rounded-[0.4rem] border p-[0.8rem] transition-colors',
'flex flex-row justify-between gap-[0.8rem]',
'bg-white-bg border-transparent',
'hover:bg-main0 hover:border-main400',
- active && 'bg-main0 border-main400'
+ active && 'bg-main0 border-main400',
+ className
)}
>
@@
- </div>
+ </button>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/client/src/shared/components/sidebar/MyLevelItem.tsx around lines 13 to
17, the component currently ignores the incoming className prop causing layout
break; add className to the props destructuring and apply it to the root element
(which should be a <button> for accessibility), merging it with any internal
classes so the passed "fixed bottom-[2.8rem]" is preserved, and ensure onClick,
active state classes, and keyboard interaction remain supported on the button.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/client/src/shared/components/sidebar/Sidebar.tsx (1)
92-111: ul 직계 자식이 div로 렌더링됨 — li로 감싸 시맨틱/A11Y 보장현재 ul 내부에 CategoryItem(div)과 CreateItem(div role="button")이 직접 렌더링됩니다. 표준 위반이며 스크린리더·스타일링에 불리합니다. 각 아이템을 li로 감싸 주세요.
- <ul className="bg-none"> - {CATEGORIES.map((c) => ( - <CategoryItem - key={c.id} - id={c.id} - label={c.label} - active={selectedCategoryId === c.id} - onClick={(id) => { - setSelectedCategoryId(id); - setActiveTab('mybookmark'); - closeMenu(); - navigate(`/my-bookmarks?categoryId=${id}`); - }} - onOptionsClick={(id, el) => openMenu(id, el)} - /> - ))} - - <CreateItem onClick={openCreatePopup} /> - </ul> + <ul className="bg-none"> + {CATEGORIES.map((c) => ( + <li key={c.id}> + <CategoryItem + id={c.id} + label={c.label} + active={selectedCategoryId === c.id} + onClick={(id) => { + setSelectedCategoryId(id); + setActiveTab('mybookmark'); + closeMenu(); + navigate(`/my-bookmarks?categoryId=${id}`); + }} + onOptionsClick={(id, el) => openMenu(id, el)} + /> + </li> + ))} + <li> + <CreateItem onClick={openCreatePopup} /> + </li> + </ul>
🧹 Nitpick comments (3)
apps/client/src/shared/components/sidebar/Sidebar.tsx (3)
113-139: z-index 중복·충돌 가능성: inline style(9999) vs 클래스(10000)inline style의 zIndex(9999)가 클래스보다 우선되어 의도와 달리 9999로 고정됩니다. 하나만 사용하세요(권장: 클래스만 사용).
- <div ref={containerRef} style={style} className="z-[10000]"> + <div ref={containerRef} style={style}>또는 useAnchoredMenu에서 zIndex를 제거하고 여기에 z-[10000]만 지정하는 방식으로 일관화하세요.
56-56: 랜드마크 라벨 추가로 내비게이션 인지성 향상aside에 aria-label을 추가하면 스크린리더가 영역을 명확히 식별합니다.
- <aside className="bg-white-bg sticky top-0 h-screen w-[24rem] border-r border-gray-300"> + <aside aria-label="사이드바 내비게이션" className="bg-white-bg sticky top-0 h-screen w-[24rem] border-r border-gray-300">
67-78: 클릭 가능한 div 기반 내비게이션 → NavLink/Link로 전환 권장키보드 접근성·시맨틱을 위해 내비 아이템을 버튼/링크로 표현하는 것이 바람직합니다. SideItem이 to prop을 받아 NavLink를 내부에서 렌더하거나, 여기서 NavLink로 감싸는 패턴을 검토해 주세요.
Also applies to: 80-91
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/client/src/shared/components/sidebar/Sidebar.tsx(1 hunks)packages/design-system/src/components/popup/Popup.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/design-system/src/components/popup/Popup.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/client/src/shared/components/sidebar/Sidebar.tsx (8)
apps/client/src/shared/hooks/useAnchoredMenu.ts (1)
useAnchoredMenu(14-89)apps/client/src/shared/utils/anchorPosition.ts (1)
rightOf(2-5)apps/client/src/shared/components/sidebar/SideItem.tsx (1)
SideItem(22-69)apps/client/src/shared/components/sidebar/AccordionItem.tsx (1)
AccordionItem(19-67)apps/client/src/shared/components/sidebar/CategoryItem.tsx (1)
CategoryItem(13-61)apps/client/src/shared/components/sidebar/CreateItem.tsx (1)
CreateItem(9-29)apps/client/src/shared/components/optionsMenuButton/OptionsMenuButton.tsx (1)
OptionsMenuButton(16-41)apps/client/src/shared/components/sidebar/MyLevelItem.tsx (1)
MyLevelItem(13-61)
jjangminii
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/client/src/shared/components/sidebar/Sidebar.tsx (1)
83-99: ul 자식 시맨틱 수정: li로 감싸기ul 직계 자식으로 div가 렌더링되고 있습니다. 각 항목을 li로 감싸 a11y와 시맨틱을 보장해 주세요. 이전 코멘트와 동일 맥락입니다.
- <ul className="bg-none"> - {CATEGORIES.map((c) => ( - <CategoryItem - key={c.id} - id={c.id} - label={c.label} - active={selectedCategoryId === c.id} - onClick={(id) => { - closeMenu(); - selectCategory(id); - }} - onOptionsClick={(id, el) => openMenu(id, el)} - /> - ))} - - <CreateItem onClick={openCreate} /> - </ul> + <ul className="bg-none"> + {CATEGORIES.map((c) => ( + <li key={c.id}> + <CategoryItem + id={c.id} + label={c.label} + active={selectedCategoryId === c.id} + onClick={(id) => { + closeMenu(); + selectCategory(id); + }} + onOptionsClick={(id, el) => openMenu(id, el)} + /> + </li> + ))} + <li> + <CreateItem onClick={openCreate} /> + </li> + </ul>
🧹 Nitpick comments (6)
apps/client/src/shared/hooks/useSidebarNav.ts (2)
8-11: 라우터 파생 상태로 동기화해 하이라이트 드리프트 방지현재 activeTab/selectedCategoryId를 로컬 state로만 관리해 뒤로가기/직접 진입 시 UI 하이라이트가 라우트와 어긋날 수 있습니다. 라우트에서 파생해 동기화하는 이펙트를 추가하는 쪽을 권장합니다.
다음 패치로 라우팅과 상태를 동기화해 주세요.
- import { useNavigate } from 'react-router-dom'; + import { useNavigate, useLocation, useSearchParams } from 'react-router-dom'; - import { useCallback, useState } from 'react'; + import { useCallback, useEffect, useState } from 'react'; export function useSidebarNav() { const navigate = useNavigate(); + const location = useLocation(); + const [searchParams] = useSearchParams(); const [activeTab, setActiveTab] = useState<SidebarTab>('remind'); const [selectedCategoryId, setSelectedCategoryId] = useState<number | null>( null ); + // URL → 상태 동기화 + useEffect(() => { + const path = location.pathname; + const tab: SidebarTab = + path.startsWith('/my-bookmarks') + ? 'mybookmark' + : path === '/level' + ? 'level' + : 'remind'; + setActiveTab(tab); + + const v = searchParams.get('categoryId'); + const n = v !== null ? Number(v) : NaN; + setSelectedCategoryId(Number.isNaN(n) ? null : n); + }, [location.pathname, searchParams]);간단 검증: 1) /my-bookmarks 직접 진입 시 북마크가 활성 표시되는지 2) 카테고리 선택 후 뒤로가기로 해제되는지 확인해 주세요.
Also applies to: 13-39
40-49: setter 공개 축소로 일관성 강화setSelectedCategoryId/setActiveTab를 외부에 노출하면 라우트 파생 로직과 충돌 위험이 있습니다. 공개를 줄이고 go* 핸들러만 제공하는 쪽을 권장합니다.
return { activeTab, selectedCategoryId, goRemind, goBookmarks, selectCategory, goLevel, - setSelectedCategoryId, - setActiveTab, };(동시에 Sidebar.tsx에서 해당 setter 호출 제거 필요 — 아래 코멘트 참고)
apps/client/src/shared/components/sidebar/OptionsMenuPortal.tsx (1)
15-25: ESC로 닫힘 지원 및 키보드 접근성 보강포털 메뉴를 ESC로 닫을 수 있도록 간단한 keydown 리스너를 추가하세요. 마우스 외 상호작용 접근성이 개선됩니다.
-import { createPortal } from 'react-dom'; +import { createPortal } from 'react-dom'; +import { useEffect } from 'react'; export default function OptionsMenuPortal({ open, style, containerRef, categoryId, getCategoryName, onEdit, onDelete, onClose, }: Props) { + useEffect(() => { + if (!open) return; + const onKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Escape') onClose(); + }; + document.addEventListener('keydown', onKeyDown); + return () => document.removeEventListener('keydown', onKeyDown); + }, [open, onClose]); if (!open || !style) return null;apps/client/src/shared/components/sidebar/PopupPortal.tsx (1)
22-23: ESC 닫힘 추가(모달 접근성 개선)모달 활성 시 ESC로 닫힐 수 있도록 keydown 리스너를 추가하면 접근성이 좋아집니다. OptionsMenuPortal과 동일한 패턴을 적용해 주세요.
-import { createPortal } from 'react-dom'; +import { createPortal } from 'react-dom'; +import { useEffect } from 'react'; @@ export default function PopupPortal({ popup, onClose, onCreateConfirm, onEditConfirm, onDeleteConfirm, }: Props) { if (!popup) return null; + useEffect(() => { + const onKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Escape') onClose(); + }; + document.addEventListener('keydown', onKeyDown); + return () => document.removeEventListener('keydown', onKeyDown); + }, [onClose]);apps/client/src/shared/components/sidebar/Sidebar.tsx (2)
66-69: 중복된 상태 초기화 제거로 단순화setSelectedCategoryId(null)은 goRemind/goBookmarks/goLevel 내부에서도 처리됩니다. 중복 호출을 제거해 의도를 선명하게 해주세요.
- setSelectedCategoryId(null); closeMenu(); goRemind();- setSelectedCategoryId(null); closeMenu(); goBookmarks();- setSelectedCategoryId(null); closeMenu(); goLevel();Also applies to: 78-81, 119-122
44-46: getCategoryName 메모이제이션(선택 사항)빈번한 리렌더에서 동일 참조 유지가 필요하면 useCallback으로 감싸는 것을 고려해 보세요. 영향이 미미하면 스킵 가능합니다.
- const getCategoryName = (id: number | null) => - CATEGORIES.find((c) => c.id === id)?.label ?? ''; + const getCategoryName = useCallback( + (id: number | null) => CATEGORIES.find((c) => c.id === id)?.label ?? '', + [] + );
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
apps/client/src/pages/level/components/LevelInfoCard.tsx(1 hunks)apps/client/src/shared/components/sidebar/AccordionItem.tsx(1 hunks)apps/client/src/shared/components/sidebar/OptionsMenuPortal.tsx(1 hunks)apps/client/src/shared/components/sidebar/PopupPortal.tsx(1 hunks)apps/client/src/shared/components/sidebar/Sidebar.tsx(1 hunks)apps/client/src/shared/hooks/useAnchoredMenu.ts(1 hunks)apps/client/src/shared/hooks/useCategoryPopups.ts(1 hunks)apps/client/src/shared/hooks/useSidebarNav.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/client/src/pages/level/components/LevelInfoCard.tsx
- apps/client/src/shared/components/sidebar/AccordionItem.tsx
- apps/client/src/shared/hooks/useAnchoredMenu.ts
🧰 Additional context used
🧬 Code graph analysis (3)
apps/client/src/shared/components/sidebar/OptionsMenuPortal.tsx (1)
apps/client/src/shared/components/optionsMenuButton/OptionsMenuButton.tsx (1)
OptionsMenuButton(16-41)
apps/client/src/shared/components/sidebar/PopupPortal.tsx (1)
apps/client/src/shared/hooks/useCategoryPopups.ts (1)
PopupState(3-7)
apps/client/src/shared/components/sidebar/Sidebar.tsx (8)
apps/client/src/shared/hooks/useSidebarNav.ts (1)
useSidebarNav(6-50)apps/client/src/shared/hooks/useAnchoredMenu.ts (1)
useAnchoredMenu(13-88)apps/client/src/shared/utils/anchorPosition.ts (1)
rightOf(2-5)apps/client/src/shared/hooks/useCategoryPopups.ts (1)
useCategoryPopups(9-24)apps/client/src/shared/components/sidebar/SideItem.tsx (1)
SideItem(22-69)apps/client/src/shared/components/sidebar/CategoryItem.tsx (1)
CategoryItem(13-61)apps/client/src/shared/components/sidebar/OptionsMenuPortal.tsx (1)
OptionsMenuPortal(15-45)apps/client/src/shared/components/sidebar/PopupPortal.tsx (1)
PopupPortal(13-65)
🔇 Additional comments (1)
apps/client/src/shared/hooks/useCategoryPopups.ts (1)
1-24: 타입 안전하고 간결합니다 — 훌륭해요분기 가능한 PopupState와 핸들러 메모이제이션이 명확합니다. 현재 요구사항에 적합합니다.
| {popup.kind === 'create' && ( | ||
| <Popup | ||
| type="input" | ||
| title="카테고리 추가하기" | ||
| left="취소" | ||
| right="추가" | ||
| placeholder="카테고리 제목을 입력해주세요" | ||
| onLeftClick={onClose} | ||
| onRightClick={() => onCreateConfirm?.()} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
입력값 전달 누락 — 생성/수정 confirm으로 텍스트가 전달되지 않습니다
현재 onCreateConfirm/onEditConfirm에 인풋 텍스트가 전달되지 않아 API 연동 시 값 손실 위험이 큽니다. 디자인시스템 Popup의 onRightClick 시그니처를 확인해 값 전달을 연결해 주세요.
옵션 A(권장, Popup이 입력값을 인자로 넘기는 경우):
-interface Props {
+interface Props {
popup: PopupState;
onClose: () => void;
- onCreateConfirm?: () => void;
- onEditConfirm?: (id: number, draft?: string) => void;
+ onCreateConfirm?: (name: string) => void;
+ onEditConfirm?: (id: number, name: string) => void;
onDeleteConfirm?: (id: number) => void;
}
@@
- onRightClick={() => onCreateConfirm?.()}
+ onRightClick={(value: string) => onCreateConfirm?.(value)}
@@
- onRightClick={() => onEditConfirm?.(popup.id)}
+ onRightClick={(value: string) => onEditConfirm?.(popup.id, value)}옵션 B(Popup이 값을 인자로 주지 않는 경우): 로컬 draft state를 두고 Popup에 value/onChange(혹은 유사 props)를 바인딩한 뒤 confirm에서 draft를 전달해 주세요.
시그니처 확인용 스크립트:
Also applies to: 38-47, 50-59
🏁 Script executed:
#!/bin/bash
# 디자인시스템 Popup Props 검색
fd -t f -a 'Popup.(tsx|ts|jsx|js)' 2>/dev/null
rg -nP -C2 'export (interface|type) .*Popup.*Props|function\s+Popup\(|onRightClick' -SLength of output: 3252
Option B 사용: 로컬 draft state에 입력값을 저장하고 Popup에 value/onChange로 바인딩한 뒤 onRightClick에서 onCreateConfirm/onEditConfirm 호출 시 해당 draft를 인자로 전달하세요
🤖 Prompt for AI Agents
In apps/client/src/shared/components/sidebar/PopupPortal.tsx around lines 26 to
36, the Popup is currently uncontrolled for text input; implement Option B by
adding a local draft state (e.g., useState<string>), pass draft as value and an
onChange handler to Popup, update draft on user input, and when the right button
is clicked call onCreateConfirm (and onEditConfirm for edit branch) with the
current draft string as an argument instead of calling them with no params.
constantly-dev
left a comment
constantly-dev
left a comment
There was a problem hiding this comment.
컴포넌트도 잘 분리해주시고, 사소한 부분들이 많아지다 보니 볼륨이 계속 커진 듯한 느낌이 있네요! 수고하셨어요 👍
코멘트 남겼으니 확인해주시고 편하게 서로 이야기 해보면 좋을 것 같아요!
| <Sidebar /> | ||
| <Outlet /> |
There was a problem hiding this comment.
sidebar 위치 잘 정의해주셨네요!! 이후 해당 PR 머지되면 사이드바 제 쪽에서 가져와서 추가 layout 작업 하겠습니다~
| @@ -0,0 +1,64 @@ | |||
| import * as React from 'react'; | |||
There was a problem hiding this comment.
해당 React 모듈 전체 안 불러도 될 것 같은데 이유가 있으셨나요?
| trailing = true, | ||
| className, | ||
| }: AccordionItemProps) { | ||
| const [internalOpen, setInternalOpen] = React.useState(defaultOpen); |
There was a problem hiding this comment.
위 React 전체 import 관련 질문과 동일!
| <Icon | ||
| name="ic_arrow_down_active" | ||
| aria-hidden | ||
| className={cn( | ||
| 'h-[2rem] w-[2rem] transition-transform', | ||
| open && 'rotate-180' | ||
| )} | ||
| /> |
There was a problem hiding this comment.
해당 arrow icon은 1.6rem인 것 같아요!
| const [internalOpen, setInternalOpen] = useState(defaultOpen); | ||
| const isOpen = open ?? internalOpen; |
There was a problem hiding this comment.
외부 open을 받아서 쓰시는 이유는 아무래도 open 상태를 컴포넌트 외부에서도 쓰이기 때문에 부모로 올리신 게 아닐까 추측해보는데, 내부에서도 open 상태를 분리하신 설계 관점에서 이유가 궁금합니다!
There was a problem hiding this comment.
밑에 코멘트에 답변 남겨뒀습니다-!
| interface Props { | ||
| open: boolean; | ||
| style?: React.CSSProperties | null; | ||
| containerRef: React.RefObject<HTMLDivElement | null>; | ||
| categoryId: number | null; | ||
| getCategoryName: (id: number | null) => string; | ||
| onEdit: (id: number, name: string) => void; | ||
| onDelete: (id: number, name: string) => void; | ||
| onClose: () => void; | ||
| } |
There was a problem hiding this comment.
interface 네이밍 수정해야 할 것 같아요
| if (!popup) return null; | ||
|
|
||
| return createPortal( | ||
| <div className="fixed inset-0 z-[11000]"> |
There was a problem hiding this comment.
z-index 토큰화의 필요성을 느끼고있습니다.. 제가 너무 무분별하게 숫자를 키운감도 있고요..
There was a problem hiding this comment.
ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ 11000은 어쩌다가...
There was a problem hiding this comment.
....그 어떤것 밑에도 깔리고싶지 않았어요 ㅎ
| import { Icon, type IconName } from '@pinback/design-system/icons'; | ||
| import { cn } from '@pinback/design-system/utils'; | ||
|
|
||
| export type IconToken = 'clock' | 'bookmark'; |
There was a problem hiding this comment.
IconToken type이 다른 곳에서도 쓰이나요? 그렇다면 type에 따로 분리를 하는게!
There was a problem hiding this comment.
사이드 아이템과 아코디언 아이템에서 사용되고 있습니다-! 이 부분은 따로 분리해두겠습니다
There was a problem hiding this comment.
해당 파일의 목적이 작은 모달..? 위치시키기 위해서 사용된건가요??
absolute 사용해도 가능할 것 같은데 어떤 이유로 해당 로직으로 구현하게 되신 것인지 궁금해요!! 어떤 것이 안됐다던지!
There was a problem hiding this comment.
이미지처럼 화면 비율(해상도)에 따라 카테고리가 최대 10개까지 늘어나면 사이드바에 스크롤이 생기는 경우가 있었습니다. 스크롤이 생겨도 팝오버가 해당 카테고리 바로 옆에 붙어서 함께 스크롤되길 원했습니다. 그런데 absolute만으로는 스크롤 컨테이너 내부에서 좌표를 잡을 때 부모 스크롤과 뷰포트 좌표가 뒤섞여 위치 계산이 깨지는 경우가 있었고, 메뉴가 스크롤 경계에 잘려 영역 밖으로 못 나가는 문제도 발생했습니다. 이를 피하려고 createPortal로 document.body에 렌더링해 부모의 overflow/스태킹 컨텍스트 영향을 벗어나게 했습니다.
| const openCreate = useCallback(() => setPopup({ kind: 'create' }), []); | ||
| const openEdit = useCallback( | ||
| (id: number, name: string) => setPopup({ kind: 'edit', id, name }), | ||
| [] | ||
| ); | ||
| const openDelete = useCallback( | ||
| (id: number, name: string) => setPopup({ kind: 'delete', id, name }), | ||
| [] | ||
| ); | ||
| const close = useCallback(() => setPopup(null), []); | ||
|
|
||
| return { popup, openCreate, openEdit, openDelete, close }; |
There was a problem hiding this comment.
useCallback을 어떤 목적으로 사용하신 건가요!
There was a problem hiding this comment.
핸들러 참조를 저장했다면, 참조가 매번 바뀌면 리스너를 계속 갈아끼우게 되니 useCallback으로 고정하면 리스너 재등록을 줄이려했는데 여기서는 핸들러 참조를 저장하지 않아서 필요하지 않을거같네요 수정하겟습니다-!
into feat/#48/SidebarRow-component
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/client/src/pages/level/Level.tsx (2)
17-17: 타입 단언(as TreeLevel) 제거 권장 — getTreeLevel 반환 타입을 명시하세요.
info.level as TreeLevel단언 대신getTreeLevel의 반환 타입에level: TreeLevel을 포함하면 안전합니다.예시(외부 파일 수정 예):
// @shared/utils/treeLevel.ts import type { TreeLevel } from '@pages/level/types/treeLevelType'; export interface TreeLevelInfo { level: TreeLevel; // ...other fields } export function getTreeLevel(acorns: number): TreeLevelInfo { // ... }이후 본 파일은 단언 없이 그대로 사용 가능합니다:
level={info.level}.
24-33: id 중복 가능성과 a11y 마크업 소량 개선 제안페이지 내 여러 곳에서 동일 패턴을 재사용할 경우
id="level-info-card"가 중복될 수 있습니다.useId()로 고유 id를 생성하고, 보조기기 인식을 위해 컨텐츠 컨테이너에role="tooltip"부여를 권장합니다.예시(본 파일 내 개선 아이디어):
// 상단 import import { useId } from 'react'; // 컴포넌트 내부 const tooltipId = useId(); <button type="button" className="peer flex items-center justify-center p-[0.4rem]" aria-describedby={tooltipId} > <Icon name="ic_info" width={20} height={20} /> </button> <div id={tooltipId} role="tooltip" className={cn( 'pointer-events-none absolute left-0 top-[3rem] z-[20]', 'opacity-0 transition-opacity duration-150', 'peer-hover:pointer-events-auto peer-focus-visible:pointer-events-auto', 'peer-hover:opacity-100 peer-focus-visible:opacity-100' )} > <LevelInfoCard /> </div>
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/client/src/pages/level/Level.tsx(1 hunks)apps/client/src/shared/components/sidebar/CreateItem.tsx(1 hunks)apps/client/src/shared/components/sidebar/MyLevelItem.tsx(1 hunks)apps/client/src/shared/components/sidebar/OptionsMenuPortal.tsx(1 hunks)apps/client/src/shared/components/sidebar/SideItem.tsx(1 hunks)apps/client/src/shared/components/sidebar/Sidebar.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/client/src/shared/components/sidebar/Sidebar.tsx
- apps/client/src/shared/components/sidebar/CreateItem.tsx
- apps/client/src/shared/components/sidebar/OptionsMenuPortal.tsx
- apps/client/src/shared/components/sidebar/MyLevelItem.tsx
- apps/client/src/shared/components/sidebar/SideItem.tsx
🔇 Additional comments (1)
apps/client/src/pages/level/Level.tsx (1)
6-6: ‘@shared/*’ alias 설정 및 기존 참조 확인 완료
앱 클라이언트의 tsconfig.json에@shared/*가 정의되어 있고 Vite는tsconfigPaths()플러그인으로 경로를 해결하며,@pages/level/utils/treeLevel참조도 모두 제거된 상태입니다.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/client/src/shared/components/sidebar/AccordionItem.tsx (2)
55-61: 아코디언 패널 A11Y 속성 추가 (region, aria-hidden/label)접힘/펼침 상태를 스크린리더에 전달해주세요. 과거 코멘트의 연장선입니다.
- <div - id={panelId} - className={cn( + <div + id={panelId} + role="region" + aria-hidden={!isOpen} + aria-label={`${label} 패널`} + className={cn( 'grid overflow-hidden transition-[grid-template-rows]', isOpen ? 'grid-rows-[1fr]' : 'grid-rows-[0fr]' )} >
45-53: 트리거↔패널 연결: aria-controls/expanded 전달 경로 마련토글 버튼에
aria-controls={panelId},aria-expanded={isOpen}를 달아주세요. SideItem에 프롭 패스를 추가하는 방식을 권장합니다.<SideItem icon={icon} label={label} active={active} trailing={trailing} open={isOpen} - onTrailingClick={toggle} + onTrailingClick={toggle} + trailingButtonProps={{ 'aria-controls': panelId, 'aria-expanded': isOpen }} - onClick={onClick} + onClick={onHeaderClick} />SideItem 측(참고용 변경):
- onTrailingClick, + onTrailingClick, + trailingButtonProps, @@ - {trailing && ( + {trailing && ( <button type="button" className="rounded-[0.4rem] p-1 focus-visible:ring-2" + {...trailingButtonProps} onClick={(e) => {Also applies to: 55-57
apps/client/src/shared/components/sidebar/Sidebar.tsx (1)
88-105: ul 직계 자식 시맨틱 위반: CategoryItem/CreateItem을 li로 감싸기현재 ul > div(button) 구조입니다. ul > li를 보장해 시맨틱/A11Y를 확보하세요. 과거 코멘트와 동일 이슈입니다.
- <ul className="bg-none"> - {CATEGORIES.map((c) => ( - <CategoryItem - key={c.id} - id={c.id} - label={c.label} - active={selectedCategoryId === c.id} - onClick={(id) => { - closeMenu(); - selectCategory(id); - }} - onOptionsClick={(id, el) => openMenu(id, el)} - /> - ))} - - <CreateItem onClick={openCreate} /> - </ul> + <ul className="list-none p-0 m-0"> + {CATEGORIES.map((c) => ( + <li key={c.id}> + <CategoryItem + id={c.id} + label={c.label} + active={selectedCategoryId === c.id} + onClick={(id) => { + closeMenu(); + selectCategory(id); + }} + onOptionsClick={(id, el) => openMenu(id, el)} + /> + </li> + ))} + <li> + <CreateItem onClick={openCreate} /> + </li> + </ul>
🧹 Nitpick comments (11)
apps/client/src/shared/components/sidebar/types/IconTokenType.ts (1)
1-1: 아이콘 토큰을 상수 배열+파생 타입으로 선언해 재사용성과 정합성 강화런타임에서도 열거가 필요하고, ICON_MAP 완전성 체크에 유리합니다.
-export type IconToken = 'clock' | 'bookmark'; +export const ICON_TOKENS = ['clock', 'bookmark'] as const; +export type IconToken = (typeof ICON_TOKENS)[number];추가로, SideItem의 ICON_MAP에 컴파일 타임 완전성 보장을 걸어두면 안전합니다(참고용, 다른 파일 수정):
// SideItem.tsx (참고) const ICON_MAP = { clock: { on: 'ic_clock_active', off: 'ic_clock_disable' }, bookmark: { on: 'ic_bookmark_active', off: 'ic_bookmark_disable' }, } as const satisfies Record<IconToken, { on: IconName; off: IconName }>;apps/client/src/shared/components/sidebar/AccordionItem.tsx (2)
62-64: gap 미적용: 컨텐츠 컨테이너에 flex/grid 지정 및 bg 클래스 보정gap은 블록 요소에선 동작하지 않습니다. 또한 배경 없음은 bg-transparent가 더 명확합니다.
- <div className="min-h-0 gap-[0.2rem] bg-none py-[0.4rem]"> + <div className="min-h-0 flex flex-col gap-[0.2rem] bg-transparent py-[0.4rem]"> {children} </div>
6-18: onClick 타입 재정의로 인한 타입 스큐 방지HTMLAttributes에 이미 onClick이 포함되어 있는데,
() => void로 재정의하면 콜백 파라미터 공변성 문제를 유발할 수 있습니다. 기존 시그니처를 존중하거나 핸들러 이름을 분리하세요.interface AccordionItemProps extends Omit<React.HTMLAttributes<HTMLDivElement>, 'children'> { @@ - onClick: () => void; + // HTMLAttributes에 onClick이 있으므로 재정의하지 않거나, + // 필요 시 헤더 클릭을 분리해 명확히 표현합니다. + onHeaderClick?: React.MouseEventHandler<HTMLDivElement>; }그리고 사용처:
- onClick={onClick} + onClick={onHeaderClick}apps/client/src/shared/components/sidebar/Sidebar.tsx (8)
52-61: 내비게이션 시맨틱 강화: aside에 aria-label 또는 nav 요소 사용보조 영역이 아닌 주요 내비게이션이라면 nav 요소가 더 적합합니다. aside를 유지한다면 role/label을 부여하세요.
- return ( - <aside className="bg-white-bg sticky top-0 h-screen w-[24rem] border-r border-gray-300"> + return ( + <aside + className="bg-white-bg sticky top-0 h-screen w-[24rem] border-r border-gray-300" + role="navigation" + aria-label="사이드바 내비게이션" + >
70-75: 중복 상태 업데이트 제거: goRemind가 이미 선택 해제 처리
useSidebarNav().goRemind()가 selectedCategoryId를 null로 설정합니다. 중복 호출로 불필요한 리렌더가 발생할 수 있습니다.- setSelectedCategoryId(null); closeMenu(); goRemind();
82-86: 중복 상태 업데이트 제거: goBookmarks가 선택 해제 포함위와 동일 맥락입니다.
- setSelectedCategoryId(null); closeMenu(); goBookmarks();
123-127: 중복 상태 업데이트 제거: goLevel이 선택 해제 포함동일 맥락.
- setSelectedCategoryId(null); closeMenu(); goLevel();
107-116: 불필요한 nullish 병합 제거
style ?? undefined는style과 동일하게 동작합니다.- style={style ?? undefined} + style={style}
139-146: 콜백 시그니처 정합성: id 파라미터 수용
PopupPortal의 onEditConfirm/onDeleteConfirm가 id를 인자로 넘겨줍니다. 시그니처를 맞춰 두면 타입 안정성이 좋아집니다.- onEditConfirm={() => { + onEditConfirm={(_id) => { // TODO: 수정 API close(); }} - onDeleteConfirm={() => { + onDeleteConfirm={(_id) => { // TODO: 삭제 API close(); }}
27-36: 라우터 파생 상태로 단일 소스화(SSOT) 고려activeTab/selectedCategoryId를 URL(pathname/query)에서 파생하면 뒤로가기/딥링크 시 하이라이트 드리프트를 방지할 수 있습니다. 현재 훅(
useSidebarNav) 내부에서 상태를 직접 관리하므로, 라우터 훅(useLocation/useSearchParams) 기반으로 재구성하는 것을 제안합니다.
56-61: 로고 A11Y: 장식 아이콘이면 aria-hidden, 브랜드 텍스트는 sr-only로현재 aria-label이 아이콘에 붙어 있습니다. 링크/헤딩이 아니라면
aria-hidden처리하고, 브랜드명은 시각적 숨김 텍스트로 제공하는 구성이 일반적입니다.- <Icon - name="logo" - aria-label="Pinback 로고" - className="h-[2.4rem] w-[8.7rem]" - /> + <span className="sr-only">Pinback</span> + <Icon name="logo" aria-hidden className="h-[2.4rem] w-[8.7rem]" />
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/client/src/shared/components/sidebar/AccordionItem.tsx(1 hunks)apps/client/src/shared/components/sidebar/SideItem.tsx(1 hunks)apps/client/src/shared/components/sidebar/Sidebar.tsx(1 hunks)apps/client/src/shared/components/sidebar/types/IconTokenType.ts(1 hunks)apps/client/src/shared/hooks/useAnchoredMenu.ts(1 hunks)apps/client/src/shared/hooks/useCategoryPopups.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/client/src/shared/hooks/useCategoryPopups.ts
- apps/client/src/shared/components/sidebar/SideItem.tsx
- apps/client/src/shared/hooks/useAnchoredMenu.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: jjangminii
PR: Pinback-Team/pinback-client#52
File: apps/client/src/shared/components/sidebar/SideItem.tsx:57-60
Timestamp: 2025-09-03T08:57:48.626Z
Learning: SideItem 컴포넌트에서 아이콘 디자인이 일반적인 기대와 반대여서, 회전 로직도 반대로 구현되었습니다. 이를 명확히 하기 위해 prop 이름을 open에서 close로 변경하여 실제 동작과 일치시켰습니다.
🧬 Code graph analysis (2)
apps/client/src/shared/components/sidebar/Sidebar.tsx (11)
apps/client/src/shared/hooks/useSidebarNav.ts (1)
useSidebarNav(6-50)apps/client/src/shared/hooks/useAnchoredMenu.ts (1)
useAnchoredMenu(13-86)apps/client/src/shared/utils/anchorPosition.ts (1)
rightOf(2-5)apps/client/src/shared/hooks/useCategoryPopups.ts (1)
useCategoryPopups(9-18)apps/client/src/shared/components/sidebar/SideItem.tsx (1)
SideItem(21-68)apps/client/src/shared/components/sidebar/AccordionItem.tsx (1)
AccordionItem(20-68)apps/client/src/shared/components/sidebar/CategoryItem.tsx (1)
CategoryItem(13-61)apps/client/src/shared/components/sidebar/CreateItem.tsx (1)
CreateItem(9-27)apps/client/src/shared/components/sidebar/OptionsMenuPortal.tsx (1)
OptionsMenuPortal(15-45)apps/client/src/shared/components/sidebar/MyLevelItem.tsx (1)
MyLevelItem(13-61)apps/client/src/shared/components/sidebar/PopupPortal.tsx (1)
PopupPortal(13-65)
apps/client/src/shared/components/sidebar/AccordionItem.tsx (3)
apps/client/src/shared/components/sidebar/types/IconTokenType.ts (1)
IconToken(1-1)packages/design-system/src/lib/utils.ts (1)
cn(108-110)apps/client/src/shared/components/sidebar/SideItem.tsx (1)
SideItem(21-68)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: storybook
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/client/src/shared/types/IconTokenType.ts (2)
1-1: ICON_MAP 키에서 타입 유도 고려(향후 아이콘 추가 시 드리프트 방지)가능하면 ICON_MAP을 shared 계층(예: shared/icons/iconMap.ts)로 올리고
export type IconToken = keyof typeof ICON_MAP로 유도하면 토큰-맵 간 불일치가 원천 차단됩니다. 레이어링만 정리되면 가장 견고합니다.
1-1: IconToken 타입 단일화 및 런타임 배열 도입 제안중복 정의 파일:
- apps/client/src/shared/types/IconTokenType.ts
- apps/client/src/shared/components/sidebar/types/IconTokenType.ts
아래와 같이 런타임 배열과 유도 타입으로 대체해 유지보수성을 높이세요:
-export type IconToken = 'clock' | 'bookmark'; +export const ICON_TOKENS = ['clock', 'bookmark'] as const; +export type IconToken = typeof ICON_TOKENS[number];중복 파일 중 하나를 제거하고, 모든 import 경로를
apps/client/src/shared/types/IconTokenType.ts로 통일하세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/client/src/shared/components/sidebar/Sidebar.tsx(1 hunks)apps/client/src/shared/types/IconTokenType.ts(1 hunks)apps/client/src/shared/utils/anchorPosition.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/client/src/shared/components/sidebar/Sidebar.tsx
- apps/client/src/shared/utils/anchorPosition.ts
3 participants
📌 Related Issues
📄 Tasks
사이드바 기본 컴포넌트 구성
SideItem 베이스(아이콘+라벨, 활성/비활성 스타일)
AccordionItem 확장(SideItem 확장)
상태 분리: active(라우팅 하이라이트) ↔ open(아코디언 열림) 독립 제어
AccordionItem: open(controlled) / defaultOpen(uncontrolled) / onOpenChange 지원
아이콘 타입 안전화
IconToken = 'clock' | 'bookmark'
ICON_MAP으로 IconToken → IconName 매핑(활성/비활성 자동 전환)
드롭다운 애니메이션
CSS Grid grid-rows-[0fr] ↔ 1fr + transition-[grid-template-rows]로 부드러운 접힘/펼침
⭐ PR Point (To Reviewer)
아이콘 타입 안전화
아이콘을 임의의 string으로 받으면 스프라이트 id에 직접 의존하게 되고, 오타/미정의 값이 런타임에서야 드러납니다. 이를 피하려고 토큰('clock' | 'bookmark')만 받도록 유니온 타입으로 제한하고, 내부에서는 ICON_MAP으로 실제 스프라이트 이름(on/off)을 매핑했습니다.
이렇게 하면 컴파일 타임에 오류를 차단 사용자는 토큰만 넘기면 되고, 실제 id 변경 시 매핑만 수정하면 되어 유지보수 용이하다 판단했습니다.
하지만 이 방식은 추후 아이콘 타입이 늘어나면 유지보수에 좋지않다고 판단되는데 어떻게 생각하시나요? 혹시 다른 좋은 방법이 있다면 공유해주세요-!
(활성/비활성 쌍도 타입으로 설정)
헤드리스+컴파운드패턴..
헤드리스+컴파운드패턴을 사용해보려했지만.. 현재 단계에서는 오히려 코드의 복잡도가 올라가고 피그마상으로 잘못 이해한 부분이 있었어서 적용하지 못했습니다.. 이떄문에 코드 수정에 시간이 걸렸고 NavLink만 사용해서 네비 로직만 사용처에서 주입하도록 했습니다 NavLink와 active 이용해서 현재 페이지일 시 활성화 디자인 분기처리 해뒀습니다
useState 상태관리
useState로 카테고리와 현재 탭바 경로 비교하면서 현재위치일 시 활성화 디자인 실행되도록 하였습니다
사이드바 완성하면서 추가적으로 생긴 파일들입니다
📷 Screenshot
2025-09-03.5.34.37.mov
2025-09-06.5.14.46.mov
Summary by CodeRabbit
새 기능
스타일
Chores