[FE-Feat] Date Picker 컴포넌트 구현 #96
Conversation
WalkthroughThis pull request introduces a new DatePicker feature. It adds various supporting files including Storybook configurations, a dedicated DatePicker context, multiple hooks (for month navigation, date selection, and highlight range), and several UI components such as headers, cells, rows, and table layouts. Additionally, it provides new CSS modules for styling and enhancements in date utilities and icon components. Minor newline additions are also included in a few files. The changes collectively build the DatePicker interface and functionality without altering existing public APIs. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant DC as DateCell
participant DS as useDateSelect Hook
participant DP as DatePicker Component
participant DT as useDatePicker Hook
U->>DC: Click on date cell
DC->>DS: Trigger selectDate(date)
DS->>DP: Call handleDateSelect(date)
DP->>DT: Update state (baseDate, calendarDates, etc.)
DT-->>DP: Return updated calendar data
DP->>U: Re-render DatePicker with updated state
sequenceDiagram
participant U as User
participant H as Header
participant MN as useMonthNavigation Hook
participant DP as DatePicker Component
U->>H: Click on chevron button
H->>MN: Invoke goToPrevMonth/goToNextMonth
MN-->>DP: Update baseDate accordingly
DP->>U: Re-render DatePicker with new month view
Possibly related issues
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
🪧 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: 6
🧹 Nitpick comments (30)
frontend/src/utils/jsxUtils.tsx (2)
1-2: Consider using direct React import for types.The
ReactNodetype is being imported from '@tanstack/react-router' when it should typically be imported directly from 'react' for better maintainability and to avoid potential version conflicts.-import type { ReactNode } from '@tanstack/react-router'; +import type { ReactNode, JSX } from 'react'; -import type { JSX } from 'react';
4-16: Consider improvements for React key generation and edge cases.The implementation looks good but could be enhanced in a few areas:
- Using array index as React key might cause issues with reconciliation if the array is modified
- No validation for empty or undefined input array
Consider this enhanced implementation:
export const intersperseElement = ( childElements: ReactNode[], separator?: JSX.Element, ): ReactNode[] => { + if (!childElements?.length) return []; const result: ReactNode[] = []; childElements.forEach((child, index) => { result.push(child); if (separator && index < childElements.length - 1) { - result.push(<separator.type {...separator.props} key={index} />); + result.push( + <separator.type + {...separator.props} + key={`separator-${index}-${separator.key ?? ''}`} + /> + ); } }); return result; };frontend/src/components/DatePicker/Table/index.tsx (1)
6-11: Consider adding error boundaries and prop types.While the implementation is clean, consider these improvements:
- Add prop types/interface to make the component more maintainable
- Wrap child components with error boundaries to gracefully handle rendering failures
Example implementation:
interface TableProps { // Add any props that might be needed in the future } const Table: React.FC<TableProps> = () => ( <ErrorBoundary fallback={<div>Something went wrong</div>}> <div className={calendarStyle}> <TableHeader /> <TableBody /> </div> </ErrorBoundary> );frontend/src/components/DatePicker/Table/Highlight/HighlightBox.tsx (1)
5-9: Consider adding prop validation and memoization.The component could benefit from these improvements:
- Add runtime validation for highlightState prop
- Memoize the component to prevent unnecessary re-renders
Example implementation:
import { memo } from 'react'; const HighlightBox = memo(({ highlightState, children }: HighlightProps) => { if (!highlightState) { throw new Error('highlightState is required'); } return ( <div className={highlightBoxStyle({ highlightState })}> {children} </div> ); }); HighlightBox.displayName = 'HighlightBox';frontend/src/components/DatePicker/Table/Highlight/HighlightGap.tsx (1)
5-9: Consider component abstraction to reduce duplication.The component structure is identical to HighlightBox. Consider creating a base component to reduce code duplication.
Example implementation:
// BaseHighlight.tsx const BaseHighlight = memo(({ highlightState, children, styleFunction }: HighlightProps & { styleFunction: (props: { highlightState: string }) => string }) => ( <div className={styleFunction({ highlightState })}> {children} </div> )); // HighlightGap.tsx const HighlightGap = (props: HighlightProps) => ( <BaseHighlight {...props} styleFunction={highlightGapStyle} /> ); // HighlightBox.tsx const HighlightBox = (props: HighlightProps) => ( <BaseHighlight {...props} styleFunction={highlightBoxStyle} /> );frontend/src/components/DatePicker/Table/Cell/DowCell.tsx (1)
1-13: LGTM! Consider adding prop types validation.The component is well-structured and follows React's functional component pattern. Consider adding prop types validation using PropTypes or TypeScript's strict mode for better type safety.
frontend/src/components/DatePicker/Header/index.css.ts (2)
3-9: Consider using symmetric padding and responsive units.The header style could be improved by:
- Using symmetric padding (both left and right) for better visual balance
- Using relative units (rem/em) instead of pixels for better responsiveness
export const headerStyle = style({ display: 'flex', - paddingLeft: '8px', + padding: '0 0.5rem', justifyContent: 'space-between', alignItems: 'center', alignSelf: 'stretch', });
11-17: Consider using relative units for dimensions.For better responsiveness across different screen sizes and font sizes:
export const chevronWrapper = style({ display: 'flex', justifyContent: 'center', alignItems: 'center', - width: 32, - height: 32, + width: '2rem', + height: '2rem', });frontend/src/components/DatePicker/DatePicker.stories.tsx (1)
5-14: Enhance Storybook documentation and add more stories.Consider adding:
- Component description and usage examples in the meta parameters
- Additional stories showcasing different states (e.g., with preselected date, disabled dates, etc.)
Add documentation and stories:
const meta: Meta = { title: 'Calendar/DatePicker', component: DatePicker, + parameters: { + docs: { + description: { + component: 'A flexible date picker component supporting both single date selection and range selection.', + }, + }, + }, argTypes: { calendarType: { control: { type: 'radio' }, options: ['select', 'range'], + description: 'Type of calendar to display - either single date selection or date range selection', }, }, } satisfies Meta<typeof DatePicker>;Would you like me to generate additional story examples for different states?
frontend/src/components/DatePicker/Table/Cell/CellWrapper.tsx (1)
14-14: Add null check for className prop.The className concatenation could result in "undefined" being added to the class list. Add a null check to handle cases where className is not provided.
- className={`${cellWrapperStyle} ${className}`} + className={`${cellWrapperStyle} ${className || ''}`}frontend/src/hooks/useDatePicker/useMonthNavigation.ts (1)
5-20: Add validation and error handling for edge cases.Consider adding:
- Validation for minimum/maximum allowed dates
- Error handling for invalid date operations
- Optional initial date parameter to support controlled components
-export const useMonthNavigation = () => { +export const useMonthNavigation = (initialDate?: Date, minDate?: Date, maxDate?: Date) => { const today = new Date(); - const [baseDate, setBaseDate] = useState(today); + const [baseDate, setBaseDate] = useState(initialDate || today); const goToPrevMonth = () => { const { month, year } = getPrevMonthInfo(baseDate); - setBaseDate(new Date(year, month)); + const newDate = new Date(year, month); + if (minDate && newDate < minDate) return; + setBaseDate(newDate); }; const goToNextMonth = () => { const { month, year } = getNextMonthInfo(baseDate); - setBaseDate(new Date(year, month)); + const newDate = new Date(year, month); + if (maxDate && newDate > maxDate) return; + setBaseDate(newDate); }; return { baseDate, goToPrevMonth, goToNextMonth }; };frontend/src/components/DatePicker/Table/index.css.ts (1)
12-18: Add responsive design support.The calendar width is fixed at 320px, which might not be optimal for all screen sizes. Consider making it responsive.
export const calendarStyle = style({ display: 'flex', - width: '320px', + width: '100%', + maxWidth: '320px', + minWidth: '280px', flexDirection: 'column', justifyContent: 'space-between', alignItems: 'center', });frontend/src/hooks/useDatePicker/useDateSelect.ts (1)
13-31: Consider adding validation for date range order.While the logic handles basic range selection, it could be improved to ensure a more robust implementation.
Consider this enhancement:
const selectDate = () => { handleDateSelect(date); if (calendarType === 'select') return; if (!highlightRange.start) { setHighlightStart(date); return; } if (!highlightRange.end) { - if (date < highlightRange.start) { + const isBeforeStart = date < highlightRange.start; + const isSameAsStart = date.getTime() === highlightRange.start.getTime(); + if (isSameAsStart) { + setHighlightStart(null); + return; + } + if (isBeforeStart) { setHighlightStart(date); return; } setHighlightEnd(date); return; } handleDateSelect(null); setHighlightStart(null); setHighlightEnd(null); };frontend/src/components/DatePicker/Table/Cell/index.css.ts (1)
7-14: Consider accessibility improvements for interactive cells.The cell wrapper style should include hover and focus states for better accessibility.
export const cellWrapperStyle = style({ display: 'flex', justifyContent: 'center', alignItems: 'center', width: '24px', height: '24px', cursor: 'default', + ':hover': { + backgroundColor: vars.color.Ref.Netural[100], + }, + ':focus': { + outline: `2px solid ${vars.color.Ref.Primary[500]}`, + outlineOffset: '2px', + }, });frontend/src/utils/date/calendar/index.ts (1)
11-12: Consider using Date.UTC for consistent date handling.The current implementation might have timezone-related edge cases.
-export const getDaysInMonth = (date: Date): number => - new Date(date.getFullYear(), date.getMonth() + 1, 0).getDate(); +export const getDaysInMonth = (date: Date): number => { + const year = date.getFullYear(); + const month = date.getMonth(); + return new Date(Date.UTC(year, month + 1, 0)).getUTCDate(); +};frontend/src/components/DatePicker/Table/Highlight/index.css.ts (1)
5-51: Consider extracting shared highlight state variants to reduce duplication.The
highlightStatevariants are identical betweenhighlightBoxStyleandhighlightGapStyle. Consider extracting these into a shared constant to improve maintainability.+const highlightStateVariants = { + none: { }, + startOfRange: { + backgroundColor: vars.color.Ref.Primary[50], + borderTopLeftRadius: vars.radius[200], + borderBottomLeftRadius: vars.radius[200], + }, + inRange: { + backgroundColor: vars.color.Ref.Primary[50], + }, + endOfRange: { + backgroundColor: vars.color.Ref.Primary[50], + borderTopRightRadius: vars.radius[200], + borderBottomRightRadius: vars.radius[200], + }, +}; export const highlightBoxStyle = recipe({ base: { padding: vars.spacing[100], }, variants: { - highlightState: { - none: { }, - startOfRange: { - backgroundColor: vars.color.Ref.Primary[50], - borderTopLeftRadius: vars.radius[200], - borderBottomLeftRadius: vars.radius[200], - }, - inRange: { - backgroundColor: vars.color.Ref.Primary[50], - }, - endOfRange: { - backgroundColor: vars.color.Ref.Primary[50], - borderTopRightRadius: vars.radius[200], - borderBottomRightRadius: vars.radius[200], - }, - }, + highlightState: highlightStateVariants, }, }); export const highlightGapStyle = recipe({ base: { flexGrow: 1, }, variants: { - highlightState: { - none: { }, - startOfRange: { - backgroundColor: vars.color.Ref.Primary[50], - borderTopLeftRadius: vars.radius[200], - borderBottomLeftRadius: vars.radius[200], - }, - inRange: { - backgroundColor: vars.color.Ref.Primary[50], - }, - endOfRange: { - backgroundColor: vars.color.Ref.Primary[50], - borderTopRightRadius: vars.radius[200], - borderBottomRightRadius: vars.radius[200], - }, - }, + highlightState: highlightStateVariants, }, });frontend/src/components/Icon/component/ArrowLeft.tsx (1)
6-6: Consider enhancing accessibility with role attribute.Add the
role="img"attribute to the SVG for better screen reader support.-<svg width={width} height={width} viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg" aria-label="arrow-left icon" fill="none" className={className} {...rest}> +<svg width={width} height={width} viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg" aria-label="arrow-left icon" role="img" fill="none" className={className} {...rest}>frontend/src/components/DatePicker/index.tsx (2)
21-21: Address the TODO comment about container style injection.The comment suggests implementing external style injection for the container. Consider adding a
classNameprop to allow style customization.Would you like me to help implement the external style injection feature?
38-44: Consider memoizing style helper functions.The
getTodayCellStyleandgetSelectedCellStylefunctions are recreated on every render. Consider usinguseMemoto optimize performance.+import { useMemo } from 'react'; const DatePicker = ({ calendarType }: DatePickerProps) => { const useDatePickerReturns = useDatePicker(); - const todayCellStyle = getTodayCellStyle(calendarType); - const selectedCellStyle = getSelectedCellStyle(calendarType); + const todayCellStyle = useMemo(() => getTodayCellStyle(calendarType), [calendarType]); + const selectedCellStyle = useMemo(() => getSelectedCellStyle(calendarType), [calendarType]);frontend/src/hooks/useDatePicker/index.ts (2)
30-32: Optimize useEffect dependencies.The
selectedDateandhighlightRangein the dependency array might cause unnecessary calendar regeneration. Consider if these dependencies are really needed for regenerating the calendar.useEffect(() => { setCalendarDates(generateMonthCalendar(baseDate)); -}, [baseDate, selectedDate, highlightRange]); +}, [baseDate]); // Only regenerate when baseDate changes
34-52: Simplify date comparison logic.The date comparison logic could be simplified using Date utility functions. Also, consider extracting the navigation logic to a separate function for better readability.
+const shouldNavigateMonth = (date: Date, baseDate: Date): 'prev' | 'next' | null => { + const startOfMonth = new Date(baseDate.getFullYear(), baseDate.getMonth(), 1); + const endOfMonth = new Date(baseDate.getFullYear(), baseDate.getMonth() + 1, 0); + + if (date < startOfMonth) return 'prev'; + if (date > endOfMonth) return 'next'; + return null; +}; const handleDateSelect = (date: Date | null) => { if (!date) { setSelectedDate(null); return; } - const currentYear = baseDate.getFullYear(); - const currentMonth = baseDate.getMonth(); - const calendarDates = getDaysInMonth(baseDate); - - const startOfCurrentMonth = new Date(currentYear, currentMonth, 1); - const endOfCurrentMonth = new Date(currentYear, currentMonth, calendarDates); - - if (date < startOfCurrentMonth) { - goToPrevMonth(); - } else if (date > endOfCurrentMonth) { - goToNextMonth(); - } + const navigation = shouldNavigateMonth(date, baseDate); + if (navigation === 'prev') goToPrevMonth(); + if (navigation === 'next') goToNextMonth(); setSelectedDate(date); };frontend/src/utils/date/date.ts (2)
101-102: Consider moving constants to a separate file.These day code constants could be moved to the date constants file alongside
WEEK_MAPfor better organization and reusability.-const SUNDAY_CODE = 0; -const SATURDAY_CODE = 6;Add to
@/constants/date.ts:export const DAY_CODES = { SUNDAY: 0, SATURDAY: 6 } as const;
109-110: Enhance the TODO comment with implementation details.The TODO comment could be more specific about the implementation plan.
-// TODO: 공휴일 OPEN API에 연결 -// export const isHoliday = (date: Date): boolean => false; +/** + * TODO: Implement holiday detection using the Holiday API + * + * Implementation plan: + * 1. Connect to the Holiday API (e.g., https://api.holiday.info) + * 2. Cache the holiday data for performance + * 3. Implement isHoliday function to check against cached data + */ +// export const isHoliday = (date: Date): boolean => false;frontend/src/utils/date/calendar/calendarGeneration.ts (2)
14-20: Optimize array partitioning for better performance.The current implementation creates a new array for each chunk. Consider using Array.from for better performance.
-const partitionArray = <T>(array: T[], size: number): T[][] => { - const chunks: T[][] = []; - for (let i = 0; i < array.length; i += size) { - chunks.push(array.slice(i, i + size)); - } - return chunks; -}; +const partitionArray = <T>(array: T[], size: number): T[][] => + Array.from({ length: Math.ceil(array.length / size) }, (_, i) => + array.slice(i * size, i * size + size) + );
79-88: Optimize memory usage in generateNextMonthDays.The function creates unnecessary intermediate variables. Consider simplifying the implementation.
const generateNextMonthDays = (existingCount: number, baseDate: Date): Date[] => { const remainingCells = (WEEK_DAYS - (existingCount % WEEK_DAYS)) % WEEK_DAYS; const { nextDate } = getNextMonthInfo(baseDate); - const [nextYear, nextMonth] = [nextDate.getFullYear(), nextDate.getMonth()]; - const dates: Date[] = []; - for (let day = FIRST_DAY; day <= remainingCells; day++) { - dates.push(new Date(nextYear, nextMonth, day)); - } - return dates; + return Array.from({ length: remainingCells }, (_, i) => + new Date(nextDate.getFullYear(), nextDate.getMonth(), FIRST_DAY + i) + ); };frontend/src/components/DatePicker/Table/Cell/DateCell.tsx (2)
34-39: Memoize inline styles to prevent unnecessary re-renders.The inline styles object is recreated on every render. Consider using useMemo to optimize performance.
+import { useMemo } from 'react'; const DateCell = ({ date, selected, baseDate, highlightState }: DateCellProps) => { const { calendarType, todayCellStyle, selectedCellStyle, } = useSafeContext(DatePickerContext); - const inlineCellStyles = assignInlineVars(cellThemeVars, { - todayCellBackgroundColor: todayCellStyle.backgroundColor ?? 'transparent', - todayCellColor: todayCellStyle.color ?? 'transparent', - selectedCellBackgroundColor: selectedCellStyle.backgroundColor ?? 'transparent', - selectedCellColor: selectedCellStyle.color ?? 'transparent', - }); + const inlineCellStyles = useMemo(() => + assignInlineVars(cellThemeVars, { + todayCellBackgroundColor: todayCellStyle.backgroundColor ?? 'transparent', + todayCellColor: todayCellStyle.color ?? 'transparent', + selectedCellBackgroundColor: selectedCellStyle.backgroundColor ?? 'transparent', + selectedCellColor: selectedCellStyle.color ?? 'transparent', + }), + [todayCellStyle, selectedCellStyle] + );
68-69: Link TODO comment to the implementation plan.The TODO comment about holiday checks should be linked to the implementation plan in the date utility file.
- // TODO: 공휴일도 함께 체크 + // TODO: Implement holiday check after the Holiday API integration is complete + // See: frontend/src/utils/date/date.tsfrontend/src/components/DatePicker/Table/Row.tsx (3)
35-36: Translate Korean comment to English for consistency.For better maintainability and collaboration, comments should be in English.
- // 마지막 셀 뒤에는 gap을 넣지 않음 + // Skip adding gap after the last cell
49-57: Move highlight state logic to a separate utility file.The highlight state logic could be reused in other components. Consider moving it to a separate utility file.
Create a new file
frontend/src/utils/highlight.ts:import { isSameDate } from './date'; import type { HighlightRange } from '@/hooks/useDatePicker/useHighlightRange'; import type { HighlightState } from '@/components/DatePicker/Table/Highlight'; export const getHighlightState = (date: Date, highlightRange: HighlightRange): HighlightState => { const { start, end } = highlightRange; if (!start || !end) return 'none'; if (isSameDate(date, start)) return 'startOfRange'; if (isSameDate(date, end)) return 'endOfRange'; if (date > start && date < end) return 'inRange'; return 'none'; };
23-44: Optimize array mapping to reduce complexity.The current implementation uses array mapping with conditional returns. Consider using a more declarative approach.
- {week.map((day, index) => { - const highlightState = getHighlightState(day, highlightRange); - const cell = ( - <HighlightBox highlightState={highlightState} key={`cell-${day.getTime()}`}> - <DateCell - baseDate={baseDate} - date={day} - highlightState={highlightState} - selected={selectedDate ? isSameDate(day, selectedDate) : false} - /> - </HighlightBox> - ); - // Skip adding gap after the last cell - if (index === week.length - 1) return cell; - const gap = ( - <HighlightGap - highlightState={getGapHighlightState(highlightState)} - key={`gap-${day.getTime()}`} - /> - ); - return [cell, gap]; - })} + {week.flatMap((day, index) => { + const highlightState = getHighlightState(day, highlightRange); + const elements = [( + <HighlightBox highlightState={highlightState} key={`cell-${day.getTime()}`}> + <DateCell + baseDate={baseDate} + date={day} + highlightState={highlightState} + selected={selectedDate ? isSameDate(day, selectedDate) : false} + /> + </HighlightBox> + )]; + + if (index < week.length - 1) { + elements.push( + <HighlightGap + highlightState={getGapHighlightState(highlightState)} + key={`gap-${day.getTime()}`} + /> + ); + } + + return elements; + })}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 870c9a9 and ba29a0b2b87924ec14d7a8043aa07292c2d1bd4a.
⛔ Files ignored due to path filters (1)
frontend/src/components/Icon/svg/arrow-left.svgis excluded by!**/*.svg
📒 Files selected for processing (35)
frontend/src/components/Calendar/Calendar.stories.ts(1 hunks)frontend/src/components/DatePicker/DatePicker.stories.tsx(1 hunks)frontend/src/components/DatePicker/DatePickerContext.ts(1 hunks)frontend/src/components/DatePicker/Header/index.css.ts(1 hunks)frontend/src/components/DatePicker/Header/index.tsx(1 hunks)frontend/src/components/DatePicker/Table/Cell/CellWrapper.tsx(1 hunks)frontend/src/components/DatePicker/Table/Cell/DateCell.tsx(1 hunks)frontend/src/components/DatePicker/Table/Cell/DowCell.tsx(1 hunks)frontend/src/components/DatePicker/Table/Cell/index.css.ts(1 hunks)frontend/src/components/DatePicker/Table/Cell/index.ts(1 hunks)frontend/src/components/DatePicker/Table/Highlight/HighlightBox.tsx(1 hunks)frontend/src/components/DatePicker/Table/Highlight/HighlightGap.tsx(1 hunks)frontend/src/components/DatePicker/Table/Highlight/index.css.ts(1 hunks)frontend/src/components/DatePicker/Table/Highlight/index.ts(1 hunks)frontend/src/components/DatePicker/Table/Row.tsx(1 hunks)frontend/src/components/DatePicker/Table/RowContainer.tsx(1 hunks)frontend/src/components/DatePicker/Table/TableBody.tsx(1 hunks)frontend/src/components/DatePicker/Table/TableHeader.tsx(1 hunks)frontend/src/components/DatePicker/Table/index.css.ts(1 hunks)frontend/src/components/DatePicker/Table/index.tsx(1 hunks)frontend/src/components/DatePicker/index.css.ts(1 hunks)frontend/src/components/DatePicker/index.tsx(1 hunks)frontend/src/components/Icon/component/ArrowLeft.tsx(1 hunks)frontend/src/components/Input/Input.stories.tsx(2 hunks)frontend/src/components/Input/MultiInput.tsx(4 hunks)frontend/src/constants/date.ts(1 hunks)frontend/src/hooks/useDatePicker/index.ts(1 hunks)frontend/src/hooks/useDatePicker/useDateSelect.ts(1 hunks)frontend/src/hooks/useDatePicker/useHighlightRange.ts(1 hunks)frontend/src/hooks/useDatePicker/useMonthNavigation.ts(1 hunks)frontend/src/utils/date/calendar/calendarGeneration.ts(1 hunks)frontend/src/utils/date/calendar/index.ts(1 hunks)frontend/src/utils/date/date.ts(1 hunks)frontend/src/utils/date/index.ts(1 hunks)frontend/src/utils/jsxUtils.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- frontend/src/components/Calendar/Calendar.stories.ts
- frontend/src/utils/date/index.ts
- frontend/src/components/DatePicker/Table/Cell/index.ts
- frontend/src/constants/date.ts
- frontend/src/components/Input/Input.stories.tsx
🧰 Additional context used
📓 Learnings (1)
frontend/src/components/DatePicker/Header/index.tsx (2)
Learnt from: hamo-o
PR: softeer5th/Team4-enDolphin#80
File: frontend/src/components/Calendar/Header/CalendarHeader.tsx:8-24
Timestamp: 2025-02-04T03:20:42.270Z
Learning: The codebase uses `useSafeContext` hook to handle React context error cases by throwing an error with message "🚨 이 컴포넌트는 ContextProvider 내부에서 사용되어야 합니다!" when the context value is null.
Learnt from: hamo-o
PR: softeer5th/Team4-enDolphin#80
File: frontend/src/components/Calendar/Header/CalendarHeader.tsx:8-24
Timestamp: 2025-02-04T03:20:42.270Z
Learning: The codebase uses `useSafeContext` hook to handle React context error cases, making explicit null checks redundant.
🪛 Biome (1.9.4)
frontend/src/components/DatePicker/Table/TableHeader.tsx
[error] 19-20: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Send PR Notification to Reviewers
🔇 Additional comments (8)
frontend/src/components/Input/MultiInput.tsx (3)
4-5: LGTM!The import statement follows the project's conventions and correctly imports the new utility.
33-33: LGTM!The replacement of the previous implementation with
intersperseElementis correct and maintains the same functionality.
45-45: LGTM!Minor formatting change that improves code readability.
frontend/src/components/DatePicker/Table/RowContainer.tsx (1)
1-11: LGTM! Clean and well-structured component.The component follows React best practices with proper TypeScript usage, children prop handling, and CSS module integration.
frontend/src/components/DatePicker/Table/Highlight/index.ts (1)
1-11: LGTM! Well-structured type definitions for range highlighting.The types and interface are well-defined, covering all necessary states for range highlighting in the calendar component.
frontend/src/hooks/useDatePicker/useDateSelect.ts (1)
4-12: LGTM! Proper context usage with error handling.The hook correctly uses
useSafeContextto handle context error cases, as per the codebase conventions.frontend/src/components/DatePicker/Table/Cell/index.css.ts (1)
16-42: LGTM! Consistent use of theme variables.The styles properly utilize theme variables for colors and maintain consistency across different cell states.
frontend/src/utils/date/calendar/index.ts (1)
19-26: LGTM! Well-documented and handles edge cases.The month navigation functions are well-documented and properly handle year transitions.
Also applies to: 33-40
dioo1461
changed the title
hamo-o
left a comment
hamo-o
left a comment
There was a problem hiding this comment.
수고하셨습니다!! disabled된 날짜 이벤트 처리는 기/디 논의 해보아요~~~
There was a problem hiding this comment.
p2;
확인해보니 calendarDates는 한군데에서만 사용되는 것 같은데, 굳이 커다란 상태로 관리하지 않고
소비자 쪽에서 baseDate를 generateMonthCalendar로 감싸서 사용하면 어떨까요?
There was a problem hiding this comment.
p3;
제공해주는 쪽에서 reset 함수를 만들어주면 한 번에 깔끔하게 처리할 수 있을 것 같습니다.
dioo1461
force-pushed
the
feature/fe/date-picker-component
branch
from
ba29a0b to
1601ef3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/src/components/DatePicker/Table/TableHeader.tsx (1)
16-26:⚠️ Potential issueFix key prop placement.
The key prop is incorrectly placed on the
DowCellinstead of the outerdiv. This could cause React reconciliation issues.- <div className={dowCellWrapperStyle}> - <DowCell className={style} key={`${label}-${i}`}> + <div key={`${label}-${i}`} className={dowCellWrapperStyle}> + <DowCell className={style}> {label} </DowCell> </div>🧰 Tools
🪛 Biome (1.9.4)
[error] 19-20: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
🧹 Nitpick comments (9)
frontend/src/components/DatePicker/Table/Row.tsx (2)
23-44: Optimize rendering performance.The current implementation creates new arrays on each render. Consider using React.memo and useMemo for better performance.
+const MemoizedDateCell = React.memo(DateCell); +const MemoizedHighlightBox = React.memo(HighlightBox); +const MemoizedHighlightGap = React.memo(HighlightGap); const Row = ({ week, baseDate, selectedDate }: RowProps) => { const { highlightRange } = useSafeContext(DatePickerContext); + const cells = useMemo(() => week.map((day, index) => { const highlightState = getHighlightState(day, highlightRange); const cell = ( - <HighlightBox highlightState={highlightState} key={`cell-${day.getTime()}`}> - <DateCell + <MemoizedHighlightBox highlightState={highlightState} key={`cell-${day.getTime()}`}> + <MemoizedDateCell baseDate={baseDate} date={day} highlightState={highlightState} selected={selectedDate ? isSameDate(day, selectedDate) : false} /> - </HighlightBox> + </MemoizedHighlightBox> ); if (index === week.length - 1) return cell; const gap = ( - <HighlightGap + <MemoizedHighlightGap highlightState={getGapHighlightState(highlightState)} key={`gap-${day.getTime()}`} /> ); return [cell, gap]; - }) + }), [week, baseDate, selectedDate, highlightRange]);
49-57: Memoize highlight state calculation.The getHighlightState function is called for each cell render. Consider memoizing the result.
+const memoizedGetHighlightState = memoize( (date: Date, highlightRange: HighlightRange): HighlightState => { const { start, end } = highlightRange; if (!start || !end) return 'none'; if (isSameDate(date, start)) return 'startOfRange'; if (isSameDate(date, end)) return 'endOfRange'; if (date > start && date < end) return 'inRange'; return 'none'; } +);frontend/src/components/DatePicker/Table/Cell/DateCell.tsx (2)
68-68: Address TODO comment for holiday checks.The TODO comment indicates missing holiday check functionality.
Would you like me to help implement the holiday check functionality using the commented-out isHoliday function from the date utils?
52-72: Simplify style selection logic.The getDateCellStyle function can be simplified using a lookup object.
+const cellStyleMap = { + selected: selectedCellStyle, + range: (highlightState: HighlightState) => + (highlightState === 'startOfRange' || highlightState === 'endOfRange') + ? selectedCellStyle + : null, + today: todayCellStyle, + otherMonth: otherMonthCellStyle, + sunday: holidayCellStyle, + saturday: saturdayCellStyle, + default: weekdayCellStyle +}; const getDateCellStyle = ( date: Date, calendarType: CalendarType, selected: boolean, baseDate: Date, highlightState: HighlightState, ) => { - if (selected) return selectedCellStyle; - if (calendarType === 'range' && ( - highlightState === 'startOfRange' || highlightState === 'endOfRange' - )) { - return selectedCellStyle; - } - if (calendarType === 'select' && isSameDate(date, new Date())) return todayCellStyle; - if (date.getMonth() !== baseDate.getMonth()) return otherMonthCellStyle; - if (isSunday(date)) return holidayCellStyle; - if (isSaturday(date)) return saturdayCellStyle; - return weekdayCellStyle; + if (selected) return cellStyleMap.selected; + if (calendarType === 'range') { + const rangeStyle = cellStyleMap.range(highlightState); + if (rangeStyle) return rangeStyle; + } + if (calendarType === 'select' && isSameDate(date, new Date())) + return cellStyleMap.today; + if (date.getMonth() !== baseDate.getMonth()) + return cellStyleMap.otherMonth; + if (isSunday(date)) return cellStyleMap.sunday; + if (isSaturday(date)) return cellStyleMap.saturday; + return cellStyleMap.default; };frontend/src/utils/date/date.ts (1)
154-155: Plan holiday API integration.The TODO comment indicates a need for holiday API integration.
Would you like me to help:
- Research and suggest suitable holiday APIs
- Create an implementation plan for the holiday check functionality
- Generate a template for the API integration?
frontend/src/utils/jsxUtils.tsx (1)
1-2: Consider importing ReactNode from 'react'.
ReactNodeis a core React type and should be imported directly from 'react' instead of '@tanstack/react-router'.-import type { ReactNode } from '@tanstack/react-router'; +import type { ReactNode, JSX } from 'react'; -import type { JSX } from 'react';frontend/src/components/DatePicker/index.tsx (2)
21-22: Consider implementing external style injection.The TODO comment aligns with the PR objectives about allowing external state management. This would improve component flexibility.
Consider adding a
containerClassNameprop to allow style injection:export interface DatePickerProps { calendarType: CalendarType; + containerClassName?: string; } -const DatePicker = ({ calendarType }: DatePickerProps) => { +const DatePicker = ({ calendarType, containerClassName }: DatePickerProps) => {
38-44: Fix typo in color variable and optimize style calculations.There's a typo in the color variable name ('Netural' should be 'Neutral') and the style calculations could be optimized.
Apply these improvements:
- ? { backgroundColor: vars.color.Ref.Primary[500], color: vars.color.Ref.Netural['White'] } + ? { backgroundColor: vars.color.Ref.Primary[500], color: vars.color.Ref.Neutral['White'] } +const memoizedTodayCellStyle = useMemo( + () => getTodayCellStyle(calendarType), + [calendarType] +); +const memoizedSelectedCellStyle = useMemo( + () => getSelectedCellStyle(calendarType), + [calendarType] +);Don't forget to add the useMemo import:
-import { useEffect, useState } from 'react'; +import { useEffect, useState, useMemo } from 'react';frontend/src/hooks/useDatePicker/index.ts (1)
34-52: Optimize date handling logic.The date comparison logic could be simplified and utilize existing utility functions.
Consider this optimization:
const handleDateSelect = (date: Date | null) => { if (!date) { setSelectedDate(null); return; } - const currentYear = baseDate.getFullYear(); - const currentMonth = baseDate.getMonth(); - const calendarDates = getDaysInMonth(baseDate); - - const startOfCurrentMonth = new Date(currentYear, currentMonth, 1); - const endOfCurrentMonth = new Date(currentYear, currentMonth, calendarDates); + const { startOfMonth, endOfMonth } = getMonthBoundaries(baseDate); - if (date < startOfCurrentMonth) { + if (date < startOfMonth) { goToPrevMonth(); - } else if (date > endOfCurrentMonth) { + } else if (date > endOfMonth) { goToNextMonth(); } setSelectedDate(date); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between ba29a0b2b87924ec14d7a8043aa07292c2d1bd4a and 1601ef30dc4a2efa5e74a0744f33b060f4b9dec0.
⛔ Files ignored due to path filters (1)
frontend/src/components/Icon/svg/arrow-left.svgis excluded by!**/*.svg
📒 Files selected for processing (35)
frontend/src/components/Calendar/Calendar.stories.ts(1 hunks)frontend/src/components/DatePicker/DatePicker.stories.tsx(1 hunks)frontend/src/components/DatePicker/DatePickerContext.ts(1 hunks)frontend/src/components/DatePicker/Header/index.css.ts(1 hunks)frontend/src/components/DatePicker/Header/index.tsx(1 hunks)frontend/src/components/DatePicker/Table/Cell/CellWrapper.tsx(1 hunks)frontend/src/components/DatePicker/Table/Cell/DateCell.tsx(1 hunks)frontend/src/components/DatePicker/Table/Cell/DowCell.tsx(1 hunks)frontend/src/components/DatePicker/Table/Cell/index.css.ts(1 hunks)frontend/src/components/DatePicker/Table/Cell/index.ts(1 hunks)frontend/src/components/DatePicker/Table/Highlight/HighlightBox.tsx(1 hunks)frontend/src/components/DatePicker/Table/Highlight/HighlightGap.tsx(1 hunks)frontend/src/components/DatePicker/Table/Highlight/index.css.ts(1 hunks)frontend/src/components/DatePicker/Table/Highlight/index.ts(1 hunks)frontend/src/components/DatePicker/Table/Row.tsx(1 hunks)frontend/src/components/DatePicker/Table/RowContainer.tsx(1 hunks)frontend/src/components/DatePicker/Table/TableBody.tsx(1 hunks)frontend/src/components/DatePicker/Table/TableHeader.tsx(1 hunks)frontend/src/components/DatePicker/Table/index.css.ts(1 hunks)frontend/src/components/DatePicker/Table/index.tsx(1 hunks)frontend/src/components/DatePicker/index.css.ts(1 hunks)frontend/src/components/DatePicker/index.tsx(1 hunks)frontend/src/components/Icon/component/ArrowLeft.tsx(1 hunks)frontend/src/components/Input/Input.stories.tsx(2 hunks)frontend/src/components/Input/MultiInput.tsx(4 hunks)frontend/src/constants/date.ts(1 hunks)frontend/src/hooks/useDatePicker/index.ts(1 hunks)frontend/src/hooks/useDatePicker/useDateSelect.ts(1 hunks)frontend/src/hooks/useDatePicker/useHighlightRange.ts(1 hunks)frontend/src/hooks/useDatePicker/useMonthNavigation.ts(1 hunks)frontend/src/utils/date/calendar/calendarGeneration.ts(1 hunks)frontend/src/utils/date/calendar/index.ts(1 hunks)frontend/src/utils/date/date.ts(3 hunks)frontend/src/utils/date/index.ts(1 hunks)frontend/src/utils/jsxUtils.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (27)
- frontend/src/components/Calendar/Calendar.stories.ts
- frontend/src/constants/date.ts
- frontend/src/components/Input/Input.stories.tsx
- frontend/src/components/DatePicker/Table/Highlight/HighlightBox.tsx
- frontend/src/components/DatePicker/Table/TableBody.tsx
- frontend/src/utils/date/index.ts
- frontend/src/components/DatePicker/Table/RowContainer.tsx
- frontend/src/components/DatePicker/Table/Cell/DowCell.tsx
- frontend/src/components/DatePicker/Table/Highlight/index.ts
- frontend/src/components/DatePicker/Header/index.css.ts
- frontend/src/components/DatePicker/Table/index.tsx
- frontend/src/components/DatePicker/Table/Highlight/index.css.ts
- frontend/src/components/DatePicker/DatePickerContext.ts
- frontend/src/components/DatePicker/Table/Highlight/HighlightGap.tsx
- frontend/src/components/DatePicker/index.css.ts
- frontend/src/hooks/useDatePicker/useMonthNavigation.ts
- frontend/src/components/DatePicker/Table/Cell/index.ts
- frontend/src/components/DatePicker/Header/index.tsx
- frontend/src/hooks/useDatePicker/useHighlightRange.ts
- frontend/src/hooks/useDatePicker/useDateSelect.ts
- frontend/src/components/Input/MultiInput.tsx
- frontend/src/components/DatePicker/Table/index.css.ts
- frontend/src/components/Icon/component/ArrowLeft.tsx
- frontend/src/components/DatePicker/DatePicker.stories.tsx
- frontend/src/utils/date/calendar/calendarGeneration.ts
- frontend/src/components/DatePicker/Table/Cell/index.css.ts
- frontend/src/utils/date/calendar/index.ts
🧰 Additional context used
🪛 Biome (1.9.4)
frontend/src/components/DatePicker/Table/TableHeader.tsx
[error] 19-20: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (10)
frontend/src/components/DatePicker/Table/TableHeader.tsx (2)
1-14: LGTM!The imports are well-organized, and the days array is cleanly structured with appropriate styles for different days of the week.
27-27: LGTM!The export statement follows React conventions.
frontend/src/components/DatePicker/Table/Cell/CellWrapper.tsx (1)
14-14: Use clsx utility for className concatenation.Replace string concatenation with the clsx utility as previously suggested.
- className={`${cellWrapperStyle} ${className}`} + className={clsx(cellWrapperStyle, className)}frontend/src/utils/date/date.ts (1)
152-153: LGTM! Clean implementation of day checks.The new utility functions with constants improve code readability and maintainability.
frontend/src/utils/jsxUtils.tsx (1)
4-16: Verify the integration with DatePicker component.Since this utility is being introduced alongside the DatePicker component, let's verify its usage:
frontend/src/components/DatePicker/index.tsx (2)
1-20: LGTM! Well-structured type definitions and imports.The type definitions are clear and properly exported, with good separation of concerns between style props and component props.
23-36: LGTM! Clean implementation with proper context usage.The component follows React best practices by using hooks for state management and context for prop drilling prevention.
frontend/src/hooks/useDatePicker/index.ts (3)
1-20: LGTM! Well-structured interface and imports.The interface clearly defines the hook's return type with all necessary properties and methods.
22-33: Consider computing calendarDates on-the-fly.As suggested in the previous review, calendarDates could be computed when needed instead of being stored in state.
Apply this refactor:
- const [calendarDates, setCalendarDates] = useState<Date[][]>( - generateMonthCalendar(baseDate), - ); + const calendarDates = useMemo( + () => generateMonthCalendar(baseDate), + [baseDate] + ); - useEffect(() => { - setCalendarDates(generateMonthCalendar(baseDate)); - }, [baseDate, selectedDate, highlightRange]);
54-58: LGTM! Well-structured return value.The hook returns all necessary values and methods as defined in the interface.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
frontend/src/components/DatePicker/Table/TableHeader.tsx (1)
6-14: Consider moving the days array to a constants file.The days array is well-structured, but consider moving it to a separate constants file (e.g.,
constants/calendar.ts) for better maintainability and potential reuse across other calendar-related components.frontend/src/components/DatePicker/Table/Cell/DateCell.tsx (3)
47-47: Address TODO: Improve cursor style handling.The TODO comment suggests finding a better way to handle cursor styles and controls. I can help implement a solution.
Would you like me to propose a solution that encapsulates cursor style and control logic into a reusable hook or utility function?
80-80: Address TODO: Add holiday check.The TODO comment indicates that holiday checking functionality needs to be implemented.
Would you like me to help implement a holiday checking utility that can be integrated with the date cell type determination?
89-103: Simplify switch case by removing redundant case.The 'weekday' case is redundant since it returns the same value as the default case.
switch (dateCellType) { case 'selected': return selectedCellStyle; case 'today': return todayCellStyle; case 'otherMonth': return otherMonthCellStyle; case 'holiday': return holidayCellStyle; case 'saturday': return saturdayCellStyle; - case 'weekday': default: return weekdayCellStyle; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 1601ef30dc4a2efa5e74a0744f33b060f4b9dec0 and 1cde2bbfa3595e4482a8f6f5a9952e81f0aa0a91.
📒 Files selected for processing (8)
frontend/src/components/DatePicker/Header/index.tsx(1 hunks)frontend/src/components/DatePicker/Table/Cell/CellWrapper.tsx(1 hunks)frontend/src/components/DatePicker/Table/Cell/DateCell.tsx(1 hunks)frontend/src/components/DatePicker/Table/Cell/DowCell.tsx(1 hunks)frontend/src/components/DatePicker/Table/Cell/index.css.ts(1 hunks)frontend/src/components/DatePicker/Table/TableBody.tsx(1 hunks)frontend/src/components/DatePicker/Table/TableHeader.tsx(1 hunks)frontend/src/hooks/useDatePicker/useDateSelect.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/components/DatePicker/Table/TableBody.tsx
- frontend/src/components/DatePicker/Header/index.tsx
- frontend/src/components/DatePicker/Table/Cell/DowCell.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
frontend/src/components/DatePicker/Table/Cell/DateCell.tsx
[error] 102-104: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
🔇 Additional comments (5)
frontend/src/components/DatePicker/Table/TableHeader.tsx (3)
1-4: LGTM! Well-organized imports.The imports are properly organized, following consistent naming conventions and clear scoping.
16-26: LGTM! Clean implementation with proper key usage.The component is well-structured with proper key placement on the outer div, addressing the previous review comment about React reconciliation.
27-27: LGTM! Proper export statement.The default export is appropriate for this main component.
frontend/src/components/DatePicker/Table/Cell/CellWrapper.tsx (1)
21-21: Useclsxutility for className concatenation.Replace string template literal with
clsxutility for better className handling.- className={`${cellWrapperStyle({ cursorType })} ${className}`} + className={clsx(cellWrapperStyle({ cursorType }), className)}frontend/src/hooks/useDatePicker/useDateSelect.ts (1)
26-38: Extract reset logic into a separate function.The reset logic is duplicated and could be extracted into a reusable function.
+ const resetSelection = () => { + handleDateSelect(null); + setHighlightStart(null); + setHighlightEnd(null); + }; + if (isSameDate(date, highlightRange.start)) { - handleDateSelect(null); - setHighlightStart(null); + resetSelection(); return; } setHighlightEnd(date); return; } - handleDateSelect(null); - setHighlightStart(null); - setHighlightEnd(null); + resetSelection();
하이라이트, 선택 기능은 구현 예정
dioo1461
force-pushed
the
feature/fe/date-picker-component
branch
from
1cde2bb to
aa95f5b
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (14)
frontend/src/components/DatePicker/Table/TableHeader.tsx (1)
6-14: Consider adding i18n support for day labels.The day labels are currently hardcoded in Korean. For better maintainability and potential internationalization, consider using an i18n library.
Example implementation using i18n:
+import { useTranslation } from 'react-i18next'; const days = [ - { label: '일', style: holidayCellStyle }, - { label: '월', style: weekdayCellStyle }, - { label: '화', style: weekdayCellStyle }, - { label: '수', style: weekdayCellStyle }, - { label: '목', style: weekdayCellStyle }, - { label: '금', style: weekdayCellStyle }, - { label: '토', style: saturdayCellStyle }, + { label: 'days.sun', style: holidayCellStyle }, + { label: 'days.mon', style: weekdayCellStyle }, + { label: 'days.tue', style: weekdayCellStyle }, + { label: 'days.wed', style: weekdayCellStyle }, + { label: 'days.thu', style: weekdayCellStyle }, + { label: 'days.fri', style: weekdayCellStyle }, + { label: 'days.sat', style: saturdayCellStyle }, ]; const TableHeader = () => { + const { t } = useTranslation(); return ( <RowContainer> - {days.map(({ label, style }, i) => ( + {days.map(({ label, style }, i) => ( <div className={dowCellWrapperStyle} key={`${label}-${i}`}> - <DowCell className={style}>{label}</DowCell> + <DowCell className={style}>{t(label)}</DowCell> </div> ))} </RowContainer> ); };frontend/src/components/DatePicker/Table/Cell/DateCell.tsx (4)
47-47: Track the TODO comment.The TODO comment about cell cursor style control should be tracked in the issue tracker.
Would you like me to create an issue to track this TODO item?
49-56: Simplify cursor style logic.The cursor style logic can be simplified by moving it to CSS using a class-based approach.
- cursorType={calendarType === 'range' && dateCellType === 'otherMonth' - ? 'not-allowed' - : 'pointer'} + className={`${getDateCellStyle(dateCellType)} ${ + calendarType === 'range' && dateCellType === 'otherMonth' + ? 'cursor-not-allowed' + : 'cursor-pointer' + }`} onClick={handleDateCellClick} style={inlineCellStyles}
80-80: Track the TODO comment.The TODO comment about holiday checking should be tracked in the issue tracker.
Would you like me to create an issue to track this TODO item about holiday checking implementation?
64-84: Add JSDoc documentation for helper functions.Consider adding JSDoc documentation to describe the purpose and parameters of the
getDateCellTypefunction.+/** + * Determines the type of date cell based on selection status, calendar type, and date comparisons. + * @param date - The date to check + * @param calendarType - The type of calendar ('select' or 'range') + * @param selected - Whether the date is selected + * @param baseDate - The current month's base date + * @param highlightState - The highlight state for range selection + * @returns The type of the date cell + */ const getDateCellType = ( date: Date, calendarType: CalendarType, selected: boolean, baseDate: Date, highlightState: HighlightState, ): DateCellType => {frontend/src/components/DatePicker/index.tsx (1)
21-21: Address the TODO comment for container style injection.The TODO comment indicates that container styles should be injectable from outside. This aligns with the PR objectives mentioning potential refactoring for better external component interaction.
Would you like me to help implement the container style injection feature? I can propose a solution that allows external style customization while maintaining default styles.
frontend/src/utils/date/calendar/index.ts (4)
1-4: Add JSDoc comments for calendar constants.Consider adding JSDoc comments to explain the purpose and usage of these constants, especially for
FIRST_MONTHandLAST_MONTHwhich use 0-based indexing.+/** + * Number of days in a week. + */ export const WEEK_DAYS = 7; +/** + * First month of the year (0-based index, represents January). + */ export const FIRST_MONTH = 0; +/** + * Last month of the year (0-based index, represents December). + */ export const LAST_MONTH = FIRST_MONTH + 11; +/** + * First day of the month. + */ export const FIRST_DAY = 1;
19-26: Add explicit return type forgetPrevMonthInfo.Consider adding an explicit return type for better type safety and documentation.
-export const getPrevMonthInfo = (date: Date) => { +export const getPrevMonthInfo = (date: Date): { month: number; year: number } => {
33-40: Refactor month navigation logic to reduce duplication.The logic in
getNextMonthInfois very similar togetPrevMonthInfo. Consider creating a shared utility function.+type MonthInfo = { month: number; year: number }; + +const getMonthInfo = (date: Date, offset: number): MonthInfo => { + const currentMonth = date.getMonth(); + const currentYear = date.getFullYear(); + + if ((offset === -1 && currentMonth === FIRST_MONTH) || + (offset === 1 && currentMonth === LAST_MONTH)) { + return { + month: offset === -1 ? LAST_MONTH : FIRST_MONTH, + year: currentYear + offset + }; + } + return { month: currentMonth + offset, year: currentYear }; +}; + +export const getPrevMonthInfo = (date: Date): MonthInfo => + getMonthInfo(date, -1); + +export const getNextMonthInfo = (date: Date): MonthInfo => + getMonthInfo(date, 1);
42-46: Add JSDoc and explicit return type forgetDateParts.Consider adding documentation and an explicit return type for better maintainability.
+/** + * Extracts year, month (0-based), and day from a Date object. + * @param date - The date to extract parts from. + * @returns An object containing the year, month, and day. + */ -export const getDateParts = (date: Date) => ({ +export const getDateParts = (date: Date): { year: number; month: number; day: number } => ({frontend/src/utils/date/calendar/calendarGeneration.ts (4)
14-20: Optimize array partitioning using Array.from.Consider using
Array.fromfor a more concise and potentially more efficient implementation.-const partitionArray = <T>(array: T[], size: number): T[][] => { - const chunks: T[][] = []; - for (let i = 0; i < array.length; i += size) { - chunks.push(array.slice(i, i + size)); - } - return chunks; -}; +const partitionArray = <T>(array: T[], size: number): T[][] => + Array.from({ length: Math.ceil(array.length / size) }, (_, i) => + array.slice(i * size, i * size + size) + );
36-47: Improve variable naming for better readability.Consider using more descriptive variable names to better convey their purpose.
const generatePreviousMonthDays = (baseDate: Date): Date[] => { const { prevDate, prevMonthDays } = getPreviousMonthInfo(baseDate); - const firstDayDow = new Date(baseDate.getFullYear(), baseDate.getMonth(), FIRST_DAY) + const firstDayOfWeek = new Date(baseDate.getFullYear(), baseDate.getMonth(), FIRST_DAY) .getDay(); const dates: Date[] = []; - for (let i = firstDayDow; i > 0; i--) { + for (let daysBeforeStart = firstDayOfWeek; daysBeforeStart > 0; daysBeforeStart--) { dates.push( new Date( prevDate.getFullYear(), prevDate.getMonth(), - prevMonthDays - i + 1 + prevMonthDays - daysBeforeStart + 1 ) ); } return dates; };
52-62: Consider using Array.from for a more functional approach.The current implementation uses imperative style. Consider using Array.from for a more functional approach.
-const generateCurrentMonthDays = (baseDate: Date): Date[] => { - const totalDays = getDaysInMonth(baseDate); - const year = baseDate.getFullYear(); - const month = baseDate.getMonth(); - - const dates: Date[] = []; - for (let day = FIRST_DAY; day <= totalDays; day++) { - dates.push(new Date(year, month, day)); - } - return dates; -}; +const generateCurrentMonthDays = (baseDate: Date): Date[] => { + const totalDays = getDaysInMonth(baseDate); + const year = baseDate.getFullYear(); + const month = baseDate.getMonth(); + + return Array.from( + { length: totalDays }, + (_, index) => new Date(year, month, index + FIRST_DAY) + ); +};
96-103: Add type annotations for better code clarity.Consider adding type annotations to intermediate variables for better code clarity and maintainability.
export const generateMonthCalendar = (baseDate: Date): Date[][] => { - const previousDays = generatePreviousMonthDays(baseDate); - const currentDays = generateCurrentMonthDays(baseDate); - const combinedDays = [...previousDays, ...currentDays]; - const nextDays = generateNextMonthDays(combinedDays.length, baseDate); - const daysArray = [...combinedDays, ...nextDays]; + const previousDays: Date[] = generatePreviousMonthDays(baseDate); + const currentDays: Date[] = generateCurrentMonthDays(baseDate); + const combinedDays: Date[] = [...previousDays, ...currentDays]; + const nextDays: Date[] = generateNextMonthDays(combinedDays.length, baseDate); + const daysArray: Date[] = [...combinedDays, ...nextDays]; return partitionArray(daysArray, WEEK_DAYS); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 1cde2bbfa3595e4482a8f6f5a9952e81f0aa0a91 and aa95f5b.
⛔ Files ignored due to path filters (1)
frontend/src/components/Icon/svg/arrow-left.svgis excluded by!**/*.svg
📒 Files selected for processing (33)
frontend/src/components/Avatar/index.css.ts(1 hunks)frontend/src/components/Calendar/Calendar.stories.ts(1 hunks)frontend/src/components/DatePicker/DatePicker.stories.tsx(1 hunks)frontend/src/components/DatePicker/DatePickerContext.ts(1 hunks)frontend/src/components/DatePicker/Header/index.css.ts(1 hunks)frontend/src/components/DatePicker/Header/index.tsx(1 hunks)frontend/src/components/DatePicker/Table/Cell/CellWrapper.tsx(1 hunks)frontend/src/components/DatePicker/Table/Cell/DateCell.tsx(1 hunks)frontend/src/components/DatePicker/Table/Cell/DowCell.tsx(1 hunks)frontend/src/components/DatePicker/Table/Cell/index.css.ts(1 hunks)frontend/src/components/DatePicker/Table/Cell/index.ts(1 hunks)frontend/src/components/DatePicker/Table/Highlight/HighlightBox.tsx(1 hunks)frontend/src/components/DatePicker/Table/Highlight/HighlightGap.tsx(1 hunks)frontend/src/components/DatePicker/Table/Highlight/index.css.ts(1 hunks)frontend/src/components/DatePicker/Table/Highlight/index.ts(1 hunks)frontend/src/components/DatePicker/Table/Row.tsx(1 hunks)frontend/src/components/DatePicker/Table/RowContainer.tsx(1 hunks)frontend/src/components/DatePicker/Table/TableBody.tsx(1 hunks)frontend/src/components/DatePicker/Table/TableHeader.tsx(1 hunks)frontend/src/components/DatePicker/Table/index.css.ts(1 hunks)frontend/src/components/DatePicker/Table/index.tsx(1 hunks)frontend/src/components/DatePicker/index.css.ts(1 hunks)frontend/src/components/DatePicker/index.tsx(1 hunks)frontend/src/components/Icon/component/ArrowLeft.tsx(1 hunks)frontend/src/constants/date.ts(1 hunks)frontend/src/hooks/useDatePicker/index.ts(1 hunks)frontend/src/hooks/useDatePicker/useDateSelect.ts(1 hunks)frontend/src/hooks/useDatePicker/useHighlightRange.ts(1 hunks)frontend/src/hooks/useDatePicker/useMonthNavigation.ts(1 hunks)frontend/src/utils/date/calendar/calendarGeneration.ts(1 hunks)frontend/src/utils/date/calendar/index.ts(1 hunks)frontend/src/utils/date/date.ts(3 hunks)frontend/src/utils/date/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (27)
- frontend/src/constants/date.ts
- frontend/src/utils/date/index.ts
- frontend/src/components/Calendar/Calendar.stories.ts
- frontend/src/components/DatePicker/Table/Highlight/index.ts
- frontend/src/components/DatePicker/index.css.ts
- frontend/src/components/DatePicker/Table/Highlight/HighlightGap.tsx
- frontend/src/components/DatePicker/Table/Highlight/HighlightBox.tsx
- frontend/src/components/DatePicker/Table/Highlight/index.css.ts
- frontend/src/components/DatePicker/Header/index.css.ts
- frontend/src/components/DatePicker/Table/index.tsx
- frontend/src/components/DatePicker/DatePickerContext.ts
- frontend/src/components/DatePicker/Table/RowContainer.tsx
- frontend/src/components/DatePicker/DatePicker.stories.tsx
- frontend/src/components/DatePicker/Table/Cell/index.ts
- frontend/src/components/Icon/component/ArrowLeft.tsx
- frontend/src/utils/date/date.ts
- frontend/src/components/DatePicker/Header/index.tsx
- frontend/src/components/DatePicker/Table/Cell/DowCell.tsx
- frontend/src/components/DatePicker/Table/Cell/CellWrapper.tsx
- frontend/src/hooks/useDatePicker/useDateSelect.ts
- frontend/src/hooks/useDatePicker/useMonthNavigation.ts
- frontend/src/components/DatePicker/Table/TableBody.tsx
- frontend/src/components/DatePicker/Table/index.css.ts
- frontend/src/components/DatePicker/Table/Cell/index.css.ts
- frontend/src/components/DatePicker/Table/Row.tsx
- frontend/src/hooks/useDatePicker/useHighlightRange.ts
- frontend/src/hooks/useDatePicker/index.ts
🧰 Additional context used
🪛 Biome (1.9.4)
frontend/src/components/DatePicker/Table/Cell/DateCell.tsx
[error] 102-104: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
🔇 Additional comments (6)
frontend/src/components/DatePicker/Table/TableHeader.tsx (3)
1-4: LGTM! Clean and well-organized imports.The imports are properly structured, using CSS modules for styling and following good practices for component imports.
16-26: LGTM! Clean and efficient implementation.The component follows React best practices with proper key placement and efficient rendering.
27-27: LGTM! Proper export statement.The default export is appropriately used for the main component.
frontend/src/components/DatePicker/Table/Cell/DateCell.tsx (2)
1-26: LGTM!The imports are well-organized and the
DateCellPropsinterface is well-defined with clear prop types.
34-39: Consider moving styles to CSS.As discussed in the previous review, since these styles won't change at runtime, they should be defined in CSS rather than using inline styles.
frontend/src/components/Avatar/index.css.ts (1)
6-9: LGTM!The container style implementation is clean and follows best practices.
| backgroundColor: vars.color.Ref.Netural['White'], | ||
| borderRadius: vars.radius['Max'], | ||
| border: `2px solid ${vars.color.Ref.Netural[500]}`, |
There was a problem hiding this comment.
Fix typo in color variable name.
There's a typo in the color variable name: "Netural" should be "Neutral".
Apply this diff to fix the typo:
- backgroundColor: vars.color.Ref.Netural['White'],
- borderRadius: vars.radius['Max'],
- border: `2px solid ${vars.color.Ref.Netural[500]}`,
+ backgroundColor: vars.color.Ref.Neutral['White'],
+ borderRadius: vars.radius['Max'],
+ border: `2px solid ${vars.color.Ref.Neutral[500]}`,
- backgroundColor: vars.color.Ref.Netural['White'],
- color: vars.color.Ref.Netural[500],
- border: `2px solid ${vars.color.Ref.Netural[100]}`,
+ backgroundColor: vars.color.Ref.Neutral['White'],
+ color: vars.color.Ref.Neutral[500],
+ border: `2px solid ${vars.color.Ref.Neutral[100]}`,Also applies to: 44-46
| const getTodayCellStyle = (calendarType: CalendarType) => calendarType === 'select' | ||
| ? { backgroundColor: vars.color.Ref.Primary[500], color: vars.color.Ref.Netural['White'] } | ||
| : { }; | ||
|
|
||
| const getSelectedCellStyle = (calendarType: CalendarType) => calendarType === 'select' | ||
| ? { backgroundColor: vars.color.Ref.Primary[100], color: vars.color.Ref.Primary[500] } | ||
| : { backgroundColor: vars.color.Ref.Primary[500], color: vars.color.Ref.Netural['White'] }; |
There was a problem hiding this comment.
Fix typo in color variable name.
There's a typo in the color variable name: "Netural" should be "Neutral".
Apply this diff to fix the typo:
- ? { backgroundColor: vars.color.Ref.Primary[500], color: vars.color.Ref.Netural['White'] }
+ ? { backgroundColor: vars.color.Ref.Primary[500], color: vars.color.Ref.Neutral['White'] }
- ? { backgroundColor: vars.color.Ref.Primary[100], color: vars.color.Ref.Primary[500] }
- : { backgroundColor: vars.color.Ref.Primary[500], color: vars.color.Ref.Netural['White'] };
+ ? { backgroundColor: vars.color.Ref.Primary[100], color: vars.color.Ref.Primary[500] }
+ : { backgroundColor: vars.color.Ref.Primary[500], color: vars.color.Ref.Neutral['White'] };📝 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.
| const getTodayCellStyle = (calendarType: CalendarType) => calendarType === 'select' | |
| ? { backgroundColor: vars.color.Ref.Primary[500], color: vars.color.Ref.Netural['White'] } | |
| : { }; | |
| const getSelectedCellStyle = (calendarType: CalendarType) => calendarType === 'select' | |
| ? { backgroundColor: vars.color.Ref.Primary[100], color: vars.color.Ref.Primary[500] } | |
| : { backgroundColor: vars.color.Ref.Primary[500], color: vars.color.Ref.Netural['White'] }; | |
| const getTodayCellStyle = (calendarType: CalendarType) => calendarType === 'select' | |
| ? { backgroundColor: vars.color.Ref.Primary[500], color: vars.color.Ref.Neutral['White'] } | |
| : { }; | |
| const getSelectedCellStyle = (calendarType: CalendarType) => calendarType === 'select' | |
| ? { backgroundColor: vars.color.Ref.Primary[100], color: vars.color.Ref.Primary[500] } | |
| : { backgroundColor: vars.color.Ref.Primary[500], color: vars.color.Ref.Neutral['White'] }; |
| /** | ||
| * 해당 달의 일수를 반환합니다. | ||
| * @param date - 기준 날짜. | ||
| * @returns 해당 달의 일수. | ||
| */ | ||
| export const getDaysInMonth = (date: Date): number => | ||
| new Date(date.getFullYear(), date.getMonth() + 1, 0).getDate(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove duplicate implementation of getDaysInMonth.
This function is duplicated in calendarGeneration.ts. Consider importing it from this file to maintain DRY principles.
| const getNextMonthInfo = (baseDate: Date) => { | ||
| const nextDate = new Date(baseDate); | ||
| nextDate.setMonth(baseDate.getMonth() + 1); | ||
| return { nextDate, prevMonthDays: getDaysInMonth(nextDate) }; | ||
| }; |
There was a problem hiding this comment.
Fix bugs in getNextMonthInfo.
The function has several issues:
- It mutates the date object
- Uses incorrect property name ('prevMonthDays' instead of 'nextMonthDays')
- Differs from index.ts implementation
-const getNextMonthInfo = (baseDate: Date) => {
- const nextDate = new Date(baseDate);
- nextDate.setMonth(baseDate.getMonth() + 1);
- return { nextDate, prevMonthDays: getDaysInMonth(nextDate) };
-};
+const getNextMonthInfo = (baseDate: Date) => {
+ const { month, year } = getNextMonthInfo(baseDate);
+ const nextDate = new Date(year, month);
+ return { nextDate, nextMonthDays: getDaysInMonth(nextDate) };
+};Committable suggestion skipped: line range outside the PR's diff.
| const getPreviousMonthInfo = (date: Date) => { | ||
| const prevDate = new Date(date); | ||
| prevDate.setMonth(date.getMonth() - 1); | ||
| return { prevDate, prevMonthDays: getDaysInMonth(prevDate) }; | ||
| }; |
There was a problem hiding this comment.
Avoid date mutation and align with index.ts implementation.
The current implementation mutates the date object and differs from getPrevMonthInfo in index.ts. Consider using the existing implementation.
-const getPreviousMonthInfo = (date: Date) => {
- const prevDate = new Date(date);
- prevDate.setMonth(date.getMonth() - 1);
- return { prevDate, prevMonthDays: getDaysInMonth(prevDate) };
-};
+const getPreviousMonthInfo = (date: Date) => {
+ const { month, year } = getPrevMonthInfo(date);
+ const prevDate = new Date(year, month);
+ return { prevDate, prevMonthDays: getDaysInMonth(prevDate) };
+};📝 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.
| const getPreviousMonthInfo = (date: Date) => { | |
| const prevDate = new Date(date); | |
| prevDate.setMonth(date.getMonth() - 1); | |
| return { prevDate, prevMonthDays: getDaysInMonth(prevDate) }; | |
| }; | |
| const getPreviousMonthInfo = (date: Date) => { | |
| const { month, year } = getPrevMonthInfo(date); | |
| const prevDate = new Date(year, month); | |
| return { prevDate, prevMonthDays: getDaysInMonth(prevDate) }; | |
| }; |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
frontend/src/components/DatePicker/Table/Cell/CellWrapper.tsx (3)
2-7: Consider using barrel exports for better import organization.The relative import path for the Text component (
../../../Text) is quite deep. Consider creating anindex.tsbarrel file in the components directory to simplify imports.-import { Text } from '../../../Text'; +import { Text } from '@/components';
9-14: Use CSSProperties type for better type safety.The
styleprop is typed asobjectwhich is too generic. Consider usingReact.CSSPropertiesfor better type safety and IDE support.+import type { PropsWithChildren, CSSProperties } from 'react'; interface CellWrapperProps extends PropsWithChildren { cursorType: 'default' | 'pointer' | 'not-allowed'; className?: string; - style?: object; + style?: CSSProperties; onClick?: () => void; }
15-29: Consider memoizing the component for better performance.Since this is a purely presentational component that receives simple props, wrapping it with
React.memocould help prevent unnecessary re-renders.-export const CellWrapper = ({ +export const CellWrapper = React.memo(({ className, cursorType, style, onClick, children, -}: CellWrapperProps) => ( +}: CellWrapperProps) => ( <div className={clsx(cellWrapperStyle({ cursorType }), className )} onClick={onClick} style={style} > <Text typo='caption'>{children}</Text> </div> -); +));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/DatePicker/Table/Cell/CellWrapper.tsx(1 hunks)
🔇 Additional comments (1)
frontend/src/components/DatePicker/Table/Cell/CellWrapper.tsx (1)
31-31: LGTM!The default export is appropriate for this single-component file.
hamo-o
left a comment
hamo-o
left a comment
There was a problem hiding this comment.
수고하셨습니다! 캘린더쪽 페이지 구현하면서 어떻게 추상화할지 같이 이야기(+추가 리팩터링)해봐요!!
#️⃣ 연관된 이슈>
📝 작업 내용> 이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)
Date Picker 컴포넌트를 구현했습니다.
calendarType: select
calendarType: range
🙏 여기는 꼭 봐주세요! > 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요
현재
useDatePicker훅 내에서 달력 정보(현재 년도, 달, 선택된 날짜 등)을 state로 관리하고 있는데, 외부와 상호작용할 수 있게 하려면 외부에서 state를 주입해주는 형태로 리팩터링해야할 것 같습니다.Summary by CodeRabbit
New Features
Rowcomponent to render rows of date cells within the DatePicker.HighlightBoxandHighlightGapcomponents to visually indicate selected date ranges.Style