Repository: Hacker0x01/react-datepicker
Pull request #4244 (2023-09-11 08:20:17)

fix: timepicker on 23 and 25-hour days
This is a fix for  #4243.

## Notes

### Tests

I would gladly add tests but time zones are involved which makes things complicated. In Node.js timezone could be changed only with environment variable which requires significant changes in test running process if we want to test several of them.

### timeClassName prop

Signature of `timeClassName` changed as currH and currM don't make sense anymore. This is a potentially breaking change and also add inconvenience for the user willing to highlight selected time.

### Time range

Time range is also prone to DST bugs (if someone accidentally would set a date on the DST boundary) and the interface looks somewhat strange to me. I think it should either accept an array of ranges or start and end times in `HH:mm` format.


Patch:
diff --git a/src/date_utils.js b/src/date_utils.js
index 10a7596e..443ee0a5 100644
--- a/src/date_utils.js
+++ b/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();
+}
diff --git a/src/index.jsx b/src/index.jsx
index 293cd85f..1ae229e3 100644
--- a/src/index.jsx
+++ b/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,
diff --git a/src/time.jsx b/src/time.jsx
index 692d9bac..cd0c475e 100644
--- a/src/time.jsx
+++ b/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,11 +185,10 @@ 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) => {
@@ -204,7 +196,7 @@ export default class Time extends React.Component {
         <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 patch:
diff --git a/test/date_utils_test.test.js b/test/date_utils_test.test.js
index 77a7c04c..e755357c 100644
--- a/test/date_utils_test.test.js
+++ b/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);
+    });
+  });
 });
diff --git a/test/time_format_test.test.js b/test/time_format_test.test.js
index e9ddd36f..7c38d579 100644
--- a/test/time_format_test.test.js
+++ b/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", () => {
diff --git a/test/timepicker_test.test.js b/test/timepicker_test.test.js
index 9a61bff0..3cfd5bee 100644
--- a/test/timepicker_test.test.js
+++ b/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(