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

front: review length field on editor #7510

Merged
merged 1 commit into from
May 24, 2024
Merged

Conversation

sim51
Copy link
Contributor

@sim51 sim51 commented May 24, 2024

fix #7489

On the editor, the length field directly impacts curves & slopes.

When changing from 10800 to 11000, you need to pass through the value 1 ,which trigger a re-computation of curves, and you lost all the data from 1 to 10800.

To avoid that, a debounce of 1.5sec has been set on this field, but it introduces a new bug : if you change the value and press enter in less than 1.5sec, the new value is not send to the backend.

So to fix those issues, this commit :

  • put the length on the top of the form with a separator
  • add a user validation mechanism via a button

The design has been done Thibaut.

@sim51 sim51 requested a review from a team as a code owner May 24, 2024 10:44
@sim51 sim51 enabled auto-merge May 24, 2024 10:44
@codecov-commenter
Copy link

codecov-commenter commented May 24, 2024

Codecov Report

Attention: Patch coverage is 0% with 72 lines in your changes are missing coverage. Please review.

Project coverage is 29.34%. Comparing base (c4b205e) to head (0e35e2d).
Report is 1 commits behind head on dev.

Files Patch % Lines
...components/LinearMetadata/FormLineStringLength.tsx 0.00% 43 Missing and 1 partial ⚠️
.../trackEdition/components/TrackEditionLeftPanel.tsx 0.00% 26 Missing ⚠️
.../src/applications/editor/components/EditorForm.tsx 0.00% 2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff              @@
##                dev    #7510      +/-   ##
============================================
- Coverage     29.34%   29.34%   -0.01%     
  Complexity     2012     2012              
============================================
  Files          1197     1197              
  Lines        147058   147050       -8     
  Branches       2889     2889              
============================================
- Hits          43156    43152       -4     
+ Misses       102202   102198       -4     
  Partials       1700     1700              
Flag Coverage Δ
core 75.07% <ø> (ø)
editoast 72.51% <ø> (-0.02%) ⬇️
front 9.76% <0.00%> (+<0.01%) ⬆️
gateway 2.41% <ø> (ø)
railjson_generator 87.49% <ø> (ø)
tests 84.23% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@SharglutDev SharglutDev left a comment

Choose a reason for hiding this comment

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

Thank you for this PR and the nice comment history in the issue.

Left just a syntax comment.

And regarding your comments in the issue, it seems we can still press "enter" in the length field, resulting to saving the form with an empty payload.

@sim51 sim51 force-pushed the bsi/front-editor-length-field branch from 4e00408 to 0e35e2d Compare May 24, 2024 12:50
Copy link
Contributor

@SharglutDev SharglutDev left a comment

Choose a reason for hiding this comment

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

Lgtm and tested. Thank your for the changes :)

@sim51 sim51 added this pull request to the merge queue May 24, 2024
Merged via the queue into dev with commit 763251a May 24, 2024
17 checks passed
@sim51 sim51 deleted the bsi/front-editor-length-field branch May 24, 2024 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

front: update tracksection length
3 participants