From e3a6bc8961873e26cd67a753ebb6d366c586cce1 Mon Sep 17 00:00:00 2001 From: Etheryte Date: Mon, 2 Oct 2023 12:53:12 +0200 Subject: [PATCH 1/5] Update datepicker insertion logic --- .../components/datetime/DateTimePicker.tsx | 24 +++++++++++++++---- .../utils/datetime/localizedMoment.test.ts | 10 +++++++- .../src/utils/datetime/localizedMoment.ts | 13 +++++++++- 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/web/html/src/components/datetime/DateTimePicker.tsx b/web/html/src/components/datetime/DateTimePicker.tsx index ba721245e62b..ec99351a337d 100644 --- a/web/html/src/components/datetime/DateTimePicker.tsx +++ b/web/html/src/components/datetime/DateTimePicker.tsx @@ -4,10 +4,10 @@ import { forwardRef, useRef } from "react"; import ReactDatePicker from "react-datepicker"; -import { localizedMoment } from "utils"; +import { localizedMoment, parseTimeString } from "utils"; // Turn this on to view internal state under the picker in the UI -const SHOW_DEBUG_VALUES = false; +const SHOW_DEBUG_VALUES = true; type InputPassthroughProps = { "data-id": string | undefined; @@ -173,8 +173,10 @@ export const DateTimePicker = (props: Props) => { portalId="time-picker-portal" ref={timePickerRef} selected={browserTimezoneValue.toDate()} - onChange={(date) => { - if (date === null) { + onChange={(date, event) => { + // If this fires without an event, it means the user picked a time from the dropdown + // This handler is only used the dropdown selection, onChangeRaw() handles regular user input + if (date === null || event) { return; } /** @@ -185,6 +187,20 @@ export const DateTimePicker = (props: Props) => { mergedDate.setHours(date.getHours(), date.getMinutes()); onChange(mergedDate); }} + onChangeRaw={(event) => { + const rawValue = event.target.value.trim(); + // TODO: Alternatively remove everything that isn't a number and a ":"? + // If the user pastes in a value or doesn't type a ":", only keep the first four numbers + // TODO: Implement + + const parsed = parseTimeString(rawValue); + if (!parsed) { + return; + } + const mergedDate = browserTimezoneValue.toDate(); + mergedDate.setHours(parsed.hours, parsed.minutes); + onChange(mergedDate); + }} showTimeSelect showTimeSelectOnly // We want the regular primary display to only show the time here, so using TIME_FORMAT is intentional diff --git a/web/html/src/utils/datetime/localizedMoment.test.ts b/web/html/src/utils/datetime/localizedMoment.test.ts index b94d9b98e29b..d5c3bca1569d 100644 --- a/web/html/src/utils/datetime/localizedMoment.test.ts +++ b/web/html/src/utils/datetime/localizedMoment.test.ts @@ -1,4 +1,4 @@ -import { localizedMoment } from "./localizedMoment"; +import { localizedMoment, parseTimeString } from "./localizedMoment"; describe("localizedMoment", () => { const validISOString = "2020-01-30T23:00:00.000Z"; @@ -82,4 +82,12 @@ describe("localizedMoment", () => { const nextWeek = localizedMoment().add(1, "week"); expect(nextWeek.calendar()).toEqual(nextWeek.format("YYYY-MM-DD")); }); + + // This function is a thin wrapper around moment so we only add basic test coverage + test("parseTimeString", () => { + expect(parseTimeString("")).toEqual(null); + expect(parseTimeString("2345")).toEqual({ hours: 23, minutes: 45 }); + expect(parseTimeString("23:45")).toEqual({ hours: 23, minutes: 45 }); + expect(parseTimeString(" 23 45 67 ")).toEqual({ hours: 23, minutes: 45 }); + }); }); diff --git a/web/html/src/utils/datetime/localizedMoment.ts b/web/html/src/utils/datetime/localizedMoment.ts index ad7449cc242c..15bc160332a4 100644 --- a/web/html/src/utils/datetime/localizedMoment.ts +++ b/web/html/src/utils/datetime/localizedMoment.ts @@ -199,6 +199,17 @@ Object.defineProperties(moment, { }, }); +const parseTimeString = function (input: string): { hours: number; minutes: number } | null { + const parsed = moment(input, ["H:m"]); + if (parsed.isValid()) { + return { + hours: parsed.hours(), + minutes: parsed.minutes(), + }; + } + return null; +}; + function localizedMomentConstructor(input?: moment.MomentInput) { // We make all inputs UTC internally and only format them for output const utcMoment = @@ -214,4 +225,4 @@ function localizedMomentConstructor(input?: moment.MomentInput) { } const localizedMoment: typeof moment = Object.setPrototypeOf(localizedMomentConstructor, moment); -export { localizedMoment }; +export { localizedMoment, parseTimeString }; From 969edb42704a9b2dcf067ba7b1b6374dcdd103c8 Mon Sep 17 00:00:00 2001 From: Etheryte Date: Mon, 2 Oct 2023 14:54:23 +0200 Subject: [PATCH 2/5] Improve test coverage, clean input values better --- .../datetime/DateTimePicker.test.tsx | 49 ++++++++++++++++++- .../components/datetime/DateTimePicker.tsx | 13 ++--- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/web/html/src/components/datetime/DateTimePicker.test.tsx b/web/html/src/components/datetime/DateTimePicker.test.tsx index 8313781819ab..a436cb97fa5f 100644 --- a/web/html/src/components/datetime/DateTimePicker.test.tsx +++ b/web/html/src/components/datetime/DateTimePicker.test.tsx @@ -61,7 +61,7 @@ describe("DateTimePicker", () => { expect(timePicker.value).toEqual("23:30"); }); - test("clearing the time input doesn't change the date (bsc#1210253)", (done) => { + test("clearing or manually editing the time input doesn't change the date (bsc#1210253, bsc#1215820)", (done) => { const validISOString = "2020-01-30T15:00:00.000Z"; let changeEventCount = 0; @@ -89,4 +89,51 @@ describe("DateTimePicker", () => { screen.getByText("16").click(); type(timePicker, "0", false); }); + + test("picking a time from the dropdown works", (done) => { + const validISOString = "2020-02-01T04:00:00.000Z"; + + const Setup = () => { + const [value, setValue] = useState(localizedMoment(validISOString)); + return ( + { + setValue(newValue); + + expect(newValue.toUserDateTimeString()).toEqual("2020-01-31 14:30"); + done(); + }} + /> + ); + }; + render(); + + const { timePicker } = getInputs(); + timePicker.click(); + screen.getByText("14:30").click(); + }); + + test("manually entering a time value works", (done) => { + const validISOString = "2020-02-01T04:00:00.000Z"; + + const Setup = () => { + const [value, setValue] = useState(localizedMoment(validISOString)); + return ( + { + setValue(newValue); + + expect(newValue.toUserDateTimeString()).toEqual("2020-01-31 23:45"); + done(); + }} + /> + ); + }; + render(); + + const { timePicker } = getInputs(); + type(timePicker, "23:45"); + }); }); diff --git a/web/html/src/components/datetime/DateTimePicker.tsx b/web/html/src/components/datetime/DateTimePicker.tsx index ec99351a337d..4970f50fc427 100644 --- a/web/html/src/components/datetime/DateTimePicker.tsx +++ b/web/html/src/components/datetime/DateTimePicker.tsx @@ -188,12 +188,14 @@ export const DateTimePicker = (props: Props) => { onChange(mergedDate); }} onChangeRaw={(event) => { - const rawValue = event.target.value.trim(); - // TODO: Alternatively remove everything that isn't a number and a ":"? - // If the user pastes in a value or doesn't type a ":", only keep the first four numbers - // TODO: Implement + // In case the user pastes a value, clean it up and cut it to max length + const rawValue = event.target.value.replaceAll(/[^\d:]/g, ""); + const cutValue = rawValue.includes(":") ? rawValue.substring(0, 5) : rawValue.substring(0, 4); + if (cutValue !== event.target.value) { + event.target.value = cutValue; + } - const parsed = parseTimeString(rawValue); + const parsed = parseTimeString(cutValue); if (!parsed) { return; } @@ -216,7 +218,6 @@ export const DateTimePicker = (props: Props) => { className="form-control" // This is used by Cucumber to interact with the component data-testid="time-picker" - maxLength={5} /> } /> From 765d07b8c054dad37ac3b2cf559ffaf377483cb9 Mon Sep 17 00:00:00 2001 From: Etheryte Date: Mon, 2 Oct 2023 14:54:52 +0200 Subject: [PATCH 3/5] Hide debug UI --- web/html/src/components/datetime/DateTimePicker.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/html/src/components/datetime/DateTimePicker.tsx b/web/html/src/components/datetime/DateTimePicker.tsx index 4970f50fc427..fb581a3c77e1 100644 --- a/web/html/src/components/datetime/DateTimePicker.tsx +++ b/web/html/src/components/datetime/DateTimePicker.tsx @@ -7,7 +7,7 @@ import ReactDatePicker from "react-datepicker"; import { localizedMoment, parseTimeString } from "utils"; // Turn this on to view internal state under the picker in the UI -const SHOW_DEBUG_VALUES = true; +const SHOW_DEBUG_VALUES = false; type InputPassthroughProps = { "data-id": string | undefined; From cff93c359d697dfae592304071df4f3c185ee0dd Mon Sep 17 00:00:00 2001 From: Etheryte Date: Mon, 2 Oct 2023 16:19:04 +0200 Subject: [PATCH 4/5] Add changes --- web/spacewalk-web.changes.eth.date-improve | 1 + 1 file changed, 1 insertion(+) create mode 100644 web/spacewalk-web.changes.eth.date-improve diff --git a/web/spacewalk-web.changes.eth.date-improve b/web/spacewalk-web.changes.eth.date-improve new file mode 100644 index 000000000000..67886c30f95b --- /dev/null +++ b/web/spacewalk-web.changes.eth.date-improve @@ -0,0 +1 @@ +- Improve datetimepicker input formatting From af40f35dc56610d993ab9b75ad6ae067ece2d5d9 Mon Sep 17 00:00:00 2001 From: Etheryte Date: Mon, 2 Oct 2023 18:40:21 +0200 Subject: [PATCH 5/5] Update format --- web/html/src/utils/datetime/localizedMoment.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/html/src/utils/datetime/localizedMoment.ts b/web/html/src/utils/datetime/localizedMoment.ts index 15bc160332a4..860ce3aff9bf 100644 --- a/web/html/src/utils/datetime/localizedMoment.ts +++ b/web/html/src/utils/datetime/localizedMoment.ts @@ -200,7 +200,7 @@ Object.defineProperties(moment, { }); const parseTimeString = function (input: string): { hours: number; minutes: number } | null { - const parsed = moment(input, ["H:m"]); + const parsed = moment(input.trim(), ["H:m", "Hm"]); if (parsed.isValid()) { return { hours: parsed.hours(),