[FE-Fix] 딤처리 레이아웃 문제, 다가오는 일정 모두보기 버튼, Date Picker 문제 해결 #272
Conversation
|
Warning Rate limit exceeded@dioo1461 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 16 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. 📥 CommitsReviewing files that changed from the base of the PR and between c7ea7212aed56e0c8412af744fffa4f20abc3231 and df831ab. 📒 Files selected for processing (21)
WalkthroughThis pull request introduces new properties and methods to the calendar context interface and updates various DatePicker and schedule-related components. It refines hooks by adding a date trimming utility and enforcing required parameters, adjusts component APIs by removing unused props and simplifying property interfaces, and refactors UI layouts for schedule displays. The changes improve consistency across styling variants and enhance month navigation and date handling through both component-level and hook-level modifications. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CalendarUI
participant SharedCalendarHook
participant MonthNavigationHook
User->>CalendarUI: Selects a date
CalendarUI->>SharedCalendarHook: handleSelectDate(date)
SharedCalendarHook->>MonthNavigationHook: setBaseDate(firstDayOfMonth)
MonthNavigationHook-->>SharedCalendarHook: Updated baseDate
SharedCalendarHook-->>CalendarUI: Returns { baseDate, gotoPrevMonth, gotoNextMonth }
Possibly related PRs
Suggested labels
Poem
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: 0
🧹 Nitpick comments (10)
frontend/src/components/DatePicker/Table/Row.tsx (1)
59-64: Consider adding JSDoc to document the highlight gap state logic.While the function rename improves clarity, adding documentation would help future maintainers understand the gap state transitions.
+/** + * Determines the highlight state for gaps between date cells. + * Returns 'inRange' for gaps after 'startOfRange' or within 'inRange', + * and 'none' for all other cases. + */ const getHighlightGapState = (highlightState: HighlightState): HighlightState => {frontend/src/features/shared-schedule/ui/UpcomingSchedules/index.tsx (1)
17-18: Document the overflow behavior in JSDoc format.The comment about relative positioning and overflow behavior would be more maintainable in JSDoc format.
- // 최상단 Flex에 relative 주면 안 됨! (overflow hidden 때문에 뷰포트가 부모여야 함) + /** + * Important: Do not set position:relative on the top-level Flex! + * The viewport must be the parent due to overflow:hidden behavior. + */frontend/src/pages/HomePage/UpcomingSection.tsx (1)
28-37: Optimize the navigate callback with useCallback.The navigate callback in the Button's onClick handler could be memoized to prevent unnecessary re-renders.
+import { useCallback } from 'react'; const UpcomingSection = () => { const { data, isPending } = useUpcomingQuery(); const schedules = data?.data ?? []; const navigate = useNavigate(); + const handleViewAll = useCallback(() => { + navigate({ to: '/upcoming-schedule' }); + }, [navigate]); // ... {schedules.length > 3 ? <Button - onClick={() => navigate({ to: '/upcoming-schedule' })} + onClick={handleViewAll} style='borderless' > 모두보기 </Button> : null}frontend/src/features/my-calendar/ui/MyDatePicker/index.tsx (1)
13-13: Consider explicit props passing instead of spreading the entire context.While spreading the entire
calendarobject works, it might pass unnecessary props to the child component. Consider explicitly passing only the required props for better maintainability and to avoid potential prop drilling.- {...calendar} + selectedDate={calendar.selectedDate} + baseDate={calendar.baseDate} + gotoPrevMonth={calendar.gotoPrevMonth} + gotoNextMonth={calendar.gotoNextMonth}frontend/src/hooks/useSharedCalendar.ts (1)
10-10: LGTM! Clean implementation of calendar sync.The implementation effectively syncs the calendar by updating the base date when a date is selected. Consider extracting the date transformation to a utility function for better reusability.
+const getFirstDayOfMonth = (date: Date) => new Date(date.getFullYear(), date.getMonth()); + const handleSelectDate = (date: Date) => { setSelected(date); - setBaseDate(new Date(date.getFullYear(), date.getMonth())); + setBaseDate(getFirstDayOfMonth(date)); };Also applies to: 14-14
frontend/src/components/DatePicker/DatePicker.stories.tsx (2)
53-55: Verify date range selection behavior.Since this component is central to the PR's date range selection requirements, please confirm that:
- Users can select the current day range
- The picker's closing behavior after range selection is properly handled (as mentioned in PR objectives)
Consider adding a story variant that demonstrates the specific date range selection behaviors mentioned in the PR objectives.
65-66: Consider i18n for label text.The Input label contains hardcoded Korean text ('기간 설정'). Consider using an internationalization solution for better maintainability and future localization support.
- label='기간 설정' + label={t('datePicker.periodSetting')}Also applies to: 71-73
frontend/src/features/timeline-schedule/ui/TimelineContent/index.css.ts (1)
13-15: Remove commented code.Instead of keeping the commented
leftproperty, consider removing it entirely. If the value needs to be preserved for future reference, document it in a separate technical document or issue.export const dimStyle = style({ position: 'absolute', - // left: -36, width: '58.5rem',frontend/src/features/timeline-schedule/ui/TimelineContent/index.tsx (2)
39-39: Remove commented event handler.Instead of keeping the commented
onScrollhandler, consider removing it entirely. If the functionality needs to be preserved for future reference, document it in a separate technical document or issue.<div className={bodyContainerStyle} - // onScroll={handleScroll} ref={scrollRef} >
49-57: Document the positioning logic.The dim overlay positioning looks correct, but consider adding a comment explaining how
getRowTopOffsetcalculates the position and height based on participant counts. This will help future maintainers understand the layout logic.{props.uncheckedParticipants.length > 0 && <div className={dimStyle} ref={dimRef} + // Position overlay after checked participants and size it based on unchecked count style={{ top: getRowTopOffset(props.checkedParticipants.length), height: getRowTopOffset(props.uncheckedParticipants.length), }} />}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between efdcbe6 and c7ea7212aed56e0c8412af744fffa4f20abc3231.
📒 Files selected for processing (19)
frontend/src/components/Calendar/context/SharedCalendarContext.ts(1 hunks)frontend/src/components/DatePicker/DatePicker.stories.tsx(4 hunks)frontend/src/components/DatePicker/DatePickerSelect.tsx(1 hunks)frontend/src/components/DatePicker/RootContainer.tsx(1 hunks)frontend/src/components/DatePicker/Table/Highlight/index.css.ts(2 hunks)frontend/src/components/DatePicker/Table/Highlight/index.ts(1 hunks)frontend/src/components/DatePicker/Table/Row.tsx(3 hunks)frontend/src/components/DatePicker/context/DatePickerRangeProvider.tsx(0 hunks)frontend/src/components/DatePicker/context/DatePickerSelectProvider.tsx(1 hunks)frontend/src/features/discussion/ui/DiscussionForm/MeetingDateDropdowns.tsx(2 hunks)frontend/src/features/my-calendar/ui/MyDatePicker/index.tsx(1 hunks)frontend/src/features/shared-schedule/ui/UpcomingSchedules/index.tsx(1 hunks)frontend/src/features/timeline-schedule/ui/TimelineContent/index.css.ts(2 hunks)frontend/src/features/timeline-schedule/ui/TimelineContent/index.tsx(3 hunks)frontend/src/hooks/useDatePicker/useDatePickerRange.ts(2 hunks)frontend/src/hooks/useDatePicker/useDatePickerSelect.ts(1 hunks)frontend/src/hooks/useSharedCalendar.ts(1 hunks)frontend/src/pages/HomePage/UpcomingSection.tsx(1 hunks)frontend/src/pages/HomePage/index.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- frontend/src/components/DatePicker/context/DatePickerRangeProvider.tsx
✅ Files skipped from review due to trivial changes (1)
- frontend/src/components/DatePicker/DatePickerSelect.tsx
🔇 Additional comments (19)
frontend/src/components/DatePicker/Table/Highlight/index.ts (1)
6-6: LGTM! The new highlight state enhances date range selection.The addition of 'startAndEnd' to HighlightState type aligns well with the PR objective of enabling current day range selection in DatePicker.Range.
frontend/src/components/DatePicker/Table/Highlight/index.css.ts (1)
27-30: LGTM! The styling for the new highlight state is well-implemented.The styling for 'startAndEnd' maintains consistency with existing highlight states and correctly uses design system variables. The empty gap style for 'startAndEnd' is appropriate since there's no gap when start and end dates are the same.
Also applies to: 56-56
frontend/src/components/DatePicker/Table/Row.tsx (1)
49-50: LGTM! The new highlight state condition is well-placed.The condition for 'startAndEnd' is correctly placed before other conditions and uses the appropriate date comparison utility.
frontend/src/pages/HomePage/index.tsx (1)
8-14: LGTM! Clean and well-structured component.The refactoring improves code organization by moving the upcoming schedules logic to a dedicated component. The component follows React best practices and maintains a clean structure.
frontend/src/features/shared-schedule/ui/UpcomingSchedules/index.tsx (1)
9-11: LGTM! Well-structured component with proper TypeScript typing.The refactoring improves separation of concerns by moving data fetching to the parent component.
frontend/src/pages/HomePage/UpcomingSection.tsx (1)
11-17: LGTM! Well-implemented loading and error states.The component properly handles loading and empty states, providing appropriate fallback UI.
frontend/src/components/DatePicker/RootContainer.tsx (1)
7-15: LGTM! Clean and focused component.The removal of the unused
refprop simplifies the component's API while maintaining its core functionality.frontend/src/components/Calendar/context/SharedCalendarContext.ts (1)
10-12: LGTM! Well-structured calendar navigation additions.The new properties for month navigation are well-typed and align perfectly with the PR's calendar sync objectives.
frontend/src/components/DatePicker/context/DatePickerSelectProvider.tsx (2)
16-17: LGTM! Simplified date selection check.The function has been simplified by directly accessing
props.selectedDateinstead of using a local variable, making the code more concise.
22-22: LGTM! Props spreading ensures complete context.Moving the spread operator to the beginning ensures all props are passed to the context while allowing specific values to override them.
frontend/src/hooks/useDatePicker/useDatePickerRange.ts (4)
16-17: LGTM! Improved date comparison accuracy.The introduction of
trimTimeensures consistent date comparisons by normalizing dates to midnight.Also applies to: 20-23
18-19: Address the UTC handling TODO.The comment indicates a temporary fix for UTC handling issues. This should be properly addressed when implementing custom Date objects.
Would you like me to create an issue to track this refactoring task?
25-33: LGTM! Clear date range selection logic.The date range selection logic has been improved with better variable naming and clearer conditions.
41-45: LGTM! Well-implemented time normalization utility.The
trimTimeutility function is a clean implementation for normalizing dates to midnight.frontend/src/hooks/useDatePicker/useDatePickerSelect.ts (1)
6-6: LGTM! Made baseDate required.Making
baseDatea required prop is appropriate as it's used in critical date calculations and month navigation.frontend/src/features/discussion/ui/DiscussionForm/MeetingDateDropdowns.tsx (2)
15-15: LGTM! Simplified validation setup.The validation setup is now more concise by directly passing the form state to the extracted validation function.
44-50: LGTM! Well-structured date range validation.The extracted
validateDateRangefunction is well-implemented with clear logic for handling:
- Null date checks
- Same-day selections
- Future date validation
frontend/src/components/DatePicker/DatePicker.stories.tsx (1)
21-21: LGTM! Clean hook usage.The simplified hook usage improves code maintainability by reducing unnecessary destructuring.
frontend/src/features/timeline-schedule/ui/TimelineContent/index.css.ts (1)
29-30: LGTM! Good use of design tokens.The addition of border radius properties using design tokens improves visual consistency.
…elect 시 DatePicker의 baseDate가 그에 맞춰 변화할 수 있도록 구현
dioo1461
force-pushed
the
bugfix/fe/first-qa
branch
from
c7ea721 to
8fa074c
Compare
#️⃣ 연관된 이슈>
📝 작업 내용> 이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)
DatePicker.Range에서 일정 범위 선택 완료되면 picker 꺼지게 하는건, 의견을 나누고 싶은 부분이 있어 보류해 두었습니다.저희 일정 조율 생성 Form에서 일정 조율 기간이 현 날짜로부터 일주일 범위로 초기화되어 있는데, 위 구현을 적용하게 되면 사용자가 기간을 재설정하기 위해 picker를 열었을 때 날짜를 선택하면 picker가 바로 꺼지게 됩니다.위 동작이 UX적으로 좋지 않을 것 같아서.. 이에 대해 의견을 나누고 싶어요. 개인적으로 전 picker가 꺼지지 않아도 괜찮을 것 같다는 입장입니다. 아니면 일정 범위를 처음에 아예 비워 놓아야 할 것 같아요🙏 여기는 꼭 봐주세요! > 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요
Summary by CodeRabbit
New Features
Refactor
Style