-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(date-selector): Refactor DateSelector component #313
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,112 @@ | ||
import { useRef } from 'react'; | ||
import dayjs from 'dayjs'; | ||
import DayPicker from 'react-day-picker'; | ||
|
||
import { useOnClickOutside } from '../../../hooks/useOnClickOutside'; | ||
import { CalendarIcon } from '../../icon/icons'; | ||
|
||
import styles from './style.module.scss'; | ||
import './datepicker.scss'; | ||
import { Button } from '../../button'; | ||
|
||
export interface CalendarProps { | ||
dateFormat: string; | ||
value?: string; | ||
onChange: (date: string) => void; | ||
yearBoundaries: { min: number; max: number }; | ||
displayCalendar?: boolean; | ||
dayjsLocale?: ILocale; | ||
firstDayOfWeek?: number; | ||
isOpen?: boolean; | ||
setCalendarOpen: (isOpen: boolean) => void; | ||
} | ||
|
||
export const Calendar = ({ | ||
dateFormat, | ||
value, | ||
onChange, | ||
yearBoundaries, | ||
displayCalendar, | ||
dayjsLocale, | ||
firstDayOfWeek, | ||
setCalendarOpen, | ||
isOpen | ||
}: CalendarProps) => { | ||
const localeDate = dayjsLocale | ||
? dayjs().locale(dayjsLocale).localeData() | ||
: dayjs().locale('en').localeData(); | ||
|
||
const localizedWeekdays = localeDate.weekdays(); | ||
const localizedWeekdaysShort = localeDate.weekdaysShort(); | ||
const localizedMonths = localeDate.months(); | ||
|
||
const calendarContainerRef = useRef<HTMLDivElement | null>(null); | ||
|
||
const calendarDefaultDate = | ||
dayjs().year() >= yearBoundaries.min && dayjs().year() <= yearBoundaries.max | ||
? dayjs().toDate() | ||
: dayjs().set('year', yearBoundaries.max).toDate(); | ||
|
||
const selectedDateInDateType = value | ||
? dayjs(value).toDate() | ||
: calendarDefaultDate; | ||
const dateCalendarFromMonth = dayjs(String(yearBoundaries.min)) | ||
.startOf('year') | ||
.toDate(); | ||
const dateCalendarToMonth = dayjs(String(yearBoundaries.max)) | ||
.endOf('year') | ||
.toDate(); | ||
|
||
useOnClickOutside(calendarContainerRef, () => setCalendarOpen(false)); | ||
|
||
if (!displayCalendar) { | ||
return null; | ||
} | ||
|
||
return ( | ||
<div | ||
className={`${styles.container} ml8`} | ||
ref={calendarContainerRef} | ||
> | ||
<Button | ||
onClick={() => setCalendarOpen(!isOpen)} | ||
data-testid="calendar-button" | ||
hideLabel | ||
variant='textColor' | ||
leftIcon={<CalendarIcon />} | ||
> | ||
Select date | ||
</Button> | ||
|
||
{isOpen && ( | ||
<DayPicker | ||
month={selectedDateInDateType} | ||
showOutsideDays={true} | ||
fromMonth={dateCalendarFromMonth} | ||
toMonth={dateCalendarToMonth} | ||
selectedDays={selectedDateInDateType} | ||
onDayClick={(date: Date) => { | ||
if ( | ||
dayjs(date).isAfter(dateCalendarFromMonth) || | ||
dayjs(date).isBefore(dateCalendarToMonth) | ||
) { | ||
const selectedDate = dayjs(date).format(dateFormat); | ||
onChange(selectedDate); | ||
setCalendarOpen(false); | ||
} | ||
}} | ||
pagedNavigation={true} | ||
disabledDays={{ | ||
before: dateCalendarFromMonth, | ||
after: dateCalendarToMonth, | ||
}} | ||
firstDayOfWeek={firstDayOfWeek} | ||
locale={dayjsLocale?.name || 'en'} | ||
months={localizedMonths} | ||
weekdaysLong={localizedWeekdays} | ||
weekdaysShort={localizedWeekdaysShort} | ||
/> | ||
)} | ||
</div> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
.container { | ||
position: relative; | ||
} |
This file was deleted.
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,7 @@ import { render } from '../../util/testUtils'; | |
|
||
import { DateSelector } from '.'; | ||
|
||
const setup = (date: string, onChange: (date: string) => void = () => {}) => { | ||
const setup = (date?: string, onChange: (date: string) => void = () => {}) => { | ||
return render( | ||
<DateSelector | ||
value={date} | ||
|
@@ -14,40 +14,138 @@ const setup = (date: string, onChange: (date: string) => void = () => {}) => { | |
}; | ||
|
||
describe('DateSelector component', () => { | ||
it('should return the selected date when clicking on the calendar', async () => { | ||
it('should show the value inside the inputs', () => { | ||
const date = '2024-01-01'; | ||
const { getByTestId } = setup(date); | ||
|
||
expect(getByTestId('date-selector-day')).toHaveValue('1'); | ||
expect(getByTestId('date-selector-month')).toHaveValue('1'); | ||
expect(getByTestId('date-selector-year')).toHaveValue('2024'); | ||
}); | ||
|
||
it('should call onChange with the value changed', async () => { | ||
const callback = jest.fn(); | ||
const { getByTestId, user } = setup(undefined, callback); | ||
|
||
await user.type(getByTestId('date-selector-day'), '5'); | ||
await user.type(getByTestId('date-selector-month'), '7'); | ||
await user.type(getByTestId('date-selector-year'), '2023'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To make sure we are not impacting accessibility, IMHO it'd be better if we stick to the actual input labels, and not rely on Is there a reason to use |
||
|
||
expect(getByTestId('date-selector-day')).toHaveValue('5'); | ||
expect(getByTestId('date-selector-month')).toHaveValue('7'); | ||
expect(getByTestId('date-selector-year')).toHaveValue('2023'); | ||
expect(callback).toHaveBeenCalledWith('2023-07-05'); | ||
}); | ||
|
||
it('should call onChange with the value changed with initial value', async () => { | ||
const callback = jest.fn(); | ||
const date = '2024-01-01'; | ||
const { getByTestId, user } = setup(date, callback); | ||
|
||
await user.type(getByTestId('date-selector-day'), '{backspace}3'); | ||
await user.type(getByTestId('date-selector-month'), '{backspace}7'); | ||
await user.type(getByTestId('date-selector-year'), '{backspace}3'); | ||
|
||
expect(getByTestId('date-selector-day')).toHaveValue('3'); | ||
expect(getByTestId('date-selector-month')).toHaveValue('7'); | ||
expect(getByTestId('date-selector-year')).toHaveValue('2023'); | ||
expect(callback).toHaveBeenCalledWith('2023-07-03'); | ||
}); | ||
|
||
it('should call onChange empty when invalid date', async () => { | ||
const callback = jest.fn(); | ||
const { getByTestId, user } = setup(undefined, callback); | ||
|
||
await user.type(getByTestId('date-selector-day'), '5'); | ||
|
||
expect(getByTestId('date-selector-day')).toHaveValue('5'); | ||
expect(callback).toHaveBeenCalledWith(''); | ||
}); | ||
|
||
it('should call onChange empty when year out of boundaries', async () => { | ||
const callback = jest.fn(); | ||
const date = '2024-01-01'; | ||
const expectedDate = '2024-01-17'; | ||
const { getByTestId, getByLabelText, user } = setup(date, callback); | ||
const button = getByTestId('calendar-button'); | ||
const { getByTestId, user } = setup(date, callback); | ||
|
||
await user.type(getByTestId('date-selector-year'), '{backspace}{backspace}30'); | ||
|
||
expect(getByTestId('date-selector-year')).toHaveValue('2030'); | ||
expect(callback).toHaveBeenCalledWith(''); | ||
}); | ||
|
||
it('should call onChange when year in boundaries after being out of boundaries', async () => { | ||
const callback = jest.fn(); | ||
const date = '2030-01-01'; | ||
const { getByTestId, user } = setup(date, callback); | ||
|
||
await user.click(button); | ||
await user.type(getByTestId('date-selector-year'), '{backspace}{backspace}23'); | ||
|
||
const calendarCell = getByLabelText(/17 2024/); | ||
expect(getByTestId('date-selector-year')).toHaveValue('2023'); | ||
expect(callback).toHaveBeenCalledWith('2023-01-01'); | ||
}); | ||
|
||
await user.click(calendarCell); | ||
it('should call onChange with the value changed', async () => { | ||
const callback = jest.fn(); | ||
const date = '2024-01-01'; | ||
const { getByTestId, user } = setup(date, callback); | ||
|
||
expect(callback).toHaveBeenCalledWith(expectedDate); | ||
await user.type(getByTestId('date-selector-day'), '{backspace}3'); | ||
await user.type(getByTestId('date-selector-month'), '{backspace}7'); | ||
await user.type(getByTestId('date-selector-year'), '{backspace}3'); | ||
|
||
expect(calendarCell).not.toBeVisible(); | ||
expect(getByTestId('date-selector-day')).toHaveValue('3'); | ||
expect(getByTestId('date-selector-month')).toHaveValue('7'); | ||
expect(callback).toHaveBeenCalledWith('2023-07-03'); | ||
}); | ||
|
||
it('should close the calendar when clicking outside', async () => { | ||
it('should navigate inputs from day to month when day is over 3', async () => { | ||
const callback = jest.fn(); | ||
const date = '2024-01-01'; | ||
const { getByTestId, getByLabelText, user } = setup(date, callback); | ||
const button = getByTestId('calendar-button'); | ||
const { getByTestId, user } = setup(date, callback); | ||
|
||
await user.type(getByTestId('date-selector-day'), '{backspace}45'); | ||
|
||
expect(getByTestId('date-selector-day')).toHaveValue('4'); | ||
expect(getByTestId('date-selector-month')).toHaveValue('5'); | ||
expect(callback).toHaveBeenCalledWith('2024-05-04'); | ||
}); | ||
|
||
describe('Calendar button', () => { | ||
it('should return the selected date when clicking on the calendar', async () => { | ||
const callback = jest.fn(); | ||
const date = '2024-01-01'; | ||
const expectedDate = '2024-01-17'; | ||
const { getByTestId, getByLabelText, user } = setup(date, callback); | ||
const button = getByTestId('calendar-button'); | ||
|
||
await user.click(button); | ||
|
||
const calendarCell = getByLabelText(/17 2024/); | ||
|
||
await user.click(calendarCell); | ||
|
||
expect(callback).toHaveBeenCalledWith(expectedDate); | ||
|
||
expect(calendarCell).not.toBeVisible(); | ||
}); | ||
|
||
it('should close the calendar when clicking outside', async () => { | ||
const callback = jest.fn(); | ||
const date = '2024-01-01'; | ||
const { getByTestId, getByLabelText, user } = setup(date, callback); | ||
const button = getByTestId('calendar-button'); | ||
|
||
await user.click(button); | ||
await user.click(button); | ||
|
||
const calendarCell = getByLabelText(/17 2024/); | ||
expect(calendarCell).toBeVisible(); | ||
const calendarCell = getByLabelText(/17 2024/); | ||
expect(calendarCell).toBeVisible(); | ||
|
||
// click outside the calendar | ||
await user.click(document.body); | ||
// click outside the calendar | ||
await user.click(document.body); | ||
|
||
expect(callback).not.toHaveBeenCalled(); | ||
expect(callback).not.toHaveBeenCalled(); | ||
|
||
expect(calendarCell).not.toBeVisible(); | ||
expect(calendarCell).not.toBeVisible(); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like we are not checking if
dateFormat
is a valid date until the component performs an update.Do you think we could fail earlier?
E.g. throw and exception during development if the
dateFormat
is incorrect.