Merged
Conversation
Walkthrough월간 보고서 기능(목록 및 상세 페이지)과 두-페이지 이미지 갤러리 뷰어, 관련 라우트·컴포넌트·스타일·이미지 자산(22개)이 추가되고 일부 외부 이미지 경로와 네비게이션 항목이 갱신되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Router
participant MonthlyReport as MonthlyReport (목록)
participant Report as Report (카드)
participant MonthlyReportDetail as MonthlyReportDetail (상세)
participant Content as Content (갤러리)
User->>Router: GET /main/monthly-report
Router->>MonthlyReport: render list
MonthlyReport->>Report: render N cards
User->>Report: click card
Report->>Router: navigate /main/monthly-report-detail
Router->>MonthlyReportDetail: render detail
MonthlyReportDetail->>Content: mount Content (pages[])
Content->>Content: set index = 0, show pages[0]/pages[1]
User->>Content: click Next
Content->>Content: index += 2, update progress
User->>Content: click Prev
Content->>Content: index -= 2, update progress
User->>MonthlyReportDetail: click header
MonthlyReportDetail->>Router: navigate /main/monthly-report
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 분
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)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
🧰 Additional context used🧬 Code graph analysis (1)frontend/src/components/external/Content.jsx (1)
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: 6
🧹 Nitpick comments (3)
frontend/src/components/external/Report.jsx (1)
5-21: 클릭 카드에 버튼/Link 패턴을 쓰면 접근성이 좋아집니다.현재는
div+onClick으로만 이동을 처리해서 키보드 사용자(탭/Enter) 입장에서는 이 요소를 활성화하기 어렵습니다.
가능하다면 다음과 같이 개선하는 것을 권장드립니다.
- 네비게이션 목적이라면
<Link to="/main/monthly-report-detail">로 래핑하고, CSS 로 카드 스타일을 적용.- 또는
<button type="button">로 바꾸고onClick={() => nav('/main/monthly-report-detail')}를 부여.- 기존처럼
div를 유지해야 한다면role="button"과tabIndex={0}를 주고,onKeyDown에 Enter/Space 대응을 추가.이렇게 하면 화면 리더 및 키보드 사용자까지 포함한 접근성이 크게 좋아집니다.
frontend/src/pages/external/MonthlyReport.jsx (1)
22-31: 게시물 개수/카드 개수 하드코딩 구조 개선 제안
4개텍스트와<Report />4개가 모두 하드코딩되어 있어서, 실제 데이터 연동 시 개수 불일치가 발생하기 쉽습니다.
차후 API 연동을 염두에 두고, 예를 들어 배열 길이를 기준으로 개수와 카드 렌더링을 같이 계산하는 형태로 바꾸는 것을 추천합니다.- <div className={styles.countSection}> - <span className={styles.count}> - <strong style={{ color: '#339FFF' }}>4개</strong>의 게시물 - </span> - </div> - <div className={styles.reportSection}> - <Report /> - <Report /> - <Report /> - <Report /> - </div> + {/* + TODO: API 연동 후 reports 데이터를 받아오도록 변경 + */} + {(() => { + const reports = [1, 2, 3, 4]; // 임시 목업 데이터 + return ( + <> + <div className={styles.countSection}> + <span className={styles.count}> + <strong style={{ color: '#339FFF' }}> + {reports.length}개 + </strong> + 의 게시물 + </span> + </div> + <div className={styles.reportSection}> + {reports.map((id) => ( + <Report key={id} /> + ))} + </div> + </> + ); + })()}frontend/src/App.jsx (1)
36-40: 외부 월간 리포트 라우트 추가 구성 적절
/main/monthly-report↔/main/monthly-report-detail라우트 구성이Main이하 외부 페이지 군과 잘 맞고,Report.jsx및MonthlyReportDetail.jsx의useNavigate경로('/main/monthly-report','/main/monthly-report-detail')와도 정합성이 있습니다. 현재 요구사항 범위에서는 별도 중첩 라우트/레이아웃 분리는 없어도 충분해 보입니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (25)
frontend/src/assets/external/external-detail-image.pngis excluded by!**/*.pngfrontend/src/assets/external/external-image.pngis excluded by!**/*.pngfrontend/src/assets/external/monthly-report.pngis excluded by!**/*.pngfrontend/src/assets/external/report/content1.pngis excluded by!**/*.pngfrontend/src/assets/external/report/content10.pngis excluded by!**/*.pngfrontend/src/assets/external/report/content11.pngis excluded by!**/*.pngfrontend/src/assets/external/report/content12.pngis excluded by!**/*.pngfrontend/src/assets/external/report/content13.pngis excluded by!**/*.pngfrontend/src/assets/external/report/content14.pngis excluded by!**/*.pngfrontend/src/assets/external/report/content15.pngis excluded by!**/*.pngfrontend/src/assets/external/report/content16.pngis excluded by!**/*.pngfrontend/src/assets/external/report/content17.pngis excluded by!**/*.pngfrontend/src/assets/external/report/content18.pngis excluded by!**/*.pngfrontend/src/assets/external/report/content19.pngis excluded by!**/*.pngfrontend/src/assets/external/report/content2.pngis excluded by!**/*.pngfrontend/src/assets/external/report/content20.pngis excluded by!**/*.pngfrontend/src/assets/external/report/content21.pngis excluded by!**/*.pngfrontend/src/assets/external/report/content3.pngis excluded by!**/*.pngfrontend/src/assets/external/report/content4.pngis excluded by!**/*.pngfrontend/src/assets/external/report/content5.pngis excluded by!**/*.pngfrontend/src/assets/external/report/content6.pngis excluded by!**/*.pngfrontend/src/assets/external/report/content7.pngis excluded by!**/*.pngfrontend/src/assets/external/report/content8.pngis excluded by!**/*.pngfrontend/src/assets/external/report/content9.pngis excluded by!**/*.pngfrontend/src/assets/external/report/report-cover.pngis excluded by!**/*.png
📒 Files selected for processing (12)
frontend/src/App.jsx(2 hunks)frontend/src/components/external/Content.jsx(1 hunks)frontend/src/components/external/Content.module.css(1 hunks)frontend/src/components/external/Report.jsx(1 hunks)frontend/src/components/external/Report.module.css(1 hunks)frontend/src/pages/external/External.module.css(1 hunks)frontend/src/pages/external/Main.jsx(2 hunks)frontend/src/pages/external/MonthlyReport.jsx(1 hunks)frontend/src/pages/external/MonthlyReport.module.css(1 hunks)frontend/src/pages/external/MonthlyReportDetail.jsx(1 hunks)frontend/src/pages/external/MonthlyReportDetail.module.css(1 hunks)frontend/src/utils/reportContent.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
frontend/src/pages/external/MonthlyReport.jsx (1)
frontend/src/components/external/Report.jsx (1)
Report(5-23)
frontend/src/components/external/Report.jsx (1)
frontend/src/pages/external/MonthlyReportDetail.jsx (1)
nav(7-7)
frontend/src/pages/external/MonthlyReportDetail.jsx (2)
frontend/src/components/external/Report.jsx (1)
nav(6-6)frontend/src/components/external/Content.jsx (1)
Content(5-52)
frontend/src/components/external/Content.jsx (1)
frontend/src/utils/reportContent.js (2)
pages(24-47)pages(24-47)
frontend/src/App.jsx (2)
frontend/src/pages/external/MonthlyReport.jsx (1)
MonthlyReport(5-35)frontend/src/pages/external/MonthlyReportDetail.jsx (1)
MonthlyReportDetail(6-22)
🔇 Additional comments (4)
frontend/src/pages/external/External.module.css (1)
9-10: 헤더 배경 이미지 경로 변경 문제 없어 보입니다.폴더 구조(
assets/external/...)와 일관되고, 기존과 동일한 배경 표시(center no-repeat)이어서 스타일/동작 상 이슈는 없어 보입니다.frontend/src/pages/external/Main.jsx (1)
2-2: 월간 리포트 메뉴 추가 방향 적절합니다.이미지 경로 변경 및
/main/monthly-report링크 추가 모두 의도에 맞게 보입니다.
App.jsx 쪽 라우트 path(/main/monthly-report)와 문자열이 정확히 일치하는지만 한 번 더 확인해 주시면 좋겠습니다.Also applies to: 21-23
frontend/src/utils/reportContent.js (1)
1-47: 현재 요구사항에는 충분히 단순하고 명시적인 구성입니다.로컬 자산을 명시적으로 import 해서
pages배열로 넘기는 구조라 Content 컴포넌트에서 사용하기 직관적입니다.
향후 페이지 수가 늘거나 줄 때에는, Content.jsx 의 2페이지 단위 넘김 로직(인덱스 +2씩 증가, progress 계산 등)과 항상 맞춰 주기만 하면 될 것 같습니다.frontend/src/App.jsx (1)
22-23: 새 월간 리포트 페이지 import 정합성 확인
MonthlyReport,MonthlyReportDetailimport 경로와 파일 구조가 외부 페이지들과 일관되고, 라우터 설정에서도 그대로 사용되고 있어 문제 없어 보입니다.
Comment on lines
+6
to
+13
| const [current, setCurrent] = useState(0); | ||
| const goPrev = () => { | ||
| if (current > 1) setCurrent(current - 2); | ||
| }; | ||
|
|
||
| const goNext = () => { | ||
| if (current < pages.length - 2) setCurrent(current + 2); | ||
| }; |
There was a problem hiding this comment.
페이지 이동 로직과 우측 화살표 비활성 조건 불일치
현재 구현에서는:
goNext가current < pages.length - 2일 때만current를 +2 하여 마지막 spread(예: 길이 22일 때current = 20)까지만 이동하고,- 우측 화살표
disabled조건은current === pages.length - 1로 되어 있어, 실제로는 마지막 spread에서도 버튼이 비활성화되지 않습니다(클릭해도 state는 안 바뀌지만, UI 상으로는 계속 활성처럼 보임).
또한 setCurrent(current ± 2)는 콜백이 아니라 클로저 값을 쓰고 있어, 향후 로직 변경 시 예기치 못한 동작 여지가 있습니다.
아래처럼 boundary를 일관되게 맞추고, 콜백 형태로 상태를 갱신하는 편이 더 안전하고 직관적입니다.
- const [current, setCurrent] = useState(0);
- const goPrev = () => {
- if (current > 1) setCurrent(current - 2);
- };
-
- const goNext = () => {
- if (current < pages.length - 2) setCurrent(current + 2);
- };
+ const [current, setCurrent] = useState(0);
+
+ const goPrev = () => {
+ setCurrent((prev) => Math.max(prev - 2, 0));
+ };
+
+ const goNext = () => {
+ setCurrent((prev) =>
+ Math.min(prev + 2, pages.length - 2),
+ );
+ };
@@
- <button
- className={styles.arrowBtn}
- onClick={goNext}
- disabled={current === pages.length - 1}
- >
+ <button
+ className={styles.arrowBtn}
+ onClick={goNext}
+ disabled={current >= pages.length - 2}
+ >추가로:
- 현재 구현은
pages.length가 짝수(항상 2장 단위로 끝나는 경우)라는 전제를 깔고 있으므로, 추후 API에서 페이지 수가 홀수일 가능성이 있다면 마지막 한 장에 대한 처리(예: 오른쪽 이미지는 조건부 렌더링)를 고려해 두면 좋겠습니다. - 접근성을 위해 좌우 화살표 버튼에
aria-label="이전 페이지",aria-label="다음 페이지"를 부여하는 것도 추천드립니다.
Also applies to: 20-21, 25-48
🤖 Prompt for AI Agents
In frontend/src/components/external/Content.jsx around lines 6–13 (and also
apply same fix to 20–21, 25–48), the page navigation boundary logic and
disabled-state checks are inconsistent and setState uses closure values; compute
a single maxIndex = pages.length - (pages.length % 2 === 0 ? 2 : 1) and use that
for both navigation guards and button disabled checks, update goPrev/goNext to
use functional state updates (setCurrent(prev => Math.max(0, Math.min(maxIndex,
prev ± 2)))), ensure the right-hand page/image is conditionally rendered when
pages.length is odd, and add aria-label="이전 페이지" / aria-label="다음 페이지" to the
arrow buttons for accessibility.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
1) 작업한 이슈번호
SISC1-227
2) 변경 요약 (What & Why)
3) 스크린샷/동영상 (UI 변경 시)
4) 상세 변경사항 (전부 다)
5) 참고사항
현재 임시로 이미지를 전부 저장하여 구현하였지만 추후 API 연동 후 로직 변경 예정입니다.
Summary by CodeRabbit
새로운 기능
스타일
✏️ Tip: You can customize this high-level summary in your review settings.