Add Elevation Graph Display #290
Conversation
|
I made this a PR to see if it was just some stupid GH action thing but no, that wasn't it. after a bit of digging it looks like it is an issue with the deps. in you installed. if you delete the package-lock the install fails for some other reason. seems like we might need to upgrade some packages to get everything lined up to fix this issue. |
|
@graue ^^^^ |
Looks like a configuration issue. |
|
after some finagling with libraries and labeling, the elevation graph now looks like this:
the public transit section is fixed width, and the bike sections are proportional to their distance. i still need to get the actual width of the element, and do label placement testing on phone screens |
There was a problem hiding this comment.
First, great work on the graph! It's looking good 🙌
I found this code hard to follow, and after half an hour of close reading I'm realizing that much of the complexity comes from interpolating elevation values that the router doesn't provide for PT legs (which was not at all obvious on first read).
Can we modify the router to provide those elevation values so this is unnecessary?
If not, could we do a first pass just to interpolate the values and nothing else, to make this simpler?
src/components/RoutesOverview.jsx
Outdated
| @@ -125,7 +125,7 @@ export default function RoutesOverview({ | |||
| className="stroke-2 text-gray-600" | |||
| /> | |||
| </Icon> | |||
| {formatDistance(route.ascend, intl)} | |||
| {formatDistance(route.ascend, intl, true)} | |||
There was a problem hiding this comment.
Would prefer an options hash and {forceFeet: true} so we don't have to look up what this means
Even better how about {isElevation: true}? Or a separate formatElevation? Since the judgement that using miles is not appropriate for vertical distances properly belongs to the formatDistance function. This also keeps it agnostic to imperial vs metric
| }; | ||
|
|
||
| const axisLeft = { | ||
| legend: 'Feet', |
There was a problem hiding this comment.
This looks like UI text so it needs to be translated. See Itinerary for examples of using intl.formatMessage
abhumbla
force-pushed
the
elevation-graph
branch
from
24d42ec to
9f12f77
Compare
abhumbla
force-pushed
the
elevation-graph
branch
from
b37b739 to
7715c33
Compare
Fixes #253 #285
Still needs better styling, will work on that before opening up to full review