-
Notifications
You must be signed in to change notification settings - Fork 2
✨ feat: 사이드바 컴포넌트 구현 #42
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
Conversation
Walkthrough이 변경 사항은 사이드바와 대시보드 관련 UI 컴포넌트, 타입 정의, 그리고 테스트 페이지 레이아웃을 추가 및 리팩토링합니다. 사이드바와 대시보드 항목, 생성 버튼 컴포넌트가 새로 도입되었으며, 관련 타입스크립트 인터페이스도 추가되었습니다. 테스트 페이지는 새로운 2단 레이아웃으로 재구성되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Sidebar
participant DashboardItem
participant CreateDashboardButton
participant Router
User->>Sidebar: 페이지 진입
Sidebar->>DashboardItem: 대시보드 목록 렌더링
User->>DashboardItem: 대시보드 항목 클릭
DashboardItem->>Sidebar: onClick(dashboardId)
Sidebar->>Router: 대시보드 상세 페이지로 이동
User->>CreateDashboardButton: 대시보드 생성 버튼 클릭
CreateDashboardButton->>Sidebar: onClick()
Sidebar->>Sidebar: (현재 콘솔 로그, 추후 모달 등 구현 예정)
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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 (5)
src/app/shared/components/common/sidebar/CreateDashboardButton.tsx (1)
11-15: 버튼type속성 명시 권장폼 내부에서 사용될 가능성을 대비해
type="button"을 명시해 두면 예기치 않게submit동작이 발생하는 것을 방지할 수 있습니다.- <button + <button + type="button" onClick={onClick} className="flex size-20 items-center justify-center rounded-6 transition-colors hover:bg-gray-50" aria-label="새 대시보드 생성"src/app/tester/page.tsx (1)
21-26:flex컨테이너 불필요 / 레이아웃 간결화 제안
Sidebar가position: fixed이므로 부모에flex를 걸어둘 필요가 없습니다.
오히려 불필요한 flex 컨텍스트로 인해 예상치 못한 정렬 이슈가 생길 수 있습니다.- <div className="flex"> + <div>또는 콘텐츠 영역에
pl-300(padding-left) 를 주어 사이드바 넓이만큼 공간을 확보하는 편이 더 단순합니다.src/app/shared/components/common/sidebar/DashboardItem.tsx (1)
17-21: 접근성과 기본 동작 개선 –type·aria-current추가
- 버튼 기본
type을 명시해 폼 안에서의 암묵적 submit 방지- 현재 선택된 대시보드임을 스크린리더에 전달하기 위해
aria-current="page"사용- <button - onClick={handleClick} + <button + type="button" + aria-current={isActive ? 'page' : undefined} + onClick={handleClick}src/app/shared/components/common/sidebar/Sidebar.tsx (2)
14-61:mockDashboards재생성 방지컴포넌트가 리렌더링될 때마다 새로운 배열·객체가 생성되어 자식
DashboardItem의 불필요한 리렌더를 유발할 수 있습니다. 상수라면 파일 최상단으로 빼거나useMemo로 메모이즈하는 편이 좋습니다.- const mockDashboards = [ +const mockDashboards = [ /* ... */ - ] +] as const
63-70: 이벤트 핸들러useCallback래핑 고려
handleDashboardClick/handleCreateDashboard가 렌더마다 새로 생성되어 하위 컴포넌트에 전달됩니다. 리렌더 최적화를 원한다면useCallback으로 래핑해 불필요한 prop 변경을 줄일 수 있습니다.- const handleDashboardClick = (dashboardId: number) => { - router.push(`/dashboard/${dashboardId}`) - } + const handleDashboardClick = useCallback( + (dashboardId: number) => router.push(`/dashboard/${dashboardId}`), + [router], + )[nitpick]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/images/logo-light2.svgis excluded by!**/*.svg
📒 Files selected for processing (5)
src/app/shared/components/common/sidebar/CreateDashboardButton.tsx(1 hunks)src/app/shared/components/common/sidebar/DashboardItem.tsx(1 hunks)src/app/shared/components/common/sidebar/Sidebar.tsx(1 hunks)src/app/shared/types/dashboard.ts(1 hunks)src/app/tester/page.tsx(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/app/shared/components/common/sidebar/CreateDashboardButton.tsx (1)
src/app/shared/types/dashboard.ts (1)
CreateDashboardButtonProps(26-28)
src/app/shared/components/common/sidebar/DashboardItem.tsx (1)
src/app/shared/types/dashboard.ts (1)
DashboardItemProps(20-24)
src/app/shared/components/common/sidebar/Sidebar.tsx (2)
src/app/shared/components/common/sidebar/CreateDashboardButton.tsx (1)
CreateDashboardButton(7-26)src/app/shared/components/common/sidebar/DashboardItem.tsx (1)
DashboardItem(7-45)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: eslint-check
🔇 Additional comments (1)
src/app/shared/types/dashboard.ts (1)
1-35: 타입 정의 전반적으로 양호도메인 모델과 컴포넌트 Props 가 명확히 분리되어 있어 가독성이 좋습니다.
추후 날짜 필드는string대신Date파싱 결과 객체로 전환할 여지가 있으나, 현 단계에서는 문제 없습니다.
dkslel1225
left a 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.
사이드바 구현 수고하셨습니다~~ 컴포넌트 분리해서 구현해주셨군요!👍👍 API 연결까지 마무리되면 가져다 쓰겠습니당⛄️
Insung-Jo
left a 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.
작업 수고 많으셨습니다! 접근성도 고려하신 부분도 보이는 거 같아서 공부가 되는 거 같습니다! 궁금한 점은 리뷰로 남겼으니 자유롭게 답변해주시면 감사하겠습니다!👍 👍
| import CreateDashboardButton from './CreateDashboardButton' | ||
| import DashboardItem from './DashboardItem' | ||
|
|
||
| export default function Sidebar(): JSX.Element { |
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.
ReactNode는 사용해본 기억이 있는데 JSX.Element는 처음 보는 거 같습니다! 어떤 타입인지 설명해주실 수 있을까요?
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.
JSX.Element는 JSX로 작성된 React 엘리먼트만을 나타내는 타입이예요. <div>, <Component /> 같은 JSX 표현식의 결과물을 의미한다고 보시면 됩니다. Sidebar 컴포넌트에서는 항상 <aside> JSX를 반환하고 조건부 렌더링이나 null 변환이 없어서 JSX.Element가 더 명확하다고 생각해서 사용했습니다.
| <div className="px-20 py-24"> | ||
| {/* 섹션 헤더 */} | ||
| <div className="mb-24 flex items-center justify-between"> | ||
| <h2 className="Text-gray text-12 font-semibold">Dash Boards</h2> | ||
| <CreateDashboardButton onClick={handleCreateDashboard} /> | ||
| </div> | ||
|
|
||
| {/* 대시보드 목록 */} | ||
| <div className="space-y-8"> | ||
| {mockDashboards.map((dashboard) => ( | ||
| <DashboardItem | ||
| key={dashboard.id} | ||
| dashboard={dashboard} | ||
| isActive={pathname === `/dashboard/${dashboard.id}`} | ||
| onClick={handleDashboardClick} | ||
| /> | ||
| ))} | ||
| </div> | ||
| </div> |
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.
이 부분은 추후에 시멘틱까지 고려한다면 nav, ul,li 태그를 활용해볼 수 있을 거 같습니다!
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.
감사합니다 :) 추후에 여유가 생기면 시멘틱까지 고려해서 수정해봐야겠네요.
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.
👍👍
| return ( | ||
| <button | ||
| type="button" | ||
| aria-current={isActive ? 'page' : undefined} |
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.
aria-label은 사용해 봤는데 aria-current는 처음 보는 거 같습니다! 이것은 어떤 역할은 하는 건가요?
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.
저도 처음에는 ARIA없이 구현을 했었는데, 따로 AI 코드리뷰 과정에서 스크린 리더 사용자들은 시각적 표현을 인식할 수 없어서 현재 위치를 알기 어렵다는 피드백을 주더라구요. 그래서 따로 웹 접근성을 고려해 한번 추가해봤습니다. 물론 기능적으로는 없어도 동작하지만요. 필수는 아니지만 그냥 습관을 기른다고 생각하는 차원에서 반영해봤어요. 리뷰 덕분에 WAI-ARIA 관련해서 이런것들이 있구나 하고 찾아보게 되었네요 :)
알면 알수록 점점 알아갈게 많아지는거 같네요...
yuj2n
left a 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.
찬호님 작업 수고하셨습니다~~
| <button | ||
| type="button" | ||
| onClick={onClick} | ||
| className="flex size-20 items-center justify-center rounded-6 transition-colors hover:bg-gray-50" |
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.
background 색상의 경우 global 스타일로 빼서 사용하면 좋지 않을까 합니다!!
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.
감사합니다 :) 중복으로 사용할 일이 생기면 global 스타일로 빼서 올려두겠습니다 👍
📌 변경 사항 개요
사이드바 컴포넌트 구현 및 대시보드 네비게이션
✨ 요약
📝 상세 내용
1. Sidebar.tsx - 메인 사이드바 컨테이너
2. DashBoardItem.tsx - 개별 대시보드 아이템
3. CreateDashboardButton.tsx - 대시보드 생성 버튼
4. dashboard.ts - 타입 정의
🔗 관련 이슈
#41
🖼️ 스크린샷
✅ 체크리스트
💡 참고 사항
Summary by CodeRabbit