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

Select multiple non-consecutive dates #4537

Merged
merged 22 commits into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
() => {
Copy link

Choose a reason for hiding this comment

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

ISSUE: no-unused-expressions (Severity: Medium)
Expected an assignment or function call and instead saw an expression.

Remediation:
This may be a non-issue if there's an existing convention and bundler configuration that I'm unaware of, but should this not be exported?

🤖 powered by PullRequest Automation 👋 verified by Ryan Lester <@buu700>

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.

ISSUE: react/jsx-no-undef (Severity: Medium)
'DatePicker' is not defined.

Remediation:
Similar to my other comment, not sure if there's a missing import statement for this.

🤖 powered by PullRequest Automation 👋 verified by Ryan Lester <@buu700>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the same for all the example files, none of them import DatePicker

Copy link

Choose a reason for hiding this comment

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

👍

Image of Ryan Lester <@buu700> Ryan Lester <@buu700>

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 @@ -921,6 +923,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 @@ -197,6 +197,23 @@ export function safeDateRangeFormat(startDate, endDate, props) {
return `${formattedStartDate} - ${formattedEndDate}`;
}

export function safeMultipleDatesFormat(dates, props) {
Copy link

Choose a reason for hiding this comment

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

Seems like a lot of care was put into defensive input handling here.

◽ Compliment

Image of Ryan Lester <@buu700> Ryan Lester <@buu700>

Copy link

Choose a reason for hiding this comment

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

Agreed - is !dates?.length a valid input state? Is "" a value that makes sense for the output of safeMultipleDatesFormat?

Image of Jacques Jacques

Copy link
Contributor Author

@tbowmo tbowmo Feb 28, 2024

Choose a reason for hiding this comment

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

If length is zero, it should return an empty string. I haven't looked that deep into the underlying code to see if null / undefined is a valid input. But I'm used to do the ?.length test in our own code, so seemed natural for me to do that.

Now that I think of it, it could probably skip a couple of if branches here, and reduce cyclic complexity a bit, by using an array join, like this

export function safeMultipleDatesFormat(dates, props) {
  if (dates?.length > 2) {
    return `${safeDateFormat(dates[0], props)} (+${dates.length - 1})`;
  }

  return (dates || []).map((date) => safeDateFormat(date, props)).join(', ')
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also seems that the rest of the safeDateFormat methods is taking care of possible null/undefined inputs

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
33 changes: 23 additions & 10 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,17 @@ 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 = () => {
if (this.props.disabledKeyboardNavigation) {
return false;
}

const isSelectedDate = this.props.selectsMultiple
Copy link

Choose a reason for hiding this comment

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

Good - you can maybe even further break this ball of hair down to one or two more if statements.

🔹 Improve Readability (Nice to have)

Image of Jacques Jacques

? this.props.selectedDates?.some((date) => this.isSameDayOrWeek(date))
: this.isSameDayOrWeek(this.props.selected);

return !isSelectedDate && this.isSameDayOrWeek(this.props.preSelection);
};

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

Expand All @@ -127,6 +132,8 @@ export default class Day extends React.Component {
),
);

isSameDayOrWeek = (other) => this.isSameDay(other) || this.isSameWeek(other);

getHighLightedClass = () => {
const { day, highlightDates } = this.props;

Expand Down Expand Up @@ -276,8 +283,14 @@ export default class Day extends React.Component {

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

isSelected = () =>
this.isSameDay(this.props.selected) || this.isSameWeek(this.props.selected);
isSelected = () => {
if (this.props.selectsMultiple) {
return this.props.selectedDates?.some((date) =>
this.isSameDayOrWeek(date),
);
}
return this.isSameDayOrWeek(this.props.selected);
};

getClassNames = (date) => {
const dayClassName = this.props.dayClassName
Expand Down
45 changes: 41 additions & 4 deletions src/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import {
isSameDay,
isMonthDisabled,
isYearDisabled,
safeMultipleDatesFormat,
getHolidaysMap,
isDateBefore,
} from "./date_utils";
Expand Down Expand Up @@ -232,6 +233,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 @@ -623,12 +626,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) {
Copy link

Choose a reason for hiding this comment

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

This would be a pre-existing issue, but is there any scenario where changedDate might be undefined? If so, consider changing this to != null (as used in code further below).

🔹 Bug (Nice to have)

Image of Ryan Lester <@buu700> Ryan Lester <@buu700>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure about this one? haven't touched this part myself..

Copy link

Choose a reason for hiding this comment

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

Ah okay, sounds good, just something that caught my attention while I was working backwards from your code to see what changedDate was. Seems out of scope of this PR either way, but may be worth someone taking a look at separately.

Image of Ryan Lester <@buu700> Ryan Lester <@buu700>

if (
Expand Down Expand Up @@ -669,6 +680,24 @@ export default class DatePicker extends React.Component {
if (isRangeFilled) {
onChange([changedDate, null], event);
}
} else if (selectsMultiple) {
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 @@ -1018,6 +1047,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 @@ -1201,7 +1232,9 @@ export default class DatePicker extends React.Component {
this.props.endDate,
this.props,
)
: safeDateFormat(this.props.selected, this.props);
: this.props.selectsMultiple
? safeMultipleDatesFormat(this.props.selectedDates, this.props)
: safeDateFormat(this.props.selected, this.props);

return React.cloneElement(customInput, {
[customInputRef]: (input) => {
Expand Down Expand Up @@ -1242,10 +1275,14 @@ export default class DatePicker extends React.Component {
clearButtonTitle,
clearButtonClassName = "",
ariaLabelClose = "Close",
selectedDates,
} = this.props;
if (
isClearable &&
(selected != null || startDate != null || endDate != null)
(selected != null ||
startDate != null ||
endDate != null ||
selectedDates?.length)
) {
return (
<button
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 @@ -331,6 +333,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
91 changes: 91 additions & 0 deletions test/datepicker_test.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1851,6 +1851,97 @@ describe("DatePicker", () => {
expect(datePicker.state.open).toBe(true);
});

describe("multiSelecting non-consecutive dates", () => {
const testDate = new Date("2024-02-02 00:00:00");

beforeAll(() => {
jest.useFakeTimers();
jest.setSystemTime(testDate);
});

afterAll(() => {
jest.useRealTimers();
});

it("should return array of dates to onChange callback when day is selected in multiSelect mode", () => {
const onChange = jest.fn();

var datePicker = TestUtils.renderIntoDocument(
<DatePicker
shouldCloseOnSelect={false}
selectsMultiple
onChange={onChange}
/>,
);
var dateInput = datePicker.input;
var node = findDOMNode(dateInput);
TestUtils.Simulate.focus(node);
var days = TestUtils.scryRenderedComponentsWithType(
datePicker.calendar,
Day,
);

// Might seem odd, but the first couple of days of the calendar is in january, so days[5] is february 2nd
TestUtils.Simulate.click(findDOMNode(days[5]));
expect(onChange).toHaveBeenCalled();
expect(onChange).toHaveBeenCalledWith([testDate], expect.anything());
});

it("should remove previously selected date from array if date is selected again", () => {
const onChange = jest.fn();
const anotherDate = new Date("2024-01-01");
var datePicker = TestUtils.renderIntoDocument(
<DatePicker
shouldCloseOnSelect={false}
selectsMultiple
selectedDates={[anotherDate, testDate]}
onChange={onChange}
/>,
);
var dateInput = datePicker.input;
var node = findDOMNode(dateInput);
TestUtils.Simulate.focus(node);
var days = TestUtils.scryRenderedComponentsWithType(
datePicker.calendar,
Day,
);

// Might seem odd, but the first couple of days of the calendar is in january, so days[5] is february 2nd
TestUtils.Simulate.click(findDOMNode(days[5]));
expect(onChange).toHaveBeenCalled();
expect(onChange).toHaveBeenCalledWith([anotherDate], expect.anything());
});

it("should add newly selected date to array of selected dates", () => {
const onChange = jest.fn();
const previouslyAddedDate = new Date("2024-01-01");

var datePicker = TestUtils.renderIntoDocument(
<DatePicker
shouldCloseOnSelect={false}
selectsMultiple
selectedDates={[previouslyAddedDate]}
onChange={onChange}
/>,
);
var dateInput = datePicker.input;
var node = findDOMNode(dateInput);
TestUtils.Simulate.focus(node);
var days = TestUtils.scryRenderedComponentsWithType(
datePicker.calendar,
Day,
);

// Might seem odd, but the first couple of days of the calendar is in january, so days[5] is february 2nd
TestUtils.Simulate.click(findDOMNode(days[5]));
expect(onChange).toHaveBeenCalled();
expect(onChange).toHaveBeenCalledWith(
[previouslyAddedDate, testDate],
expect.anything(),
);
});
});

describe("selectsRange with inline", () => {
it("should change dates of range when dates are empty", () => {
const selected = utils.newDate();
Expand Down
Loading
Loading