Skip to content

ui-charts: display a GOV in a GET in Storybook - #11636

Merged
jacomyal merged 13 commits into
devfrom
ajy/945
May 16, 2025
Merged

ui-charts: display a GOV in a GET in Storybook#11636
jacomyal merged 13 commits into
devfrom
ajy/945

Conversation

@jacomyal

@jacomyal jacomyal commented Apr 30, 2025

Copy link
Copy Markdown
Contributor

This PR addresses issue #945. It is huge, and has been designed to be reviewed each commit one at a time.

It was originally submitted here, on the OSRD-UI former repository.

The core parts are:

  • Improving Manchette and SpaceTimeChart to ease the track-occupancy diagram integration
  • Creating a new TrackOccupancyStandalone component, that properly uses useManchetteWithSpaceTimeChart to get a track-occupancy diagram, with the SpaceTimeChart interactions (panning and zooming)
  • Adding a new story to showcase how to display a TrackOccupancyCanvas and a TrackOccupancyCanvas within a split section, using useManchetteWithSpaceTimeChart
  • Re-implementing clicking trains in the TrackOccupancyCanvas layer, using picking

@jacomyal
jacomyal requested review from SharglutDev and clarani April 30, 2025 13:54
@jacomyal
jacomyal requested a review from a team as a code owner April 30, 2025 13:54
@github-actions github-actions Bot added the area:front Work on Standard OSRD Interface modules label Apr 30, 2025
@clarani clarani changed the title front: ui-charts: display a GOV in a GET in Storybook ui-charts: display a GOV in a GET in Storybook May 5, 2025
@clarani clarani self-assigned this May 6, 2025
clarani
clarani reviewed May 6, 2025
View reviewed changes
Comment thread front/ui/ui-charts/src/spaceTimeChart/components/PathLayer.tsx
Comment thread front/ui/ui-charts/src/spaceTimeChart/components/PathLayer.tsx Outdated
Comment thread front/ui/ui-charts/src/spaceTimeChart/utils/scales.ts Outdated
Comment thread front/ui/ui-charts/src/manchette/hooks/useManchetteWithSpaceTimeChart.tsx Outdated
clarani
clarani reviewed May 7, 2025
View reviewed changes
Comment thread front/ui/ui-charts/src/trackOccupancyDiagram/components/TrackOccupancyStandalone.tsx
Comment thread front/ui/ui-charts/src/trackOccupancyDiagram/components/TrackOccupancyStandalone.tsx
Comment thread front/ui/storybook/stories/ui-charts/trackOccupancyDiagram/rendering.stories.tsx Outdated
Comment thread front/ui/storybook/stories/ui-charts/trackOccupancyDiagram/rendering.stories.tsx
Comment thread front/ui/storybook/stories/ui-charts/trackOccupancyDiagram/rendering.stories.tsx Outdated
Comment thread front/ui/ui-charts/src/trackOccupancyDiagram/components/layers/OccupancyZonesLayer.tsx Outdated
Comment thread front/ui/ui-charts/src/trackOccupancyDiagram/components/layers/OccupancyZonesLayer.tsx Outdated
Comment thread front/ui/ui-charts/src/trackOccupancyDiagram/components/layers/OccupancyZonesLayer.tsx Outdated
Comment thread front/ui/ui-charts/src/trackOccupancyDiagram/components/layers/OccupancyZonesLayer.tsx Outdated
Comment thread ...ui/ui-charts/src/trackOccupancyDiagram/components/helpers/drawElements/drawOccupancyZones.ts
SharglutDev
SharglutDev reviewed May 7, 2025
View reviewed changes
Comment thread front/ui/ui-charts/src/manchette/hooks/useManchetteWithSpaceTimeChart.tsx
Comment thread front/ui/ui-charts/src/spaceTimeChart/components/PathLayer.tsx Outdated
SharglutDev
SharglutDev reviewed May 9, 2025
View reviewed changes
Comment thread front/ui/ui-charts/src/trackOccupancyDiagram/components/TrackOccupancyStandalone.tsx Outdated
jacomyal added a commit that referenced this pull request May 12, 2025
@jacomyal
ui-charts: integrates various minor changes
5a9c2fc 
This commit addresses various discussions opened by reviewers on PR
#11636

Details:
- Adds train selection in track-occupancy.stories.tsx
- Fixes "+XXX trains" label placement in OccupancyZonesLayer
- Adds various comments and renames some variables and functions to
  improve code readability

Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
@jacomyal
jacomyal requested review from SharglutDev and clarani May 12, 2025 15:19
jacomyal added a commit that referenced this pull request May 13, 2025
@jacomyal
ui-charts: integrates various minor changes
0b07b58 
This commit addresses various discussions opened by reviewers on PR
#11636

Details:
- Adds train selection in track-occupancy.stories.tsx
- Fixes "+XXX trains" label placement in OccupancyZonesLayer
- Adds various comments and renames some variables and functions to
  improve code readability

Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
clarani
clarani reviewed May 13, 2025
View reviewed changes
Comment thread front/ui/storybook/stories/ui-charts/spaceTimeChart/helpers/paths.ts
Comment thread front/ui/storybook/stories/ui-charts/manchetteWithSpaceTimeChart/track-occupancy.stories.tsx
clarani
clarani reviewed May 13, 2025
View reviewed changes

@clarani clarani left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The more trains label is not centered anymore
image

jacomyal added a commit that referenced this pull request May 13, 2025
@jacomyal
ui-charts: integrates various minor changes
dbdaa54 
This commit addresses various discussions opened by reviewers on PR
#11636

Details:
- Adds train selection in track-occupancy.stories.tsx
- Fixes "+XXX trains" label placement in OccupancyZonesLayer
- Adds various comments and renames some variables and functions to
  improve code readability

Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
jacomyal added a commit that referenced this pull request May 13, 2025
@jacomyal
ui-charts: integrates various minor changes
565e9a2 
This commit addresses various discussions opened by reviewers on PR
#11636

Details:
- Fixes "+XXX trains" label placement in OccupancyZonesLayer, so that
  it's properly centered on displayed zones of the pack
- Replaces `XXX * KILOMETER` with `positionKmToMm(XXX)` for clarity
@jacomyal

jacomyal commented May 13, 2025

Copy link
Copy Markdown
Contributor Author

The more trains label is not centered anymore

Oh, ok, I misread your comment on the "+X trains" label coordinates. I just pushed a fix.

@jacomyal
jacomyal requested a review from clarani May 13, 2025 14:53
jacomyal added a commit that referenced this pull request May 13, 2025
@jacomyal
ui-charts: integrates various minor changes
d2f4029 
This commit addresses various discussions opened by reviewers on PR
#11636

Details:
- Adds train selection in track-occupancy.stories.tsx
- Fixes "+XXX trains" label placement in OccupancyZonesLayer
- Adds various comments and renames some variables and functions to
  improve code readability

Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
jacomyal added a commit that referenced this pull request May 13, 2025
@jacomyal
ui-charts: integrates various minor changes
793dc0f 
This commit addresses various discussions opened by reviewers on PR
#11636

Details:
- Fixes "+XXX trains" label placement in OccupancyZonesLayer, so that
  it's properly centered on displayed zones of the pack
- Replaces `XXX * KILOMETER` with `positionKmToMm(XXX)` for clarity
jacomyal added a commit that referenced this pull request May 14, 2025
@jacomyal
ui-charts: integrates various minor changes
96fef3d 
This commit addresses various discussions opened by reviewers on PR
#11636

Details:
- Adds train selection in track-occupancy.stories.tsx
- Fixes "+XXX trains" label placement in OccupancyZonesLayer
- Adds various comments and renames some variables and functions to
  improve code readability

Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
jacomyal added a commit that referenced this pull request May 14, 2025
@jacomyal
ui-charts: integrates various minor changes
e9a467d 
This commit addresses various discussions opened by reviewers on PR
#11636

Details:
- Fixes "+XXX trains" label placement in OccupancyZonesLayer, so that
  it's properly centered on displayed zones of the pack
- Replaces `XXX * KILOMETER` with `positionKmToMm(XXX)` for clarity

Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
SharglutDev
SharglutDev approved these changes May 14, 2025
View reviewed changes

@SharglutDev SharglutDev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm and tested (tested on ubuntu), great job ✅
Lets see if it works for the mac users :)

clarani
clarani approved these changes May 14, 2025
View reviewed changes

@clarani clarani left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ✅

jacomyal added 13 commits May 16, 2025 14:42
@jacomyal
ui-charts: improves splitting manchette and STC
9ee9fdc 
Details:
- Adds splitting waypoints in the operational points given to the STC
  from useManchetteWithSpaceTimeChart, and handles not rendering them in
  the STC code directly
- Adds flatSteps in the SpaceTimeChartContext, as a Set<number>
- Steps rendering path segments on pauses on flat steps, since we now
  assume they are rendered directly in the split section layer itself

Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
@jacomyal
ui-charts: makes space-time chart more resilient
9b98f31 
This commit allows the SpaceTimeChart to work with space scales that
just contain one single flat step. The idea is to allow using the
SpaceTimeChart, to only render a TrackOccupancyDiagram, while still
having the other interesting interactions (zooming and panning,
basically).

Details:
- Adds new hideTimeCaptions param
- Improves utils to allow the SpaceTimeChart to work with only one
  waypoint, and a flat step on it

Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
@jacomyal
ui-charts: makes manchette more resilient
5fd4fee 
This commit aims at making Manchette and useManchetteWithSpaceTimeChart
work with a single flat step, on a single waypoint. This will allow them
to be used to render a TrackOccupancyDiagram, with all the interesting
interactions (zooming and panning).

Details:
- Adds various options to useManchetteWithSpaceTimeChart, to allow
  disabling zooming, panning or time captions
- Allows overriding vertical padding in useManchetteWithSpaceTimeChart,
  to exactly fit one single flat step, to the pixel
- Improves helpers (and their tests) to work in the edge case where
  there is just one waypoint, and a flat step on it

Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
@jacomyal
ui-charts: adds new TrackOccupancyStandalone
1cc625c 
This commit basically replaces the previous
trackOccupancyDiagram/rendering story, with a new component,
TrackOccupancyStandalone, that actually calls
useManchetteWithSpaceTimeChart instead of mimicking it. The main benefit
is to get proper interactions, such as panning on the time axis.

Also, this commit is very impactful, because many things were done quite
customly for track occupancy drawing, when the SpaceTimeChart APIs
provide many tools to help rendering additional layers on top of them.

One final note, before getting into the detail: This commit breaks
selecting a train with the mouse on the TrackOccupancyDiagram, because
this was done in the rendering loop, rather than using picking (as it
should).

Details:
- Replaces all time and space to pixels conversions from track occupancy
  layers, with code that uses proper tools from SpaceTimeChartContext
- Removes mouse collision detection (it was done in the rendering
  process, which meant that rendering was depending on the
  mousePositions, causing many excess renderings)
- Updates OccupancyZone: id becomes trainId, and arrivalTime and
  departureTime become numbers instead of dates
- Adds new TrackOccupancyStandalone component
- Updates rendering story to use new component

Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
@jacomyal
ui-charts: fixes some manchette stories
936a85d 
Some code from useManchetteWithSpaceTimeChart stories was out of date,
this commit fixes those occurances.

Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
@jacomyal
ui-charts: normalizes millimeters in stories
659480c 
There were some weird issues due to the fact that data for the
manchette and useManchetteWithSpaceTimeChart were in millimeters, and
data for the SpaceTimeChart were in meters.

This commit migrates everything SpaceTimeChart-related to millimeters.

Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
@jacomyal
ui-charts: drafts TOD within STC story
4e200e4 
This commit addresses ticket:
osrd-project/osrd-confidential#945

Details:
- Adds possibility to add some top padding to TrackOccupancyCanvas
- Adds possibility to display a close button in the top right of the
  TrackOccupancyCanvas
- Allows giving children to TrackOccupancyManchette
- Adds Waypoint (as WaypointComponent) to exports from ui-charts
- Creates new trac-occupancy story, that shows how to display
  TrackOccupancy diagrams within split sections of the Manchette and the
  SpaceTimeChart

Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
@jacomyal
ui-charts: restores clicking trains in the TOD
9cf660b 
Details:
- Implements back clicks detection in the TrackOccupancy diagram, using
  the picking framework from the SpaceTimeDiagram
- Adds an example showing how it works in the
  trackOccupancyDiagram/rendering story

Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
@jacomyal
ui-charts: adds top and bottom borders to TOD
bc04fc7 
Details:
- Adds top and bottom borders to TrackOccupancyCanvas
- Adds top and bottom borders to TrackOccupancyManchette
- Removes these borders when using TrackOccupancyStandalone

Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
@jacomyal
ui-charts: integrates various minor changes
9312e39 
This commit addresses various discussions opened by reviewers on PR
#11636

Details:
- Adds train selection in track-occupancy.stories.tsx
- Fixes "+XXX trains" label placement in OccupancyZonesLayer
- Adds various comments and renames some variables and functions to
  improve code readability

Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
@jacomyal
ui-charts: draws times in TrackOccupancyStandalone
933a456 
This commit adds times graduations in the bottom of the
TrackOccupancyStandalone component.

Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
@jacomyal
ui-charts: fixes shift+wheel on SpaceTimeChart
cb41c5b 
Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
@jacomyal
ui-charts: integrates various minor changes
5a9e83e 
This commit addresses various discussions opened by reviewers on PR
#11636

Details:
- Fixes "+XXX trains" label placement in OccupancyZonesLayer, so that
  it's properly centered on displayed zones of the pack
- Replaces `XXX * KILOMETER` with `positionKmToMm(XXX)` for clarity

Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
@jacomyal
jacomyal added this pull request to the merge queue May 16, 2025
github-merge-queue Bot pushed a commit that referenced this pull request May 16, 2025
@jacomyal
ui-charts: integrates various minor changes
776cba0 
This commit addresses various discussions opened by reviewers on PR
#11636

Details:
- Adds train selection in track-occupancy.stories.tsx
- Fixes "+XXX trains" label placement in OccupancyZonesLayer
- Adds various comments and renames some variables and functions to
  improve code readability

Signed-off-by: Alexis Jacomy <alexis.jacomy@gmail.com>
Merged via the queue into dev with commit c654cc3 May 16, 2025
@jacomyal
jacomyal deleted the ajy/945 branch May 16, 2025 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Yohh Yohh Yohh left review comments
@Akctarus Akctarus Akctarus left review comments
@SharglutDev SharglutDev SharglutDev approved these changes
+1 more reviewer
@clarani clarani clarani approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jacomyal jacomyal

@clarani clarani

Labels

area:front Work on Standard OSRD Interface modules

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jacomyal @Yohh @clarani @Akctarus @SharglutDev