-
Notifications
You must be signed in to change notification settings - Fork 46
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 option menu to manchette #8775
Conversation
ed881a9
to
541423b
Compare
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 #8775 +/- ##
============================================
- Coverage 36.92% 36.89% -0.04%
Complexity 2242 2242
============================================
Files 1255 1257 +2
Lines 116971 117069 +98
Branches 3270 3272 +2
============================================
Hits 43190 43190
- Misses 71836 71932 +96
- Partials 1945 1947 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
541423b
to
4314175
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.
nice, a few suggestions first
Also I was wondering about the reusability of the menu : |
I'm a bit confused, in this case why don't we create a component in OSRD-UI? witch could be called as a child of manchette and used for each case |
What I meant was : are looking exactly the same except the items inside and the width of the menu. Maybe it should be better to have only one generic component for that to avoid code duplication. Second question is : do we want it in osrd-ui or do we consider that it's purely an osrd component and we keep in osrd code base. |
4314175
to
caed404
Compare
I've taken the liberty to rename the PR, I hope that's fine. |
bb8ccf0
to
32196de
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.
Yes that's exactly why. For now the space time chart header is inside the osrd-ui components and I can't access it but the other issue will be done, I would be able to create it on osrd side and put the button at the right place. |
f9736fa
to
df2bd35
Compare
10f5c2d
to
1919fac
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.
Just small suggestions
front/src/modules/simulationResult/components/SpaceTimeChart/ManchetteMenuButton.tsx
Outdated
Show resolved
Hide resolved
front/src/modules/simulationResult/components/SpaceTimeChart/ManchetteMenuButton.tsx
Show resolved
Hide resolved
front/src/modules/simulationResult/components/SpaceTimeChart/ManchetteMenuButton.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.
LGTM
1926750
to
bff4546
Compare
Signed-off-by: SharglutDev <[email protected]>
PR contains : - a button to toggle the menu - menu has only one option to show/hide ops Signed-off-by: SharglutDev <[email protected]>
bff4546
to
beb51be
Compare
close #8687
As requested by @thibautsailly, the manchette menu button should remain hidden until the waypoint modal is completed (#8627 and #8628)
To test the component, uncomment it