Skip to content

front: handle unrecognized track in itinerary modal#16634

Open
theocrsb wants to merge 2 commits into
devfrom
tce/front/fix-handle-unrecognized-track-in-itinerary-modal
Open

front: handle unrecognized track in itinerary modal#16634
theocrsb wants to merge 2 commits into
devfrom
tce/front/fix-handle-unrecognized-track-in-itinerary-modal

Conversation

@theocrsb
Copy link
Copy Markdown
Contributor

@theocrsb theocrsb commented May 7, 2026

closes #16553

import this train to test it
unknowntrack.json

At the moment, the track names that have been added are not yet visible in the drop-down list. This will be resolved with the latest PR (once the back-end PR has been merged).

@github-actions github-actions Bot added the area:front Work on Standard OSRD Interface modules label May 7, 2026
@theocrsb theocrsb force-pushed the tce/front/fix-handle-unrecognized-track-in-itinerary-modal branch 4 times, most recently from 52d3532 to 03a2d9e Compare May 11, 2026 10:07
@theocrsb theocrsb requested a review from RomainValls May 11, 2026 10:09
@theocrsb theocrsb force-pushed the tce/front/fix-handle-unrecognized-track-in-itinerary-modal branch 3 times, most recently from eed9b21 to e69b46f Compare May 11, 2026 13:34
@theocrsb theocrsb requested review from Akctarus and SarahBellaha May 12, 2026 05:51
@theocrsb theocrsb force-pushed the tce/front/fix-handle-unrecognized-track-in-itinerary-modal branch from e69b46f to d38fb4a Compare May 12, 2026 05:52
@theocrsb theocrsb marked this pull request as ready for review May 12, 2026 05:52
@theocrsb theocrsb requested a review from a team as a code owner May 12, 2026 05:52
Comment on lines +297 to +298
// blackAlpha10
background-color: rgba(0, 0, 0, 0.1);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you use black10 ?

Suggested change
// blackAlpha10
background-color: rgba(0, 0, 0, 0.1);
background-color: var(--black10);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Black10 is different. where we using transparency

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can totally use var(--black10) 👍
rgba(0, 0, 0, 0.1) means you are using the black color with 10% opacity

@theocrsb theocrsb force-pushed the tce/front/fix-handle-unrecognized-track-in-itinerary-modal branch from d28d7ab to e30e887 Compare May 12, 2026 13:58
Signed-off-by: theocrsb <theo_crosbie@yahoo.fr>
@theocrsb theocrsb force-pushed the tce/front/fix-handle-unrecognized-track-in-itinerary-modal branch from e30e887 to 3d3c0cf Compare May 12, 2026 14:01
@theocrsb theocrsb requested a review from Akctarus May 12, 2026 14:01
Signed-off-by: theocrsb <theo_crosbie@yahoo.fr>
@theocrsb theocrsb force-pushed the tce/front/fix-handle-unrecognized-track-in-itinerary-modal branch from 3d3c0cf to 88e1116 Compare May 12, 2026 16:25
Copy link
Copy Markdown
Contributor

@Akctarus Akctarus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested, LGTM

"moveLocationOnMap": "Move location on the map",
"movePointOnMap": "Move this waypoint using the map",
"next": "Next",
"noComputation": "there will be no computation for this train",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"noComputation": "there will be no computation for this train",
"noComputation": "There will be no computation for this train.",

"moveLocationOnMap": "Déplacer le point remarquable sur la carte",
"movePointOnMap": "Déplacez ce point en utilisant la carte",
"next": "Suivant",
"noComputation": "il n'y aura pas de calcul pour ce train",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"noComputation": "il n'y aura pas de calcul pour ce train",
"noComputation": "Il n'y aura pas de calcul pour ce train.",

return;
}

const isTrackNameUnknown = local_track_name
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this naming/logic combination a bit odd: isTrackNameUnknown is computed through a negated condition, which makes the code less intuitive to read.
I think it would be clearer to go back to:

const isValidLocalTrackName = local_track_name
          ? matchedOp?.parts.some((part) => {
              const track = trackSectionsById[part.track];
              if (!track) return false;
              return local_track_name === part.local_track_name;
            })
          : false;

Also, if local_track_name is undefined, i believe isValidLocalTrackName should be false rather than true.
You can then use it l.139 with a negation :)

...currentData,
path: currentData.path.map((item) =>
item.id === stepId && item.location.type !== 'track_offset'
? { ...item, location: { ...item.location, local_track_name: trackName || undefined } }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
? { ...item, location: { ...item.location, local_track_name: trackName || undefined } }
? { ...item, location: { ...item.location, local_track_name: trackName } }

itineraryModalIsOpen: boolean;
setItineraryModalIsOpen: (isOpen: boolean) => void;
displayTrainScheduleManagement: string;
trainScheduleToEditData?: TrainScheduleToEditData;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can only pass the trainScheduleId here

Comment on lines +178 to +185
await dispatch(
osrdEditoastApi.endpoints.putTrainSchedulesById.initiate({
id: trainScheduleId,
trainSchedule: {
...currentData,
path: currentData.path.map((item) =>
item.id === stepId && item.location.type !== 'track_offset'
? { ...item, location: { ...item.location, local_track_name: trackName || undefined } }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for this, you can sort of re-use a part of the updatePacedTrain helper by spliting it like :

export async function updateTrainSchedule(dispatch: AppDispatch, id: number, trainSchedule: TrainSchedule) {
  await dispatch(
    osrdEditoastApi.endpoints.putTrainSchedulesById.initiate({
      id,
      trainSchedule,
    })
  ).unwrap();
}

async function updatePacedTrain(dispatch: AppDispatch, id: number, trainSchedule: TrainSchedule) {
  if (trainSchedule.paced?.exceptions && trainSchedule.paced.exceptions.length > 0) {
    console.error(
      'updatePacedTrain: exceptions should not be included in the paced field. Use exception endpoints instead.'
    );
  }
  await updateTrainSchedule(dispatch, id, trainSchedule);
}

Comment on lines +297 to +298
// blackAlpha10
background-color: rgba(0, 0, 0, 0.1);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can totally use var(--black10) 👍
rgba(0, 0, 0, 0.1) means you are using the black color with 10% opacity

{hasInvalidPathStepDisplay && (
<div key={`invalid-op-${bannerWiggle}`}>
<Banner type="error" message={t('alertInvalidOP')} />
<Banner type="info" message={`${t('alertInvalidOP')} ${t('noComputation')}`} />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<Banner type="info" message={`${t('alertInvalidOP')} ${t('noComputation')}`} />
<Banner type="info" message={`${t('alertInvalidOP')}. ${t('noComputation')}`} />

You can either add the missing period between the two sentences here or, better, directly in the translation file :)

{!hasInvalidPathStepDisplay && pathfindingError && (
<div key={`pathfinding-${bannerWiggle}`}>
<Banner type="error" message={pathfindingError} />
<Banner type="info" message={`${pathfindingError} ${t('noComputation')}`} />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<Banner type="info" message={`${pathfindingError} ${t('noComputation')}`} />
<Banner type="info" message={`${pathfindingError}. ${t('noComputation')}`} />

Same

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:front Work on Standard OSRD Interface modules

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Front: Handle unrecognized track in itinerary modal

4 participants