-
Notifications
You must be signed in to change notification settings - Fork 52
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
Add time to map itin summary overlay #1254
Add time to map itin summary overlay #1254
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really good!
@@ -57,7 +70,8 @@ const Card = styled.div` | |||
} | |||
} | |||
div { | |||
margin-top: -0px!important; | |||
margin-top: -0px !important; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I doing think this -
is doing anything!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch haha
@@ -112,7 +126,8 @@ function getUniquePoint( | |||
|
|||
const selfDistance = distance(point, centerOfLine) | |||
// maximize distance from all other points while minimizing distance to center of our own line | |||
const averageDistance = totalDistance / otherMidpoints.length - selfDistance | |||
const averageDistance = | |||
1.2 * (totalDistance / otherMidpoints.length) - selfDistance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this 1.2 do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This prioritizes the labels not overlapping slightly above them being in the center of the line. I'm not super happy with this value, so we'll probably tweak it more in the future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description:
Adds a time duration to the itinerary summary overlay! Follows SEPTA style
PR Checklist: