-
Notifications
You must be signed in to change notification settings - Fork 45
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
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 #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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this 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.
front/src/applications/editor/components/LinearMetadata/FormLineStringLength.tsx
Outdated
Show resolved
Hide resolved
front/src/applications/editor/components/LinearMetadata/FormLineStringLength.tsx
Outdated
Show resolved
Hide resolved
4e00408
to
0e35e2d
Compare
There was a problem hiding this 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 :)
fix #7489
On the editor, the length field directly impacts curves & slopes.
When changing from
10800
to11000
, you need to pass through the value1
,which trigger a re-computation of curves, and you lost all the data from1
to10800
.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 :
The design has been done Thibaut.