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: rework layer activation #5607

Merged
merged 1 commit into from
Nov 13, 2023
Merged

Conversation

SarahBellaha
Copy link
Contributor

@SarahBellaha SarahBellaha commented Nov 6, 2023

Closes #5483
Closes #5593 (same as #5306)
Closes #5644

@SarahBellaha SarahBellaha added kind:bug Something isn't working area:front Work on Standard OSRD Interface modules module:infra-editor Infra Edition labels Nov 6, 2023
@SarahBellaha SarahBellaha requested a review from a team as a code owner November 6, 2023 16:05
Copy link

codecov bot commented Nov 6, 2023

Codecov Report

Merging #5607 (43e9d5c) into dev (0de0f5b) will increase coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##                dev    #5607   +/-   ##
=========================================
  Coverage     19.57%   19.57%           
  Complexity     2322     2322           
=========================================
  Files           886      886           
  Lines        105832   105818   -14     
  Branches       2572     2572           
=========================================
  Hits          20716    20716           
+ Misses        83607    83593   -14     
  Partials       1509     1509           
Flag Coverage Δ
front 8.30% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
front/src/applications/editor/nav.tsx 0.00% <0.00%> (ø)
...ls/rangeEdition/catenary/CatenaryEditionLayers.tsx 0.00% <0.00%> (ø)
front/src/applications/editor/Editor.tsx 0.00% <0.00%> (ø)
...Edition/speedSection/SpeedSectionEditionLayers.tsx 0.00% <0.00%> (ø)
...src/applications/editor/components/LayersModal.tsx 0.00% <0.00%> (ø)

@SarahBellaha SarahBellaha linked an issue Nov 7, 2023 that may be closed by this pull request
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.

Thank you for this PR !

Noticed some problems while testing :

  • on left panel, when opening the electrification or speed limit form (which are not rendered like the rest of the editor), the layer is not activated. In fact, it seems to hide all the current activated layers.

image

  • if the layer pops when toggling the switch. Is it still relevant to have the cancel and confirm button ?

image

  • in the layers modal, we can for example activate the signal layer without the track section layer whereas this behavior is different when clicking in the left panel (track sections seems to be required to display signals)

image

@SarahBellaha SarahBellaha force-pushed the sba/front/rework-layer-activation branch from 313c555 to a9f622f Compare November 9, 2023 12:28
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.

Good job for the bug fix of catenary and speed limit layer !
Left 2 comments and you need to remove both unused useMemo

@SarahBellaha SarahBellaha force-pushed the sba/front/rework-layer-activation branch 2 times, most recently from 95ac784 to dad9c1c Compare November 9, 2023 13:43
@SharglutDev
Copy link
Contributor

Seen with PO, we can remove the "Confirm" and "Cancel" button from the layers modal

@SarahBellaha SarahBellaha force-pushed the sba/front/rework-layer-activation branch from 16f64c5 to 0a724fd Compare November 10, 2023 20: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.

Lgtm. Tested in local, everything works great, good job :)

Copy link
Contributor

@clarani clarani left a comment

Choose a reason for hiding this comment

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

Left only one comment, very very nice work ! 🎉

Copy link
Contributor

@clarani clarani left a comment

Choose a reason for hiding this comment

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

LGTM !! 🎉

@SarahBellaha SarahBellaha force-pushed the sba/front/rework-layer-activation branch from 4b9716f to 43e9d5c Compare November 13, 2023 13:36
@SarahBellaha SarahBellaha added this pull request to the merge queue Nov 13, 2023
Merged via the queue into dev with commit 2ed39a8 Nov 13, 2023
@SarahBellaha SarahBellaha deleted the sba/front/rework-layer-activation branch November 13, 2023 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:front Work on Standard OSRD Interface modules kind:bug Something isn't working module:infra-editor Infra Edition
Projects
None yet
3 participants