Skip to content

Conversation

@oesnuj
Copy link
Member

@oesnuj oesnuj commented Nov 19, 2025

🔗 연관된 이슈

#857


📝 작업 내용

1️⃣ 11월 패치노트 배너 이미지 추가

데스크탑/모바일 배너 이미지를 4배 해상도 PNG로 추가했습니다.
3개의 배너로 구성했어요:

  1. 모든 동아리 한곳에 모으다
  2. 지금 바로 모아동에서 시작하세요
  3. 11월 패치노트 (Notion 링크 연결)
데스크탑 모바일
데스크탑 배너 모바일 배너



2️⃣ 배너 데이터 구조 개선

파일 이동: constants/banners.tsBanner/bannerData.ts

배너 데이터를 추가할 때 constants 폴더에서 찾기 어려웠어요.
배너 컴포넌트랑 같은 폴더에 두는 게 더 직관적일 것 같아서 옮겼습니다.


desktop/mobile 배열 중복 제거

기존에는 id, linkTo, alt 같은 값들이 desktop/mobile 배열에 각각 중복으로 들어가 있었습니다.
하나의 배열에 desktopImage/mobileImage만 분리해서 관리하도록 변경했어요. (54줄 → 42줄)


네비게이션 버튼 의존성 제거

PhotoList/PhotoModal에서 Banner 컴포넌트의 버튼 아이콘을 import 해서 쓰고 있었는데,
이건 좀 이상한 의존성이라 각 컴포넌트가 assets/images/icons에서 직접 import 하도록 수정했습니다.



3️⃣ Banner 컴포넌트 리팩토링

배너 데이터를 props로 받지 않고 컴포넌트 내부에서 직접 import 하도록 변경했습니다.
MainPage/components에서만 쓰이는데 props로 넘기니까 데이터 흐름 추적이 어려웠어요.
어디서 이미지가 오는지 한눈에 보이도록 개선했습니다.

이미지 import 네이밍도 개선했어요: banner_desktop1AllClubsDesktopImage
배너 ID도 의미 있는 이름으로 변경했습니다 (예: all-clubs-in-one-place, start-with-moadong)

💡 배너 ID는 Claude에게 추천받았습니다


4️⃣ 접근성 개선

네비게이션 버튼에 aria-label을 추가했습니다 ('이전 배너', '다음 배너')
배너 이미지에도 의미있는 alt 텍스트를 추가했어요.


🔍 중점 리뷰 요청 사항

  1. 모바일/데스크탑 배너를 하나의 배열로 통합한 구조가 적절한지 검토 부탁드립니다
  2. bannerData.ts 파일명이 적절한지 의견 부탁드립니다

🫡 참고사항

현재 설계 선택 이유

현재는 모바일/데스크탑 배너가 동일한 내용으로 이미지만 다릅니다.
따라서 통합 구조가 적합하다고 판단했어요.
중복 제거를 통해 유지보수성도 크게 향상됐습니다.

향후 고려사항

모바일과 데스크탑에서 완전히 다른 배너를 보여줘야 할 경우,
desktop/mobile 배열 분리 구조로 다시 돌아갈 필요가 있을 것 같습니다.

- constants/banners.ts → Banner/bannerData.ts로 이동
- desktop/mobile 배열 분리 제거, 단일 배열에 desktopImage/mobileImage 포함하여 중복 제거
- Banner 사용하던 prev/next 버튼을 PhotoList/PhotoModal에서도 직접 import하도록 변경
- 기존 constants에서 버튼을 가져오던 방식 제거
- BannerProps 인터페이스 제거 (불필요한 props 주입 방지)
- BANNER_DATA를 컴포넌트 내부에서 직접 import하여 사용
- 네비게이션 버튼(prev/next)을 assets에서 직접 import
- BannerWrapper에서 불필요한 background-color: transparent 제거
- 중복되거나 의미 없는 스타일 속성 정리
- 데스크탑/모바일 배너 이미지를 4배 해상도로 교체하여 선명도 향상
@vercel
Copy link

vercel bot commented Nov 19, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
moadong Ready Ready Preview Comment Nov 19, 2025 6:46am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 19, 2025

Warning

.coderabbit.yaml has a parsing error

The CodeRabbit configuration file in this repository has a parsing error and default settings were used instead. Please fix the error(s) in the configuration file. You can initialize chat with CodeRabbit to get help with the configuration file.

💥 Parsing errors (1)
Validation error: Invalid regex pattern for base branch. Received: "**" at "reviews.auto_review.base_branches[0]"
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Walkthrough

배너 시스템을 재구성하여 상수 기반 props에서 자체 포함 데이터 소스로 전환했습니다. SlideButton 배열을 제거하고 별도의 SVG 이미지를 사용하도록 업데이트했으며, 배너 데이터를 새로운 bannerData.ts 모듈로 중앙화했습니다.

Changes

Cohort / File(s) 변경 요약
배너 상수 제거
frontend/src/constants/banners.ts
DesktopBannerImageList, MobileBannerImageList, SlideButton 공개 상수 및 해당 이미지 임포트 제거
배너 데이터 중앙화
frontend/src/pages/MainPage/components/Banner/bannerData.ts
새로운 BANNERS 배열 및 BannerItem 인터페이스 정의; 데스크톱/모바일 배너 이미지, ID, 링크, alt 텍스트 포함
Banner 컴포넌트 리팩토링
frontend/src/pages/MainPage/components/Banner/Banner.tsx, frontend/src/pages/MainPage/components/Banner/Banner.styles.ts, frontend/src/pages/MainPage/MainPage.tsx
Banner 컴포넌트에서 props 제거 및 로컬 bannerData 사용으로 전환; BannerProps 타입 제거; Banner.styles에서 제네릭 타입 제거
네비게이션 아이콘 마이그레이션
frontend/src/pages/ClubDetailPage/components/PhotoList/PhotoList.tsx, frontend/src/pages/ClubDetailPage/components/PhotoList/PhotoModal/PhotoModal.tsx
SlideButton 배열 참조를 개별 PrevButton/NextButton SVG 임포트로 교체

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 분

주의 필요 영역:

  • Banner.tsx의 새로운 데이터 소스 로직 및 렌더링 흐름 검증
  • bannerData.ts의 배너 메타데이터 정확성 (이미지 경로, 링크 대상, alt 텍스트)
  • 두 개의 PhotoList 관련 파일에서 아이콘 마이그레이션의 일관성
  • 모바일/데스크톱 조건부 렌더링의 동작 확인

Possibly related PRs

Suggested labels

✨ Feature, 🎨 Design, 💻 FE

Suggested reviewers

  • lepitaaar
  • seongje973
  • seongwon030

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed PR 제목은 주요 변경 사항인 '11월 패치노트 배너 추가' 및 'Banner 컴포넌트 구조 개선'을 명확하게 요약하고 있으며, 변경 사항의 핵심을 정확히 반영합니다.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/#857-patchnote-banner-2025-11

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@oesnuj oesnuj added ✨ Feature 기능 개발 💻 FE Frontend labels Nov 19, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (1)
frontend/src/pages/MainPage/components/Banner/bannerData.ts (1)

16-39: 배너 데이터 구조가 적절하나, URL 관리 개선을 고려하세요.

배너 데이터가 잘 구조화되어 있고 alt 텍스트도 설명적입니다. 다만 Line 36의 Notion URL이 매우 긴 문자열로 하드코딩되어 있습니다. 향후 URL 변경이나 여러 곳에서 재사용이 필요한 경우를 대비하여 별도 상수로 분리하는 것을 고려할 수 있습니다.

현재 구조에서는 각 배너가 고유한 URL을 가질 수 있으므로 즉시 변경이 필요한 것은 아닙니다.

예시 (선택사항):

+const PATCH_NOTE_URL = 'https://honorable-cough-8f9.notion.site/1e8aad232096804f9ea9ee4f5cf0cd10';
+
 const BANNERS: BannerItem[] = [
   // ...
   {
     id: 'patch-note-november-2025',
     desktopImage: PatchNoteDesktopImage,
     mobileImage: PatchNoteMobileImage,
-    linkTo: 'https://honorable-cough-8f9.notion.site/1e8aad232096804f9ea9ee4f5cf0cd10',
+    linkTo: PATCH_NOTE_URL,
     alt: '모아동 11월 패치노트 안내 - 지원서 관리 및 메인페이지 개편',
   },
 ];
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 412cc8f and f7c6182.

⛔ Files ignored due to path filters (2)
  • frontend/src/assets/images/banners/banner_desktop3.png is excluded by !**/*.png
  • frontend/src/assets/images/banners/banner_mobile3.png is excluded by !**/*.png
📒 Files selected for processing (7)
  • frontend/src/constants/banners.ts (0 hunks)
  • frontend/src/pages/ClubDetailPage/components/PhotoList/PhotoList.tsx (2 hunks)
  • frontend/src/pages/ClubDetailPage/components/PhotoList/PhotoModal/PhotoModal.tsx (3 hunks)
  • frontend/src/pages/MainPage/MainPage.tsx (1 hunks)
  • frontend/src/pages/MainPage/components/Banner/Banner.styles.ts (1 hunks)
  • frontend/src/pages/MainPage/components/Banner/Banner.tsx (3 hunks)
  • frontend/src/pages/MainPage/components/Banner/bannerData.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • frontend/src/constants/banners.ts
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/pages/MainPage/components/Banner/bannerData.ts (1)
frontend/src/pages/MainPage/components/Banner/Banner.styles.ts (1)
  • BannerItem (41-51)
🔇 Additional comments (8)
frontend/src/pages/ClubDetailPage/components/PhotoList/PhotoList.tsx (1)

10-11: 네비게이션 아이콘 개선이 적절합니다.

SlideButton에서 전용 SVG 자산(PrevButton, NextButton)으로 전환하고 명확한 alt 텍스트를 추가하여 접근성이 향상되었습니다.

Also applies to: 71-71, 76-76

frontend/src/pages/ClubDetailPage/components/PhotoList/PhotoModal/PhotoModal.tsx (1)

1-4: 접근성 개선 및 React 19 호환성이 우수합니다.

React import 제거는 React 19의 새로운 JSX 변환과 호환되며, aria-label을 버튼 요소에 직접 배치하여 스크린 리더 사용자를 위한 접근성이 향상되었습니다.

Also applies to: 67-80

frontend/src/pages/MainPage/components/Banner/bannerData.ts (1)

8-14: BannerItem 인터페이스 설계가 우수합니다.

데스크톱/모바일 이미지를 하나의 객체로 통합한 구조는 코드 중복을 줄이고 유지보수성을 향상시킵니다. 타입 정의도 명확합니다.

frontend/src/pages/MainPage/MainPage.tsx (1)

55-55: 컴포넌트 단순화가 적절합니다.

Banner 컴포넌트가 props 없이 자체 데이터를 관리하도록 변경하여 관심사 분리가 개선되었고, MainPage의 책임이 감소했습니다.

frontend/src/pages/MainPage/components/Banner/Banner.styles.ts (1)

24-24: 불필요한 타입 제거가 적절합니다.

Banner 컴포넌트가 더 이상 props를 받지 않으므로 BannerProps 제네릭 타입을 제거하여 코드가 더 명확해졌습니다.

frontend/src/pages/MainPage/components/Banner/Banner.tsx (3)

6-10: 중앙화된 데이터 구조로의 리팩토링이 우수합니다.

Banner 컴포넌트가 props 대신 중앙화된 BANNERS 데이터 소스를 사용하도록 변경하여:

  • 컴포넌트 응집도 향상 (데이터와 컴포넌트가 같은 폴더)
  • 코드 중복 감소 (desktop/mobile 배열 통합)
  • 컴포넌트 사용이 더 간단해짐

전용 네비게이션 SVG 자산(PrevButton, NextButton) 사용도 적절합니다.

Also applies to: 12-12


36-41: 접근성 개선이 훌륭합니다.

네비게이션 버튼에 aria-label("이전 배너", "다음 배너")을 추가하여 스크린 리더 사용자를 위한 접근성이 크게 향상되었습니다. 내부 <img> 태그의 빈 alt는 버튼에 이미 레이블이 있으므로 적절합니다.


55-67: 조건부 이미지 렌더링 및 배너 클릭 처리가 적절합니다.

  • banner.id를 key로 사용하여 React 렌더링 최적화 ✓
  • isMobile에 따라 적절한 이미지 선택 ✓
  • banner.linkTo 존재 여부로 클릭 가능 여부 제어 ✓
  • 설명적인 banner.alt 텍스트 사용으로 접근성 향상 ✓

Copy link
Member

@seongwon030 seongwon030 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

배너 변경 좋습니다

@oesnuj oesnuj merged commit ad24725 into develop-fe Nov 20, 2025
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

💻 FE Frontend ✨ Feature 기능 개발

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants