Fixes #38962 - Update DateTimePicker to pf5 #10802

MariaAga · 2025-12-16T10:42:21Z

The original task states:
Deprecate the 3 components: TimePicker, DatePicker and DateTimePicker
Investigate where the components are used so we can plan the work to replace their usage

But the components are used in erb forms, so they should still be wrapped.
I only found them being used in the generate report erb. but in the future we might need to reuse the datetime picker, which PF doesnt supply directly.
foreman_remote_job_execution uses a different type of a datetime picker, thats because in there users can select a date and a time separately, but in some form, it might be necessary to have the date and time in the same text field, like in the generate page.

Copilot

Pull request overview

This PR modernizes the DateTimePicker, DatePicker, and TimePicker components by migrating from custom implementations to PatternFly 5 native components. The changes simplify the codebase significantly by removing numerous custom date/time handling components and replacing them with PF5's built-in functionality.

Key Changes

Replaced custom date/time pickers with PatternFly 5 components (CalendarMonth, DatePicker, TimePicker)
Migrated test suite from Enzyme to React Testing Library
Removed over 1000 lines of custom date handling logic and components
Added helper functions for date/time formatting

Reviewed changes

Copilot reviewed 54 out of 54 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
dateTimeHelpers.js	New utility file with formatTime and formatDate helper functions
DateTimePicker.js	Converted to functional component using PF5 CalendarMonth and custom TimePicker
DatePicker.js	Simplified to wrap PF5 DatePicker component
TimePicker.js	Simplified to wrap PF5 TimePicker component
*.test.js files	Migrated from Enzyme to React Testing Library
date-time-picker.scss	Removed legacy styles, added minimal wrapper styles
DateComponents/*	Deleted custom date picker components (MonthView, YearView, DecadeView, etc.)
TimeComponents/*	Deleted custom time picker components
form_helper.rb	Added time_local_f helper method

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

webpack/assets/javascripts/react_app/components/common/DateTimePicker/DateTimePicker.js

MariaAga · 2025-12-16T11:16:28Z

TODO: fix

and add a validator to allow only future date (if prop is passed)

MariaAga · 2025-12-17T10:03:54Z

Fixed review comments, added isFutureOnly prop, fixed the extra 0 bug

Copilot

Pull request overview

Copilot reviewed 54 out of 54 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

webpack/assets/javascripts/react_app/components/common/DateTimePicker/DateTimePicker.js

Copilot · 2025-12-17T10:06:58Z

webpack/assets/javascripts/react_app/components/common/DateTimePicker/DateTimePicker.js

+      }
+    };
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [valueDate, valueTime, value, required, isFutureOnly]);


The interval cleanup is missing onChange in the dependency array but uses it. The useEffect at line 43 uses the onChange callback (line 54) but doesn't include it in the dependency array (line 70). This violates the rules of hooks and could lead to calling a stale onChange function. Either add onChange to the dependency array or wrap it with useCallback in the parent component.

ekohl · 2025-12-17T12:01:16Z

webpack/assets/javascripts/react_app/components/common/DateTimePicker/DateTimePicker.js

+  const [valueDate, setValueDate] = useState(formatDate(initialValue));
+  const [valueTime, setValueTime] = useState(formatTime(initialValue));


I don't mean to block the review here and certainly not block, but I notice you store a formatted date in the store. Isn't it possible to store a Date object instead of a string? My main concern is that you can't deal with timezones this way which can be important.

Can you give an example? formatDate and formatTime use a New Date( object

When I read the flow you take initialValue (which could be timezone aware) deconstruct that into simple strings for both date and time. Then reuse that into a string value. Could the TextInput itself be fed a Date instance with a format function? Effectively removing the formatDate and formatTime helpers.

Out of my interest I tried to read up on what HTML can do. https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/input/datetime-local uses the value 2018-06-12T19:30 which is the ISO 8601 format. So perhaps pass that around as much as possible?

Then I don't know what PatternFly does, but I noticed https://www.patternfly.org/components/forms/text-input/ supports datetime-local too (instead of text). Is that only in PF6?

And this is still for me to understand how we deal with dates, not a formal review.

I will say that from my tests by editing the field with a different timezone all worked as expected, and the server respected the correct timezone (set the generate report to 3 mins in the future and it only downloaded 3 mins later)

pondrejk · 2025-12-17T12:24:02Z

Heya, looked at packit, not sure how much in progress this is, but here are some observations anyway:

there's one downstream test related, the way it is written now doesn't break robottelo, though not sure if it would make sense for users to display time and date in one field and then time again in a separate field

if we drop time from the first field I'd need to make some adjustments in airgun which is ok, but I'd need unique id for the time input field as well
I cant select from the calendar popup and the popup doesn't disappear if I click away from it (have to click calendar icon again to roll it up)

clear all button clears the first field only

MariaAga · 2025-12-17T12:54:22Z

@pondrejk

clear all button clears the first field only

Bug in PF patternfly/patternfly-react#6569
There is no way to empty the time field. I'll take a look if I can workaround it.

I cant select from the calendar popup

Fixed

and the popup doesn't disappear if I click away from it (have to click calendar icon again to roll it up)

I based my work on the PF design, and it doesnt disappear on click away as well
https://v5-archive.patternfly.org/components/date-and-time/date-and-time-picker

pondrejk · 2025-12-17T13:17:43Z

https://v5-archive.patternfly.org/components/date-and-time/date-and-time-picker

strangely that second example there with Date and time range picker doesn't have this problem, not sure why

MariaAga · 2025-12-17T13:35:32Z

strangely that second example there with Date and time range picker doesn't have this problem, not sure why

@pondrejk
Is there a preference to close on clickout side? I can add it if thats what we expect

MariaAga · 2025-12-17T13:43:28Z

todo: editing the time when date is empty should populate the date

pondrejk · 2025-12-17T13:45:08Z

Is there a preference to close on clickout side? I can add it if thats what we expect

I suspect users could see it as a bug, for example if they change focus without changing date and the popover will prevent interaction with another field/dropdown, something like here with time select:

MariaAga · 2025-12-17T14:18:19Z

Fixed: calendar close on clickout side and on date select, selecting a time when there is no date sets the date to today.

pondrejk · 2025-12-17T14:54:50Z

webpack/assets/javascripts/react_app/components/common/DateTimePicker/DateTimePicker.js

+          </InputGroupItem>
+          <InputGroupItem className="date-picker-input-item">
+            <TimePicker
+              value={valueTime}


nitpick, maybe some ouiaId here as well?, because right now the time picker input has id=""

there is no ouiaId prop for timepicker, instead I gave it

id={`${id}-time-picker`}

pondrejk

ack from the functional side, UI works nice, the time offset is honored when creating a report. Also positive PRT result in SatelliteQE/robottelo#20541 proves the automation is not affected and the tested scenario still works

ofedoren

Thanks, @MariaAga and @pondrejk !

Just for future reference sake:

Why is the time shown twice? In patternfly docs there are examples where it's either shown together or completely separate. We currently have it mixed.
The old UI had a Today button, but it doesn't seem to be present now. Does clear button do it instead?

MariaAga · 2025-12-18T18:30:49Z

Why is the time shown twice? In patternfly docs there are examples where it's either shown together or completely separate. We currently have it mixed.

I didnt like that the PF docs created a new time selector which was just a dropdown with times, and I wanted to reuse existing components, so I put the timepicker component as it is.
The time needs to be in the date-time field so the form will have a date-time value, the second time field is mostly for comfort and editing.

The old UI had a Today button, but it doesn't seem to be present now. Does clear button do it instead?

No, clear only removes the date, PF example just have buttons outside the date picker to reset and clear the date https://v5-archive.patternfly.org/components/date-and-time/date-picker#controlled-required
Not sure if we want more buttons on the form, but what about something like this? (only for the date time picker)

ofedoren · 2025-12-19T13:03:23Z

what about something like this? (only for the date time picker)

Honestly, I'm not sure if this button is being used by the users, but at least it would feel the same since we already have such button.

MariaAga · 2025-12-19T16:41:27Z

Added a today button inside the datepicker. Moved the clear button for consistency, and also since the clear only clears the date

MariaAga requested a review from Copilot December 16, 2025 10:42

pr-processor bot added the Not yet reviewed label Dec 16, 2025

github-actions bot added the UI label Dec 16, 2025

Copilot started reviewing on behalf of MariaAga December 16, 2025 10:42 View session

Copilot AI reviewed Dec 16, 2025

View reviewed changes

pr-processor bot removed the Not yet reviewed label Dec 16, 2025

MariaAga force-pushed the pf5-datetime branch from cbd93fe to e5e958e Compare December 17, 2025 10:02

MariaAga requested a review from Copilot December 17, 2025 10:03

Copilot started reviewing on behalf of MariaAga December 17, 2025 10:03 View session

Copilot AI reviewed Dec 17, 2025

View reviewed changes

MariaAga force-pushed the pf5-datetime branch from e5e958e to 093f82f Compare December 17, 2025 10:12

ekohl reviewed Dec 17, 2025

View reviewed changes

MariaAga force-pushed the pf5-datetime branch from 093f82f to 93e63e2 Compare December 17, 2025 12:48

MariaAga force-pushed the pf5-datetime branch from 93e63e2 to 870cfa6 Compare December 17, 2025 14:17

pr-processor bot added the Waiting on contributor label Dec 17, 2025

pondrejk reviewed Dec 17, 2025

View reviewed changes

pondrejk mentioned this pull request Dec 17, 2025

prt commit, no merge please SatelliteQE/robottelo#20541

Draft

Fixes #38962 - Update DateTimePicker to pf5

28107f1

MariaAga force-pushed the pf5-datetime branch from 870cfa6 to 28107f1 Compare December 17, 2025 15:22

pr-processor bot added Needs re-review and removed Waiting on contributor labels Dec 17, 2025

pr-processor bot removed the Needs re-review label Dec 17, 2025

pondrejk approved these changes Dec 17, 2025

View reviewed changes

ofedoren previously approved these changes Dec 18, 2025

View reviewed changes

Refs #38962 - move clear,today buttons to be inside datetimepicker

22f5ca1

MariaAga dismissed ofedoren’s stale review via 22f5ca1 December 19, 2025 16:39

		const [valueDate, setValueDate] = useState(formatDate(initialValue));
		const [valueTime, setValueTime] = useState(formatTime(initialValue));

Fixes #38962 - Update DateTimePicker to pf5 #10802

Are you sure you want to change the base?

Fixes #38962 - Update DateTimePicker to pf5 #10802

Conversation

MariaAga commented Dec 16, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MariaAga commented Dec 16, 2025

Uh oh!

MariaAga commented Dec 17, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

ekohl Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

MariaAga Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

ekohl Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

MariaAga Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

pondrejk commented Dec 17, 2025

Uh oh!

MariaAga commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pondrejk commented Dec 17, 2025

Uh oh!

MariaAga commented Dec 17, 2025

Uh oh!

MariaAga commented Dec 17, 2025

Uh oh!

pondrejk commented Dec 17, 2025

Uh oh!

MariaAga commented Dec 17, 2025

Uh oh!

pondrejk Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

MariaAga Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

pondrejk left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ofedoren left a comment

Choose a reason for hiding this comment

Uh oh!

MariaAga commented Dec 18, 2025

Uh oh!

ofedoren commented Dec 19, 2025

Uh oh!

MariaAga commented Dec 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

MariaAga commented Dec 17, 2025 •

edited

Loading

pondrejk left a comment •

edited

Loading