[feature] 글 영역 박스를 제거하고 레이아웃 구분선을 추가한다#738
Conversation
…MOA-213 [feature] 사이드바가 스크롤을 따라온다
suhyun113
requested review from
Zepelown,
alsdddk,
lepitaaar,
oesnuj and
seongwon030
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
AdminPage 레이아웃 스타일frontend/src/pages/AdminPage/AdminPage.styles.ts |
새 Divider export 추가(세로 1px, position: sticky, top: 98px, height: calc(100vh - 98px), margin: 0 34px). AdminPageContainer에서 gap: 34px 제거, align-items: flex-start 추가. Content의 border/border-radius 제거; 기존 패딩(62px 58px) 및 width/max-width 유지. |
AdminPage 컴포넌트 변경frontend/src/pages/AdminPage/AdminPage.tsx |
Divider를 임포트하고 사이드바와 콘텐츠 사이에 <Styled.Divider /> 삽입하여 레이아웃에 적용. |
Sidebar 스타일 조정frontend/src/pages/AdminPage/components/SideBar/SideBar.styles.ts |
SidebarWrapper에 position: sticky; top: 98px; height: fit-content; 추가(스크롤과의 시각적 일치 목적). |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related issues
- MOA-230 — 요구 사항(글 영역 박스 제거 및 레이아웃 구분선 추가, 사이드바 스티키 처리)과 본 PR의 변경 내용이 일치함.
- [feature] MOA-230 글 영역 박스를 제거하고 레이아웃 구분선을 추가한다 #731 — 동일한 UI 변화(콘텐츠 박스 제거 및 레이아웃 구분선/사이드바 sticky)와 직접적으로 관련됨.
Possibly related PRs
- [feature] 사이드바가 스크롤을 따라온다 #730 — 동일한
Sidebar.styles.ts변경(사이드바 sticky)과 겹침. - [feature] 사이드바 내용을 계층 구조로 변경한다 #711 — Divider 관련 스타일/익스포트 변경과 코드 영역이 겹침.
Suggested reviewers
- Zepelown
- lepitaaar
✨ Finishing touches
- 📝 Generate Docstrings
🧪 Generate unit tests
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feature/#731-add-layout-divider-MOA-230
📜 Recent review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
frontend/src/pages/AdminPage/AdminPage.styles.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/pages/AdminPage/AdminPage.styles.ts
Tip
👮 Agentic pre-merge checks are now available in preview!
Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
- Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
- Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.
Please see the documentation for more information.
Example:
reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post.
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 help to get the list of available commands and usage tips.
Pre-merge checks
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Linked Issues Check | ✅ Passed | 링크된 이슈 MOA-230의 주요 요구사항인 글 영역 박스 제거, 레이아웃 구분선 추가, 사이드바/콘텐츠 스크롤 개선에 대해 PR 변경사항은 코드 수준에서 주요 항목을 반영하고 있습니다: Content에서 테두리/둥근 모서리 제거, Divider 스타일 추가 및 AdminPage에 삽입, SidebarWrapper에 sticky 속성 적용으로 고정 동작을 조정한 점이 확인됩니다. 다만 PR 설명에 '상단 영역 스크롤 처리 불확실'이 남아 있어 스크롤 엣지케이스(특히 상단 노출/오프셋)는 디자인 검토와 QA로 추가 확인이 필요합니다. |
| Out of Scope Changes Check | ✅ Passed | 수정 파일들은 AdminPage 및 SideBar 관련 스타일과 레이아웃에 국한되어 있어 링크된 이슈의 목적(글 영역 박스 제거, 구분선 추가, 스크롤 동작 개선)과 직접 연관되며 범위 외 변경은 발견되지 않습니다. 다만 요약에서 AdminPage.styles.ts에 Divider를 새로 추가했다고 하나 AdminPage.tsx는 SideBar.styles에서 Divider를 import한 것으로 기재되어 있어 모듈 경로/내보내기 일관성은 확인이 필요합니다. |
| Docstring Coverage | ✅ Passed | No functions found in the changes. Docstring coverage check skipped. |
| Title Check | ✅ Passed | 제목 "[feature] 글 영역 박스를 제거하고 레이아웃 구분선을 추가한다"는 PR의 주요 변경사항인 글 영역 박스 제거와 사이드바-콘텐츠 사이 구분선 추가를 명확히 요약합니다. 표현이 간결하고 구체적이며 불필요한 파일 목록이나 이모지 같은 잡음이 없어 히스토리에서 변경 의도를 빠르게 파악할 수 있습니다. 따라서 PR의 목적과 주요 변경을 잘 반영하고 있습니다. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
frontend/src/pages/AdminPage/components/SideBar/SideBar.styles.ts (3)
11-13: sticky 오프셋(98px) 매직 넘버 상수화 + sticky 컨테이너 overflow 확인 요청
- 98px이 여러 파일에 중복됩니다. 상수(또는 테마 토큰)로 추출해 동일하게 참조하세요.
height: fit-content;는 없어도 자연 동작할 가능성이 높습니다. 필요성 재검토 바랍니다.- 부모 컨테이너에
overflow가 설정되어 있으면 sticky가 무력화됩니다. 상위 레이아웃의 overflow 유무를 확인해 주세요.변경 예시(해당 파일 내부 상수 사용):
+const HEADER_HEIGHT_PX = 98; + export const SidebarWrapper = styled.aside` ... - top: 98px; + top: ${HEADER_HEIGHT_PX}px; - height: fit-content; + /* 필요 시 유지, 아닌 경우 제거 고려 */ + /* height: fit-content; */ `;
4-6: align-items 값이 유효하지 않습니다: 'left' → 'flex-start'로 수정 필요CSS 표준에
align-items: left는 없습니다. 브라우저가 무시하여 레이아웃 의도와 달라질 수 있습니다.export const SidebarWrapper = styled.aside` display: flex; flex-direction: column; - align-items: left; + align-items: flex-start; ... `;
55-61: font-weight: medium 미지원 값 사용 → 수치(500/600) 또는 토큰으로 교체
font-weight: medium은 표준이 아니며 무시될 수 있습니다. 디자인 가이드에 맞춰 500(=Medium) 등 수치 사용을 권장합니다.export const SidebarCategoryTitle = styled.p` ... - font-weight: medium; + font-weight: 500; ... `; export const SidebarButton = styled.button` ... - font-weight: medium; + font-weight: 500; ... &.active { background-color: rgba(255, 117, 67, 1); color: white; - font-weight: medium; + font-weight: 600; /* 강조용(필요 시 조정) */ } `;Also applies to: 76-78, 81-85
frontend/src/pages/AdminPage/AdminPage.styles.ts (2)
9-17: 세로 구분선 컴포넌트 네이밍 충돌 방지 + 매직 넘버(98/34) 상수화
- 동일 리포 내
Divider(가로선, SideBar)와 이름이 겹쳐 혼동됩니다.VerticalDivider등으로 명확히 해주세요.98px,34px중복 사용을 상수/테마 토큰화하세요.- 모바일 환경을 고려하면
100vh대신100dvh검토를 권장합니다. (관리자 화면이 데스크톱 한정이라면 현 상태 유지 가능)-export const Divider = styled.div` +export const VerticalDivider = styled.div` - position: sticky; - top: 98px; + position: sticky; + top: ${HEADER_HEIGHT_PX}px; width: 1px; - height: calc(100vh - 98px); + /* 모바일 대응 필요 시: height: calc(100dvh - ${HEADER_HEIGHT_PX}px); */ + height: calc(100vh - ${HEADER_HEIGHT_PX}px); background-color: #dcdcdc; - margin: 0 34px; + margin: 0 ${LAYOUT_GAP_PX}px; flex-shrink: 0; `;파일 상단 등 적절한 위치에 상수 선언(또는 테마 토큰 사용):
const HEADER_HEIGHT_PX = 98; const LAYOUT_GAP_PX = 34;
23-23: 콘텐츠 패딩 값 상수화/토큰화 권장
62px 58px 62px 58px역시 매직 넘버입니다. 의미 있는 이름의 상수나 테마 스페이싱 스케일을 사용해 가독성과 유지보수성을 높이세요.export const Content = styled.main` width: 100%; max-width: 977px; - padding: 62px 58px 62px 58px; + padding: ${CONTENT_PADDING}; `;추가:
const CONTENT_PADDING = '62px 58px 62px 58px';frontend/src/pages/AdminPage/AdminPage.tsx (2)
8-8: 혼동을 야기하는 중복 Divider import 제거현재 JSX에서는
Styled.Divider(스타일 파일의 Divider)를 사용하고 있으며,SideBar.styles의Divider는 불필요합니다. 제거하여 혼란과 린트 경고를 방지하세요.-import { Divider } from './components/SideBar/SideBar.styles';
29-29: 세로 구분선 명확화(네이밍 변경 시 사용처 반영)위에서 제안한 대로 컴포넌트명을
VerticalDivider로 변경했다면, 사용처도 함께 업데이트하세요.- <Styled.Divider /> + <Styled.VerticalDivider />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
frontend/src/pages/AdminPage/AdminPage.styles.ts(1 hunks)frontend/src/pages/AdminPage/AdminPage.tsx(2 hunks)frontend/src/pages/AdminPage/components/SideBar/SideBar.styles.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (frontend/.cursorrules)
frontend/**/*.{ts,tsx}: Replace magic numbers with named constants for clarity.
Replace complex or nested ternary operators with if/else statements or IIFEs for readability.
Assign complex boolean conditions to named variables.
Use consistent return types for similar functions and hooks.
Avoid hidden side effects; functions should only perform actions implied by their signature (Single Responsibility Principle).
Use unique, descriptive names for custom wrappers and functions to avoid ambiguity.
Define constants near related logic or ensure names link them clearly.
Files:
frontend/src/pages/AdminPage/AdminPage.tsxfrontend/src/pages/AdminPage/components/SideBar/SideBar.styles.tsfrontend/src/pages/AdminPage/AdminPage.styles.ts
frontend/**/*.tsx
📄 CodeRabbit inference engine (frontend/.cursorrules)
frontend/**/*.tsx: Abstract complex logic/interactions into dedicated components or higher-order components (HOCs).
Separate significantly different conditional UI/logic into distinct components.
Colocate simple, localized logic or use inline definitions to reduce context switching.
Choose field-level or form-level cohesion based on form requirements.
Break down broad state management into smaller, focused hooks or contexts.
Use component composition instead of props drilling.
Files:
frontend/src/pages/AdminPage/AdminPage.tsx
🧬 Code graph analysis (1)
frontend/src/pages/AdminPage/AdminPage.styles.ts (1)
frontend/src/pages/AdminPage/components/SideBar/SideBar.styles.ts (1)
Divider(40-46)
🔇 Additional comments (1)
frontend/src/pages/AdminPage/AdminPage.styles.ts (1)
6-6: 컨테이너 교차축 정렬 조정 LGTM
align-items: flex-start;로 사이드바/콘텐츠 상단 정렬이 맞춰져, sticky 요소와 시각적 불연속이 줄어듭니다.
| padding: 62px 58px; | ||
| max-width: 977px; | ||
| padding: 62px 58px 62px 58px; |
There was a problem hiding this comment.
margin, padding 등과 같은 CSS 속성은 동일한 패턴일 경우 축약형(shorthand)으로 표현할 수 있습니다.
기존에 padding: 62px 58px;로 표현되어 있는데, 풀어쓰신(padding: 62px 58px 62px 58px;) 특별한 이유가 있으실까요?
There was a problem hiding this comment.
특별한 이유는 없었어요!!
축약형으로 되는지는 잘 몰랐고, 제가 보기 편해서 그렇게 작성했습니다!
새로운 걸 알게되었네요! 감사합니다아
다시 수정해서 올릴게요~
suhyun113
changed the title
suhyun113
changed the title
4 participants
#️⃣연관된 이슈
📝작업 내용
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit