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: integrate speedspacechart version two #8146

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

Yohh
Copy link
Contributor

@Yohh Yohh commented Jul 22, 2024

new logic

  • the functions to format the actual data to hydrate the GEV V2 are set in SpeedSpaceChart module in a helpers file
  • a electricalProfilesDesignValues const has been added in the same folder, in consts.ts
  • the new style logic has been added in his own file then imported in style.scss

integration

  • SimulationResultsV2 and StdcmResultsV2 now display the new GEV via SpeedSpaceChartV2
  • the logic to prepare and hydrate the GEV V2 is kept in SpeedSpaceChartV2

tests

  • every format() function, isNotCompatible() and getProfileValue() have been tested

next

  • ui-speedspacechart needs some fixes for the GEV v2 to be well displayed (see this PR):
    • the all height should be adapted when one or more linear layers is/are display
    • the front-interactivy-layer has a useless mt-2 tailwind rule set to !imortant that overrides the needed css
    • the settings panel height doesn't follows the GEV V2 height
    • speedLimits and temporarySpeedLimits checkboxes should be disable
    • type: 'non_electrified' is missing for ElectrificationValues
  • the format() helpers should be tested
  • the width isn't responsive yet

close #7735
close #7734

@codecov-commenter
Copy link

codecov-commenter commented Jul 22, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 44.23077% with 174 lines in your changes missing coverage. Please review.

Project coverage is 28.45%. Comparing base (ac01ddf) to head (ab175a5).
Report is 3 commits behind head on dev.

Files Patch % Lines
...t/components/SpeedSpaceChart/SpeedSpaceChartV2.tsx 0.00% 63 Missing and 1 partial ⚠️
...lationResult/components/SpeedSpaceChart/helpers.ts 63.81% 29 Missing and 26 partials ⚠️
...nt/src/applications/stdcm/views/StdcmResultsV2.tsx 0.00% 14 Missing ⚠️
...perationalStudies/views/v2/SimulationResultsV2.tsx 0.00% 13 Missing ⚠️
...helpers/formatPowerRestrictionRangesWithHandled.ts 72.41% 2 Missing and 6 partials ⚠️
...t/components/SpeedSpaceChart/useSpeedSpaceChart.ts 0.00% 8 Missing ⚠️
...tionResult/components/SpeedSpaceChart/d3Helpers.ts 0.00% 4 Missing ⚠️
...onResult/components/SpeedSpaceChart/prepareData.ts 0.00% 1 Missing and 1 partial ⚠️
...mulationResult/components/SpeedSpaceChart/utils.ts 0.00% 2 Missing ⚠️
...ront/src/applications/stdcm/views/StdcmResults.tsx 0.00% 1 Missing ⚠️
... and 3 more

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

Additional details and impacted files
@@             Coverage Diff              @@
##                dev    #8146      +/-   ##
============================================
+ Coverage     28.24%   28.45%   +0.21%     
- Complexity     2116     2123       +7     
============================================
  Files          1300     1301       +1     
  Lines        158309   157770     -539     
  Branches       3177     3213      +36     
============================================
+ Hits          44715    44897     +182     
+ Misses       111687   110935     -752     
- Partials       1907     1938      +31     
Flag Coverage Δ
core 75.43% <ø> (+0.03%) ⬆️
editoast 70.40% <ø> (-0.01%) ⬇️
front 10.21% <44.23%> (+0.18%) ⬆️
gateway 2.03% <ø> (ø)
railjson_generator 87.49% <ø> (ø)
tests 73.18% <ø> (ø)

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.

@Yohh Yohh force-pushed the yoh/speedspacechart-version-two-integration branch from c7aebcc to 34be2ec Compare July 22, 2024 15:14
@Yohh Yohh requested review from clarani and Caracol3 July 23, 2024 10:15
@Yohh Yohh force-pushed the yoh/speedspacechart-version-two-integration branch 2 times, most recently from 779bb77 to 183f438 Compare July 23, 2024 10:49
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.

So nice to finally see this new chart in osrd, thank you for that !

Just started the review but here are some comments/remarks

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.

Finished the review, thank you for the work. See comments.

Maybe I missed something but on stdcm, the speedspacechart is empty

Capture d’écran 2024-07-23 à 19 35 54

@Yohh
Copy link
Contributor Author

Yohh commented Jul 23, 2024

Finished the review, thank you for the work. See comments.

Maybe I missed something but on stdcm, the speedspacechart is empty

Capture d’écran 2024-07-23 à 19 35 54

I don't understand, it's not empty for me...
image

@Yohh Yohh marked this pull request as ready for review July 24, 2024 14:37
@Yohh Yohh requested a review from a team as a code owner July 24, 2024 14:37
@Yohh Yohh requested a review from SharglutDev July 24, 2024 14:43
@Yohh Yohh force-pushed the yoh/speedspacechart-version-two-integration branch from 5b2fcf9 to 0151b31 Compare July 24, 2024 14:53
@Yohh Yohh force-pushed the yoh/speedspacechart-version-two-integration branch 8 times, most recently from 1d0209d to 56087e8 Compare July 24, 2024 21:56
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.

Still some bugs on the speed space chart itselft (so not related to your integration PR) but Lgtm and tested, great job :)

@Yohh Yohh force-pushed the yoh/speedspacechart-version-two-integration branch from 3ec242a to 0f5c92f Compare July 25, 2024 09:56
- add SpeedSpaceChart module
- add format functions in /applications/operationalStudies/utils.st
- add _uiSpeedSpaceChart.scss, import it in style.scss
- add mmToKm() in /utils/physics.ts

- isolate new logic in SpeedSpaceChartV2.tsx
- adapt StdcmResultsV2.tsx to new SpeedSpaceChartV2 props

- manage GEV width by retrieving container width

- add translations in json files and use it in SpeedSpaceChartV2

- add tests for helpers.ts

- replace declivities with slopes in translations
- remove GevPreparedDataV2 type
- adapt formatPowerRestrictionRangesWithHandled.ts to directly hydrate the GEV V2
- fix power restriction tests
@Yohh Yohh force-pushed the yoh/speedspacechart-version-two-integration branch from 0f5c92f to ab175a5 Compare July 25, 2024 10:09
@Yohh Yohh enabled auto-merge July 25, 2024 10:10
@Yohh Yohh added this pull request to the merge queue Jul 25, 2024
Merged via the queue into dev with commit e741331 Jul 25, 2024
20 checks passed
@Yohh Yohh deleted the yoh/speedspacechart-version-two-integration branch July 25, 2024 10:29
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.

osrd-ui: GEV V2 integration in OSRD osrd-ui: add vertical enlarger button to GEV V2
7 participants