[feature] 관리자 모집 기간 설정 시 분 단위 시간 선택을 지원한다#1293
Conversation
This reverts commit 4d9eb2d.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Warning
|
| Cohort / File(s) | Summary |
|---|---|
RecruitEditTab 스타일 frontend/src/pages/AdminPage/tabs/RecruitEditTab/RecruitEditTab.styles.ts |
AlwaysRecruitButton의 공개 prop을 { $active: boolean }에서 { $isAlwaysActive: boolean }로 변경하고 기본(비활성) 색상/배경/테두리 스타일을 회색 계열로 변경. $isAlwaysActive일 때 활성 스타일 적용 로직 추가. |
RecruitEditTab 로직 frontend/src/pages/AdminPage/tabs/RecruitEditTab/RecruitEditTab.tsx |
기존 Calendar 사용을 DateTimeRangePicker로 교체. isAlwaysRecruiting 상태 추가, 시작/종료 동기화 및 backup/restore 로직 도입, 초기화 useEffect 및 ISO 저장 방식으로 정리. 버튼 ARIA/명명 및 핸들러 리네임 반영. |
기존 Calendar 제거 frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/Calendar/Calendar.tsx, .../Calendar.styles.ts |
기존 Calendar 컴포넌트 및 관련 스타일 파일(react-datepicker 통합 스타일, 커스텀 헤더 등) 전체 삭제. |
DateTimeRangePicker 및 스타일 추가 frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimeRangePicker.tsx, .../DateTimeRangePicker.styles.ts |
새 DateTimeRangePicker 컴포넌트와 스타일 추가. 시작/종료 입력 표시, 패널 토글, 외부 클릭 처리, disabledEnd 지원 등 UI 상호작용 구현. |
DatePickerPanel 추가 frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DatePickerPanel.tsx, .../DatePickerPanel.styles.ts |
inline react-datepicker 기반 DatePickerPanel 추가. ResizeObserver로 높이 보고, 월 이동/날짜 선택 핸들러 포함. |
DateTimePanel 추가 frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimePanel.tsx, .../DateTimePanel.styles.ts |
DatePickerPanel과 TimePickerPanel을 조합해 날짜+시간 선택 UI(헤더 월 네비게이션 포함) 제공. |
TimePickerPanel 추가 frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/TimePickerPanel.tsx, .../TimePickerPanel.styles.ts |
시/분 스크롤 선택 UI(00–23시, 00–59분) 추가. 초기 스크롤 정렬 및 선택 강조 처리 포함. |
유틸리티 추가 및 테스트 frontend/src/utils/recruitmentDateFormatter.ts, frontend/src/utils/recruitmentDateFomatter.test.ts |
`formatRecruitmentDateTime(date?: Date |
Sequence Diagram(s)
sequenceDiagram
participant Admin as RecruitEditTab
participant Picker as DateTimeRangePicker
participant Panel as DateTimePanel
participant Calendar as DatePickerPanel
participant Time as TimePickerPanel
Admin->>Picker: 렌더(recruitmentStart, recruitmentEnd, disabledEnd)
Picker->>Picker: 입력 클릭 → activePicker 설정
Picker->>Panel: 패널 열기(date, onChangeDate, viewMonth)
Panel->>Calendar: 인라인 캘린더 렌더링(selected, viewMonth)
Panel->>Time: 시간/분 스크롤 렌더링(selected, height)
Calendar-->>Panel: 날짜 선택(onChangeDate)
Time-->>Panel: 시간/분 선택(onChangeDate)
Panel-->>Picker: 합쳐진 Date 반환(onChangeDate)
Picker-->>Admin: recruitmentStart/recruitmentEnd 변경 콜백 호출
Admin->>Admin: isAlwaysRecruiting 토글 → backup/restore 실행
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
- [fix] 관리자 페이지 모집 기간 캘린더 레이아웃 오류를 수정한다 #1149: Calendar.styles 제거/대체 작업과 react-datepicker 스타일 변경에 직접 연관.
- [feature] 관리자 페이지 모집기간 선택에 상시모집 처리 버튼을 추가한다 #774: "상시모집" 처리 및 AlwaysRecruitButton 관련 스타일/prop 변경과 중복 가능.
- [fix] 모집기간 설정 시간 보정 #690: RecruitEditTab의 recruitmentStart/recruitmentEnd 초기화 및 저장 흐름 변경과 연관성 높음.
Suggested labels
✅ Test
Suggested reviewers
- seongwon030
- lepitaaar
- Zepelown
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | PR 제목이 변경사항의 핵심을 명확하게 요약하고 있습니다. '관리자 모집 기간 설정 시 분 단위 시간 선택을 지원한다'는 전체 변경사항의 주요 목표를 정확하게 반영하고 있습니다. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
feature/#1165-admin-recruitment-minute-time-MOA-624
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.
Tip
CodeRabbit can use Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.
Add a .trivyignore file to your project to customize which findings Trivy reports.
🎨 UI 변경사항을 확인해주세요
2개 스토리 변경 · 전체 56개 스토리 · 22개 컴포넌트 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimeRangePicker.styles.ts (1)
11-34: 접근성을 위한 포커스 스타일 및 비활성화 커서 추가 권장키보드 네비게이션 사용자를 위한 포커스 스타일과 비활성화 상태의 커서 스타일이 누락되어 있습니다.
♻️ 제안된 개선 사항
&:disabled { background-color: ${colors.gray[400]}; color: ${colors.gray[500]}; + cursor: not-allowed; } + + &:focus-visible { + outline: 2px solid ${colors.primary[800]}; + outline-offset: 2px; + } `;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimeRangePicker.styles.ts` around lines 11 - 34, The Input styled component is missing keyboard focus styles and a disabled cursor; update the Input styled.button (component named Input) to add a clear focus state (e.g., :focus-visible or :focus with an accessible outline/box-shadow using theme colors) so keyboard users can see focus, and add cursor: not-allowed to the &:disabled block so disabled buttons convey non-interactivity; ensure the focus style does not conflict with the existing $isActive styles.frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/TimePickerPanel.tsx (1)
62-65: 인라인 스타일이 styled-component와 중복됩니다.
ScrollList에overflow-y: auto가 이미 정의되어 있으므로 인라인 스타일의overflowY: 'auto'는 중복입니다.position: 'relative'가 필요하다면 styled-component에 추가하는 것이 좋습니다.♻️ 제안하는 수정
TimePickerPanel.styles.ts의ScrollList에position: relative를 추가하고:export const ScrollList = styled.div<{ $containerHeight: number }>` height: ${({ $containerHeight }) => $containerHeight}px; overflow-y: auto; margin: 20px 0; + position: relative;
TimePickerPanel.tsx에서 인라인 스타일을 제거:<Styled.ScrollList $containerHeight={height} ref={hourListRef} - style={{ overflowY: 'auto', position: 'relative' }} ><Styled.ScrollList $containerHeight={height} ref={minuteListRef} - style={{ overflowY: 'auto', position: 'relative' }} >Also applies to: 81-84
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/TimePickerPanel.tsx` around lines 62 - 65, Styled.ScrollList is receiving an inline style that duplicates its styled-component rules (overflow-y: auto) and also sets position: 'relative'; update the ScrollList styled-component in TimePickerPanel.styles.ts to include position: relative (and keep its overflow-y definition), then remove the inline style object from Styled.ScrollList in TimePickerPanel.tsx (the instances using hourListRef and minuteListRef around the given lines and the similar block at 81-84) so layout is driven solely by the styled-component.frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimePanel.styles.ts (1)
5-14:left: 0속성이 중복 선언되어 있습니다.Line 7과 Line 13에서
left: 0이 두 번 선언되어 있습니다. 하나를 제거해 주세요.♻️ 제안하는 수정
export const Panel = styled.div<{ $alignRight?: boolean }>` position: absolute; top: 55px; left: 0; background: ${colors.base.white}; border-radius: 12px; box-shadow: 0 8px 20px rgba(0, 0, 0, 0.18); overflow: hidden; z-index: 10; - left: 0; right: auto;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimePanel.styles.ts` around lines 5 - 14, In DateTimePanel.styles.ts (the styled block used by DateTimePanel), remove the duplicate CSS property declaration so only a single "left: 0" remains; find the style rule in the DateTimePanel.styles (or the styled component) and delete the redundant left: 0 line to avoid duplicate declarations.frontend/src/pages/AdminPage/tabs/RecruitEditTab/RecruitEditTab.styles.ts (1)
30-36: 조건부 스타일링 패턴이 다른 파일들과 일관되지 않습니다.다른 스타일 파일들(예:
DateTimePanel.styles.ts,TimePickerPanel.styles.ts)에서는css헬퍼를 사용하고 있습니다. 일관성을 위해 동일한 패턴을 사용하는 것을 권장합니다.♻️ 제안하는 수정
+import styled, { css } from 'styled-components'; -import styled from 'styled-components';${({ $isAlwaysActive }) => - $isAlwaysActive && - ` - color: ${colors.base.white}; - background-color: ${colors.primary[800]}; - border: none; - `} + $isAlwaysActive && + css` + color: ${colors.base.white}; + background-color: ${colors.primary[800]}; + border: none; + `}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/AdminPage/tabs/RecruitEditTab/RecruitEditTab.styles.ts` around lines 30 - 36, Replace the inline conditional template string with the styled-components css helper to match other files: import css from styled-components (or named css) and change the block that checks the $isAlwaysActive prop (the current ${({ $isAlwaysActive }) => $isAlwaysActive && `...` } expression) to return css`...` instead of a raw template string, preserving the same properties (color, background-color, border) and the $isAlwaysActive prop name so the pattern aligns with DateTimePanel.styles.ts and TimePickerPanel.styles.ts.frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimePanel.tsx (1)
17-20: 부모 컴포넌트의date값 변경 시viewMonth동기화 고려
viewMonth는 컴포넌트 마운트 시에만date로 초기화되므로, 피커가 열려있는 상태에서 부모의date값이 변경되면viewMonth와 실제 선택된 날짜가 불일치할 수 있습니다. 현재 구조에서는 피커 타입 전환 시 컴포넌트가 다시 마운트되어viewMonth가 재초기화되지만, 동일한 타입의 피커가 열려있는 상태에서 외부 업데이트가 발생할 경우 이 문제가 나타날 수 있습니다.필요하다면
useEffect로date변경을 감지하여viewMonth를 동기화하는 것을 고려해 주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimePanel.tsx` around lines 17 - 20, The viewMonth state (initialized with useState in DateTimePanel) is only set once from the incoming date prop, so if the parent updates date while the picker remains mounted the displayed month can get out of sync; add a useEffect that watches the date prop and calls setViewMonth(date || new Date()) to synchronize viewMonth when date changes (keep the existing early return if !date), referencing the viewMonth and setViewMonth state variables and the date prop in DateTimePanel.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DatePickerPanel.styles.ts`:
- Around line 35-40: In the CSS selector block that hides datepicker UI
(.react-datepicker__triangle, .react-datepicker__navigation,
.react-datepicker__header, .react-datepicker__curront-month) there is a typo:
change the class selector .react-datepicker__curront-month to
.react-datepicker__current-month so the rule applies correctly to the current
month element.
In
`@frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimeRangePicker.tsx`:
- Around line 57-60: When disabledEnd becomes true the end panel must be closed;
add a useEffect inside DateTimeRangePicker that watches disabledEnd and
activePicker and, if disabledEnd && activePicker === 'end', programmatically
close the end picker (call the existing togglePicker to clear the active picker
or call the component's setActivePicker/close handler so activePicker is no
longer 'end'). Ensure the same guard is applied for any render/visibility logic
that checks activePicker so the panel cannot remain open while disabledEnd is
true.
In `@frontend/src/pages/AdminPage/tabs/RecruitEditTab/RecruitEditTab.tsx`:
- Around line 63-68: The current branch replaces empty/unspecified recruitment
fields with new Date(), causing unintended saved dates; instead, call
recruitmentDateParser for clubDetail.recruitmentStart and
clubDetail.recruitmentEnd and preserve its null return for blank/"미정" values
(i.e., remove the fallback to new Date()), so parsedStart and parsedEnd become
whatever recruitmentDateParser returns (null or a Date); also update any
downstream usage of parsedStart/parsedEnd in this component (e.g., form initial
values or submit handlers) to handle nulls if necessary.
- Around line 89-112: The functional updater passed to setIsAlwaysRecruiting
currently performs side effects (mutating backupRangeRef.current and calling
setRecruitmentStart/setRecruitmentEnd) which must be moved out; change the
updater used in the event handler so it only returns the toggled boolean
(compute nextMode inside the updater or just toggle based on prior value) and
then, immediately after calling setIsAlwaysRecruiting, run the side-effect logic
in the handler or in a useEffect that watches isAlwaysRecruiting: when enabling,
set backupRangeRef.current = { start: recruitmentStart, end: recruitmentEnd };
when disabling, read backupRangeRef.current and call
setRecruitmentStart(baseDate) and setRecruitmentEnd(...) using
isFarFuture(backup.end) to decide the end value. Ensure all mutations and
setRecruitment* calls are removed from the function passed to
setIsAlwaysRecruiting.
---
Nitpick comments:
In
`@frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimePanel.styles.ts`:
- Around line 5-14: In DateTimePanel.styles.ts (the styled block used by
DateTimePanel), remove the duplicate CSS property declaration so only a single
"left: 0" remains; find the style rule in the DateTimePanel.styles (or the
styled component) and delete the redundant left: 0 line to avoid duplicate
declarations.
In
`@frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimePanel.tsx`:
- Around line 17-20: The viewMonth state (initialized with useState in
DateTimePanel) is only set once from the incoming date prop, so if the parent
updates date while the picker remains mounted the displayed month can get out of
sync; add a useEffect that watches the date prop and calls setViewMonth(date ||
new Date()) to synchronize viewMonth when date changes (keep the existing early
return if !date), referencing the viewMonth and setViewMonth state variables and
the date prop in DateTimePanel.
In
`@frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimeRangePicker.styles.ts`:
- Around line 11-34: The Input styled component is missing keyboard focus styles
and a disabled cursor; update the Input styled.button (component named Input) to
add a clear focus state (e.g., :focus-visible or :focus with an accessible
outline/box-shadow using theme colors) so keyboard users can see focus, and add
cursor: not-allowed to the &:disabled block so disabled buttons convey
non-interactivity; ensure the focus style does not conflict with the existing
$isActive styles.
In
`@frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/TimePickerPanel.tsx`:
- Around line 62-65: Styled.ScrollList is receiving an inline style that
duplicates its styled-component rules (overflow-y: auto) and also sets position:
'relative'; update the ScrollList styled-component in TimePickerPanel.styles.ts
to include position: relative (and keep its overflow-y definition), then remove
the inline style object from Styled.ScrollList in TimePickerPanel.tsx (the
instances using hourListRef and minuteListRef around the given lines and the
similar block at 81-84) so layout is driven solely by the styled-component.
In `@frontend/src/pages/AdminPage/tabs/RecruitEditTab/RecruitEditTab.styles.ts`:
- Around line 30-36: Replace the inline conditional template string with the
styled-components css helper to match other files: import css from
styled-components (or named css) and change the block that checks the
$isAlwaysActive prop (the current ${({ $isAlwaysActive }) => $isAlwaysActive &&
`...` } expression) to return css`...` instead of a raw template string,
preserving the same properties (color, background-color, border) and the
$isAlwaysActive prop name so the pattern aligns with DateTimePanel.styles.ts and
TimePickerPanel.styles.ts.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
frontend/src/pages/AdminPage/tabs/RecruitEditTab/RecruitEditTab.styles.tsfrontend/src/pages/AdminPage/tabs/RecruitEditTab/RecruitEditTab.tsxfrontend/src/pages/AdminPage/tabs/RecruitEditTab/components/Calendar/Calendar.styles.tsfrontend/src/pages/AdminPage/tabs/RecruitEditTab/components/Calendar/Calendar.tsxfrontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DatePickerPanel.styles.tsfrontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DatePickerPanel.tsxfrontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimePanel.styles.tsfrontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimePanel.tsxfrontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimeRangePicker.styles.tsfrontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimeRangePicker.tsxfrontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/TimePickerPanel.styles.tsfrontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/TimePickerPanel.tsxfrontend/src/utils/recruitmentDateFormatter.ts
💤 Files with no reviewable changes (2)
- frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/Calendar/Calendar.tsx
- frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/Calendar/Calendar.styles.ts
| const parsedStart = clubDetail.recruitmentStart | ||
| ? recruitmentDateParser(clubDetail.recruitmentStart) | ||
| : null; | ||
| const end = clubDetail.recruitmentEnd | ||
| : new Date(); | ||
| const parsedEnd = clubDetail.recruitmentEnd | ||
| ? recruitmentDateParser(clubDetail.recruitmentEnd) | ||
| : null; | ||
| const isAlways = isFarFuture(end); | ||
|
|
||
| if (isAlways) { | ||
| setAlways(true); | ||
| backupRangeRef.current = { start, end }; | ||
| setRecruitmentStart(start ?? now); | ||
| } else { | ||
| setRecruitmentStart(start ?? now); | ||
| setRecruitmentEnd(end ?? now); | ||
| } | ||
| : new Date(); |
There was a problem hiding this comment.
빈 모집기간을 현재 시각으로 치환하면 저장 시 데이터가 오염될 수 있습니다.
recruitmentDateParser는 빈 문자열/미정을 null로 처리하도록 되어 있는데, 현재 분기에서는 빈 값이 new Date()로 들어가 의도치 않게 모집기간이 생성됩니다. 빈 값은 null을 유지하도록 파싱 경로를 통일해 주세요.
수정 예시
- const parsedStart = clubDetail.recruitmentStart
- ? recruitmentDateParser(clubDetail.recruitmentStart)
- : new Date();
- const parsedEnd = clubDetail.recruitmentEnd
- ? recruitmentDateParser(clubDetail.recruitmentEnd)
- : new Date();
+ const parsedStart = recruitmentDateParser(clubDetail.recruitmentStart ?? '');
+ const parsedEnd = recruitmentDateParser(clubDetail.recruitmentEnd ?? '');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/AdminPage/tabs/RecruitEditTab/RecruitEditTab.tsx` around
lines 63 - 68, The current branch replaces empty/unspecified recruitment fields
with new Date(), causing unintended saved dates; instead, call
recruitmentDateParser for clubDetail.recruitmentStart and
clubDetail.recruitmentEnd and preserve its null return for blank/"미정" values
(i.e., remove the fallback to new Date()), so parsedStart and parsedEnd become
whatever recruitmentDateParser returns (null or a Date); also update any
downstream usage of parsedStart/parsedEnd in this component (e.g., form initial
values or submit handlers) to handle nulls if necessary.
| setIsAlwaysRecruiting((prevMode) => { | ||
| const nextMode = !prevMode; | ||
| const now = new Date(); | ||
| if (nextMode) { | ||
| // 상시모집 활성화 시 현재 날짜 백업 | ||
| backupRangeRef.current = { | ||
| start: recruitmentStart, | ||
| end: recruitmentEnd, | ||
| }; | ||
| } else { | ||
| // 상시모집 비활성화 | ||
| const { start, end } = backupRangeRef.current; | ||
| const backupWasAlways = isFarFuture(end); | ||
| if (backupWasAlways) { | ||
| // 백업이 상시모집인 경우 | ||
| const base = start ?? now; | ||
| setRecruitmentStart(base); | ||
| setRecruitmentEnd(base); | ||
| // 상시모집 비활성화 시 백업 데이터 복구 | ||
| const backup = backupRangeRef.current; | ||
| const baseDate = backup.start || now; | ||
|
|
||
| setRecruitmentStart(baseDate); | ||
|
|
||
| if (isFarFuture(backup.end)) { | ||
| setRecruitmentEnd(baseDate); | ||
| } else { | ||
| // 백업이 상시모집이 아닌 경우 | ||
| setRecruitmentStart(start ?? now); | ||
| setRecruitmentEnd(end ?? now); | ||
| setRecruitmentEnd(backup.end || now); | ||
| } | ||
| } | ||
| return !prev; | ||
| return nextMode; | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
React 19 useState functional updater Strict Mode re-invocation side effects
💡 Result:
In React 19, if you run your app under <StrictMode> in development, React may invoke your useState functional updater twice (e.g. setCount(c => /*...*/)), specifically because Strict Mode intentionally double-calls “functions that should be pure” to help you detect hidden side effects. This includes functions you pass to useState and to state “set” functions. [1]
What this means for setState(prev => next)
- The updater function must be pure: compute
nextfromprevonly. - Do not perform side effects inside the updater (logging, analytics, mutations, network calls, writing to storage, etc.), because Strict Mode may run it twice and expose those side effects. [1]
What to do instead
- Keep the updater as a pure calculation (no mutation of
prev, no external writes). [1] - Move side effects to:
- an event handler (often the best place), or
- a
useEffectthat reacts to the committed state change (note: effects are also intentionally re-run an extra setup/cleanup cycle in dev Strict Mode). [1]
Source
[1] React docs: <StrictMode> (development-only double invocation and what gets double-called) https://react.dev/reference/react/StrictMode
setIsAlwaysRecruiting 함수형 업데이터 내부의 부수효과를 분리해야 합니다.
React 공식 문서에 따르면, Strict Mode의 개발 환경에서 useState의 함수형 업데이터는 의도적으로 두 번 호출됩니다. 업데이터는 순수 함수여야 하므로 이전 상태에서 다음 상태를 계산하는 로직만 포함해야 합니다. 현재 코드는 업데이터 내부에서 backupRangeRef.current 변경과 setRecruitmentStart, setRecruitmentEnd 호출이 함께 실행되는데, 이는 부수효과에 해당하므로 두 번 실행될 위험이 있습니다.
nextMode 계산을 먼저 한 후, 그 결과에 따라 ref 변경과 다른 상태 업데이트를 이벤트 핸들러에서 또는 useEffect에서 분리하여 처리하는 것이 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/AdminPage/tabs/RecruitEditTab/RecruitEditTab.tsx` around
lines 89 - 112, The functional updater passed to setIsAlwaysRecruiting currently
performs side effects (mutating backupRangeRef.current and calling
setRecruitmentStart/setRecruitmentEnd) which must be moved out; change the
updater used in the event handler so it only returns the toggled boolean
(compute nextMode inside the updater or just toggle based on prior value) and
then, immediately after calling setIsAlwaysRecruiting, run the side-effect logic
in the handler or in a useEffect that watches isAlwaysRecruiting: when enabling,
set backupRangeRef.current = { start: recruitmentStart, end: recruitmentEnd };
when disabling, read backupRangeRef.current and call
setRecruitmentStart(baseDate) and setRecruitmentEnd(...) using
isFarFuture(backup.end) to decide the end value. Ensure all mutations and
setRecruitment* calls are removed from the function passed to
setIsAlwaysRecruiting.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimePanel.styles.ts (2)
24-44: 색상은 토큰으로 통일하면 테마 유지보수가 더 쉬워집니다.
Header/NavButton의color: white;를colors.base.white로 맞추면 파일 내 색상 사용 규칙이 일관됩니다.제안 diff
export const Header = styled.div` display: grid; grid-template-columns: 40px 1fr 40px 60px 60px; height: 44px; background: ${colors.primary[800]}; - color: white; + color: ${colors.base.white}; align-items: center; text-align: center; `; export const NavButton = styled.button` background: none; border: none; - color: white; + color: ${colors.base.white}; font-size: 20px; cursor: pointer;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimePanel.styles.ts` around lines 24 - 44, Replace literal "white" color usage with the project's color token for consistency: in the styled components Header and NavButton change their color declarations from "color: white;" to use colors.base.white (i.e. update the color property in Header and NavButton to colors.base.white) so the file follows the tokenized color convention.
4-22:Panel의 중복left선언은 하나로 정리하는 게 좋습니다.Line 7과 Line 13에
left: 0;가 중복되어 있어요. 동작에는 영향이 없지만 정렬 분기 가독성이 떨어집니다.제안 diff
export const Panel = styled.div<{ $alignRight?: boolean }>` position: absolute; top: 55px; left: 0; background: ${colors.base.white}; border-radius: 12px; box-shadow: 0 8px 20px rgba(0, 0, 0, 0.18); overflow: hidden; z-index: 10; - left: 0; right: auto;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimePanel.styles.ts` around lines 4 - 22, The styled component Panel has duplicate left: 0 declarations; remove the redundant one so the default positioning sets left: 0 only once, and keep the existing alignment branch that sets right: 0 and left: auto when the $alignRight prop is true (i.e., update the Panel styled.div to have a single left: 0 and keep the ${({ $alignRight }) => ...} block unchanged to flip to left: auto when $alignRight is truthy).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimePanel.styles.ts`:
- Around line 24-44: Replace literal "white" color usage with the project's
color token for consistency: in the styled components Header and NavButton
change their color declarations from "color: white;" to use colors.base.white
(i.e. update the color property in Header and NavButton to colors.base.white) so
the file follows the tokenized color convention.
- Around line 4-22: The styled component Panel has duplicate left: 0
declarations; remove the redundant one so the default positioning sets left: 0
only once, and keep the existing alignment branch that sets right: 0 and left:
auto when the $alignRight prop is true (i.e., update the Panel styled.div to
have a single left: 0 and keep the ${({ $alignRight }) => ...} block unchanged
to flip to left: auto when $alignRight is truthy).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
frontend/src/pages/AdminPage/tabs/RecruitEditTab/RecruitEditTab.styles.tsfrontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimePanel.styles.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/pages/AdminPage/tabs/RecruitEditTab/RecruitEditTab.styles.ts
There was a problem hiding this comment.
♻️ Duplicate comments (1)
frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimeRangePicker.tsx (1)
73-83:⚠️ Potential issue | 🟡 Minor
disabledEnd전환 시 패널 렌더 가드가 없어 한 프레임 노출될 수 있습니다.현재는
useEffect로 닫기 때문에disabledEnd=true로 바뀐 직후 한 번은 종료 패널이 렌더될 수 있습니다. 렌더 조건에서도disabledEnd를 함께 확인해 즉시 차단해 주세요.수정 예시
- {activePicker && ( + {(activePicker === 'start' || (activePicker === 'end' && !disabledEnd)) && ( <DateTimePanel $alignRight={activePicker === 'end'} date={activePicker === 'start' ? recruitmentStart : recruitmentEnd} onChangeDate={ activePicker === 'start' ? onChangeRecruitmentStart : onChangeRecruitmentEnd } /> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimeRangePicker.tsx` around lines 73 - 83, The DateTimePanel can briefly render one frame when disabledEnd flips true because the current guard only checks activePicker; update the render condition around DateTimePanel to also verify disabledEnd (i.e., only render when activePicker && !(activePicker === 'end' && disabledEnd)) so the end panel is blocked immediately; keep existing useEffect close logic but add this immediate check referencing activePicker, disabledEnd, DateTimePanel, recruitmentEnd, and onChangeRecruitmentEnd to prevent that one-frame flash.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimeRangePicker.tsx`:
- Around line 73-83: The DateTimePanel can briefly render one frame when
disabledEnd flips true because the current guard only checks activePicker;
update the render condition around DateTimePanel to also verify disabledEnd
(i.e., only render when activePicker && !(activePicker === 'end' &&
disabledEnd)) so the end panel is blocked immediately; keep existing useEffect
close logic but add this immediate check referencing activePicker, disabledEnd,
DateTimePanel, recruitmentEnd, and onChangeRecruitmentEnd to prevent that
one-frame flash.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DatePickerPanel.styles.tsfrontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimeRangePicker.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DatePickerPanel.styles.ts
| import { colors } from '@/styles/theme/colors'; | ||
| import 'react-datepicker/dist/react-datepicker.css'; | ||
|
|
||
| const commonCellLayout = css` |
There was a problem hiding this comment.
여기서 styled가 아닌 css를 쓴 이유가 있었나요?
There was a problem hiding this comment.
react-datepicker 내부 클래스에 스타일을 적용해야해서
styled 컴포넌트로 분리하기 보다는 css helper를 사용해 공통 스타일을 묶었습니다.
react-datepicker가 생성하는 DOM 구조에 직접 스타일을 덮어쓰는 방식이라
컴포넌트로 분리하기보다는 css 블록을 interpolation하는 형태가 더 자연스럽다고 판단했습니다ㅏ
There was a problem hiding this comment.
제가 이해하기로는 interpolation은 css안에 js코드가 들어가는 일명 props를 전달하는 방식인 경우를 뜻하는걸로 봤어요.
그렇다면 css helper를 사용한 것이지, interpolation과는 거리가 멀지 않나요?
There was a problem hiding this comment.
말씀하신 대로 props를 통한 동적 제어가 대표적 interpolation 방식입니다.
다만, styled-components 공식문서를 보면 css helper가 생성하는 결과물을 interpolations를 포함하는 형태로 정의하고 있어서 이 역시 포함된다고 생각했습니다.
이번에는 외부 라이브러리를 사용했기 때문에 이미 생성한 클래스를 제어하기 위해 새로운 styled 컴포넌트로 분리하는 것보다도 react-datepicker가 생성하는 기존 클래스의 속성을 덮어쓰는 것이 더 맞는 것 같아서 스타일 속성만 공유하는 css 방식이 더 직관적이라 생각했습니다.
There was a problem hiding this comment.
styled-components가 이미 interpolations를 포함하는 형태군요. 덕분에 styled-components 내부 동작을 더 잘 이해할 수 있었어요. styled-components 내부동작을 노션에도 정리했습니다.
|
|
||
| const observer = new ResizeObserver(() => { | ||
| onHeightChange(monthElement.offsetHeight); | ||
| }); |
There was a problem hiding this comment.
window resize랑 다르게 특정 요소 크기를 관찰하는 용도군요.
이전의 height와 다른 경우에만 state를 업뎃하면 매번 state를 최신화하는 것보다 낫지 않을까요?
There was a problem hiding this comment.
ResizeObserver가 이벤트를 여러 번 발생시킬 수 있다는 점을 고려하면 이전 height와 비교 후 state를 업데이트하는 방식이 더 효율적일 것 같네여! 반영하겠습니다~
| interface DateTimePanelProps { | ||
| date: Date | null; | ||
| onChangeDate: (date: Date) => void; | ||
| $alignRight?: boolean; |
There was a problem hiding this comment.
styled componenets의 특성을 잘 파악하시고 쓰셨군요 👍
| const formattedYearMonth = `${viewMonth.getFullYear()}.${String( | ||
| viewMonth.getMonth() + 1, | ||
| ).padStart(2, '0')}`; |
There was a problem hiding this comment.
여기선 useMemo로 연산비용을 최적화할 수 있어요. 물론 자주 같은 값을 사용하지 않을 것 같아서 불필요할 수도 있지만 생각나서 적어봅니다. useMemo
There was a problem hiding this comment.
말씀해주신 것처럼 useMemo로 감쌀 수도 있지만 해당 연산이 문자열 포맷 정도라 비용이 매우 작아서 사용하지 않았습니다ㅏ 추후 포맷 로직이 복잡해진다면 useMemo 적용도 고려해보겠습니다. 감사합니당
| return format(date, 'yyyy년 MM월 dd일 (eee) HH:mm', { locale: ko }); | ||
| }; |
There was a problem hiding this comment.
라이브러리 유틸을 사용하는 경우에는 테스트코드가 필요할까요?
There was a problem hiding this comment.
현재 유틸은 date-fns의 format을 단순히 감싸는 수준이라... 별도의 로직이 거의 없어서 테스트 코드는 추가하지 않았습니다.
포맷 규칙이 변경되거나 로직이 추가될 경우 테스트 코드도 함께 작성하면 좋을 것 같습니다!
There was a problem hiding this comment.
이 정도로 단순한 포맷팅 로직은 date-fns에 의존하기보다 직접 구현하고 테스트로 보장하는 편이 더 낫다고 생각했습니다.
util 함수는 재사용 범위가 넓기에, 간단한 동작은 외부 의존성을 줄이고 내부에서 안정성있게 관리하는 쪽이 유지보수에 더 유리하다고 봤어요.
지금 format이 재사용되는 범위를 파악해도 좋을 것 같고요.
There was a problem hiding this comment.
말씀주신 대로 이 정도의 포맷팅은 직접 구현하여 외부 의존성을 줄이는게 좋을 것 같네요!
한번 date-fns 없이 구현하고 테스트 코드로 안정성을 보장하는 방향으로 수정해볼게요.
| const updateHour = (hour: number) => { | ||
| const nextDate = new Date(selectedDate); | ||
| nextDate.setHours(hour); | ||
| onChangeDate(nextDate); |
There was a problem hiding this comment.
Date객체가 mutable이라 객체를 직접 수정하는 방식보단 복사본 만들어서 업뎃하는게 어떨까요?
There was a problem hiding this comment.
Date 객체가 mutable이라 selectedDate를 new Date로 복사한 뒤 업데이트하도록 작성을 했습니다.
현재 코드도 그 방식으로 작성했다고 생각했는데, 혹시 제가 놓친 부분이 있을까요?
| useLayoutEffect(() => { | ||
| const scrollTimer = setTimeout(() => { | ||
| alignScrollToCenter(hourListRef.current, selectedDate.getHours()); | ||
| alignScrollToCenter(minuteListRef.current, selectedDate.getMinutes()); | ||
| }, 0); |
There was a problem hiding this comment.
useLayoutEffect로 깜빡임 방지한 것 좋습니다.
혹시 setTimeout에 0을 걸어둔 게 어떤 의도인가요?
There was a problem hiding this comment.
스크롤 위치 계산 시점에 DOM 레이아웃이 완전히 반영되지 않는 경우가 있어
렌더 이후 실행을 보장하기 위해 setTimeout(0)을 사용했습니다.
There was a problem hiding this comment.
비슷한 목적이라면 requestAnimationFrame을 사용하는 방법도 있을 것 같습니다.
There was a problem hiding this comment.
requestAnimationFrame 을 활용하면 브라우저가 화면을 그리기 직전에 실행이 보장되어, 스크롤 위치 계산 시 최신의 레이아웃 정보를 더 안전하게 참조할 수 있겠네요!
반영했습니다! 6c3bb8e
There was a problem hiding this comment.
setTimeout 사용 시 어떤 잠재적인 문제가 있다고 보셨는지 궁금합니다.
There was a problem hiding this comment.
setTimeout이 렌더링 타이밍과 동기화되지 않아 불안정할 수 있다고 생각했습니다.
렌더 직후에 실행이 보장되는 requestAnimationFrame으로 변경했습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/utils/recruitmentDateFomatter.test.ts (1)
1-1: 테스트 파일명 오탈자는 맞춰두는 편이 좋습니다.
recruitmentDateFomatter.test.ts의Fomatter는 오탈자로 보입니다.recruitmentDateFormatter.test.ts로 맞추면 검색성과 파일 일관성이 좋아집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/utils/recruitmentDateFomatter.test.ts` at line 1, Rename the test file from recruitmentDateFomatter.test.ts to recruitmentDateFormatter.test.ts to fix the typo and ensure consistency with the module import; update any references to the test file name in CI/config (if present) so the imported symbol formatRecruitmentDateTime from "./recruitmentDateFormatter" resolves cleanly and the filename matches the formatter module name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/utils/recruitmentDateFomatter.test.ts`:
- Around line 4-40: Prettier flagged an indentation/formatting mismatch in the
test file; run the project's Prettier/formatter on
frontend/src/utils/recruitmentDateFomatter.test.ts (or fix the inconsistent
indentation around the last test where jest.setSystemTime(mockDate) is placed)
so the beforeEach/afterEach blocks, all it() blocks, and the date test lines
(references: formatRecruitmentDateTime, jest.setSystemTime) use the project's
configured spacing and indent rules; apply the formatter or re-indent the file
consistently and re-run lint/CI.
---
Nitpick comments:
In `@frontend/src/utils/recruitmentDateFomatter.test.ts`:
- Line 1: Rename the test file from recruitmentDateFomatter.test.ts to
recruitmentDateFormatter.test.ts to fix the typo and ensure consistency with the
module import; update any references to the test file name in CI/config (if
present) so the imported symbol formatRecruitmentDateTime from
"./recruitmentDateFormatter" resolves cleanly and the filename matches the
formatter module name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c40fe2a8-7a41-43a3-95de-56fb90a0e6d8
📒 Files selected for processing (3)
frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/TimePickerPanel.tsxfrontend/src/utils/recruitmentDateFomatter.test.tsfrontend/src/utils/recruitmentDateFormatter.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/utils/recruitmentDateFormatter.ts
- frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/TimePickerPanel.tsx
lepitaaar
left a comment
There was a problem hiding this comment.
아르키 프론트엔드 시니어 코드리뷰(자동) 승인합니다.
lepitaaar
left a comment
There was a problem hiding this comment.
핵심 리뷰
- [Medium] frontend/src/pages/AdminPage/tabs/RecruitEditTab/RecruitEditTab.tsx | @@ -20,82 +20,95 @@ const RecruitEditTab = () => { | useEffect 의존성 배열 점검 필요. stale closure/누락 의존성 확인 권장
- [Medium] frontend/src/pages/AdminPage/tabs/RecruitEditTab/components/DateTimeRangePicker/DateTimeRangePicker.tsx | @@ -0,0 +1,88 @@ | interval/listener cleanup 누락 여부 확인 필요
수정 위치는 각 항목의 '파일 | @@ hunk @@' 기준으로 반영하면 됩니다.
#️⃣연관된 이슈
📝작업 내용
기존의
react-datepicker기반 시간 선택 방식에서 발하던 제약 사항(30분 단위 설정, 디자인 일관성 부족)을 해결하기 위해, 커스텀TimePickerPanel을 도입하고 전반적인 UI/UX를 개선했습니다.주요 변경 사항
1. 시간/분 선택 단위로 수정
2. 시간 선택 피커 스크롤 중앙 정렬
3. 달력 높이 연동 가변 레이아웃
DatePickerPanel)의 월별 행 수(4~6주)에 따라 변화하는 높이를 실시간으로 감지하도록 했습니다.ResizeObserver를 통해 측정된 높이 값을 오른쪽 시간 피커에 전달하여, 전체 패널의 높이가 불균형해지지 않도록 가변적인 스크롤 영역을 제공했습니다.시작 날짜가 마감 날짜보다 늦어지거나, 마감 날짜가 시작 날짜보다 빨라질 경우 상대 날짜를 자동으로 동일하게 맞춰주는 로직을 추가하여 데이터 무결성을 보장했습니다.
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit
새로운 기능
버그 수정 / 동작 개선
스타일