Repository navigation
[FE] SISC1-198 [FEAT] 헤더 적용 및 토글 사이드바 전체 페이지 적용 - #81
Conversation
Walkthrough헤더(UI)를 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Header
participant Layout
participant Sidebar
rect rgba(100,150,240,0.08)
User->>Header: 메뉴 버튼 클릭
Header->>Layout: onToggleSidebar() 호출
Layout->>Layout: isSidebarOpen 토글
Layout->>Header: isOpen prop 갱신
Layout->>Sidebar: isOpen prop 갱신
Sidebar->>Sidebar: open/closed 클래스 적용
end
rect rgba(120,200,160,0.06)
User->>Header: 브랜드(로고/타이틀) 클릭
Header->>Header: useNavigate 호출 (경로 이동)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
frontend/src/components/Layout.jsx (1)
19-27: 선택적 리팩토링: CSS 모듈 사용을 고려하세요.인라인 스타일이 작동하지만, 일관성을 위해 CSS 모듈로 추출하는 것을 권장합니다. 특히
paddingTop과marginLeft값은 유지보수성을 위해 CSS 변수나 모듈 클래스로 관리하는 것이 좋습니다.예시:
Layout.module.css파일을 생성하고:.mainContent { display: flex; position: relative; padding-top: 90px; transition: margin-left 0.3s ease; } .mainContent.sidebarOpen { margin-left: 264px; }그리고 컴포넌트에서 사용:
+import styles from './Layout.module.css'; -<main - style={{ - display: 'flex', - position: 'relative', - paddingTop: '90px', - marginLeft: isSidebarOpen && !isRoot ? '264px' : '0', - transition: 'margin-left 0.3s ease', - }} -> +<main className={`${styles.mainContent} ${isSidebarOpen && !isRoot ? styles.sidebarOpen : ''}`}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
frontend/src/components/Header.jsx(1 hunks)frontend/src/components/Header.module.css(1 hunks)frontend/src/components/Layout.jsx(1 hunks)frontend/src/components/Sidebar.jsx(1 hunks)frontend/src/components/Sidebar.module.css(0 hunks)frontend/src/pages/Home.jsx(0 hunks)frontend/src/pages/Home.module.css(0 hunks)
💤 Files with no reviewable changes (3)
- frontend/src/pages/Home.module.css
- frontend/src/pages/Home.jsx
- frontend/src/components/Sidebar.module.css
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/components/Header.jsx (2)
frontend/src/pages/Home.jsx (1)
nav(7-37)frontend/src/App.jsx (1)
App(15-34)
frontend/src/components/Layout.jsx (2)
frontend/src/components/Header.jsx (1)
Header(5-35)frontend/src/components/Sidebar.jsx (1)
Sidebar(4-122)
🔇 Additional comments (3)
frontend/src/components/Layout.jsx (1)
2-18: 잘 구현되었습니다!헤더와 사이드바 상태 관리가 깔끔하게 구현되었습니다.
useState를 사용한 토글 로직과 Props 전달이 올바르게 작동합니다.frontend/src/components/Header.jsx (1)
10-19: 훌륭한 접근성 구현입니다!메뉴 버튼에
aria-label과aria-expanded속성을 올바르게 사용하여 스크린 리더 사용자를 위한 접근성이 잘 구현되었습니다.frontend/src/components/Sidebar.jsx (1)
4-122: 훌륭한 리팩토링입니다!상태를 부모 컴포넌트로 이동시키고 Props 기반 렌더링으로 변경한 것은 좋은 설계입니다. 컴포넌트가 더 재사용 가능하고 테스트하기 쉬워졌습니다.
aria-hidden속성과NavLink의 활성/비활성 스타일링도 올바르게 구현되었습니다.
| <div className={styles.right}> | ||
| <button className={styles.login} onClick={() => nav('/login')}> | ||
| 로그인 | ||
| </button> | ||
| <button className={styles.signUp} onClick={() => nav('signup')}> | ||
| 회원가입 | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
경로 표기 방식을 일관되게 유지하세요.
로그인 버튼은 절대 경로('/login')를 사용하지만, 회원가입 버튼은 상대 경로('signup')를 사용합니다. 동일한 레벨의 라우트이므로 일관성을 위해 둘 다 절대 경로를 사용하는 것이 좋습니다.
다음 diff를 적용하세요:
<button className={styles.login} onClick={() => nav('/login')}>
로그인
</button>
-<button className={styles.signUp} onClick={() => nav('signup')}>
+<button className={styles.signUp} onClick={() => nav('/signup')}>
회원가입
</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 className={styles.right}> | |
| <button className={styles.login} onClick={() => nav('/login')}> | |
| 로그인 | |
| </button> | |
| <button className={styles.signUp} onClick={() => nav('signup')}> | |
| 회원가입 | |
| </button> | |
| </div> | |
| <div className={styles.right}> | |
| <button className={styles.login} onClick={() => nav('/login')}> | |
| 로그인 | |
| </button> | |
| <button className={styles.signUp} onClick={() => nav('/signup')}> | |
| 회원가입 | |
| </button> | |
| </div> |
🤖 Prompt for AI Agents
In frontend/src/components/Header.jsx around lines 25 to 32, the route usage is
inconsistent: the login button uses an absolute path ('/login') while the signup
button uses a relative path ('signup'); update the signup button to use the
absolute path '/signup' (i.e., change the nav call to navigate to '/signup') so
both buttons use consistent absolute route notation.
| font-weight: 600; | ||
| font-size: 18.53px; | ||
| line-height: 120%; | ||
| letter-spacing: -3%; |
There was a problem hiding this comment.
잘못된 CSS 문법을 수정하세요.
letter-spacing 속성에 -3%가 사용되었지만, 이는 유효하지 않은 CSS 문법입니다. letter-spacing은 길이 단위(px, em, rem 등)만 허용하며 퍼센트 값은 지원하지 않습니다. 브라우저가 이 속성을 무시하게 됩니다.
다음 diff를 적용하여 유효한 단위로 변경하세요:
.title {
font-weight: 600;
font-size: 18.53px;
line-height: 120%;
- letter-spacing: -3%;
+ letter-spacing: -0.03em;
color: white;
}
.login {
border: none;
border-right: 1px solid #a3a3a3;
background: none;
color: #ffffff;
cursor: pointer;
font-weight: 500;
font-size: 16px;
line-height: 120%;
- letter-spacing: -3%;
+ letter-spacing: -0.03em;
width: 90px;
}
.signUp {
border: none;
border-left: 1px solid #a3a3a3;
background: none;
color: #ffffff;
cursor: pointer;
font-weight: 500;
font-size: 16px;
line-height: 120%;
- letter-spacing: -3%;
+ letter-spacing: -0.03em;
width: 100px;
}Also applies to: 76-76, 89-89
🤖 Prompt for AI Agents
In frontend/src/components/Header.module.css at lines 39, 76 and 89, the
letter-spacing values use the invalid percentage unit (-3%); replace those
percent values with a valid CSS length unit (for example -0.03em or -3px) so the
property is recognized by browsers — update each occurrence to a concrete length
unit consistent with the design (choose em for scalable type or px for fixed
spacing).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/components/Sidebar.jsx (1)
18-27: 주석 처리된 코드 제거 필요주석 처리된 "홈" 링크 코드는 데드 코드입니다. 필요 시 Git 히스토리에서 복원할 수 있으므로 제거하는 것이 좋습니다.
<ul> - {/* <li> - <NavLink - to="/" - className={({ isActive }) => - isActive ? styles['active-link'] : styles['inactive-link'] - } - > - 홈 - </NavLink> - </li> */} <li> <NavLink
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/src/components/Header.jsx(1 hunks)frontend/src/components/Header.module.css(1 hunks)frontend/src/components/Layout.jsx(1 hunks)frontend/src/components/Sidebar.jsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/components/Layout.jsx
- frontend/src/components/Header.module.css
- frontend/src/components/Header.jsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/components/Sidebar.jsx (3)
frontend/src/components/Header.jsx (2)
isRoot(7-7)nav(8-8)frontend/src/components/Layout.jsx (1)
isRoot(9-9)frontend/src/pages/Attendance.jsx (1)
nav(15-15)
🔇 Additional comments (2)
frontend/src/components/Sidebar.jsx (2)
1-4: 상태 관리 제거 및 제어 컴포넌트로 리팩토링 완료내부 상태와 불필요한 의존성을 제거하고 부모 컴포넌트로부터
isOpen,isRootprops를 받도록 변경한 것은 올바른 설계입니다. 관심사의 분리가 잘 이루어졌습니다.
14-137: 네비게이션 구조 및 링크 구현 양호고정된 메뉴 섹션 구조와
NavLink의isActive콜백을 활용한 스타일링 패턴이 일관되고 명확합니다. 코드가 깔끔하게 정리되어 있습니다.
| <div | ||
| className={`${styles.homeSidebarMenu} ${ | ||
| !isOpen && isRoot ? styles.closed : styles.open | ||
| }`} | ||
| aria-hidden={!isOpen} | ||
| > |
There was a problem hiding this comment.
className 조건 로직 오류
9-11번 줄의 조건식이 잘못되었습니다. 현재 로직은 다음과 같이 동작합니다:
- 사이드바가 닫혀있고(
isOpen=false) 루트가 아닌 페이지에서는styles.open이 적용됨 - 이는 의도와 반대되는 동작입니다
다음 diff를 적용하여 수정하세요:
<div
className={`${styles.homeSidebarMenu} ${
- !isOpen && isRoot ? styles.closed : styles.open
+ isOpen ? styles.open : styles.closed
}`}
aria-hidden={!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 | |
| className={`${styles.homeSidebarMenu} ${ | |
| !isOpen && isRoot ? styles.closed : styles.open | |
| }`} | |
| aria-hidden={!isOpen} | |
| > | |
| <div | |
| className={`${styles.homeSidebarMenu} ${ | |
| isOpen ? styles.open : styles.closed | |
| }`} | |
| aria-hidden={!isOpen} | |
| > |
🤖 Prompt for AI Agents
In frontend/src/components/Sidebar.jsx around lines 8 to 13, the conditional for
className is inverted: it currently applies styles.open when the sidebar is
closed on non-root pages. Change the condition to check !isRoot instead of
isRoot so the closed class is applied when the sidebar is closed AND the page is
not the root (i.e., use !isOpen && !isRoot ? styles.closed : styles.open); if
you actually need different behavior on the root page, make that logic explicit
and comment it.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/components/Header.jsx(1 hunks)frontend/src/components/Layout.jsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/Layout.jsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/components/Header.jsx (1)
frontend/src/components/Layout.jsx (1)
isRoot(9-9)
🔇 Additional comments (2)
frontend/src/components/Header.jsx (2)
1-29: 과거 리뷰 코멘트가 더 이상 유효하지 않습니다.이전 리뷰에서 지적된 로그인/회원가입 버튼이 현재 코드에는 존재하지 않습니다. 해당 기능이 제거되었거나 다른 위치로 이동한 것으로 보입니다.
9-20: 접근성 구현이 잘 되어 있습니다.메뉴 버튼에
aria-label과aria-expanded속성을 적절히 사용하여 스크린 리더 사용자를 위한 접근성을 잘 고려했습니다.
| <div className={styles.brand} onClick={() => nav('/')}> | ||
| <img className={styles.logo} src={Logo} alt="세종투자연구회 로고" /> | ||
| <span className={styles.title}>세종투자연구회</span> | ||
| </div> |
There was a problem hiding this comment.
브랜드 영역을 키보드로 접근 가능하도록 변경하세요.
현재 div 요소에 onClick 핸들러를 사용하고 있어 키보드 사용자가 탭 키로 접근하거나 엔터 키로 활성화할 수 없습니다. 버튼 또는 링크 요소를 사용해야 합니다.
다음 diff를 적용하여 접근성을 개선하세요:
- <div className={styles.brand} onClick={() => nav('/')}>
+ <button
+ className={styles.brand}
+ onClick={() => nav('/')}
+ aria-label="홈으로 이동"
+ >
<img className={styles.logo} src={Logo} alt="세종투자연구회 로고" />
<span className={styles.title}>세종투자연구회</span>
- </div>
+ </button>참고: CSS에서 button 요소의 기본 스타일을 제거해야 할 수 있습니다:
.brand {
background: none;
border: none;
cursor: pointer;
/* 기존 스타일 유지 */
}🤖 Prompt for AI Agents
In frontend/src/components/Header.jsx around lines 21 to 24, replace the
clickable div with a semantic, keyboard-focusable element (preferably a <button
type="button"> or an <a> if it navigates) so keyboard users can tab to and
activate the brand; keep the existing onClick={() => nav('/')} handler, add a
clear aria-label (e.g., aria-label="홈으로 이동"), and remove any JS-only key
handlers; update CSS to neutralize button defaults (background: none; border:
none; cursor: pointer;) so visual styling is preserved.
1 participant
1) 작업한 이슈번호
SISC1-198
2) 변경 요약 (What & Why)
3) 스크린샷/동영상 (UI 변경 시)
4) 상세 변경사항 (전부 다)
5) 참고사항
Summary by CodeRabbit
릴리스 노트
새로운 기능
제거
개선 사항