-
Notifications
You must be signed in to change notification settings - Fork 40
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: add checks for input size in the scenario modal #7920
front: add checks for input size in the scenario modal #7920
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 #7920 +/- ##
============================================
- Coverage 28.42% 28.39% -0.03%
Complexity 2059 2059
============================================
Files 1264 1266 +2
Lines 155100 155174 +74
Branches 3053 3055 +2
============================================
- Hits 44082 44060 -22
- Misses 109197 109291 +94
- Partials 1821 1823 +2
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.
Nice job :)
I left a comment on project, but you can do the same for study and scenario
5304436
to
adcfd8f
Compare
Thank you for your feedback ! |
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.
Almost perfect, can you just rename the functions checkFields => checkProjectFields, checkScenarioFields, checkStudyFields ? Otherwise we have 3 functions checkFields which are exported, it's not very practical
adcfd8f
to
40ef040
Compare
Of course it's done |
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 ✅ tested
close #5935