Feat/132/create edit gathering api : popOverCalendar, 약속만들기 시간선택 동작 개선 #142
Conversation
yulrang
requested review from
ChoYenii,
HaeJungg,
User850413 and
minkyung5x5
|
Caution Review failedThe pull request is closed. Walkthrough이 변경 사항은 Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Warning Rate limit exceeded@yulrang has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 46 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
src/components/common/input/date-time-picker/date-time-picker.stories.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't determine the plugin "react-hooks" uniquely.
Please remove the "plugins" setting from either config or remove either plugin installation. If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (2)
src/components/common/input/pop-over-calendar/index.tsx (2)
14-18: 불필요한 상태 관리 개선 필요
inputTheme을 상태로 관리하는 대신opened값에서 직접 계산하는 것이 더 효율적일 것 같습니다.다음과 같이 수정하는 것을 제안합니다:
-const [inputTheme, setInputTheme] = useState(opened ? 'dark' : 'light'); +const inputTheme = opened ? 'dark' : 'light';
58-58: 날짜 표시 형식 개선 필요현재 날짜 표시 형식이 단순합니다. 더 읽기 좋은 형식으로 개선하면 좋을 것 같습니다.
날짜 포맷팅을 위해
date-fns라이브러리를 사용하거나 별도의 포맷터 함수를 만드는 것을 제안합니다:-<span>{date ? `${date.getMonth() + 1}월 ${date.getDate()}일` : '날짜 선택'}</span> +<span>{date ? format(date, 'yyyy년 M월 d일') : '날짜 선택'}</span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/components/common/input/calendar-filter/index.tsx(2 hunks)src/components/common/input/date-time-picker/index.tsx(2 hunks)src/components/common/input/pop-over-calendar/index.tsx(2 hunks)
🔇 Additional comments (2)
src/components/common/input/calendar-filter/index.tsx (1)
77-77: rest props 사용에 대한 주의가 필요합니다.
{...rest}를 통해 전달되는 props가 기존 Calendar 컴포넌트의 중요한 props를 덮어쓸 수 있습니다. 특히 getDayProps, renderDay, styles 등의 주요 기능에 영향을 줄 수 있으므로 주의가 필요합니다.
src/components/common/input/pop-over-calendar/index.tsx (1)
42-47: 접근성과 사용성이 잘 고려된 구현입니다
포털 사용, 키보드 접근성, 외부 클릭 처리 등이 적절하게 구현되어 있습니다.
| export default function CalendarFilter({ | ||
| value, | ||
| toDoDates, | ||
| onChange, | ||
| ...rest | ||
| }: CalendarFilterProps) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Props 타입 정의 개선이 필요합니다.
CalendarFilterProps 인터페이스에 추가된 rest props의 타입이 정의되어 있지 않습니다. 타입 안정성을 위해 다음과 같은 개선이 필요합니다.
export interface CalendarFilterProps {
value: Date;
toDoDates: Date[] | null;
onChange: (date: Date) => void;
+ // Calendar 컴포넌트의 props 타입을 확장
+ extends Omit<CalendarProps, 'value' | 'onChange'>;
}Committable suggestion skipped: line range outside the PR's diff.
| useEffect(() => { | ||
| document.addEventListener('click', handleOutsideClick); | ||
| return () => document.removeEventListener('click', handleOutsideClick); | ||
| }, []); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
의존성 배열 개선 필요
useEffect의 의존성 배열에 close 함수가 누락되어 있어 스테일 클로저 문제가 발생할 수 있습니다.
다음과 같이 수정하는 것을 제안합니다:
useEffect(() => {
document.addEventListener('click', handleOutsideClick);
return () => document.removeEventListener('click', handleOutsideClick);
-}, []);
+}, [close]);Committable suggestion skipped: line range outside the PR's diff.
| e.stopPropagation(); | ||
| setDate(new Date()); | ||
| onChange(new Date()); | ||
| close(); | ||
| }; | ||
| const handleSubmit = () => { | ||
| const handleSubmit = (e: React.MouseEvent<HTMLButtonElement>) => { | ||
| e.stopPropagation(); | ||
| onChange(date); | ||
| close(); | ||
| }; | ||
|
|
||
| const handleOutsideClick = (e: MouseEvent) => { | ||
| if (popOver.current && !popOver.current.contains(e.target as Node)) { | ||
| close(); | ||
| } | ||
| }; | ||
| useEffect(() => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
초기화 동작 개선 필요
handleClear 함수에서 현재 날짜로 초기화하는 것보다 날짜를 완전히 비우는 것이 사용자 경험에 더 적합할 것 같습니다.
다음과 같이 수정하는 것을 제안합니다:
const handleClear = (e: React.MouseEvent<HTMLButtonElement>) => {
e.stopPropagation();
- setDate(new Date());
- onChange(new Date());
+ setDate(null);
+ onChange(null);
close();
};이를 위해 PopOverProps 인터페이스도 수정이 필요합니다:
export interface PopOverProps {
- value: Date;
+ value: Date | null;
- onChange: (date: Date) => void;
+ onChange: (date: Date | null) => void;
}Committable suggestion skipped: line range outside the PR's diff.
| const newDate = new Date(selected); | ||
|
|
||
| newDate.setHours(Number(hour)); | ||
| newDate.setMinutes(Number(minute)); | ||
|
|
||
| const kstDate = new Date(newDate.getTime() - newDate.getTimezoneOffset() * 60000).toISOString(); | ||
|
|
||
| setSelected(newDate); | ||
| onChange(newDate); | ||
|
|
||
| // KST 기준 ISO 문자열 전달 | ||
| onChange(kstDate); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
시간대 처리 로직 개선이 필요합니다.
현재 구현에서 몇 가지 개선이 필요한 부분이 있습니다:
- 수동으로 KST 변환하는 대신 dayjs를 활용하면 더 안전하게 처리할 수 있습니다.
- 자정 근처에서 날짜가 변경될 수 있는 엣지 케이스 처리가 필요합니다.
다음과 같이 개선하는 것을 제안드립니다:
- const newDate = new Date(selected);
-
- newDate.setHours(Number(hour));
- newDate.setMinutes(Number(minute));
-
- const kstDate = new Date(newDate.getTime() - newDate.getTimezoneOffset() * 60000).toISOString();
-
- setSelected(newDate);
-
- // KST 기준 ISO 문자열 전달
- onChange(kstDate);
+ const newDate = dayjs(selected)
+ .hour(Number(hour))
+ .minute(Number(minute))
+ .tz('Asia/Seoul');
+
+ setSelected(newDate.toDate());
+ onChange(newDate.toISOString());이렇게 수정하면:
- dayjs의 타임존 기능을 활용하여 더 안정적인 시간 처리가 가능합니다
- 날짜 변경선 근처에서의 엣지 케이스도 자동으로 처리됩니다
Committable suggestion skipped: line range outside the PR's diff.
| export interface DateTimePickerProps { | ||
| fullDate: Date; | ||
| onChange: (date: Date) => void; | ||
| onChange: (date: string) => void; |
There was a problem hiding this comment.
💡 Codebase verification
Date 타입 변경으로 인한 영향도가 확인되었습니다
현재 DateTimePicker 컴포넌트를 사용하는 두 곳에서 모두 Date 객체를 사용하고 있습니다:
date-time-picker.stories.tsx:useState<Date>()사용create-gathering-form/index.tsx:new Date()를 fullDate prop으로 전달
이 변경으로 인해 다음 작업이 필요합니다:
- stories 파일의 state 타입과 예제 코드 수정
- create-gathering-form의 date 처리 로직 수정
🔗 Analysis chain
인터페이스 변경에 대한 마이그레이션 가이드가 필요합니다.
onChange 콜백의 타입을 Date에서 string으로 변경하는 것은 Breaking Change입니다. 이 컴포넌트를 사용하는 모든 곳에서 수정이 필요할 것 같습니다.
다음 스크립트로 영향을 받는 컴포넌트들을 확인해보겠습니다:
Let me check the actual usage of DateTimePicker in these files to understand the impact.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# DateTimePicker 컴포넌트의 사용처를 검색
rg -l "DateTimePicker" src/
Length of output: 249
Script:
#!/bin/bash
# Check how DateTimePicker is being used in the stories and actual implementation
rg "DateTimePicker" src/components/common/input/date-time-picker/date-time-picker.stories.tsx -A 5
rg "DateTimePicker" src/app/\(crew\)/crew/detail/\[id\]/_components/create-gathering/create-gathering-form/index.tsx -A 5
Length of output: 1473
🔖 Issue Ticket
#132
✍️ Description
✅ Checklist
PR
Test
Summary by CodeRabbit
새로운 기능
CalendarFilter컴포넌트가 추가 속성을 수용하도록 수정되었습니다.DateTimePicker의onChange콜백이Date객체 대신 문자열을 수용하도록 변경되었습니다.PopOverCalendar컴포넌트가 외부 클릭 시 닫히도록 개선되었습니다.버그 수정
DateTimePicker의onChange기능이 업데이트되었습니다.문서화