[FE-Feat] Date Picker 리팩터링 #134
Conversation
WalkthroughThe changes reorganize and refactor the DatePicker component and its related functionality. The main DatePicker now delegates rendering to its subcomponents DatePicker.Select and DatePicker.Range, with the corresponding stories updated. Obsolete context and hooks (e.g., useDatePicker, useDateSelect) have been removed, and new hooks (useDatePickerRange and useDatePickerSelect) are introduced. Internal names and types have been standardized (e.g., CalendarType → DatePickerType) and import paths adjusted, while table components and the Header have been updated to use the revised context and naming conventions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DatePickerSelect
participant useDatePickerSelect
participant useMonthNavigation
User->>DatePickerSelect: Clicks date cell
DatePickerSelect->>useDatePickerSelect: onDateCellClick(date)
useDatePickerSelect->>useMonthNavigation: Validate date & update baseDate
useMonthNavigation-->>useDatePickerSelect: Return updated calendar dates
useDatePickerSelect-->>DatePickerSelect: Return selectedDate and highlight info
DatePickerSelect->>User: Render updated date selection
sequenceDiagram
participant User
participant DatePickerRange
participant useDatePickerRange
participant useHighlightRange
User->>DatePickerRange: Clicks date cell for range selection
DatePickerRange->>useDatePickerRange: onDateCellClick(date)
useDatePickerRange->>useHighlightRange: Update highlightRange (start/end)
useHighlightRange-->>useDatePickerRange: Return updated range data
DatePickerRange->>User: Render selected date range
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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: 3
🧹 Nitpick comments (11)
frontend/src/components/DatePicker/index.tsx (2)
2-2: Consider importing ReactNode directly from 'react'.
It's unusual to import ReactNode from '@tanstack/react-router'. This might introduce confusion and possible version mismatches.
9-14: Interface looks good.
The new CommonDatePickerProps aligns with a modular approach. Consider adding brief JSDoc comments explaining each prop.frontend/src/components/DatePicker/context/DatePickerContext.ts (1)
7-14: Unified interface for both select and range modes.
While it’s convenient to have a single interface, consider documenting which properties apply uniquely to “select” vs. “range.”frontend/src/hooks/useDatePicker/useMonthNavigation.ts (1)
8-8: Consider keeping method names consistent with common patterns.The method names have been changed from
goToPrevMonth/goToNextMonthtogotoPrevMonth/gotoNextMonth. While both are valid, the previous naming convention with "To" is more commonly used in React components.- const gotoPrevMonth = () => { + const goToPrevMonth = () => { - const gotoNextMonth = () => { + const goToNextMonth = () => {Also applies to: 13-13
frontend/src/hooks/useDatePicker/useDatePickerRange.ts (1)
25-44: Consider adding validation for date range boundaries.The click handling logic could benefit from additional validation to ensure the date range doesn't exceed reasonable boundaries (e.g., maximum range of 1 year).
const onDateCellClick = (date: Date) => { const { start, end } = highlightRange; + const MAX_RANGE_DAYS = 365; + if (!start) { setHighlightStart(date); return; } if (!end) { if (date < start) { setHighlightStart(date); } else if (isSameDate(date, start)) { setHighlightStart(null); } else { + const daysDifference = Math.floor((date.getTime() - start.getTime()) / (1000 * 60 * 60 * 24)); + if (daysDifference > MAX_RANGE_DAYS) { + return; + } setHighlightEnd(date); } return; } setHighlightStart(null); setHighlightEnd(null); };frontend/src/components/DatePicker/DatePickerSelect.tsx (1)
35-37: Consider adding ARIA attributes for better accessibility.The trigger div should have appropriate ARIA attributes to indicate it's an expandable/collapsible element.
- <div onClick={() => setIsOpen(prev => !prev)}> + <div + onClick={() => setIsOpen(prev => !prev)} + role="button" + aria-expanded={isOpen} + aria-haspopup="true" + >frontend/src/hooks/useDatePicker/useDatePickerSelect.ts (1)
46-48: Consider extracting week range calculation to a utility function.The week range calculation logic could be moved to a separate utility function for better reusability.
+const getWeekRange = (date: Date): [Date, Date] => { + const weekDates = formatDateToWeekDates(date); + return [weekDates[0], weekDates[weekDates.length - 1]]; +}; - const weekDates = formatDateToWeekDates(date); - const [startOfWeek, endOfWeek] = [weekDates[0], weekDates[weekDates.length - 1]]; + const [startOfWeek, endOfWeek] = getWeekRange(date);frontend/src/components/DatePicker/DatePickerRange.tsx (1)
12-12: Rename interface to follow naming convention.The interface name should be suffixed with
Propsto maintain consistency with TypeScript naming conventions.-export interface DatePickerRange extends CommonDatePickerProps { +export interface DatePickerRangeProps extends CommonDatePickerProps {frontend/src/components/DatePicker/Table/Row.tsx (1)
17-17: Consider maintaining consistent naming.While the local rename from
weekDatestoweekminimizes internal changes, it introduces two different names for the same data. Consider usingweekDatesconsistently throughout to improve code clarity.-const Row = ({ weekDates: week, baseDate }: RowProps) => { +const Row = ({ weekDates, baseDate }: RowProps) => {frontend/src/components/DatePicker/Table/Cell/DateCell.tsx (2)
47-48: Consider extracting disabled state logic.The disabled state logic could be moved to a helper function for better reusability and maintainability.
+const isDateCellDisabled = (calendarType: DatePickerType, dateCellType: DateCellType) => + calendarType === 'range' && dateCellType === 'otherMonth'; const DateCell = ({ ... }: DateCellProps) => { ... - const isDisabled = calendarType === 'range' && dateCellType === 'otherMonth'; + const isDisabled = isDateCellDisabled(calendarType, dateCellType); ... };
77-77: Address TODO: Implement holiday checking.The TODO comment indicates missing functionality for holiday checking. This is important for proper date cell styling.
Would you like me to help implement the holiday checking functionality? I can suggest an implementation that:
- Accepts a list of holiday dates
- Checks if a given date is in the holiday list
- Applies appropriate styling
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
frontend/src/components/DatePicker/DatePicker.stories.tsx(1 hunks)frontend/src/components/DatePicker/DatePickerContext.ts(0 hunks)frontend/src/components/DatePicker/DatePickerRange.tsx(1 hunks)frontend/src/components/DatePicker/DatePickerSelect.tsx(1 hunks)frontend/src/components/DatePicker/Header/index.tsx(3 hunks)frontend/src/components/DatePicker/Table/Cell/DateCell.tsx(4 hunks)frontend/src/components/DatePicker/Table/Row.tsx(2 hunks)frontend/src/components/DatePicker/Table/TableBody.tsx(1 hunks)frontend/src/components/DatePicker/context/DatePickerContext.ts(1 hunks)frontend/src/components/DatePicker/index.tsx(1 hunks)frontend/src/hooks/useDatePicker/index.ts(0 hunks)frontend/src/hooks/useDatePicker/useDatePickerRange.ts(1 hunks)frontend/src/hooks/useDatePicker/useDatePickerSelect.ts(1 hunks)frontend/src/hooks/useDatePicker/useDateSelect.ts(0 hunks)frontend/src/hooks/useDatePicker/useHighlightRange.ts(1 hunks)frontend/src/hooks/useDatePicker/useMonthNavigation.ts(1 hunks)
💤 Files with no reviewable changes (3)
- frontend/src/components/DatePicker/DatePickerContext.ts
- frontend/src/hooks/useDatePicker/useDateSelect.ts
- frontend/src/hooks/useDatePicker/index.ts
🧰 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.
🔇 Additional comments (26)
frontend/src/components/DatePicker/index.tsx (4)
4-5: Neat import of subcomponents.
Importing DatePickerRange and DatePickerSelect here neatly organizes the subcomponents.
7-7: Clearer naming.
Renaming to 'DatePickerType' clearly differentiates the component's usage modes.
17-18: Explicit usage guidance.
Throwing an error is a direct way to ensure the user selects the correct subcomponent. This aligns with an "API guard" pattern.
21-22: Attaching subcomponents provides flexibility.
Exposing “Select” and “Range” on the main object is a convenient pattern for multiple variant usage.frontend/src/components/DatePicker/Table/TableBody.tsx (4)
4-4: Updated import path.
Switching to the new context path is consistent with the refactoring.
8-8: Destructuring context usage.
Pulling out calendarDates and baseDate from the context is clean; consider verifying if any additional props are needed.
14-14: Renamed prop.
Renaming “week” to “weekDates” improves clarity.
20-20: Maintain default export.
Exporting as default is straightforward and matches typical usage patterns.frontend/src/components/DatePicker/context/DatePickerContext.ts (2)
1-4: Clean imports.
Importing React's createContext and the external types sets a clear foundation for the new context.
16-16: Context creation.
Creating the DatePickerContext with a null default aligns well with a safe context strategy.frontend/src/hooks/useDatePicker/useHighlightRange.ts (2)
11-12: LGTM! Type annotation improves type safety.The addition of the
HighlightRangetype annotation for theprevparameter enhances type safety and code clarity.
17-17: LGTM! Exposing setHighlightRange provides more flexibility.Including
setHighlightRangein the return object gives consumers more control over the highlight range state.frontend/src/hooks/useDatePicker/useMonthNavigation.ts (1)
5-6: LGTM! Improved initialization flexibility.The addition of the optional
initialBaseDateparameter and its usage in state initialization provides better control over the initial state.frontend/src/components/DatePicker/DatePicker.stories.tsx (1)
20-32: LGTM! Well-structured stories demonstrating component usage.The stories effectively demonstrate the usage of both Select and Range variants of the DatePicker component, including state management and event handling.
Also applies to: 34-45
frontend/src/hooks/useDatePicker/useDatePickerRange.ts (1)
20-23: LGTM! Efficient calendar generation with useMemo.The use of
useMemofor calendar generation optimizes performance by preventing unnecessary recalculations.frontend/src/components/DatePicker/DatePickerSelect.tsx (1)
39-39: 🏁 Script executed:#!/bin/bash fd -e css -e scss "DatePickerSelect"Length of output: 36
frontend/src/components/DatePicker/Header/index.tsx (1)
12-12: LGTM! Good use of useSafeContext.The component correctly uses
useSafeContextfor context error handling, as established in the codebase.frontend/src/components/DatePicker/DatePickerRange.tsx (2)
24-28: LGTM! Good use of useCallback.The
isDateSelectedfunction is correctly memoized usinguseCallbackwith proper dependencies.
46-46: 🏁 Script executed:#!/bin/bash fd -e css -e scss "DatePickerRange"Length of output: 35
frontend/src/components/DatePicker/Table/Row.tsx (4)
5-5: LGTM! Import path restructuring.The updated import path follows a better organization pattern.
13-13: LGTM! Improved prop naming.The rename from
weektoweekDatesbetter describes the data being passed.
18-18: LGTM! Improved state management.Moving date selection logic to the context aligns with the PR's objective of state injection and improves component modularity.
26-31: LGTM! Simplified DateCell props.The change improves component modularity by delegating date selection state to the context.
frontend/src/components/DatePicker/Table/Cell/DateCell.tsx (3)
4-4: LGTM! Improved imports organization.The changes improve code organization by:
- Centralizing theme variables
- Using more structured import paths
Also applies to: 7-8
30-39: LGTM! Improved component structure.The restructuring enhances code readability and aligns with the PR's state injection pattern.
52-53: LGTM! Improved interaction handling.The changes properly handle disabled state in both visual feedback (cursor) and interaction (click handler).
| export default meta; | ||
|
|
||
| type Story = StoryObj<typeof DatePicker>; | ||
| // export default meta; |
There was a problem hiding this comment.
Remove duplicate export statement.
Line 16 contains a duplicate export statement that should be removed.
- // export default meta;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // export default meta; |
| return; | ||
| } | ||
|
|
||
| const { year, month } = getDateParts(baseDate!); |
There was a problem hiding this comment.
Add null check for baseDate.
The non-null assertion (!) on baseDate could lead to runtime errors if baseDate is undefined.
- const { year, month } = getDateParts(baseDate!);
+ const currentDate = baseDate ?? new Date();
+ const { year, month } = getDateParts(currentDate);📝 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 { year, month } = getDateParts(baseDate!); | |
| const currentDate = baseDate ?? new Date(); | |
| const { year, month } = getDateParts(currentDate); |
| const getTodayCellStyle = (calendarType: DatePickerType) => calendarType === 'select' | ||
| ? { backgroundColor: vars.color.Ref.Primary[500], color: vars.color.Ref.Netural['White'] } | ||
| : { }; | ||
|
|
||
| const getSelectedCellStyle = (calendarType: DatePickerType) => 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'.
- ? { 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[500], color: vars.color.Ref.Netural['White'] };
+ : { 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: DatePickerType) => calendarType === 'select' | |
| ? { backgroundColor: vars.color.Ref.Primary[500], color: vars.color.Ref.Netural['White'] } | |
| : { }; | |
| const getSelectedCellStyle = (calendarType: DatePickerType) => 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: DatePickerType) => calendarType === 'select' | |
| ? { backgroundColor: vars.color.Ref.Primary[500], color: vars.color.Ref.Neutral['White'] } | |
| : { }; | |
| const getSelectedCellStyle = (calendarType: DatePickerType) => 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'] }; |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
frontend/src/components/DatePicker/index.tsx (1)
21-23: Consider using English for error messages.The error message is currently in Korean. Consider using English to ensure broader accessibility across development teams.
- throw new Error('!!! DatePicker.Select, DatePicker.Range를 사용해주세요 !!!'); + throw new Error('!!! Please use DatePicker.Select or DatePicker.Range !!!');frontend/src/components/DatePicker/DatePicker.stories.tsx (1)
24-24: Consider explicit prop passing instead of spreading.While spreading props works, explicitly passing them would make the component usage more transparent and maintainable.
// For Default story - {...monthNavigation} + baseDate={monthNavigation.baseDate} + gotoNextMonth={monthNavigation.gotoNextMonth} + gotoPrevMonth={monthNavigation.gotoPrevMonth} // For Range story - {...monthNavigation} - {...highlightProps} + baseDate={monthNavigation.baseDate} + gotoNextMonth={monthNavigation.gotoNextMonth} + gotoPrevMonth={monthNavigation.gotoPrevMonth} + highlightRange={highlightProps.highlightRange} + setHighlightStart={highlightProps.setHighlightStart} + setHighlightEnd={highlightProps.setHighlightEnd}Also applies to: 35-36
frontend/src/hooks/useDatePicker/useDatePickerRange.ts (1)
25-44: Add validation for invalid date ranges.The click handler should validate that the end date isn't more than a reasonable range (e.g., 1 year) from the start date to prevent invalid selections.
const onDateCellClick = (date: Date) => { const { start, end } = highlightRange; if (!start) { setHighlightStart(date); return; } if (!end) { + const maxRange = new Date(start); + maxRange.setFullYear(maxRange.getFullYear() + 1); + if (date < start) { setHighlightStart(date); + } else if (date > maxRange) { + // Either show an error or set to max allowed date + return; } else if (isSameDate(date, start)) { setHighlightStart(null); } else { setHighlightEnd(date); } return; } setHighlightStart(null); setHighlightEnd(null); };frontend/src/components/DatePicker/DatePickerRange.tsx (2)
11-15: LGTM! Consider adding JSDoc comments.The interface is well-defined and extends CommonDatePickerProps appropriately with range-specific functionality.
Consider adding JSDoc comments to document the purpose of each prop:
export interface DatePickerRange extends CommonDatePickerProps { + /** The currently selected date range */ highlightRange: HighlightRange; + /** Callback to set the start date of the range */ setHighlightStart: (date: Date | null) => void; + /** Callback to set the end date of the range */ setHighlightEnd: (date: Date | null) => void; }
29-52: Add error boundaries and loading states.The component should handle potential errors from useDatePickerRange and provide visual feedback during loading.
Consider wrapping the component with an error boundary and adding loading states:
import { ErrorBoundary } from '@/components/ErrorBoundary'; const DatePickerRange = ({ highlightRange, ...props }: DatePickerRange) => { // ... existing code ... if (calendarDates === undefined) { return <div>Loading...</div>; } return ( <ErrorBoundary fallback={<div>Error loading date picker</div>}> <DatePickerContext.Provider> {/* ... existing JSX ... */} </DatePickerContext.Provider> </ErrorBoundary> ); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
frontend/src/components/DatePicker/DatePicker.stories.tsx(1 hunks)frontend/src/components/DatePicker/DatePickerRange.tsx(1 hunks)frontend/src/components/DatePicker/DatePickerSelect.tsx(1 hunks)frontend/src/components/DatePicker/Table/Cell/CellWrapper.tsx(1 hunks)frontend/src/components/DatePicker/Table/Row.tsx(2 hunks)frontend/src/components/DatePicker/context/DatePickerContext.ts(1 hunks)frontend/src/components/DatePicker/index.tsx(1 hunks)frontend/src/hooks/useDatePicker/useDatePickerRange.ts(1 hunks)frontend/src/hooks/useDatePicker/useDatePickerSelect.ts(1 hunks)frontend/src/hooks/useDatePicker/useHighlightRange.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/src/components/DatePicker/Table/Cell/CellWrapper.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/src/hooks/useDatePicker/useHighlightRange.ts
- frontend/src/hooks/useDatePicker/useDatePickerSelect.ts
- frontend/src/components/DatePicker/context/DatePickerContext.ts
- frontend/src/components/DatePicker/Table/Row.tsx
🔇 Additional comments (5)
frontend/src/components/DatePicker/index.tsx (1)
7-19: Well-structured type definitions!The interfaces are well-defined and align perfectly with the PR objectives. The
CommonDatePickerPropsinterface provides a good foundation for both Select and Range variants.frontend/src/components/DatePicker/DatePicker.stories.tsx (1)
16-39: Well-structured stories demonstrating component usage!The stories effectively demonstrate both Select and Range variants with proper state management using custom hooks.
frontend/src/components/DatePicker/DatePickerSelect.tsx (1)
39-39: Define containerStyle using CSS modules or styled-components.The
containerStyleclassName suggests inline styles, which should be moved to a proper styling solution.❌ Verification inconclusive
🏁 Script executed:
#!/bin/bash # Check if there's a styles file for the DatePicker fd -e css -e scss -e styled.ts "DatePicker"Length of output: 43
Issue: Missing proper styling definition for "containerStyle".
The search for any related styling file (CSS/SCSS/styled-components) for the DatePicker yielded no results. This means that the
"containerStyle"class applied in the<div>is not backed by a proper CSS module or styled-component. Please update the code by either defining a CSS module (e.g.,DatePickerSelect.module.css) and importing it or by refactoring the style into a styled-component.frontend/src/components/DatePicker/DatePickerRange.tsx (2)
18-18: Review the isOpen state initialization logic.The current initialization
!props.triggermight lead to unexpected behavior. If trigger is undefined, the picker will always be open by default.Consider explicitly checking for trigger existence:
- const [isOpen, setIsOpen] = useState(!props.trigger); + const [isOpen, setIsOpen] = useState(props.trigger === undefined);
54-54: LGTM!The default export is appropriate for this component.
| const calendarDates = useMemo( | ||
| () => generateMonthCalendar(baseDate ?? new Date()), | ||
| [baseDate], | ||
| ); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider timezone-safe default date handling.
Using new Date() directly might cause timezone-related issues. Consider using a utility function that ensures consistent timezone handling.
🏁 Script executed:
#!/bin/bash
# Check if there's a date utility function in the codebase
rg -l "new Date\(\)" --type tsLength of output: 384
Timezone Safety Concern in Default Date Initialization
- In frontend/src/hooks/useDatePicker/useDatePickerRange.ts (lines 20–23), the fallback uses
new Date()directly, which can introduce timezone inconsistencies. - A search confirmed similar usage in several files, suggesting a systemic reliance on
new Date()without any wrapper or utility ensuring consistent, timezone-safe date handling. - Consider refactoring this and related instances (e.g., in useCalendar.ts, DateCell.tsx, etc.) to a common utility that guarantees proper timezone adjustments.
| <div onClick={() => setIsOpen(prev => !prev)}> | ||
| {props.trigger} | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessibility attributes to the trigger.
The click handler div should have proper accessibility attributes for better keyboard navigation and screen reader support.
- <div onClick={() => setIsOpen(prev => !prev)}>
+ <div
+ role="button"
+ tabIndex={0}
+ aria-expanded={isOpen}
+ aria-haspopup="true"
+ onClick={() => setIsOpen(prev => !prev)}
+ onKeyDown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ setIsOpen(prev => !prev);
+ }
+ }}
+ >📝 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.
| <div onClick={() => setIsOpen(prev => !prev)}> | |
| {props.trigger} | |
| </div> | |
| <div | |
| role="button" | |
| tabIndex={0} | |
| aria-expanded={isOpen} | |
| aria-haspopup="true" | |
| onClick={() => setIsOpen(prev => !prev)} | |
| onKeyDown={(e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| setIsOpen(prev => !prev); | |
| } | |
| }} | |
| > | |
| {props.trigger} | |
| </div> |
| {isOpen && ( | ||
| <div className='containerStyle'> | ||
| <Header /> | ||
| <Table /> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add click outside handling for the dropdown.
The date picker should close when clicking outside to follow standard dropdown behavior.
+import { useEffect, useRef } from 'react';
+
const DatePickerSelect = ({ selectedDate, ...props }: DatePickerSelectProps) => {
const [isOpen, setIsOpen] = useState(!props.trigger);
+ const containerRef = useRef<HTMLDivElement>(null);
+
+ useEffect(() => {
+ const handleClickOutside = (event: MouseEvent) => {
+ if (containerRef.current && !containerRef.current.contains(event.target as Node)) {
+ setIsOpen(false);
+ }
+ };
+
+ document.addEventListener('mousedown', handleClickOutside);
+ return () => document.removeEventListener('mousedown', handleClickOutside);
+ }, []);
// ... rest of the component
return (
<DatePickerContext.Provider value={...}>
<div onClick={...}>{props.trigger}</div>
{isOpen && (
- <div className='containerStyle'>
+ <div ref={containerRef} className='containerStyle'>📝 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.
| {isOpen && ( | |
| <div className='containerStyle'> | |
| <Header /> | |
| <Table /> | |
| </div> | |
| )} | |
| import { useEffect, useRef } from 'react'; | |
| import { useState } from 'react'; | |
| // ... other necessary imports | |
| const DatePickerSelect = ({ selectedDate, ...props }: DatePickerSelectProps) => { | |
| const [isOpen, setIsOpen] = useState(!props.trigger); | |
| const containerRef = useRef<HTMLDivElement>(null); | |
| useEffect(() => { | |
| const handleClickOutside = (event: MouseEvent) => { | |
| if (containerRef.current && !containerRef.current.contains(event.target as Node)) { | |
| setIsOpen(false); | |
| } | |
| }; | |
| document.addEventListener('mousedown', handleClickOutside); | |
| return () => document.removeEventListener('mousedown', handleClickOutside); | |
| }, []); | |
| // ... rest of the component | |
| return ( | |
| <DatePickerContext.Provider value={/* ... context value ... */}> | |
| <div onClick={/* ... click handler logic ... */}>{props.trigger}</div> | |
| {isOpen && ( | |
| <div ref={containerRef} className='containerStyle'> | |
| <Header /> | |
| <Table /> | |
| </div> | |
| )} | |
| </DatePickerContext.Provider> | |
| ); | |
| }; | |
| export default DatePickerSelect; |
| <div onClick={() => setIsOpen(prev => !prev)}> | ||
| {props.trigger} | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessibility attributes to the clickable div.
The div needs proper keyboard interaction and ARIA attributes for accessibility.
Apply these changes:
- <div onClick={() => setIsOpen(prev => !prev)}>
+ <div
+ role="button"
+ tabIndex={0}
+ aria-expanded={isOpen}
+ aria-haspopup="dialog"
+ onClick={() => setIsOpen(prev => !prev)}
+ onKeyDown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ setIsOpen(prev => !prev);
+ }
+ }}
+ >
{props.trigger}
</div>📝 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.
| <div onClick={() => setIsOpen(prev => !prev)}> | |
| {props.trigger} | |
| </div> | |
| <div | |
| role="button" | |
| tabIndex={0} | |
| aria-expanded={isOpen} | |
| aria-haspopup="dialog" | |
| onClick={() => setIsOpen(prev => !prev)} | |
| onKeyDown={(e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| setIsOpen(prev => !prev); | |
| } | |
| }} | |
| > | |
| {props.trigger} | |
| </div> |
| {props.trigger} | ||
| </div> | ||
| {isOpen && ( | ||
| <div className='containerStyle'> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Define or import the containerStyle className.
The className 'containerStyle' appears to be undefined.
Please verify if this style is defined in a CSS module or styled-components. If not, consider:
- <div className='containerStyle'>
+ <div className={styles.container}>Run this script to check for style definitions:
🏁 Script executed:
#!/bin/bash
# Search for containerStyle definition in CSS/SCSS files
rg -t css -t scss 'containerStyle'
# Search for style imports in the current file's directory
rg -t typescript -t javascript 'containerStyle' "$(dirname "frontend/src/components/DatePicker/DatePickerRange.tsx")"Length of output: 247
I'll run a revised script using file globbing to search for any occurrences of “containerStyle” in potential style files and in the component directory. This should help us verify whether the “containerStyle” class is defined somewhere in the codebase.
Once you execute this script, please share the output so we can further verify whether the containerStyle class is defined or imported.
🏁 Script executed:
#!/bin/bash
echo "Searching for containerStyle in .css files:"
find . -type f -iname "*.css" -exec grep -H "containerStyle" {} \;
echo "Searching for containerStyle in .scss files:"
find . -type f -iname "*.scss" -exec grep -H "containerStyle" {} \;
echo "Searching for containerStyle in .tsx and .ts files in the DatePicker directory:"
find frontend/src/components/DatePicker -type f \( -iname "*.ts" -o -iname "*.tsx" \) -exec grep -H "containerStyle" {} \;Length of output: 912
I'll run a script to check whether the DatePickerRange.tsx file imports styles from index.css.ts (or a similar module) so that the defined style containerStyle can be correctly used.
Please share the output so we can further verify if the style is properly imported.
🏁 Script executed:
#!/bin/bash
echo "Looking for CSS module import in DatePickerRange.tsx:"
rg "index\.css\.ts" frontend/src/components/DatePicker/DatePickerRange.tsxLength of output: 193
Issue Found: Missing Import for Defined Style
The investigation shows that the style is defined in the file frontend/src/components/DatePicker/index.css.ts (exporting containerStyle), but neither the DatePickerRange.tsx nor the other relevant component files import it. Instead of using a string literal (className='containerStyle'), you need to import the style and apply it correctly.
- Location:
frontend/src/components/DatePicker/DatePickerRange.tsx - Suggested Fix:
- Add an import in
DatePickerRange.tsx:import { containerStyle } from './index.css.ts';
- Update the JSX to use the imported style:
- <div className='containerStyle'> + <div className={containerStyle}>
- Add an import in
| <div onClick={() => setIsOpen(prev => !prev)}> | ||
| {props.trigger} | ||
| </div> |
There was a problem hiding this comment.
p2;
trigger 위치 스타일링이 데이트피커 바로 위에 되어야 할 것 같습니다! 크기 줄어드는 것도 수정해주시면 좋을 것 같아요
There was a problem hiding this comment.
p3; 트리거 있는 버전 + 공용 컨텍스트 쓰는 버전도 추가해주시면 좋을 것 같습니다.
| const calendarDates = useMemo( | ||
| () => generateMonthCalendar(baseDate ?? new Date()), | ||
| [baseDate], |
There was a problem hiding this comment.
p3;
요게 쓰이는 곳은 한군데밖에 없는 것 같습니다. generateMonthCalendar 함수를 테이블에서 실행해서 컨텍스트에서 관리하는 상태를 줄여볼 수 있을 것 같습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/src/components/DatePicker/index.tsx (1)
16-18: Consider using English for error messages.The error message is currently in Korean. Consider using English for better accessibility across international development teams.
- throw new Error('!!! DatePicker.Select, DatePicker.Range를 사용해주세요 !!!'); + throw new Error('!!! Please use DatePicker.Select or DatePicker.Range !!!');frontend/src/hooks/useDatePicker/useDatePickerRange.ts (1)
16-35: Consider adding comments and descriptive variable names.While the logic is correct, the code could be more self-documenting. Consider:
- Adding comments explaining the different states and their transitions
- Using more descriptive variable names for better readability
const onDateCellClick = (date: Date) => { const { start, end } = highlightRange; + // If no start date is set, set it to the clicked date if (!start) { setHighlightStart(date); return; } + // If only start date is set, handle the second click if (!end) { + // If clicked date is before start, update start date if (date < start) { setHighlightStart(date); + // If clicked date is same as start, clear selection } else if (isSameDate(date, start)) { setHighlightStart(null); + // If clicked date is after start, set it as end date } else { setHighlightEnd(date); } return; } + // If both dates are set, clear the selection setHighlightStart(null); setHighlightEnd(null); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
frontend/src/components/DatePicker/DatePickerRange.tsx(1 hunks)frontend/src/components/DatePicker/DatePickerSelect.tsx(1 hunks)frontend/src/components/DatePicker/Table/Highlight/index.ts(1 hunks)frontend/src/components/DatePicker/Table/Row.tsx(2 hunks)frontend/src/components/DatePicker/Table/TableBody.tsx(1 hunks)frontend/src/components/DatePicker/context/DatePickerContext.ts(1 hunks)frontend/src/components/DatePicker/index.tsx(1 hunks)frontend/src/hooks/useDatePicker/useDatePickerRange.ts(1 hunks)frontend/src/hooks/useDatePicker/useDatePickerSelect.ts(1 hunks)frontend/src/hooks/useDatePicker/useHighlightRange.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- frontend/src/hooks/useDatePicker/useHighlightRange.ts
- frontend/src/components/DatePicker/context/DatePickerContext.ts
- frontend/src/hooks/useDatePicker/useDatePickerSelect.ts
- frontend/src/components/DatePicker/DatePickerRange.tsx
- frontend/src/components/DatePicker/Table/Row.tsx
🔇 Additional comments (8)
frontend/src/components/DatePicker/Table/Highlight/index.ts (1)
8-11: LGTM!The
HighlightRangeinterface is well-defined and properly typed for managing date range selection.frontend/src/components/DatePicker/index.tsx (1)
9-14: LGTM!The refactoring to a composite component with Select and Range variants improves modularity and reusability. The CommonDatePickerProps interface is well-defined with clear prop types.
Also applies to: 20-21
frontend/src/components/DatePicker/DatePickerSelect.tsx (3)
34-36: Add accessibility attributes to the trigger.The click handler div should have proper accessibility attributes for better keyboard navigation and screen reader support.
37-42: Add click outside handling for the dropdown.The date picker should close when clicking outside to follow standard dropdown behavior.
16-46: LGTM!The component is well-structured with clear separation of concerns:
- Uses custom hooks for date selection logic
- Properly manages internal state
- Provides comprehensive context to child components
frontend/src/components/DatePicker/Table/TableBody.tsx (3)
2-6: LGTM! Clean import organization.The import changes reflect good separation of concerns with the calendar generation logic moved to a utility function, and the context properly organized in a dedicated context directory.
8-19: LGTM! Clean implementation with improved prop naming.The component implementation is now more focused with:
- Simplified context usage
- Clear separation of calendar generation logic
- Improved prop naming (weekDates is more descriptive than week)
9-10: Verify baseDate null handling.The fallback to
new Date()whenbaseDateis null could lead to unexpected behavior ifbaseDateis undefined in the context. Consider whether this fallback aligns with the expected behavior.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for DatePickerContext provider to understand baseDate initialization ast-grep --pattern 'const DatePickerContext = createContext<DatePickerContextType>($$$)'Length of output: 90
I'll now generate additional shell scripts to search for the definition of DatePickerContext and inspect how its value for baseDate is initialized. This will help us determine whether the fallback (
new Date()) might be problematic or if the context always supplies a valid value.Please run the following scripts:
Once we have these results, we can verify how baseDate is initialized and whether the fallback to
new Date()is intentional or might lead to unexpected behavior.
🏁 Script executed:
#!/bin/bash # Look for files with "DatePickerContext" in their name to locate the provider definition echo "Searching for files containing 'DatePickerContext':" fd -e ts -e tsx -e js "DatePickerContext" . # Search within the repository for the provider initialization (using createContext) for DatePickerContext echo "Searching for DatePickerContext provider definition using createContext:" rg "createContext.*DatePickerContext" -n .Length of output: 582
Verified baseDate null handling in TableBody.tsx.
- The provider file (
frontend/src/components/DatePicker/context/DatePickerContext.ts) initializes the context with a default value ofnull.- As a result, if the provider does not supply a specific
baseDate, the component’s use of the nullish coalescing operator (baseDate ?? new Date()) will indeed fall back to the current date.- This behavior appears intentional: when no explicit
baseDateis provided via a wrapping provider, the calendar is generated using today's date.- Ensure that this fallback aligns with the overall design expectations—if a strictly controlled date is required, the provider should always supply a valid
baseDate.
className을 주입하지 않으면, default 스타일이 적용됨
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
frontend/src/components/DatePicker/DatePickerSelect.tsx (1)
26-28: Add accessibility attributes to the trigger wrapper.The clickable div needs proper keyboard interaction and ARIA attributes for accessibility.
- <div className={triggerWrapperStyle} onClick={() => setIsOpen(true)}> + <div + className={triggerWrapperStyle} + role="button" + tabIndex={0} + aria-expanded={isOpen} + aria-haspopup="dialog" + aria-label="Select date" + onClick={() => setIsOpen(true)} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + setIsOpen(true); + } + }} + >📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<div className={triggerWrapperStyle} role="button" tabIndex={0} aria-expanded={isOpen} aria-haspopup="dialog" aria-label="Select date" onClick={() => setIsOpen(true)} onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); setIsOpen(true); } }} > {trigger} {isOpen && (frontend/src/components/DatePicker/DatePickerRange.tsx (1)
30-32: Add accessibility attributes to the trigger wrapper.The clickable div needs proper keyboard interaction and ARIA attributes for accessibility.
- <div className={triggerWrapperStyle} onClick={() => setIsOpen(true)}> + <div + className={triggerWrapperStyle} + role="button" + tabIndex={0} + aria-expanded={isOpen} + aria-haspopup="dialog" + aria-label="Select date range" + onClick={() => setIsOpen(true)} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + setIsOpen(true); + } + }} + >
🧹 Nitpick comments (13)
frontend/src/components/DatePicker/Table/TableBody.tsx (1)
13-24: Consider optimizing the key generation.While using
toISOString()for key generation is reliable, it might be more efficient to use the week's index or a combination of year/month/weekNumber for the key, especially if the component re-renders frequently.Consider this alternative approach:
- key={`week-${week[0].toISOString()}`} + key={`week-${baseDate.getFullYear()}-${baseDate.getMonth()}-${index}`}The layout structure using Flex with explicit direction and gap properties is clean and maintainable.
frontend/src/components/DatePicker/index.css.ts (1)
5-18: Consider theming for width property and consistent border usage.
Currently, thewidthis hardcoded to286. You might consider using a theme-based variable (just like you're doing for spacing) to maintain consistency. Also, you usevars.color.Ref.Netural[100]insideborder: `1px ${...}`,but rely on direct numeric values for other properties. Harmonizing these can help standardize styling across the app.frontend/src/components/DatePicker/datePicker.stories.css.ts (1)
1-6: Unify sizing with theming.
If your design system uses spacing variables, you might consider referencing them forwidthandheight, just like forgapor padding in other components, to ensure consistency across the codebase.frontend/src/components/Calendar/context/SharedCalendarContext.ts (1)
10-10: Optionally define a default object.
Usingnullas the default can be valid, but you could provide a default implementation if you want to avoid null checks elsewhere. A custom hook pattern can also help manage initialization logic.frontend/src/components/DatePicker/RootContainer.tsx (1)
7-10: Consider using more explicit typing for the ref prop.While the current implementation works, using
RefObject<HTMLDivElement | null>could be more precise asRefObject<HTMLDivElement>since the ref object itself is never null, only its current value can be.interface RootContainerProps extends PropsWithChildren { - ref: RefObject<HTMLDivElement | null>; + ref: RefObject<HTMLDivElement>; className?: string; }frontend/src/hooks/useSharedCalendar.ts (1)
7-20: Consider timezone handling and performance optimizations.A few suggestions to improve the hook:
- Consider using a date library (like date-fns) for consistent timezone handling.
- Memoize the returned object to prevent unnecessary re-renders.
export const useSharedCalendar = (): CalendarSharedInfo => { const [selected, setSelected] = useState(new Date()); const handleSelectDate = useCallback((date: Date) => { setSelected(date); }, []); + return useMemo(() => ({ - return { selectedDate: selected, selectedWeek: formatDateToWeekDates(selected), handleSelectDate, today: new Date(), + }), [selected, handleSelectDate]); - }; };frontend/src/components/DatePicker/index.tsx (2)
2-2: Consider importing ReactNode from 'react' instead.The ReactNode type is available directly from 'react', no need to import it from @tanstack/react-router.
-import type { ReactNode } from '@tanstack/react-router'; +import type { ReactNode } from 'react';
17-19: Consider using English for error messages.For consistency and better international developer experience, error messages should be in English.
const DatePicker = () => { - throw new Error('!!! DatePicker.Select, DatePicker.Range를 사용해주세요 !!!'); + throw new Error('Please use DatePicker.Select or DatePicker.Range instead of DatePicker directly'); };frontend/src/components/DatePicker/context/DatePickerRangeProvider.tsx (1)
28-42: Consider adding error boundaries.While the implementation is clean, consider adding error boundaries to handle potential runtime errors gracefully.
+import { ErrorBoundary } from '@/components/ErrorBoundary'; return ( + <ErrorBoundary fallback={<div>Failed to load date picker</div>}> <DatePickerContext.Provider value={{ calendarType: 'range', onDateCellClick, isDateSelected, selectedDate: null, highlightRange, ...props, }} > {children} </DatePickerContext.Provider> + </ErrorBoundary> );frontend/src/components/DatePicker/Table/Highlight/index.css.ts (1)
8-9: Use CSS variables for dimensions.Using fixed pixel values makes the component less flexible. Consider using CSS variables from the theme for better maintainability and consistency.
- width: 32, - height: 32, + width: vars.spacing[800], // Adjust the spacing token as needed + height: vars.spacing[800], // Adjust the spacing token as neededAnd similarly for the gap style:
- height: 32, + height: vars.spacing[800], // Adjust the spacing token as neededAlso applies to: 33-33
frontend/src/components/DatePicker/DatePicker.stories.tsx (3)
15-20: Add documentation for story variants.The meta configuration should include descriptions for each story variant to help other developers understand their purpose and usage.
const meta = { title: 'Calendar/DatePicker', component: DatePicker.Select, + parameters: { + docs: { + description: { + component: 'A flexible date picker component that supports single date and range selection.', + }, + }, + }, } satisfies Meta<typeof DatePicker.Select>;
22-106: Add more story variants for error and disabled states.Consider adding examples that demonstrate:
- Error states (e.g., invalid date selection)
- Disabled states (e.g., disabled dates, disabled trigger)
- Loading states
- Different locales
103-103: Remove commented-out code.Clean up the commented-out Calendar component reference.
- {/* <Calendar {...props}/> */}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
frontend/src/components/Calendar/context/SharedCalendarContext.ts(1 hunks)frontend/src/components/DatePicker/DatePicker.stories.tsx(1 hunks)frontend/src/components/DatePicker/DatePickerRange.tsx(1 hunks)frontend/src/components/DatePicker/DatePickerSelect.tsx(1 hunks)frontend/src/components/DatePicker/Header/index.css.ts(1 hunks)frontend/src/components/DatePicker/RootContainer.tsx(1 hunks)frontend/src/components/DatePicker/Table/Cell/index.css.ts(1 hunks)frontend/src/components/DatePicker/Table/Highlight/index.css.ts(2 hunks)frontend/src/components/DatePicker/Table/TableBody.tsx(1 hunks)frontend/src/components/DatePicker/Table/index.css.ts(2 hunks)frontend/src/components/DatePicker/Table/index.tsx(1 hunks)frontend/src/components/DatePicker/context/DatePickerRangeProvider.tsx(1 hunks)frontend/src/components/DatePicker/context/DatePickerSelectProvider.tsx(1 hunks)frontend/src/components/DatePicker/datePicker.stories.css.ts(1 hunks)frontend/src/components/DatePicker/index.css.ts(1 hunks)frontend/src/components/DatePicker/index.tsx(1 hunks)frontend/src/hooks/useSharedCalendar.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/src/components/DatePicker/Table/index.tsx
🔇 Additional comments (13)
frontend/src/components/DatePicker/Table/TableBody.tsx (2)
2-2: LGTM! Import changes align with refactoring goals.The updated imports reflect better organization with the context in a dedicated directory and introduce necessary utilities for the refactored component.
Also applies to: 4-4, 6-6
10-11: Verify the null handling approach for baseDate.While providing a fallback to
new Date()is reasonable, consider whether this default behavior should be documented or handled more explicitly, as it could affect the parent component's state management.Consider:
- Adding a comment explaining the fallback behavior
- Validating baseDate earlier in the component tree
- Documenting this behavior in the component's interface
const { baseDate } = useSafeContext(DatePickerContext); + // Fallback to current date if baseDate is not provided const calendarDates = generateMonthCalendar(baseDate ?? new Date());frontend/src/components/DatePicker/index.css.ts (2)
20-24: Looks good!
TherootContainerStyleis clear and consistent with the theming approach, providing clarity on layout.
26-28: Simple & effective.
Positioning the trigger wrapper asrelativegives flexibility for nested absolutely positioned elements in the future. The style definition looks correct.frontend/src/components/DatePicker/datePicker.stories.css.ts (1)
8-11: Straightforward input styling.
This approach is clear. If you frequently adjust input dimensions, referencing them via theme variables could streamline updates and keep design aligned.frontend/src/components/Calendar/context/SharedCalendarContext.ts (1)
3-8: Interface is well-defined.
Including all relevant date properties (e.g.,selectedDate,selectedWeek) and associated handlers clarifies the intended usage.frontend/src/components/DatePicker/Header/index.css.ts (1)
3-7: LGTM! Good use of design system tokens.The change from hardcoded padding to theme-based spacing improves maintainability and ensures visual consistency across the application.
frontend/src/components/DatePicker/index.tsx (1)
9-15: LGTM! Good use of compound components pattern.The refactoring to use compound components (Select and Range) with shared common props improves the component's flexibility and maintainability.
Also applies to: 21-22
frontend/src/components/DatePicker/Table/index.css.ts (2)
12-18: LGTM! Improved responsive design.The change from fixed width to
100%enhances the component's flexibility and responsiveness.
20-25: LGTM! Enhanced layout control.Adding
height: '100%'improves layout control and consistency.frontend/src/components/DatePicker/context/DatePickerSelectProvider.tsx (1)
11-35: LGTM! Clean implementation of the provider component.The component effectively manages date selection state and provides it through context.
Let's verify the hook implementation:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Check if the hook is properly implemented ast-grep --pattern $'export function useDatePickerSelect($_) { $$$ }'Length of output: 72
I'll now run new scripts to search for the hook definition in alternative forms—both as a function declaration and as an arrow function—to ensure we properly verify its implementation.
Once we see the output from these commands, we can determine whether the hook is implemented as expected.
🏁 Script executed:
#!/bin/bash # Search for a function declaration of useDatePickerSelect rg -n "export function useDatePickerSelect" . echo "-----" # Search for an arrow function (const-based) declaration of useDatePickerSelect rg -n "export const useDatePickerSelect" .Length of output: 210
I'll now run additional scripts to verify that the hook returns both
highlightRangeandonDateCellClickas they are used in the provider component.
🏁 Script executed:
#!/bin/bash echo "Searching for 'highlightRange' in the hook implementation:" rg -n "highlightRange" frontend/src/hooks/useDatePicker/useDatePickerSelect.ts echo "-----" echo "Searching for 'onDateCellClick' in the hook implementation:" rg -n "onDateCellClick" frontend/src/hooks/useDatePicker/useDatePickerSelect.tsLength of output: 662
Verified: Provider and Hook Implementation Are Consistent
- The hook
useDatePickerSelectis implemented as an arrow function returning bothhighlightRangeandonDateCellClick, as confirmed by the searches.- The returned values are correctly deconstructed and passed into the
DatePickerContext.Providerin the provider component.frontend/src/components/DatePicker/context/DatePickerRangeProvider.tsx (1)
22-26: LGTM! Well-optimized date selection check.Good use of useCallback for memoization of the isDateSelected function.
frontend/src/components/DatePicker/Table/Cell/index.css.ts (1)
13-14: Verify unit handling for numeric values.The change from string ('24px') to number (24) might affect how the CSS is interpreted. Please verify that vanilla-extract properly handles numeric values for width and height.
✅ Verification successful
🌐 Web query:
How does vanilla-extract handle numeric values for width and height properties?💡 Result:
Vanilla Extract automatically converts numeric values for dimensional properties like
widthandheightto pixel units (px) by default[1][2]. This behavior applies to most CSS properties requiring units, while unitless properties likeflexGroworopacityaccept raw numbers directly[1].Key Handling Characteristics:
Automatic Pixel Conversion
Numeric values without explicit units get converted to pixels:const box = style({ width: 100, // → 100px height: 200 // → 200px });Unitless Property Support
Properties likelineHeight,zIndex, andflexGrowretain raw numbers:const flexItem = style({ flexGrow: 1, zIndex: 100 });Explicit Unit Overrides
Use string literals for non-pixel units:const container = style({ width: '50%', height: 'calc(100vh - 20px)' });Advanced Use Cases
- Sprinkles Utility handles responsive numeric values through predefined scales[4]:
const responsiveWidth = sprinkles({ width: { mobile: 320, desktop: 1024 } // Converts to px at each breakpoint });- CSS Variables enable dynamic values when combined with
assignInlineVars[7]:import { assignInlineVars } from '@vanilla-extract/dynamic'; const dynamicWidth = style({ vars: { '--custom-width': '50vw' }, width: 'var(--custom-width)' });For mixed unit requirements (e.g., viewport units), always use string literals to bypass automatic pixel conversion[1][7].
Citations:
- 1: https://vanilla-extract.style/documentation/styling/
- 2: https://www.dhiwise.com/post/the-future-of-web-development-vanilla-extract-css
- 3: https://github.com/vanilla-extract-css/vanilla-extract/blob/master/site/src/system/styles/sprinkles.css.ts
- 4: https://vanilla-extract.style/documentation/packages/sprinkles/
- 5: https://v7.mantine.dev/styles/vanilla-extract
- 6: https://www.sitepoint.com/data-types-in-sass/
- 7: vanilla-extract-css/vanilla-extract#1098
- 8: https://qwik.dev/docs/integrations/styled-vanilla-extract/
- 9: https://www.thisdot.co/blog/introduction-to-vanilla-extract-for-css
- 10: https://vanilla-extract.style/documentation/api/style/
Numeric value handling verified for vanilla-extract
Vanilla Extract automatically converts numeric values for properties like
widthandheighttopx. The change from a string ('24px') to a number (24) is correct and will be interpreted as24px.
| import type { DatePickerSelectProps } from '../DatePickerSelect'; | ||
| import { DatePickerContext } from './DatePickerContext'; | ||
|
|
||
| interface DatePickerRangeProviderProps extends PropsWithChildren, DatePickerSelectProps {} |
There was a problem hiding this comment.
Fix interface naming inconsistency.
The interface name DatePickerRangeProviderProps doesn't match the component's purpose (SelectProvider).
-interface DatePickerRangeProviderProps extends PropsWithChildren, DatePickerSelectProps {}
+interface DatePickerSelectProviderProps extends PropsWithChildren, DatePickerSelectProps {}📝 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.
| interface DatePickerRangeProviderProps extends PropsWithChildren, DatePickerSelectProps {} | |
| interface DatePickerSelectProviderProps extends PropsWithChildren, DatePickerSelectProps {} |
#️⃣ 연관된 이슈>
📝 작업 내용> 이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)
🙏 여기는 꼭 봐주세요! > 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요
Summary by CodeRabbit
New Features
DatePicker.SelectandDatePicker.Rangecomponents for enhanced date selection options.DatePickerRangecomponent for managing date ranges with improved context management.useDatePickerRangehook for handling date range selection logic.useDatePickerSelecthook for managing date selection within the date picker component.SharedCalendarContextfor managing shared calendar information across components.Bug Fixes
DateCellcomponent for better performance and clarity.Refactor
Documentation
DatePickercomponent to improve clarity and usability in demonstrations.