front: stdcm: persist user-entered date and time in the interface #8942
Conversation
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## dev #8942 +/- ##
============================================
- Coverage 37.13% 37.11% -0.03%
Complexity 2211 2211
============================================
Files 1260 1260
Lines 115076 115189 +113
Branches 3230 3234 +4
============================================
+ Hits 42736 42752 +16
- Misses 70407 70500 +93
- Partials 1933 1937 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
front/src/utils/date.ts
Outdated
| date: Date, | ||
| { hours, minutes }: { hours: number; minutes: number } |
There was a problem hiding this comment.
not really an issue, just a suggestion: we could create an explicit type instead of repeating those parameters
| @@ -302,19 +302,21 @@ export function buildCommonConfReducers<S extends OsrdConfState>(): CommonConfRe | |||
| state.startTime = action.payload; | |||
| }, | |||
| updateOrigin(state: Draft<S>, action: PayloadAction<ArrayElement<S['pathSteps']>>) { | |||
| const prevOriginArrivalType = state.pathSteps[0]?.arrivalType; | |||
There was a problem hiding this comment.
| const prevOriginArrivalType = state.pathSteps[0]?.arrivalType; | |
| const prevOriginArrivalType = state.pathSteps.at(0)?.arrivalType; |
| } | ||
| : null; | ||
| state.pathSteps = updateOriginPathStep(state.pathSteps, newPoint, true); | ||
| }, | ||
| updateDestination(state: Draft<S>, action: PayloadAction<ArrayElement<S['pathSteps']>>) { | ||
| const prevDestinationArrivalType = state.pathSteps[state.pathSteps.length - 1]?.arrivalType; |
There was a problem hiding this comment.
| const prevDestinationArrivalType = state.pathSteps[state.pathSteps.length - 1]?.arrivalType; | |
| const prevDestinationArrivalType = state.pathSteps.at(- 1)?.arrivalType; |
front/src/utils/date.ts
Outdated
| * @param {{ hours: number; minutes: number }} - An object containing the hours and the minutes of the arrival time. | ||
| * @returns {string} The ISO formatted arrival date string. | ||
| */ | ||
| export const generateISOArrival = ( |
There was a problem hiding this comment.
I think the function could be used in contexts other than just arrival dates, so I suggest renaming it to something more generic. What do you think?
| export const generateISOArrival = ( | |
| export const generateISODateFromDateTime = ( |
| const updateOriginPoint = (pathStep: PathStep | null) => { | ||
| dispatch(updateOrigin(pathStep)); | ||
| if (arrivalDateInput && arrivalTimeInput) { | ||
| const newOpArrival = generateISOArrival(arrivalDateInput, arrivalTimeInput); | ||
| dispatch(updateOriginArrival(newOpArrival)); | ||
| } |
There was a problem hiding this comment.
To avoid dispatching two actions when arrivalDate and arrivalTime are defined, what do you think about dispatching a single updateOrigin(pathStep) with the pathStep already having the new arrival time set?
The same logic can be applied for updateDestinationPoint
| const updateOriginPoint = (pathStep: PathStep | null) => { | |
| dispatch(updateOrigin(pathStep)); | |
| if (arrivalDateInput && arrivalTimeInput) { | |
| const newOpArrival = generateISOArrival(arrivalDateInput, arrivalTimeInput); | |
| dispatch(updateOriginArrival(newOpArrival)); | |
| } | |
| const updateOriginPoint = (pathStep: PathStep | null) => { | |
| if (!pathStep || !arrivalDateInput || !arrivalTimeInput) { | |
| return dispatch(updateOrigin(pathStep)); | |
| } | |
| dispatch( | |
| updateOrigin({ | |
| ...pathStep, | |
| arrival: generateISOArrival(arrivalDateInput, arrivalTimeInput), | |
| }) | |
| ); | |
| }; |
|
|
||
| const [arrivalDateInput, setArrivalDateInput] = useState<Date | undefined>(undefined); | ||
| const [arrivalTimeInput, setArrivalTimeInput] = useState<{ hours: number; minutes: number }>({ | ||
| hours: 0, | ||
| minutes: 0, | ||
| }); | ||
|
|
There was a problem hiding this comment.
I noticed that these states are duplicated in StdcmDestination and StdcmOrigin, so we might want to move them up to the parent StdcmConfig. Then, we can pass the necessary data to StdcmDestination and StdcmOrigin.
To simplify things, I suggest unifying the variables. Instead of having arrivalDateInput and arrivalTimeInput, we could use a single variable called scheduleConstraint (you can suggest another name). This could have the following type:
type ScheduleConstraint = {
date: Date;
hour: number;
minutes: number;
}Then, StdcmConfig can manage this new state variable (scheduleConstraint) and pass it to StdcmDestination and StdcmOrigin.
Closes STDCM: Persist user-entered times & dates after a pathstep changed