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: quick fix for speed label #5531

Merged
merged 3 commits into from
Nov 27, 2023
Merged

Conversation

alexandredamiron
Copy link
Contributor

#closes #5496

This is a quick fix which will remove badly formatted time and give a shadow to the speed to it will remained visible

Correct behavior correction need a rewrite of train display logic

@alexandredamiron alexandredamiron requested a review from a team as a code owner October 30, 2023 19:21
@codecov
Copy link

codecov bot commented Oct 30, 2023

Codecov Report

Attention: 15 lines in your changes are missing coverage. Please review.

Comparison is base (d800049) 19.61% compared to head (714ad17) 19.62%.
Report is 6 commits behind head on dev.

Files Patch % Lines
...onents/SimulationResultsMap/TrainHoverPosition.tsx 0.00% 15 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                dev    #5531      +/-   ##
============================================
+ Coverage     19.61%   19.62%   +0.01%     
  Complexity     2332     2332              
============================================
  Files           907      908       +1     
  Lines        107693   107847     +154     
  Branches       2623     2626       +3     
============================================
+ Hits          21124    21169      +45     
- Misses        85013    85119     +106     
- Partials       1556     1559       +3     
Flag Coverage Δ
front 8.63% <0.00%> (+0.03%) ⬆️

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.

@SharglutDev
Copy link
Contributor

Seems your changes are not taken into account when testing. You probably need to fix the conflict first.

@alexandredamiron alexandredamiron force-pushed the adn/fixMapTrainSpeedLabel branch from e5afd67 to 8830e7c Compare November 3, 2023 16:23
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.

Is the contrast enough ? Maybe we should ask the UX what he thinks

image

@thibautsailly
Copy link

The text-shadow should have a spread value to increase contrast:
text-shadow: 0 0 0 rgba(255,255,255,0.83);
Screenshot 2023-11-16 at 15 43 39

@alexandredamiron alexandredamiron force-pushed the adn/fixMapTrainSpeedLabel branch from 8830e7c to 2d6e172 Compare November 16, 2023 14:52
@alexandredamiron
Copy link
Contributor Author

@thibautsailly I have to succes implementing that

Capture d’écran 2023-11-16 180814

@nicolaswurtz
Copy link
Contributor

Thx ! But why remove time ? time is as necessary as speed, could you let (and correct format if needed) time ?

@nicolaswurtz nicolaswurtz self-assigned this Nov 23, 2023
@alexandredamiron alexandredamiron force-pushed the adn/fixMapTrainSpeedLabel branch from 2d6e172 to a8c8974 Compare November 23, 2023 13:57
@nicolaswurtz nicolaswurtz self-requested a review November 24, 2023 16:49
@SharglutDev
Copy link
Contributor

I still don't see @thibautsailly proposal

image

@alexandredamiron alexandredamiron added this pull request to the merge queue Nov 27, 2023
Merged via the queue into dev with commit 8be84ad Nov 27, 2023
@alexandredamiron alexandredamiron deleted the adn/fixMapTrainSpeedLabel branch November 27, 2023 14:10
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.

Weird display next to the train on the map
4 participants