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

font: display kps #5402

Merged
merged 9 commits into from
Oct 25, 2023
Merged

font: display kps #5402

merged 9 commits into from
Oct 25, 2023

Conversation

nicolaswurtz
Copy link
Contributor

@nicolaswurtz nicolaswurtz commented Oct 19, 2023

Close #5222

It should display KP on all maps for:

  • detectors
  • bufferstops
  • signals
  • psl signs (broken on angle)

Operational points kps should be displayed when backend will be updated.

@nicolaswurtz nicolaswurtz requested a review from a team as a code owner October 19, 2023 12:49
@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

Merging #5402 (7246a6c) into dev (f7fe126) will decrease coverage by 0.03%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##                dev    #5402      +/-   ##
============================================
- Coverage     18.77%   18.74%   -0.03%     
  Complexity     2317     2317              
============================================
  Files           856      857       +1     
  Lines        103023   103177     +154     
  Branches       2403     2404       +1     
============================================
  Hits          19344    19344              
- Misses        82338    82491     +153     
- Partials       1341     1342       +1     
Flag Coverage Δ
front 7.10% <0.00%> (-0.02%) ⬇️

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

Files Coverage Δ
front/src/common/Map/Settings/MapSettings.tsx 0.00% <ø> (ø)
...src/common/Map/Layers/extensions/SNCF/SNCF_PSL.tsx 0.00% <0.00%> (ø)
front/src/common/Map/Layers/Detectors.tsx 0.00% <0.00%> (ø)
...ont/src/common/Map/Settings/MapSettingsSignals.tsx 0.00% <0.00%> (ø)
front/src/common/Map/Layers/BufferStops.tsx 0.00% <0.00%> (ø)
front/src/common/Map/Layers/Signals.tsx 0.00% <0.00%> (ø)
...mmon/Map/Layers/extensions/SNCF/SNCF_PSL_SIGNS.tsx 0.00% <0.00%> (ø)
front/src/common/Map/Layers/GeoJSONs.tsx 0.00% <0.00%> (ø)
...ront/src/common/Map/Settings/MapSettingsLayers.tsx 0.00% <0.00%> (ø)
front/src/common/Map/Layers/OperationalPoints.tsx 0.00% <0.00%> (ø)
... and 2 more

@sim51
Copy link
Contributor

sim51 commented Oct 19, 2023

Is it useful to have them at this level of zoom ?

image
image

Should we have the PK also on the editor ?

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.

Tested in local and it seems that even when the signals layer is disabled, the PK from the signal is still displayed.

image

@nicolaswurtz nicolaswurtz changed the title Nwz/show pks font: display kps Oct 23, 2023
@nicolaswurtz
Copy link
Contributor Author

Is it useful to have them at this level of zoom ?

image image

Should we have the PK also on the editor ?

Corrected !

@nicolaswurtz
Copy link
Contributor Author

Tested in local and it seems that even when the signals layer is disabled, the PK from the signal is still displayed.

image

corrected

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, nice feature !

@sim51
Copy link
Contributor

sim51 commented Oct 24, 2023

You replaced the map item's label (generally the item's ID) for Detector & BufferStop by the KP value.

For me (as a not rails guy), it was hard to figure out that the label is the KP and not the ID/name of the object.
Can we prefix the value by KP ?

Moreover on signals, KPs are displayed just under the signal name. So there is 2 ways of how a KP is displayed on the map.

@nicolaswurtz nicolaswurtz force-pushed the nwz/show-pks branch 2 times, most recently from 9db3e61 to 82d0031 Compare October 24, 2023 14:32
@nicolaswurtz
Copy link
Contributor Author

nicolaswurtz commented Oct 24, 2023

You replaced the map item's label (generally the item's ID) for Detector & BufferStop by the KP value.
For me (as a not rails guy), it was hard to figure out that the label is the KP and not the ID/name of the object. Can we prefix the value by KP ?

As the KP becomes a label "KP" mention is useless because it's firstly a label.

Moreover on signals, KPs are displayed just under the signal name. So there is 2 ways of how a KP is displayed on the map.

KPs are centered under centered signals, lefted or righted along signal's position.

@sim51
Copy link
Contributor

sim51 commented Oct 24, 2023

As discussed with @nicolaswurtz :

  • KP as label makes sense for rails expert (also without the KP prefix).
  • There is a general bug of signals, so my comment on the 'stops' is not linked to this PR

@nicolaswurtz nicolaswurtz added this pull request to the merge queue Oct 25, 2023
Merged via the queue into dev with commit c5ea4c2 Oct 25, 2023
@nicolaswurtz nicolaswurtz deleted the nwz/show-pks branch October 25, 2023 17:38
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.

front: display PK in the carto
4 participants