[feature] 사용자가 지원하기를 눌렀을때 지원가능한 지원서들이 모달로 뜬다#787
Conversation
suhyun113
requested review from
alsdddk,
lepitaaar,
oesnuj,
seongje973 and
seongwon030
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
모달 컴포넌트 및 스타일 frontend/src/components/application/modals/ApplicationSelectModal.tsx, frontend/src/components/application/modals/ApplicationSelectModal.styles.ts, frontend/src/components/common/Modal/Modal.tsx, frontend/src/components/common/Modal/Modal.styles.ts |
ApplicationSelectModal 컴포넌트 및 공통 Modal과 스타일 추가·수정(오버레이, 헤더, 바디, 백드롭 클릭 제어, 스크롤 락 등). |
API 변경 및 신규 유틸 frontend/src/apis/application/getApplication.ts, frontend/src/apis/application/getApplicationOptions.ts |
getApplication 시그니처에 applicationFormId 추가하고 엔드포인트를 /api/club/{clubId}/apply/{applicationFormId}로 변경. 신규 getApplicationOptions(clubId)로 지원서 목록 조회 및 에러 메시지 파싱 로직 추가. |
타입 정의 frontend/src/types/application.ts |
ApplicationForm 인터페이스(id, title) 추가. |
모의 API 및 데이터/상수 frontend/src/mocks/api/apply.ts, frontend/src/mocks/data/mockData.ts, frontend/src/mocks/constants/clubApi.ts |
apply mock 응답 스키마를 중첩된 data 형태로 변경하고 /api/:clubId/applications GET 엔드포인트 추가. mockOptions 및 여러 CLUB_* 상수 추가. |
훅·컨텍스트 업데이트 frontend/src/hooks/queries/application/useGetApplication.ts, frontend/src/context/AdminClubContext.tsx |
useGetApplication이 (clubId?, applicationFormId?) 형태로 확장(쿼리Key·enabled 변경). AdminClubContext에 applicationFormId와 setter 상태 추가. |
페이지·라우팅·컴포넌트 변경 frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx, frontend/src/pages/ApplicationFormPage/ApplicationFormPage.tsx, frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantDetailPage/ApplicantDetailPage.tsx, frontend/src/pages/AdminPage/tabs/ApplicationEditTab/ApplicationEditTab.tsx, frontend/src/App.tsx |
ClubApplyButton이 getApplicationOptions로 옵션을 조회해 0/1/다중 분기(모달 오픈 포함). ApplicationFormPage 경로에 applicationFormId 파라미터 추가(/application/:clubId/:applicationFormId) 및 useGetApplication에 전달. 관리자 페이지들에서 applicationFormId를 사용하도록 훅 호출·렌더 가드 변경. |
Sequence Diagram(s)
sequenceDiagram
actor User
participant ClubApplyButton
participant API as getApplicationOptions
participant Modal as ApplicationSelectModal
participant Router as Router/ApplicationFormPage
User->>ClubApplyButton: 지원하기 클릭
ClubApplyButton->>API: GET /api/club/{clubId}/apply (getApplicationOptions)
API-->>ClubApplyButton: ApplicationForm[] (옵션 목록)
alt 옵션 0개
ClubApplyButton->>ClubApplyButton: 아무 동작 없음
else 옵션 1개
ClubApplyButton->>Router: navigate /application/{clubId}/{applicationFormId}
else 옵션 >=2개
ClubApplyButton->>Modal: 모달 열기 (options)
User->>Modal: 옵션 선택
Modal->>Router: navigate /application/{clubId}/{applicationFormId}
end
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
주의 집중 필요 영역:
getApplication/getApplicationOptions의 HTTP 에러 파싱 및 일관성 검증useGetApplication의 쿼리Key·enabled 변경이 캐싱·리패칭에 미치는 영향- Modal(오버레이/백드롭)과 ApplicationSelectModal의 접근성·스크롤 락 동작 검증
- ApplicationFormPage의 STORAGE_KEY 변경으로 인한 로컬스토리지 충돌 여부
- mock API 엔드포인트·응답 형식 변경이 로컬 개발/테스트에 미치는 영향
Possibly related issues
- [feature] MOA-288 사용자가 지원하기를 눌렀을때 지원가능한 지원서들이 모달로 뜬다 #784 — 사용자가 지원하기를 눌렀을때 지원가능한 지원서들이 모달로 뜬다: 본 PR은 동일한 기능(지원서 옵션 모달 표시)을 구현합니다.
Possibly related PRs
- [fix] 지원서 미등록 시 alert 반복 버그 수정 및 외부 링크/지원서 분기 처리 #605 — ClubApplyButton 관련 변경: ClubApplyButton의 클릭/네비게이션 흐름 변경과 직접 연관.
- [feature] 동아리 지원하기 기능 #536 — getApplication 변경: getApplication 시그니처·엔드포인트 수정과 코드 레벨 연관.
- [feature] 동아리 지원서 타입 설계 및 Mock API 개발 #418 — mocks 및 apply API 변경: mock API·데이터 구조 변경과 직접적인 연관.
Suggested labels
📬 API
Suggested reviewers
- seongwon030
- lepitaaar
- oesnuj
Pre-merge checks and finishing touches
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | PR 제목이 변경사항의 핵심을 명확하게 설명하고 있습니다. 사용자가 지원하기 버튼을 클릭했을 때 지원 가능한 지원서들이 모달로 표시되는 기능을 직관적으로 전달합니다. |
| Linked Issues check | ✅ Passed | PR에서 MOA-288 이슈의 모든 주요 코딩 요구사항을 충족합니다: (1) 여러 지원서 있을 때 모달 추가 MOA-288, (2) ApplicationSelectModal 컴포넌트 구현 MOA-288, (3) getApplicationOptions API 함수 추가 MOA-288, (4) ClubApplyButton에서 지원서 개수별 로직 처리 MOA-288, (5) Mock 데이터 기반 구현 MOA-288. |
| Out of Scope Changes check | ✅ Passed | 모든 변경사항이 지원서 선택 모달 기능과 관련 있습니다. Modal 공통 컴포넌트, 지원서 API 함수, AdminClubContext 확장, 경로 파라미터 추가 등 모두 핵심 기능 구현에 필요한 범위 내 변경입니다. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✨ 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/#784-add-application-select-modal-MOA-288
📜 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/apis/application/getApplicationOptions.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/apis/application/getApplicationOptions.ts
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (6)
frontend/src/apis/application/getApplication.ts (1)
2-6: TODO 주석 추가: Mock API(@/mocks/constants/clubApi의API_BASE) 사용이 임시 구현이므로 실제 API 전환 시@/constants/api의API_BASE_URL로 변경하세요.frontend/src/components/common/Modal/Modal.tsx (2)
21-21: 불필요한 조건부 렌더링을 제거하세요.
title은 필수 prop이므로 조건부 체크가 불필요합니다.다음 diff를 적용하세요:
- {title && <Styled.Title>{title}</Styled.Title>} + <Styled.Title>{title}</Styled.Title>
13-30: ESC 키 처리와 포커스 관리를 추가하는 것을 고려하세요.현재 모달은 오버레이 클릭으로만 닫을 수 있습니다. 접근성 향상을 위해:
- ESC 키로 모달을 닫을 수 있어야 합니다
- 모달이 열릴 때 포커스가 모달 내부로 이동해야 합니다
- 모달이 닫힐 때 포커스가 트리거 요소로 복귀해야 합니다
이는 WCAG 2.1 지침을 준수하기 위한 권장사항입니다.
예시 구현:
import { MouseEvent, ReactNode, useEffect, useRef } from "react"; const Modal = ({ isOpen, onClose, title, description, children }: ModalProps) => { const containerRef = useRef<HTMLDivElement>(null); useEffect(() => { if (!isOpen) return; const handleEscape = (e: KeyboardEvent) => { if (e.key === 'Escape') onClose(); }; // 포커스를 모달로 이동 containerRef.current?.focus(); document.addEventListener('keydown', handleEscape); return () => document.removeEventListener('keydown', handleEscape); }, [isOpen, onClose]); if (!isOpen) return null; return ( <Styled.Overlay isOpen={isOpen} onClick={onClose}> <RemoveScroll enabled={isOpen}> <Styled.Container ref={containerRef} tabIndex={-1} isOpen={isOpen} onClick={(e: MouseEvent<HTMLDivElement>) => e.stopPropagation()} role="dialog" aria-modal="true" > {/* 나머지 코드 */} </Styled.Container> </RemoveScroll> </Styled.Overlay> ); }frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx (2)
32-42:openByOption에서 undefined option 처리를 개선하세요.
openByOption함수가option이 undefined일 때 적절히 처리하지 않습니다. 현재는 Line 114에서 명시적으로 전달하지만, 타입 안정성을 위해 함수 시그니처를 명확히 하는 것이 좋습니다.다음 diff를 적용하여 타입 안정성을 개선하세요:
- const openByOption = (option?: ApplicationOption) => { + const openByOption = (option: ApplicationOption) => { if (!clubId) return; if (option?.url) { // 외부 폼 window.open(option.url, '_blank'); } else { // 내부 폼 navigate(`/application/${clubId}`); } setIsOpen(false); };
54-86: 복잡한 에러 처리 로직을 단순화하는 것을 고려하세요.현재 중첩된 try-catch 구조는 가독성이 떨어지며 유지보수가 어렵습니다. 코딩 가이드라인에 따라 복잡한 조건부 로직을 명확한 if/else 문이나 헬퍼 함수로 분리하는 것을 고려하세요.
예시 리팩토링:
const handleClick = async () => { trackEvent(EVENT_NAME.CLUB_APPLY_BUTTON_CLICKED); if (deadlineText === RECRUITMENT_STATUS.CLOSED) { alert(`현재 ${clubDetail.name} 동아리는 모집 기간이 아닙니다.`); return; } try { const list = await getApplicationOptions(clubId); handleApplicationOptions(list); } catch { await handleFallbackApplication(); } }; const handleApplicationOptions = (list: ApplicationOption[]) => { if (list.length === 1) { openByOption(list[0]); } else if (list.length >= 2) { setOptions(list); setIsOpen(true); } else { setOptions([]); setIsOpen(true); } }; const handleFallbackApplication = async () => { try { await getApplication(clubId); navigate(`/application/${clubId}`); } catch { const externalForm = clubDetail.externalApplicationUrl?.trim(); if (externalForm) { window.open(externalForm, '_blank'); } else { setOptions([]); setIsOpen(true); } } };frontend/src/components/common/Modal/Modal.styles.ts (1)
14-22: 모바일 반응형 디자인을 개선하세요.
min-width: 500px는 작은 화면(예: 스마트폰)에서 문제가 될 수 있습니다. 모바일 환경을 고려한 반응형 디자인이 필요합니다.다음 diff를 적용하여 반응형을 개선하세요:
export const Container = styled.div<{ isOpen: boolean }>` - min-width: 500px; + width: 90%; + max-width: 500px; max-height: 90vh; background: #fff; border-radius: 10px; overflow: hidden; box-shadow: ${({ isOpen }) => (isOpen ? '0 18px 44px rgba(0,0,0,.22)' : 'none')}; transition: transform .2s ease, box-shadow .2s ease; + + @media (max-width: 768px) { + width: 95%; + max-width: none; + } `;
📜 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 ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
frontend/package.json(1 hunks)frontend/src/apis/application/getApplication.ts(1 hunks)frontend/src/apis/application/getApplicationOptions.ts(1 hunks)frontend/src/components/application/modals/ApplicationSelectModal.styles.ts(1 hunks)frontend/src/components/application/modals/ApplicationSelectModal.tsx(1 hunks)frontend/src/components/common/Modal/Modal.styles.ts(1 hunks)frontend/src/components/common/Modal/Modal.tsx(1 hunks)frontend/src/mocks/api/apply.ts(3 hunks)frontend/src/mocks/constants/clubApi.ts(1 hunks)frontend/src/mocks/data/mockData.ts(2 hunks)frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx(4 hunks)frontend/src/types/application.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/components/application/modals/ApplicationSelectModal.tsxfrontend/src/mocks/data/mockData.tsfrontend/src/mocks/constants/clubApi.tsfrontend/src/types/application.tsfrontend/src/mocks/api/apply.tsfrontend/src/apis/application/getApplicationOptions.tsfrontend/src/components/application/modals/ApplicationSelectModal.styles.tsfrontend/src/apis/application/getApplication.tsfrontend/src/components/common/Modal/Modal.tsxfrontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsxfrontend/src/components/common/Modal/Modal.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/components/application/modals/ApplicationSelectModal.tsxfrontend/src/components/common/Modal/Modal.tsxfrontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx
🧠 Learnings (1)
📚 Learning: 2025-03-19T05:18:07.818Z
Learnt from: seongwon030
PR: Moadong/moadong#195
File: frontend/src/pages/AdminPage/AdminPage.tsx:7-7
Timestamp: 2025-03-19T05:18:07.818Z
Learning: AdminPage.tsx에서 현재 하드코딩된 클럽 ID('67d2e3b9b15c136c6acbf20b')는 로그인 기능 구현 후 동적으로 가져오는 방식으로 수정될 예정입니다.
Applied to files:
frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx
🧬 Code graph analysis (6)
frontend/src/components/application/modals/ApplicationSelectModal.tsx (1)
frontend/src/types/application.ts (1)
ApplicationOption(60-64)
frontend/src/mocks/data/mockData.ts (2)
frontend/src/types/application.ts (1)
ApplicationOption(60-64)frontend/src/mocks/constants/clubApi.ts (4)
CLUB_BOB(6-6)CLUB_IVF(5-5)CLUB_BACK(7-7)CLUB_TEST(8-8)
frontend/src/mocks/api/apply.ts (4)
frontend/src/mocks/data/mockData.ts (2)
mockData(26-124)mockOptions(126-144)frontend/src/mocks/constants/clubApi.ts (1)
API_BASE(1-1)frontend/src/mocks/utils/validateClubId.ts (1)
validateClubId(1-4)frontend/src/mocks/constants/error.ts (1)
ERROR_MESSAGE(1-5)
frontend/src/apis/application/getApplicationOptions.ts (1)
frontend/src/mocks/constants/clubApi.ts (1)
API_BASE(1-1)
frontend/src/apis/application/getApplication.ts (1)
frontend/src/mocks/constants/clubApi.ts (1)
API_BASE(1-1)
frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx (1)
frontend/src/types/application.ts (1)
ApplicationOption(60-64)
🔇 Additional comments (7)
frontend/package.json (1)
37-37: LGTM!모달의 스크롤 잠금 기능을 위한 적절한 의존성 추가입니다.
frontend/src/mocks/api/apply.ts (2)
78-88: LGTM!새로운 applications 엔드포인트 구현이 잘 되어있습니다. clubId 검증과 nullish coalescing 연산자 사용이 적절합니다.
27-34: getApplication이 응답의 data를 반환하도록 이미 구현되어 있어 호출부 수정 불필요합니다.frontend/src/mocks/constants/clubApi.ts (1)
5-8: LGTM!매직 문자열 대신 명명된 상수를 사용하여 코딩 가이드라인을 잘 따르고 있습니다.
frontend/src/apis/application/getApplicationOptions.ts (1)
2-6: Mock API 사용이 임시인지 확인이 필요합니다.
getApplication.ts와 마찬가지로, 이 함수도 프로덕션 코드에서 mock API base를 사용하고 있습니다. 실제 API로 전환할 때를 대비해 TODO 주석 추가를 권장합니다.frontend/src/mocks/data/mockData.ts (1)
126-144: LGTM!Mock 데이터가 잘 구조화되어 있고
ApplicationOption인터페이스를 적절히 활용하고 있습니다. 내부/외부 링크 구분도 명확합니다.frontend/src/types/application.ts (1)
60-64: LGTM!새로운
ApplicationOption인터페이스가 명확하게 정의되어 있고, 주석으로url필드의 의미를 잘 설명하고 있습니다.
| export const EmptyMessage = styled.div` | ||
| padding: 16px 8px; | ||
| color: #9D9D9D; | ||
| text-align: center; | ||
| font-weight: 600; | ||
| `; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
매직 컬러 값을 상수로 추출하세요.
Line 5의 #9D9D9D는 매직 값입니다. 코딩 가이드라인에 따라 명명된 상수로 대체해야 합니다.
색상 테마 파일을 생성하거나 기존 테마 파일을 사용하세요:
// theme/colors.ts 또는 유사한 파일에서
export const COLORS = {
textSecondary: '#9D9D9D',
// ...
} as const;그런 다음 스타일에서 사용:
+import { COLORS } from '@/theme/colors';
+
export const EmptyMessage = styled.div`
padding: 16px 8px;
- color: #9D9D9D;
+ color: ${COLORS.textSecondary};
text-align: center;
font-weight: 600;
`;🤖 Prompt for AI Agents
In frontend/src/components/application/modals/ApplicationSelectModal.styles.ts
around lines 3 to 8, the color literal `#9D9D9D` is a magic value; extract it
into a theme/colors file (e.g., create or update theme/colors.ts exporting a
COLORS object with textSecondary: '#9D9D9D' as const) and replace the literal
with an imported COLORS.textSecondary in the styled component; ensure the new
file is exported and imported with the correct relative path.
| export const OptionButton = styled.button` | ||
| width: 100%; | ||
| padding: 18px 20px; | ||
| border-radius: 10px; | ||
| border: 1px solid #DCDCDC; | ||
| background: #fff; | ||
| font-weight: 600; | ||
| font-size: 16px; | ||
| cursor: pointer; | ||
| transition: background-color .15s ease, color .15s ease, border-color .15s ease; | ||
|
|
||
| &:hover { | ||
| background: #ff7a00; | ||
| color: #fff; | ||
| border-color: #ff7a00; | ||
| } | ||
| `; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
매직 컬러 값을 상수로 추출하세요.
Lines 19, 27, 29에 하드코딩된 색상 값들(#DCDCDC, #ff7a00)은 매직 값입니다. 코딩 가이드라인에 따라 명명된 상수로 대체해야 합니다.
색상 테마 파일을 생성하거나 기존 테마 파일을 사용하세요:
// theme/colors.ts 또는 유사한 파일에서
export const COLORS = {
borderGray: '#DCDCDC',
primary: '#ff7a00',
white: '#fff',
// ...
} as const;그런 다음 스타일에서 사용:
+import { COLORS } from '@/theme/colors';
+
export const OptionButton = styled.button`
width: 100%;
padding: 18px 20px;
border-radius: 10px;
- border: 1px solid #DCDCDC;
- background: #fff;
+ border: 1px solid ${COLORS.borderGray};
+ background: ${COLORS.white};
font-weight: 600;
font-size: 16px;
cursor: pointer;
transition: background-color .15s ease, color .15s ease, border-color .15s ease;
&:hover {
- background: #ff7a00;
- color: #fff;
- border-color: #ff7a00;
+ background: ${COLORS.primary};
+ color: ${COLORS.white};
+ border-color: ${COLORS.primary};
}
`;🤖 Prompt for AI Agents
In frontend/src/components/application/modals/ApplicationSelectModal.styles.ts
around lines 15 to 31, replace the hardcoded color literals (#DCDCDC, #ff7a00,
#fff) with named constants from a shared theme/colors file (create one as
theme/colors.ts exporting a COLORS object if it doesn't exist). Import the
COLORS constants at the top of this file and use them for border, background and
hover colors (e.g., COLORS.borderGray, COLORS.primary, COLORS.white) so no magic
color values remain in the styled component.
frontend/src/components/application/modals/ApplicationSelectModal.tsx
Outdated
Show resolved
Hide resolved
| if (!isOpen) return null; | ||
|
|
||
| return ( | ||
| <Styled.Overlay isOpen={isOpen} onClick={onClose} aria-modal="true"> |
There was a problem hiding this comment.
aria-modal 속성을 Container로 이동하세요.
ARIA 명세에 따르면 aria-modal="true"는 모달 컨테이너에 적용되어야 하며, 오버레이가 아닌 실제 대화 상자 요소에 위치해야 합니다.
다음 diff를 적용하세요:
- <Styled.Overlay isOpen={isOpen} onClick={onClose} aria-modal="true">
+ <Styled.Overlay isOpen={isOpen} onClick={onClose}>
<RemoveScroll enabled={isOpen}>
- <Styled.Container isOpen={isOpen} onClick={(e: MouseEvent<HTMLDivElement>) => e.stopPropagation()}>
+ <Styled.Container isOpen={isOpen} onClick={(e: MouseEvent<HTMLDivElement>) => e.stopPropagation()} role="dialog" aria-modal="true">📝 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.
| <Styled.Overlay isOpen={isOpen} onClick={onClose} aria-modal="true"> | |
| <Styled.Overlay isOpen={isOpen} onClick={onClose}> | |
| <RemoveScroll enabled={isOpen}> | |
| <Styled.Container | |
| isOpen={isOpen} | |
| onClick={(e: MouseEvent<HTMLDivElement>) => e.stopPropagation()} | |
| role="dialog" | |
| aria-modal="true" | |
| > |
🤖 Prompt for AI Agents
In frontend/src/components/common/Modal/Modal.tsx around line 17, the
aria-modal="true" attribute is currently placed on the Overlay but per ARIA it
must be on the actual modal container. Remove aria-modal from Styled.Overlay and
add aria-modal="true" to the modal Container element (the element that has
role="dialog" or represents the dialog content); keep the Overlay's
onClick={onClose} behavior unchanged. Ensure the container retains appropriate
role and focus handling while the overlay remains just for
backdrop/click-to-close.
frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx
Outdated
Show resolved
Hide resolved
frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
frontend/src/components/common/Modal/Modal.tsx (1)
24-26: ARIA 속성 위치 수정 및 role 속성 추가 필요
aria-modal="true"는 Overlay가 아닌 실제 모달 컨테이너(Container)에 위치해야 합니다.- Container에
role="dialog"속성이 누락되었습니다. 스크린 리더가 모달을 인식하려면 필수입니다.다음 diff를 적용하세요:
- <Styled.Overlay isOpen={isOpen} onClick={handleOverlayClick} aria-modal="true"> + <Styled.Overlay isOpen={isOpen} onClick={handleOverlayClick}> <RemoveScroll enabled={isOpen}> - <Styled.Container isOpen={isOpen} onClick={(e: MouseEvent<HTMLDivElement>) => e.stopPropagation()}> + <Styled.Container isOpen={isOpen} onClick={(e) => e.stopPropagation()} role="dialog" aria-modal="true">참고: 라인 26의 명시적 타입 어노테이션(
MouseEvent<HTMLDivElement>)도 제거했습니다. TypeScript가 자동으로 추론할 수 있습니다.
📜 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 (2)
frontend/src/components/application/modals/ApplicationSelectModal.tsx(1 hunks)frontend/src/components/common/Modal/Modal.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/application/modals/ApplicationSelectModal.tsx
🧰 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/components/common/Modal/Modal.tsx
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/components/common/Modal/Modal.tsx
🧠 Learnings (1)
📚 Learning: 2025-07-20T11:48:50.207Z
Learnt from: seongwon030
Repo: Moadong/moadong PR: 541
File: frontend/src/pages/ClubDetailPage/components/ClubDetailHeader/ClubDetailHeader.tsx:41-43
Timestamp: 2025-07-20T11:48:50.207Z
Learning: moadong 프로젝트는 Next.js가 아닌 순수 React + react-router-dom 기반의 CSR(Client-Side Rendering) SPA이므로, window 객체 사용에 대한 SSR 호환성 문제를 제기하지 않아야 합니다.
Applied to files:
frontend/src/components/common/Modal/Modal.tsx
| export interface ModalProps { | ||
| isOpen: boolean; | ||
| onClose: () => void; | ||
| title: string; | ||
| description?: string; | ||
| children?: ReactNode; | ||
| onBackdropClick?: () => boolean | void; | ||
| } |
There was a problem hiding this comment.
title 속성 타입 불일치
title은 필수 속성(string)으로 정의되었지만, 28번 줄에서 조건부 렌더링(title &&)을 사용하고 있습니다. 빈 문자열을 허용하려면 타입을 선택적으로 변경하거나, 항상 타이틀을 표시하도록 로직을 수정해야 합니다.
다음 중 하나를 적용하세요:
옵션 1: title을 선택적 속성으로 변경
- title: string;
+ title?: string;옵션 2: 조건부 렌더링 제거 (title이 항상 제공된다면)
- {title && <Styled.Title>{title}</Styled.Title>}
+ <Styled.Title>{title}</Styled.Title>📝 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.
| export interface ModalProps { | |
| isOpen: boolean; | |
| onClose: () => void; | |
| title: string; | |
| description?: string; | |
| children?: ReactNode; | |
| onBackdropClick?: () => boolean | void; | |
| } | |
| export interface ModalProps { | |
| isOpen: boolean; | |
| onClose: () => void; | |
| title?: string; | |
| description?: string; | |
| children?: ReactNode; | |
| onBackdropClick?: () => boolean | void; | |
| } |
🤖 Prompt for AI Agents
In frontend/src/components/common/Modal/Modal.tsx around lines 5 to 12, the
ModalProps defines title as a required string but the component uses conditional
rendering (title && ...) at line 28; either make title optional in the type or
always render the title. Fix by choosing one: Option A — change title: string to
title?: string in ModalProps and keep the conditional render; Option B — remove
the conditional rendering and always render the title (ensuring callers always
pass a non-empty string).
| const Modal = ({ isOpen, onClose, title, description, children, onBackdropClick }: ModalProps) => { | ||
| if (!isOpen) return null; | ||
|
|
||
| const handleOverlayClick = () => { | ||
| const result = onBackdropClick?.(); | ||
| if (result === false) return; | ||
| onClose(); | ||
| }; | ||
|
|
||
| return ( | ||
| <Styled.Overlay isOpen={isOpen} onClick={handleOverlayClick} aria-modal="true"> | ||
| <RemoveScroll enabled={isOpen}> | ||
| <Styled.Container isOpen={isOpen} onClick={(e: MouseEvent<HTMLDivElement>) => e.stopPropagation()}> | ||
| <Styled.Header> | ||
| {title && <Styled.Title>{title}</Styled.Title>} | ||
| <Styled.IconButton aria-label="close" type="button" onClick={onClose}>✕</Styled.IconButton> | ||
| </Styled.Header> | ||
| {description && <Styled.Description>{description}</Styled.Description>} | ||
| <Styled.Body>{children}</Styled.Body> | ||
| </Styled.Container> | ||
| </RemoveScroll> | ||
| </Styled.Overlay> | ||
| ); | ||
| } |
There was a problem hiding this comment.
필수 접근성 기능 누락: ESC 키 지원 및 포커스 관리
접근성을 위해 다음 기능들이 필요합니다:
- ESC 키로 모달 닫기: 키보드 사용자를 위한 필수 기능
- 포커스 트랩: Tab 키로 모달 외부로 포커스가 이동하지 않도록 제한
- 포커스 복원: 모달이 닫힐 때 원래 위치로 포커스 반환
다음 중 하나를 선택하세요:
옵션 1: 직접 구현
ESC 키 핸들러 추가:
useEffect(() => {
if (!isOpen) return;
const handleEscape = (e: KeyboardEvent) => {
if (e.key === 'Escape') {
onClose();
}
};
window.addEventListener('keydown', handleEscape);
return () => window.removeEventListener('keydown', handleEscape);
}, [isOpen, onClose]);포커스 관리를 위해서는 focus-trap-react 같은 라이브러리를 고려하거나, 직접 구현해야 합니다.
옵션 2 (권장): Radix UI Dialog 사용
접근성 기능이 내장된 헤드리스 UI 라이브러리(Radix UI Dialog)를 사용하면 ESC 키, 포커스 트랩, 포커스 복원이 자동으로 처리됩니다.
🤖 Prompt for AI Agents
frontend/src/components/common/Modal/Modal.tsx around lines 14 to 37: the modal
is missing required accessibility features—add ESC key handling to close the
modal, implement a focus trap while the modal is open and restore focus to the
previously focused element when it closes; to fix, on open capture
document.activeElement, add a keydown listener that calls onClose() when Escape
is pressed, wrap the modal content with a focus-trap (or use a library such as
focus-trap-react or switch to Radix UI Dialog which provides ESC, focus trapping
and focus restoration out of the box), ensure click handlers still stop
propagation, and on modal close restore focus to the saved element and clean up
listeners/trap.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/src/components/application/modals/ApplicationSelectModal.tsx (2)
5-11: onBackdropClick의 반환 타입을 더 명확하게 개선하세요.
onBackdropClick?: () => boolean | void타입은 다소 모호합니다. void를 반환할 수도 있고 boolean을 반환할 수도 있어서, 호출하는 측에서 언제 무엇을 반환해야 하는지 불명확합니다.다음 중 하나로 개선하는 것을 권장합니다:
옵션 1: boolean만 반환하도록 변경
- onBackdropClick?: () => boolean | void; + onBackdropClick?: () => boolean;그리고 핸들러에서:
const handleOverlayClick = () => { - const result = onBackdropClick?.(); - if (result === false) return; + if (onBackdropClick?.() === false) return; onClose(); };옵션 2: 명확한 의도를 가진 네이밍
- onBackdropClick?: () => boolean | void; + shouldCloseOnBackdropClick?: () => boolean;As per coding guidelines (일관된 반환 타입 사용).
26-26: onClick 핸들러의 중괄호를 제거하세요.단일 표현식을 반환하는 화살표 함수에서는 중괄호가 불필요합니다.
다음 diff를 적용하세요:
- <Styled.OptionButton key={option.id} onClick={() => {onSelect(option);}}> + <Styled.OptionButton key={option.id} onClick={() => onSelect(option)}>
📜 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 (2)
frontend/src/components/application/modals/ApplicationSelectModal.tsx(1 hunks)frontend/src/components/common/Modal/Modal.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/common/Modal/Modal.tsx
🧰 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/components/application/modals/ApplicationSelectModal.tsx
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/components/application/modals/ApplicationSelectModal.tsx
🧠 Learnings (10)
📚 Learning: 2025-07-19T05:09:10.702Z
Learnt from: CR
Repo: Moadong/moadong PR: 0
File: frontend/.cursorrules:0-0
Timestamp: 2025-07-19T05:09:10.702Z
Learning: Applies to frontend/**/*.tsx : Choose field-level or form-level cohesion based on form requirements.
Applied to files:
frontend/src/components/application/modals/ApplicationSelectModal.tsx
📚 Learning: 2025-07-19T05:09:10.702Z
Learnt from: CR
Repo: Moadong/moadong PR: 0
File: frontend/.cursorrules:0-0
Timestamp: 2025-07-19T05:09:10.702Z
Learning: Applies to frontend/**/*.tsx : Abstract complex logic/interactions into dedicated components or higher-order components (HOCs).
Applied to files:
frontend/src/components/application/modals/ApplicationSelectModal.tsx
📚 Learning: 2025-07-19T05:09:10.702Z
Learnt from: CR
Repo: Moadong/moadong PR: 0
File: frontend/.cursorrules:0-0
Timestamp: 2025-07-19T05:09:10.702Z
Learning: Applies to frontend/**/*.tsx : Separate significantly different conditional UI/logic into distinct components.
Applied to files:
frontend/src/components/application/modals/ApplicationSelectModal.tsx
📚 Learning: 2025-07-19T05:09:10.702Z
Learnt from: CR
Repo: Moadong/moadong PR: 0
File: frontend/.cursorrules:0-0
Timestamp: 2025-07-19T05:09:10.702Z
Learning: Applies to frontend/**/*.tsx : Use component composition instead of props drilling.
Applied to files:
frontend/src/components/application/modals/ApplicationSelectModal.tsx
📚 Learning: 2025-07-19T05:09:10.702Z
Learnt from: CR
Repo: Moadong/moadong PR: 0
File: frontend/.cursorrules:0-0
Timestamp: 2025-07-19T05:09:10.702Z
Learning: Applies to frontend/**/*.tsx : Colocate simple, localized logic or use inline definitions to reduce context switching.
Applied to files:
frontend/src/components/application/modals/ApplicationSelectModal.tsx
📚 Learning: 2025-07-19T05:09:10.702Z
Learnt from: CR
Repo: Moadong/moadong PR: 0
File: frontend/.cursorrules:0-0
Timestamp: 2025-07-19T05:09:10.702Z
Learning: Applies to frontend/**/*.{ts,tsx} : Use consistent return types for similar functions and hooks.
Applied to files:
frontend/src/components/application/modals/ApplicationSelectModal.tsx
📚 Learning: 2025-07-19T05:09:10.702Z
Learnt from: CR
Repo: Moadong/moadong PR: 0
File: frontend/.cursorrules:0-0
Timestamp: 2025-07-19T05:09:10.702Z
Learning: Applies to frontend/**/*.tsx : Break down broad state management into smaller, focused hooks or contexts.
Applied to files:
frontend/src/components/application/modals/ApplicationSelectModal.tsx
📚 Learning: 2025-07-20T11:48:50.207Z
Learnt from: seongwon030
Repo: Moadong/moadong PR: 541
File: frontend/src/pages/ClubDetailPage/components/ClubDetailHeader/ClubDetailHeader.tsx:41-43
Timestamp: 2025-07-20T11:48:50.207Z
Learning: moadong 프로젝트는 Next.js가 아닌 순수 React + react-router-dom 기반의 CSR(Client-Side Rendering) SPA이므로, window 객체 사용에 대한 SSR 호환성 문제를 제기하지 않아야 합니다.
Applied to files:
frontend/src/components/application/modals/ApplicationSelectModal.tsx
📚 Learning: 2025-07-19T05:09:10.702Z
Learnt from: CR
Repo: Moadong/moadong PR: 0
File: frontend/.cursorrules:0-0
Timestamp: 2025-07-19T05:09:10.702Z
Learning: Applies to frontend/**/*.{ts,tsx} : Use unique, descriptive names for custom wrappers and functions to avoid ambiguity.
Applied to files:
frontend/src/components/application/modals/ApplicationSelectModal.tsx
📚 Learning: 2025-07-19T05:09:10.702Z
Learnt from: CR
Repo: Moadong/moadong PR: 0
File: frontend/.cursorrules:0-0
Timestamp: 2025-07-19T05:09:10.702Z
Learning: Applies to frontend/**/*.{ts,tsx} : Avoid hidden side effects; functions should only perform actions implied by their signature (Single Responsibility Principle).
Applied to files:
frontend/src/components/application/modals/ApplicationSelectModal.tsx
🧬 Code graph analysis (1)
frontend/src/components/application/modals/ApplicationSelectModal.tsx (1)
frontend/src/types/application.ts (1)
ApplicationForm(60-63)
🔇 Additional comments (3)
frontend/src/components/application/modals/ApplicationSelectModal.tsx (3)
1-3: import 구문이 깔끔합니다.필요한 의존성만 정확히 임포트되어 있고, 이전 리뷰에서 지적된 미사용 import도 제거되었습니다.
18-32: Early Return 패턴과 컴포넌트 분리가 잘 적용되었습니다.이전 리뷰에서 제안된 대로 OptionsList를 별도 컴포넌트로 분리하고 Early Return 패턴을 사용하여 가독성이 크게 향상되었습니다.
34-52: 컴포넌트 구조와 합성이 잘 설계되었습니다.Modal과 OptionsList를 적절히 합성하여 관심사 분리가 잘 이루어졌습니다. handleOverlayClick 로직도 로컬화되어 있어 코드 가독성이 좋습니다.
As per coding guidelines (컴포넌트 합성 사용, 단순하고 지역적인 로직 배치).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx (1)
74-84: 에러 처리 시 사용자 피드백이 중복될 수 있습니다.Lines 80-83에서 external URL이 없을 때 빈 옵션으로 모달을 열고(Line 81) 동시에 alert를 표시합니다(Line 83).
ApplicationSelectModal이 빈 옵션에 대해 "지원 가능한 지원서 없음" 메시지를 표시한다면, alert와 모달 메시지가 중복될 수 있습니다.모달이 이미 적절한 메시지를 표시한다면 alert를 제거하는 것을 고려하세요:
} catch (e) { const externalApplicationUrl = clubDetail.externalApplicationUrl?.trim(); if (externalApplicationUrl) { window.open(externalApplicationUrl, '_blank'); return; } setOptions([]); setIsOpen(true); console.error('지원서 옵션 조회 중 오류가 발생했습니다.', e); - alert('지원서 정보를 불러오는 중 오류가 발생했습니다. 다시 시도해주세요.'); }또는 에러 상황을 모달에 명확히 전달하려면 options 대신 별도의 error 상태를 사용하는 것을 고려할 수 있습니다.
📜 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/components/application/modals/ApplicationSelectModal.tsx(1 hunks)frontend/src/components/common/Modal/Modal.tsx(1 hunks)frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/components/common/Modal/Modal.tsx
- frontend/src/components/application/modals/ApplicationSelectModal.tsx
🧰 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/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx
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/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx
🧠 Learnings (10)
📚 Learning: 2025-07-19T05:09:10.702Z
Learnt from: CR
Repo: Moadong/moadong PR: 0
File: frontend/.cursorrules:0-0
Timestamp: 2025-07-19T05:09:10.702Z
Learning: Applies to frontend/**/*.tsx : Separate significantly different conditional UI/logic into distinct components.
Applied to files:
frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx
📚 Learning: 2025-07-19T05:09:10.702Z
Learnt from: CR
Repo: Moadong/moadong PR: 0
File: frontend/.cursorrules:0-0
Timestamp: 2025-07-19T05:09:10.702Z
Learning: Applies to frontend/**/*.tsx : Break down broad state management into smaller, focused hooks or contexts.
Applied to files:
frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx
📚 Learning: 2025-07-19T05:09:10.702Z
Learnt from: CR
Repo: Moadong/moadong PR: 0
File: frontend/.cursorrules:0-0
Timestamp: 2025-07-19T05:09:10.702Z
Learning: Applies to frontend/**/*.tsx : Abstract complex logic/interactions into dedicated components or higher-order components (HOCs).
Applied to files:
frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx
📚 Learning: 2025-07-19T05:09:10.702Z
Learnt from: CR
Repo: Moadong/moadong PR: 0
File: frontend/.cursorrules:0-0
Timestamp: 2025-07-19T05:09:10.702Z
Learning: Applies to frontend/**/*.{ts,tsx} : Use consistent return types for similar functions and hooks.
Applied to files:
frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx
📚 Learning: 2025-07-19T05:05:10.196Z
Learnt from: seongwon030
Repo: Moadong/moadong PR: 548
File: frontend/src/pages/ClubDetailPage/ClubDetailPage.tsx:17-57
Timestamp: 2025-07-19T05:05:10.196Z
Learning: ClubDetailPage.tsx에서 notJoinedClubNames 배열의 하드코딩은 의도적인 설계 결정입니다. 개발자가 명시적으로 하드코딩을 선택했으므로 이에 대한 리팩토링 제안을 하지 않아야 합니다.
Applied to files:
frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx
📚 Learning: 2025-07-19T05:09:10.702Z
Learnt from: CR
Repo: Moadong/moadong PR: 0
File: frontend/.cursorrules:0-0
Timestamp: 2025-07-19T05:09:10.702Z
Learning: Applies to frontend/**/*.tsx : Choose field-level or form-level cohesion based on form requirements.
Applied to files:
frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx
📚 Learning: 2025-07-19T05:09:10.702Z
Learnt from: CR
Repo: Moadong/moadong PR: 0
File: frontend/.cursorrules:0-0
Timestamp: 2025-07-19T05:09:10.702Z
Learning: Applies to frontend/**/*.tsx : Colocate simple, localized logic or use inline definitions to reduce context switching.
Applied to files:
frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx
📚 Learning: 2025-07-19T05:09:10.702Z
Learnt from: CR
Repo: Moadong/moadong PR: 0
File: frontend/.cursorrules:0-0
Timestamp: 2025-07-19T05:09:10.702Z
Learning: Applies to frontend/**/*.{ts,tsx} : Define constants near related logic or ensure names link them clearly.
Applied to files:
frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx
📚 Learning: 2025-03-19T05:18:07.818Z
Learnt from: seongwon030
Repo: Moadong/moadong PR: 195
File: frontend/src/pages/AdminPage/AdminPage.tsx:7-7
Timestamp: 2025-03-19T05:18:07.818Z
Learning: AdminPage.tsx에서 현재 하드코딩된 클럽 ID('67d2e3b9b15c136c6acbf20b')는 로그인 기능 구현 후 동적으로 가져오는 방식으로 수정될 예정입니다.
Applied to files:
frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx
📚 Learning: 2025-09-21T02:23:27.796Z
Learnt from: seongwon030
Repo: Moadong/moadong PR: 744
File: frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx:47-48
Timestamp: 2025-09-21T02:23:27.796Z
Learning: ClubApplyButton 컴포넌트에서 ShareButton은 항상 렌더링되어야 하므로 정적 import를 사용하는 것이 적절함. 동적 import는 불필요함.
Applied to files:
frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx
🧬 Code graph analysis (1)
frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx (1)
frontend/src/types/application.ts (1)
ApplicationForm(60-63)
🔇 Additional comments (6)
frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx (6)
8-11: LGTM!새로운 모달 기능에 필요한 imports가 모두 적절하게 추가되었습니다.
28-30: LGTM!모달 상태 관리가 적절하게 구현되었습니다.
35-45: LGTM!내부 지원서로 이동하는 로직이 명확하고 에러 처리도 적절합니다.
47-51:void키워드 사용으로 인한 Promise 처리 누락을 확인하세요.Line 50에서
void goWithForm(option.id)를 사용하면 async 함수의 Promise가 무시되며, 에러가 발생해도 이 함수는 알 수 없습니다. 현재goWithForm내부에 alert 처리가 있어 사용자에게는 에러가 전달되지만, 프로그래밍 관점에서는 명시적으로 에러를 처리하는 것이 더 명확합니다.다음과 같이 명시적으로 처리하는 것을 고려해보세요:
const openByOption = (option?: ApplicationForm) => { if (!option) return; - void goWithForm(option.id); + goWithForm(option.id).catch(() => { + // goWithForm 내부에서 이미 alert 처리됨 + }); };또는 async/await 방식:
- const openByOption = (option?: ApplicationForm) => { + const openByOption = async (option?: ApplicationForm) => { if (!option) return; - void goWithForm(option.id); + await goWithForm(option.id); };
32-32: LGTM!적절한 가드 절입니다.
111-116: LGTM!
ApplicationSelectModal이 적절하게 렌더링되고 있으며, 필요한 props가 모두 올바르게 전달되었습니다.
| if (list.length <= 0) { | ||
| return; | ||
| } | ||
|
|
||
| if (list.length === 1) { | ||
| await goWithForm(list[0].id); | ||
| return; | ||
| } | ||
| window.open(externalFormLink, '_blank', 'noopener,noreferrer'); | ||
| setOptions(list); | ||
| setIsOpen(true); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
매직 넘버를 상수로 교체하고 빈 목록 처리를 개선하세요.
Line 64, 68에서 매직 넘버 0과 1을 사용하고 있습니다. 코딩 가이드라인에 따라 명명된 상수로 교체해야 합니다. 또한 Line 64-66에서 지원서가 없을 때 사용자에게 피드백이 없어 혼란을 줄 수 있습니다.
다음과 같이 개선하세요:
+ const NO_APPLICATIONS = 0;
+ const SINGLE_APPLICATION = 1;
+
const handleClick = async () => {
trackEvent(EVENT_NAME.CLUB_APPLY_BUTTON_CLICKED);
if (deadlineText === RECRUITMENT_STATUS.CLOSED) {
alert(`현재 ${clubDetail.name} 동아리는 모집 기간이 아닙니다.`);
return;
}
try {
const list = await getApplicationOptions(clubId);
- if (list.length <= 0) {
+ if (list.length === NO_APPLICATIONS) {
+ alert('현재 지원 가능한 지원서가 없습니다.');
return;
}
- if (list.length === 1) {
+ if (list.length === SINGLE_APPLICATION) {
await goWithForm(list[0].id);
return;
}
setOptions(list);
setIsOpen(true);Based on coding guidelines
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx
around lines 64 to 73, replace the magic numbers 0 and 1 with named constants
(e.g., NO_APPLICATIONS = 0, SINGLE_APPLICATION = 1) and refactor the empty-list
branch to provide user feedback instead of silently returning (for example show
a toast/error message or disable the apply action and log context), keep the
single-item path to call goWithForm(list[0].id) and return, and otherwise
setOptions(list) and setIsOpen(true); ensure constants are declared near the top
of the module and update any comparisons to use them.
4 participants
#️⃣연관된 이슈
📝작업 내용
이를 위해 재사용 가능한 공통
Modal컴포넌트를 분리하여 제작했으며, 지원서 목록을 조회하고(getApplicationOptions), 특정 지원서 정보를 가져오는(getApplication) 신규 API 함수 2개를 추가했습니다.최종적으로 이 기능들을 조합하여
ClubApplyButton의 '지원하기' 버튼 클릭 시 분기 처리 로직을 개선했습니다.1️⃣ 공통
Modal컴포넌트 제작 (Modal.tsx)도입 이유 : 기존에는 모달이 없었습니다. 그러나 현재 모달 없이 임시로 띄우는 경고 메시지를 추후 모달로 띄우기 위해 필요하다고 판단했습니다. 현재 구현한
ApplicationSelectModal뿐만 아니라, 향후 프로젝트 전반에서 사용될 알림(Alert), 확인(Confirm) 등의 다양한 모달에서 일관된 UI/UX를 제공하고 재사용성을 높이기 위해 공통Modal컴포넌트를 제작했습니다.주요 기능 및 사용법
isOpenprop으로 모달의 열림/닫힘 상태를 제어onCloseprop으로 닫기 버튼(X)의 동작 정의title과description전달해 헤더 영역 구성childrenprop을 통해 모달의 본문(Body) 영역에 커스텀 컴포넌트나 엘리먼트 삽입 가능useEffect를 사용해 모달이 열려있는 동안document.body의 스크롤을hidden으로 처리onBackdropClickprop을 통해 오버레이(배경) 클릭 시 동작 제어 가능(기본값 :onClose)-> 예시 사용법
2️⃣ 지원서 API 연동 코드 추가
지원서 로직 처리에 필요한 클럽 지원서 사용자 API 함수 2개를 추가했습니다.
GET /api/club/{clubId}/apply: 클럽의 활성화된 지원서 목록 불러오기->
getApplicationOptions(clubId): 동아리 ID(clubId)를 기반으로 해당 동아리가 보유한 지원서 목록(id, title 배열)을 조회합니다.GET /api/club/{clubId}apply/{applicationFormId}: 클럽 지원서 양식 불러오기->
getApplication(clubId, applicationFormId): 특정 지원서(formId)의 상세 정보를 조회합니다.3️⃣ '지원하기' 버튼 로직 개선 (
ClubApplyButton.tsx)기존의 단일 지원서 이동 로직을
getApplicationOptionsAPI와 연동하여 다음과 같이 개선했습니다.지원하기버튼 클릭 시getApplicationOptionsAPI를 호출합니다.ApplicationSelectModal을 엽니다.goWithForm함수가 실행되어 해당 지원서 페이지로 이동합니다.goWithForm을 즉시 실행하여 바로 해당 지원서 페이지로 이동합니다.ApplicationSelectModal을 열어 "지원 가능한 분야가 없습니다."라는 메시지를 표시합니다.externalApplicationUrl이 있는 경우, 기존 로직대로 외부 링크로 이동합니다.🧑💻 개발 회고 (Mock Data 작업 경험)
이번 작업은 백엔드 API가 완성되기 전, Mock Data를 기반으로 선행 개발을 진행했습니다.
ApplicationSelectModal의 UI를 미리 구현하고, 지원서 개수에 따른(0개, 1개, N개) 분기 로직을 사전에 테스트해볼 수 있었습니다.결과적으로 Mock Data를 사용해본 것 자체는 좋은 경험이었으나, 재작업을 줄이기 위해서는 개발 초기 백엔드 팀과 API 응답을 조기에 명확히 합의하고 그에 맞춰 개발하는 것이 중요하다는 것을 알게되었습니다.
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit
새로운 기능
스타일 / UX
버그 수정 / 안내