-
Notifications
You must be signed in to change notification settings - Fork 51
front: add alternative solutions proposals for stdcm #10970
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## dev #10970 +/- ##
==========================================
+ Coverage 80.81% 80.83% +0.01%
==========================================
Files 1120 1122 +2
Lines 113171 113310 +139
Branches 759 758 -1
==========================================
+ Hits 91461 91592 +131
- Misses 21655 21663 +8
Partials 55 55
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
e02e324
to
91bc2fa
Compare
91bc2fa
to
20f9637
Compare
9e3d9f6
to
6778dae
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.
As discussed, I think it would be easier to handle directly the results in your new functions handleSuccess and handleConflicts in order to remove useStdcmResults and the useEffect in StdcmView
771a931
to
2ae0425
Compare
2ae0425
to
9c22f1a
Compare
9c22f1a
to
6caa9d4
Compare
2f2fec9
to
13a2a27
Compare
8bd9d88
to
547117b
Compare
front/src/applications/stdcm/components/StdcmResults/StdcmDebugResults.tsx
Outdated
Show resolved
Hide resolved
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.
tested, lgtm !
front/src/reducers/osrdconf/stdcmConf/stdcmConfReducers.spec.ts
Outdated
Show resolved
Hide resolved
front/src/applications/stdcm/utils/formatSimulationReportSheet.ts
Outdated
Show resolved
Hide resolved
front/src/reducers/osrdconf/stdcmConf/stdcmConfReducers.spec.ts
Outdated
Show resolved
Hide resolved
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 good 😄
Only 1 question @thibautsailly, here is the mockup:
Is it ok for you if we have the banner takes all the width ? Can we harmonize the padding top and bottom around the main div ?
Discussed with @thibautsailly:
|
ee8f4b2
to
cfa5a3c
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, only 4 small comments to fix.
Can you squash your commits and ask a review in the tests channel ?
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 ✅
Please, can you squash your commits ? You should only merge 2 commits
… empty while consolidating overtakes Signed-off-by: Achrafmohye <[email protected]>
7c4f595
to
a45370d
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 ✅
Signed-off-by: Achrafmohye <[email protected]>
a45370d
to
7d81c7f
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 ✅
closes #10572