Merge pull request #4244 from lemming/timepicker-dst-fix

fix: timepicker on 23 and 25-hour days

src/date_utils.js

@@ -818,3 +818,47 @@ export function getYearsPeriod(
   const startPeriod = endPeriod - (yearItemNumber - 1);
   return { startPeriod, endPeriod };
 }
+
+export function getHoursInDay(d) {
+  const startOfDay = new Date(d.getFullYear(), d.getMonth(), d.getDate());
+  const startOfTheNextDay = new Date(
+    d.getFullYear(),
+    d.getMonth(),
+    d.getDate(),
+    24
+  );
+
+  return Math.round((+startOfTheNextDay - +startOfDay) / 3_600_000);
+}
+
+/**
+ * Returns the start of the minute for the given date
+ *
+ * NOTE: this function is a DST and timezone-safe analog of `date-fns/startOfMinute`
+ * do not make changes unless you know what you're doing
+ *
+ * See comments on https://github.com/Hacker0x01/react-datepicker/pull/4244
+ * for more details
+ *
+ * @param {Date} d date
+ * @returns {Date} start of the minute
+ */
+export function startOfMinute(d) {
+  const seconds = d.getSeconds();
+  const milliseconds = d.getMilliseconds();
+
+  return toDate(d.getTime() - seconds * 1000 - milliseconds);
+}
+
+/**
+ * Returns whether the given dates are in the same minute
+ *
+ * This function is a DST and timezone-safe analog of `date-fns/isSameMinute`
+ *
+ * @param {Date} d1
+ * @param {Date} d2
+ * @returns {boolean}
+ */
+export function isSameMinute(d1, d2) {
+  return startOfMinute(d1).getTime() === startOfMinute(d2).getTime();
+}

src/index.jsx

@@ -686,10 +686,12 @@ export default class DatePicker extends React.Component {
     const selected = this.props.selected
       ? this.props.selected
       : this.getPreSelection();
-    let changedDate = setTime(selected, {
-      hour: getHours(time),
-      minute: getMinutes(time),
-    });
+    let changedDate = this.props.selected
+      ? time
+      : setTime(selected, {
+          hour: getHours(time),
+          minute: getMinutes(time),
+        });
 
     this.setState({
       preSelection: changedDate,

src/time.jsx

@@ -3,17 +3,15 @@ import PropTypes from "prop-types";
 import {
   getHours,
   getMinutes,
-  setHours,
-  setMinutes,
   newDate,
   getStartOfDay,
   addMinutes,
   formatDate,
-  isBefore,
-  isEqual,
   isTimeInDisabledRange,
   isTimeDisabled,
   timesToInjectAfter,
+  getHoursInDay,
+  isSameMinute,
 } from "./date_utils";
 
 export default class Time extends React.Component {
@@ -91,20 +89,16 @@ export default class Time extends React.Component {
     this.props.onChange(time);
   };
 
-  isSelectedTime = (time, currH, currM) =>
-    this.props.selected &&
-    currH === getHours(time) &&
-    currM === getMinutes(time);
+  isSelectedTime = (time) =>
+    this.props.selected && isSameMinute(this.props.selected, time);
 
-  liClasses = (time, currH, currM) => {
+  liClasses = (time) => {
     let classes = [
       "react-datepicker__time-list-item",
-      this.props.timeClassName
-        ? this.props.timeClassName(time, currH, currM)
-        : undefined,
+      this.props.timeClassName ? this.props.timeClassName(time) : undefined,
     ];
 
-    if (this.isSelectedTime(time, currH, currM)) {
+    if (this.isSelectedTime(time)) {
       classes.push("react-datepicker__time-list-item--selected");
     }
 
@@ -160,19 +154,18 @@ export default class Time extends React.Component {
     const format = this.props.format ? this.props.format : "p";
     const intervals = this.props.intervals;
 
-    const base = getStartOfDay(newDate(this.props.selected));
-    const multiplier = 1440 / intervals;
+    const activeDate =
+      this.props.selected || this.props.openToDate || newDate();
+
+    const base = getStartOfDay(activeDate);
     const sortedInjectTimes =
       this.props.injectTimes &&
       this.props.injectTimes.sort(function (a, b) {
         return a - b;
       });
 
-    const activeDate =
-      this.props.selected || this.props.openToDate || newDate();
-    const currH = getHours(activeDate);
-    const currM = getMinutes(activeDate);
-    const activeTime = setHours(setMinutes(base, currM), currH);
+    const minutesInDay = 60 * getHoursInDay(activeDate);
+    const multiplier = minutesInDay / intervals;
 
     for (let i = 0; i < multiplier; i++) {
       const currentTime = addMinutes(base, i * intervals);
@@ -192,19 +185,18 @@ export default class Time extends React.Component {
 
     // Determine which time to focus and scroll into view when component mounts
     const timeToFocus = times.reduce((prev, time) => {
-      if (isBefore(time, activeTime) || isEqual(time, activeTime)) {
+      if (time.getTime() <= activeDate.getTime()) {
         return time;
-      } else {
-        return prev;
       }
+      return prev;
     }, times[0]);
 
     return times.map((time, i) => {
       return (
         <li
           key={i}
           onClick={this.handleClick.bind(this, time)}
-          className={this.liClasses(time, currH, currM)}
+          className={this.liClasses(time)}
           ref={(li) => {
             if (time === timeToFocus) {
               this.centerLi = li;
@@ -213,11 +205,9 @@ export default class Time extends React.Component {
           onKeyDown={(ev) => {
             this.handleOnKeyDown(ev, time);
           }}
-          tabIndex={time === timeToFocus ? "0" : "-1"}
+          tabIndex={time === timeToFocus ? 0 : -1}
           role="option"
-          aria-selected={
-            this.isSelectedTime(time, currH, currM) ? "true" : undefined
-          }
+          aria-selected={this.isSelectedTime(time) ? "true" : undefined}
         >
           {formatDate(time, format, this.props.locale)}
         </li>

test/date_utils_test.test.js

@@ -4,6 +4,7 @@ import {
   addDays,
   subDays,
   isEqual,
+  isSameMinute,
   isSameDay,
   isSameMonth,
   isSameQuarter,
@@ -37,6 +38,7 @@ import {
   safeDateRangeFormat,
   getHolidaysMap,
   arraysAreEqual,
+  startOfMinute,
 } from "../src/date_utils";
 import setMinutes from "date-fns/setMinutes";
 import setHours from "date-fns/setHours";
@@ -1223,4 +1225,27 @@ describe("date_utils", () => {
       expect(arraysAreEqual(array1, array2)).toBe(false);
     });
   });
+
+  describe("isSameMinute", () => {
+    it("should return true if two dates are within the same minute", () => {
+      const d1 = new Date(2020, 10, 10, 10, 10, 10); // Nov 10, 2020 10:10:10
+      const d2 = new Date(2020, 10, 10, 10, 10, 20); // Nov 10, 2020 10:10:20
+      expect(isSameMinute(d1, d2)).toBe(true);
+    });
+
+    it("should return false if two dates aren't within the same minute", () => {
+      const d1 = new Date(2020, 10, 10, 10, 10, 10); // Nov 10, 2020 10:10:10
+      const d2 = new Date(2020, 10, 10, 10, 11, 10); // Nov 10, 2020 10:11:10
+      expect(isSameMinute(d1, d2)).toBe(false);
+    });
+  });
+
+  describe("startOfMinute", () => {
+    it("should properly find the start of the minute", () => {
+      const d = new Date(2020, 10, 10, 10, 10, 10); // Nov 10, 2020 10:10:10
+      const expected = new Date(2020, 10, 10, 10, 10, 0); // Nov 10, 2020 10:10:00
+
+      expect(startOfMinute(d)).toEqual(expected);
+    });
+  });
 });

test/time_format_test.test.js

@@ -121,7 +121,7 @@ describe("TimeComponent", () => {
       var timeListItem = timeComponent.find(
         ".react-datepicker__time-list-item--selected",
       );
-      expect(timeListItem.at(0).prop("tabIndex")).toBe("0");
+      expect(timeListItem.at(0).prop("tabIndex")).toBe(0);
     });
 
     it("should not add the aria-selected property to a regular item", () => {
@@ -151,7 +151,7 @@ describe("TimeComponent", () => {
       var timeListItem = timeComponent.find(
         ".react-datepicker__time-list-item",
       );
-      expect(timeListItem.at(0).prop("tabIndex")).toBe("-1");
+      expect(timeListItem.at(0).prop("tabIndex")).toBe(-1);
     });
 
     it("when no selected time, should focus the time closest to the opened time", () => {
@@ -169,7 +169,7 @@ describe("TimeComponent", () => {
         timeListItem
           .findWhere((node) => node.type() && node.text() === "09:00")
           .prop("tabIndex"),
-      ).toBe("0");
+      ).toBe(0);
     });
 
     it("when no selected time, should call calcCenterPosition with centerLi ref, closest to the opened time", () => {

test/timepicker_test.test.js

@@ -57,10 +57,7 @@ describe("TimePicker", () => {
   });
 
   it("should show different colors for times", () => {
-    const handleTimeColors = (time, currH, currM) => {
-      if (!Number.isInteger(currH) || !Number.isInteger(currM)) {
-        return "wrong";
-      }
+    const handleTimeColors = (time) => {
       return time.getHours() < 12 ? "red" : "green";
     };
     const timePicker = TestUtils.renderIntoDocument(