Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle invalid values in stdcm inputs #9374

Closed
2 tasks
SharglutDev opened this issue Oct 17, 2024 · 10 comments · Fixed by #9574 or #10740
Closed
2 tasks

Handle invalid values in stdcm inputs #9374

SharglutDev opened this issue Oct 17, 2024 · 10 comments · Fixed by #9574 or #10740
Assignees
Labels
area:front Work on Standard OSRD Interface modules kind:enhancement Improvement of existing features

Comments

@SharglutDev
Copy link
Contributor

SharglutDev commented Oct 17, 2024

Description and goal

Now that we have some inputs with number in stdcm, we need to handle some edge cases.

To be confirmed by @thibautsailly, but we probably want to handle this with the statusWithMessage prop in inputs and blocking the simulation button while in error like in the rest of the form.

If we do that, it will need a refacto of the css of the stdcm consist part to keep matching the mockup.

Acceptance criteria

  • The following fileds with invalid or empty inputs display a warning message when focusing out. They are not displayed if no traction engine is selected.
    • tonnage (min = weight of traction engine + towed rolling stock, max = 10000t).
      🇫‍🇷 "Le tonnage total doit être compris entre ... et ...t".
      🇬‍🇧 "The total weight must be between ... and ...t"
    • length (min = length of traction engine + towed rolling stock, max = 750m).
      🇫‍🇷 "La longueur totale doit être comprise entre ... et 750m"
      🇬‍🇧 "The total length must be between ... and 750m"
    • max speed (min = 30km/h, max = the value computed depending on traction engine, towed rs, and composition code).
      🇫‍🇷 "La vitesse max. doit être comprise entre 30 et ...km/h"
      🇬‍🇧 "The max speed must be between 30 and ...km/h"
  • When an invalid value is entered in the consist card, the whole column gets larger to accommodate the display of the field error state. This pushes the operational points column to the right, and reduces the map's width ; all margins stay the same.
    When the value becomes valid, all columns widths go back to their default values. All these interactions happen with a transition easeinout 300ms.
@SharglutDev SharglutDev added area:front Work on Standard OSRD Interface modules kind:enhancement Improvement of existing features labels Oct 17, 2024
@SharglutDev SharglutDev changed the title Handle negative numbers in stdcm inputs Handle invalid values in stdcm inputs Oct 17, 2024
@thibautsailly
Copy link

When an invalid value is entered in the consist card, the whole column gets larger to accommodate the display of the field error state. This pushes the operational points column to the right, and reduces the map's width ; all margins stay the same.
When the value becomes valid, all columns widths go back to their default values.

@Wadjetz
Copy link
Member

Wadjetz commented Nov 18, 2024

@maelysLeratRosso Do we have a defined maximum speed limit, similar to the constraints for mass and length? Could you also provide more details about the rules for "the value computed depending on traction engine, towed rs, and composition code" ? I believe the towed rolling stock does not have a maximum speed.

@maelysLeratRosso
Copy link

maelysLeratRosso commented Nov 18, 2024

@Wadjetz

  • No defined maximum speed limit
  • A defined minimum speed limit of 30km/h
  • "the value computed depending on traction engine, towed rs, and composition code" : vMAX = minimum between
    • max speed of traction engine (if defined and not 0)
    • max speed of the towed rolling stock (if defined and not 0) -> yes, the towed rolling stocks have max speeds
  • The composition code can be ignored for now

see #9712

@maelysLeratRosso
Copy link

maelysLeratRosso commented Jan 9, 2025

We reopen this issue because of :

  • this bug Picking some rolling stocks triggers unwanted warning #10306
  • Image : the last acceptance criteria is not satisfied : "When an invalid value is entered in the consist card, the whole column gets larger to accommodate the display of the field error state. This pushes the operational points column to the right, and reduces the map's width ; all margins stay the same.When the value becomes valid, all columns widths go back to their default values. All these interactions happen with a transition easeinout 300ms."

@SharglutDev
Copy link
Contributor Author

the last acceptance criteria is not satisfied : "When an invalid value is entered in the consist card, the whole column gets larger to accommodate the display of the field error state. This pushes the operational points column to the right, and reduces the map's width ; all margins stay the same.When the value becomes valid, all columns widths go back to their default values. All these interactions happen with a transition easeinout 300ms."

Sorry about that @maelysLeratRosso , there has been a terrible misunderstanding. A few weeks ago, as @Wadjetz tried to implement the issue with the initial AC like this one, it looked really hard to integrate in the app.
After some discussions with @thibautsailly, he designed a new way to show these errors/warnings with a tooltip which doesn't require the columns to resize and was way easier to integrate in the app.
The app should now match this mockup : https://www.sketch.com/s/15e0909c-f17d-4200-a770-b9fc1154fa7e/a/4owY3Kq

We totally not integrated you in these discussions, which is really bad, I'm sorry 😕

@maelysLeratRosso
Copy link

ooooooooh ok no problem !

Then I reopen it only for this problem :

@SharglutDev SharglutDev self-assigned this Jan 10, 2025
@SharglutDev
Copy link
Contributor Author

Should be fixed in #10313

@maelysLeratRosso
Copy link

Previous bug is now solved, but there is now another one which prevents this issue from being closed :

#10409

@SharglutDev
Copy link
Contributor Author

Should be fixed with #10413

@thibautsailly
Copy link

The validity check should be performed on focus out, not while the user is typing. Being presented an error message because you haven't finished entering the value you want to add is not a great experience. When it happens multiple times a day, it's rough.

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 kind:enhancement Improvement of existing features
Projects
None yet
4 participants