Skip to content
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

Support selecting multiple dates #3999

Merged
merged 11 commits into from
Feb 28, 2024
5 changes: 5 additions & 0 deletions docs-site/src/components/Examples/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ import selectsRangeWithDisabledDates from "../../examples/selectsRangeWithDisabl
import CalendarStartDay from "../../examples/calendarStartDay";
import ExternalForm from "../../examples/externalForm";
import CalendarIcon from "../../examples/calendarIcon";
import SelectsMultiple from "../../examples/selectsMultiple";
import CalendarIconExternal from "../../examples/calendarIconExternal";
import CalendarIconSvgIcon from "../../examples/calendarIconSvgIcon";
import ToggleCalendarOnIconClick from "../../examples/toggleCalendarOnIconClick";
Expand Down Expand Up @@ -484,6 +485,10 @@ export default class exampleComponents extends React.Component {
title: "Specific Time Range",
component: ExcludeTimePeriod,
},
{
title: "Select multiple dates",
component: SelectsMultiple,
},
{
title: "Strict parsing",
component: StrictParsing,
Expand Down
15 changes: 15 additions & 0 deletions docs-site/src/examples/selectsMultiple.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
() => {
const [selectedDates, setSelectedDates] = useState([new Date()]);
const onChange = (dates) => {
setSelectedDates(dates);
};
return (
<DatePicker
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the example show where to import this from?

🔸 Reduce Future Bugs (Important)

Image of Graham C Graham C

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The examples added here are automatically published to https://reactdatepicker.com/

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but how does someone who what line to write in the code to get to the DatePicker component. I am suggesting we add an import line at the top of this example. I guess this is an existing problem with all the examples, so probably out of scope to change.

Image of Graham C Graham C

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally I would, but I just copied what the other examples did.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we can start by leading with example here and add the import to show how to get it done.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me.

Image of Graham C Graham C

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That will fix the automation complaining about DatePicker not being defined...

Image of Andy W Andy W

selectedDates={selectedDates}
selectsMultiple
onChange={onChange}
shouldCloseOnSelect={false}
disabledKeyboardNavigation
/>
);
};
4 changes: 4 additions & 0 deletions src/calendar.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ export default class Calendar extends React.Component {
selectsStart: PropTypes.bool,
selectsRange: PropTypes.bool,
selectsDisabledDaysInRange: PropTypes.bool,
selectsMultiple: PropTypes.bool,
selectedDates: PropTypes.arrayOf(PropTypes.instanceOf(Date)),
showMonthDropdown: PropTypes.bool,
showPreviousMonths: PropTypes.bool,
showMonthYearDropdown: PropTypes.bool,
Expand Down Expand Up @@ -920,6 +922,8 @@ export default class Calendar extends React.Component {
selectsEnd={this.props.selectsEnd}
selectsRange={this.props.selectsRange}
selectsDisabledDaysInRange={this.props.selectsDisabledDaysInRange}
selectsMultiple={this.props.selectsMultiple}
selectedDates={this.props.selectedDates}
showWeekNumbers={this.props.showWeekNumbers}
startDate={this.props.startDate}
endDate={this.props.endDate}
Expand Down
17 changes: 17 additions & 0 deletions src/date_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,23 @@ export function safeDateRangeFormat(startDate, endDate, props) {
return `${formattedStartDate} - ${formattedEndDate}`;
}

export function safeMultipleDatesFormat(dates, props) {
if (!dates || !dates.length) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (!dates || !dates.length) {
if (!dates?.length) {

return "";
}
const formattedFirstDate = safeDateFormat(dates[0], props);
if (dates.length === 1) {
return formattedFirstDate;
}
if (dates.length === 2) {
const formattedSecondDate = safeDateFormat(dates[1], props);
return `${formattedFirstDate}, ${formattedSecondDate}`;
}

const extraDatesCount = dates.length - 1;
return `${formattedFirstDate} (+${extraDatesCount})`;
}

// ** Date Setters **

export function setTime(date, { hour = 0, minute = 0, second = 0 }) {
Expand Down
39 changes: 28 additions & 11 deletions src/day.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ export default class Day extends React.Component {
showWeekPicker: PropTypes.bool,
showWeekNumber: PropTypes.bool,
selectsDisabledDaysInRange: PropTypes.bool,
selectsMultiple: PropTypes.bool,
selectedDates: PropTypes.arrayOf(PropTypes.instanceOf(Date)),
startDate: PropTypes.instanceOf(Date),
renderDayContents: PropTypes.func,
handleOnKeyDown: PropTypes.func,
Expand Down Expand Up @@ -93,14 +95,25 @@ export default class Day extends React.Component {

isSameDay = (other) => isSameDay(this.props.day, other);

isKeyboardSelected = () =>
!this.props.disabledKeyboardNavigation &&
!(
this.isSameDay(this.props.selected) ||
this.isSameWeek(this.props.selected)
) &&
(this.isSameDay(this.props.preSelection) ||
this.isSameWeek(this.props.preSelection));
isKeyboardSelected = () => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO it would be good to leave a brief comment in the code here describing the intended condition. I think it's pretty hard to parse out why this code is as it is (at least without substantially more context).

🔸 Improve Readability (Important)

Image of Eric E Eric E

if (this.props.disabledKeyboardNavigation) {
return false;
}
if (this.props.selectsMultiple) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @nandorojo 👋
Was digging into this PR to investigate "select multiple months" and I think we need logic similar to this for the months and weeks selection, otherwise there's no selection styles there.
Happy to help with it if I can

const isSelectedDate = (this.props.selectedDates || []).some((date) =>
this.isSameDay(date)
);
return !isSelectedDate && this.isSameDay(this.props.preSelection);
} else {
return !(
this.isSameDay(this.props.selected) ||
this.isSameWeek(this.props.selected)
) && (
this.isSameDay(this.props.preSelection) ||
this.isSameWeek(this.props.preSelection)
);
}
Comment on lines +107 to +115
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the else clause should not be necessary, as the if above ends with a return, so we can skip an indentation here

Suggested change
} else {
return !(
this.isSameDay(this.props.selected) ||
this.isSameWeek(this.props.selected)
) && (
this.isSameDay(this.props.preSelection) ||
this.isSameWeek(this.props.preSelection)
);
}
}
return !(
this.isSameDay(this.props.selected) ||
this.isSameWeek(this.props.selected)
) && (
this.isSameDay(this.props.preSelection) ||
this.isSameWeek(this.props.preSelection)
);

};

isDisabled = () => isDayDisabled(this.props.day, this.props);

Expand Down Expand Up @@ -275,9 +288,13 @@ export default class Day extends React.Component {
};

isCurrentDay = () => this.isSameDay(newDate());

isSelected = () =>
this.isSameDay(this.props.selected) || this.isSameWeek(this.props.selected);

isSelected = () => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the kind of logic update where it would be great to have unit tests. are there any unit tests?

🔸 Improve Test Coverage (Important)

Image of Andy W Andy W

if (this.props.selectsMultiple && this.props.selectedDates) {
return this.props.selectedDates.some((date) => this.isSameDay(date));
}
return this.isSameDay(this.props.selected) || this.isSameWeek(this.props.selected);
};
Comment on lines +292 to +297
Copy link
Contributor

@tbowmo tbowmo Feb 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks sketchy, as it will fall through to checking on props.selected, if this.props.selectsMultiple is true, and this.props.selectedDates is undefined.

Perhaps something in the line of this instead (I added test on this.isSameWeek() as well, haven't dived enough into the code to see if that actually is necessary?

Suggested change
isSelected = () => {
if (this.props.selectsMultiple && this.props.selectedDates) {
return this.props.selectedDates.some((date) => this.isSameDay(date));
}
return this.isSameDay(this.props.selected) || this.isSameWeek(this.props.selected);
};
isSelected = () => {
if (this.props.selectsMultiple) {
return this.props.selectedDates?.some((date) => this.isSameDay(date) || this.isSameWeek(date)) ?? false;
}
return this.isSameDay(this.props.selected) || this.isSameWeek(this.props.selected);
};


getClassNames = (date) => {
const dayClassName = this.props.dayClassName
Expand Down
39 changes: 37 additions & 2 deletions src/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import {
isSameDay,
isMonthDisabled,
isYearDisabled,
safeMultipleDatesFormat,
getHolidaysMap,
isDateBefore,
} from "./date_utils";
Expand Down Expand Up @@ -230,6 +231,8 @@ export default class DatePicker extends React.Component {
selectsStart: PropTypes.bool,
selectsRange: PropTypes.bool,
selectsDisabledDaysInRange: PropTypes.bool,
selectsMultiple: PropTypes.bool,
selectedDates: PropTypes.arrayOf(PropTypes.instanceOf(Date)),
showMonthDropdown: PropTypes.bool,
showPreviousMonths: PropTypes.bool,
showMonthYearDropdown: PropTypes.bool,
Expand Down Expand Up @@ -621,12 +624,20 @@ export default class DatePicker extends React.Component {
}
}

const { onChange, selectsRange, startDate, endDate } = this.props;
const {
onChange,
selectsRange,
startDate,
endDate,
selectsMultiple,
selectedDates,
} = this.props;

if (
!isEqual(this.props.selected, changedDate) ||
this.props.allowSameDay ||
selectsRange
selectsRange ||
selectsMultiple
) {
if (changedDate !== null) {
if (
Expand Down Expand Up @@ -667,6 +678,26 @@ export default class DatePicker extends React.Component {
if (isRangeFilled) {
onChange([changedDate, null], event);
}
} else if (selectsMultiple) {
const noSelectedDates = !selectedDates || selectedDates.length === 0;

if (noSelectedDates) {
Comment on lines +682 to +684
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tend to prefer these type of checks here

Suggested change
const noSelectedDates = !selectedDates || selectedDates.length === 0;
if (noSelectedDates) {
if (!selectedDates?.length) {

onChange([changedDate], event);
} else {
const isChangedDateAlreadySelected = selectedDates.some(
(selectedDate) => isSameDay(selectedDate, changedDate)
);

if (isChangedDateAlreadySelected) {
const nextDates = selectedDates.filter(
(selectedDate) => !isSameDay(selectedDate, changedDate)
);

onChange(nextDates, event);
} else {
onChange([...selectedDates, changedDate], event);
}
}
} else {
onChange(changedDate, event);
}
Expand Down Expand Up @@ -1011,6 +1042,8 @@ export default class DatePicker extends React.Component {
selectsStart={this.props.selectsStart}
selectsEnd={this.props.selectsEnd}
selectsRange={this.props.selectsRange}
selectsMultiple={this.props.selectsMultiple}
selectedDates={this.props.selectedDates}
startDate={this.props.startDate}
endDate={this.props.endDate}
excludeDates={this.props.excludeDates}
Expand Down Expand Up @@ -1194,6 +1227,8 @@ export default class DatePicker extends React.Component {
this.props.endDate,
this.props,
)
: this.props.selectsMultiple
? safeMultipleDatesFormat(this.props.selectedDates, this.props)
: safeDateFormat(this.props.selected, this.props);

return React.cloneElement(customInput, {
Expand Down
4 changes: 4 additions & 0 deletions src/month.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ export default class Month extends React.Component {
selectsStart: PropTypes.bool,
selectsRange: PropTypes.bool,
selectsDisabledDaysInRange: PropTypes.bool,
selectsMultiple: PropTypes.bool,
selectedDates: PropTypes.arrayOf(PropTypes.instanceOf(Date)),
showWeekNumbers: PropTypes.bool,
startDate: PropTypes.instanceOf(Date),
setOpen: PropTypes.func,
Expand Down Expand Up @@ -330,6 +332,8 @@ export default class Month extends React.Component {
selectsEnd={this.props.selectsEnd}
selectsRange={this.props.selectsRange}
selectsDisabledDaysInRange={this.props.selectsDisabledDaysInRange}
selectsMultiple={this.props.selectsMultiple}
selectedDates={this.props.selectedDates}
showWeekNumber={this.props.showWeekNumbers}
showWeekPicker={this.props.showWeekPicker}
startDate={this.props.startDate}
Expand Down
4 changes: 4 additions & 0 deletions src/week.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ export default class Week extends React.Component {
selectsEnd: PropTypes.bool,
selectsStart: PropTypes.bool,
selectsRange: PropTypes.bool,
selectsMultiple: PropTypes.bool,
selectedDates: PropTypes.arrayOf(PropTypes.instanceOf(Date)),
selectsDisabledDaysInRange: PropTypes.bool,
showWeekNumber: PropTypes.bool,
showWeekPicker: PropTypes.bool,
Expand Down Expand Up @@ -166,6 +168,8 @@ export default class Week extends React.Component {
showWeekPicker={this.props.showWeekPicker}
showWeekNumber={this.props.showWeekNumber}
selectsDisabledDaysInRange={this.props.selectsDisabledDaysInRange}
selectsMultiple={this.props.selectsMultiple}
selectedDates={this.props.selectedDates}
startDate={this.props.startDate}
endDate={this.props.endDate}
dayClassName={this.props.dayClassName}
Expand Down
36 changes: 36 additions & 0 deletions test/multiple_selected_dates.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import React from "react";
import DatePicker from "../src";
import { render } from "@testing-library/react";

describe("Multiple Dates Selected", function () {
function getDatePicker(extraProps) {
return render(
<DatePicker
selectsMultiple
onChange={() => {}}
shouldCloseOnSelect={false}
disabledKeyboardNavigation
{...extraProps}
/>,
);
}

it("should handle text format for two selected dates", () => {
const datePicker = getDatePicker({
selectsMultiple: true,
selectedDates: [new Date("2024/01/01"), new Date("2024/01/15")],
});

expect(datePicker.getByRole("textbox").value).toBe("01/01/2024, 01/15/2024");
});


it("should handle text format for more than two selected dates", () => {
const datePicker = getDatePicker({
selectsMultiple: true,
selectedDates: [new Date("2024/01/01"), new Date("2024/01/15"), new Date("2024/03/15"), ],
});

expect(datePicker.getByRole('textbox').value).toBe("01/01/2024 (+2)");
});
martijnrusschen marked this conversation as resolved.
Show resolved Hide resolved
});