WIP: feat(TripPlanner): transit leg and itinerary locations #2251
Conversation
okay, not quite there yet heh
|
Could we do something even dumber than that, and just have some logic at time-of-rendering that tells a walking segment not to render the locations at either end unless it's the beginning or end of an itinerary? |
There was a problem hiding this comment.
This looks so great right now!
I have a thought about <.place />. What if, instead of wrapping each <.leg /> in two places (which guarantees that all places will get doubled up, as it currently is), we wrapped only transit legs with <.place />'s and left walking legs place-less. We would then need to add a location-dot-icon place at the beginning and the end of the whole itinerary.
(If we do ☝️then it probably makes sense to have a <.transit_leg /> include its places and leave them out of the main itinerary_detail function, but that's kind of a minor thing.)
In order to get that fully 100% correct, I think we'd need to add a data transformation layer to solve (at least) two corner-cases:
- Transferring without walking, which can happen if you need to transfer at a bus stop that serves multiple routes, or if you need to switch green line trains in the Central Subway. (This would involve having a "waiting leg", which hasn't been fully designed yet)
- If the itinerary starts or ends exactly at a station or stop, then we don't want to double up the locations (like in the screenshot below)
IMO ☝️ these corner cases and the data transformation layer can happen as fast-follow-up PR's, rather than getting lumped into this one. If we get rid of the guaranteed duplication of <.place />'s, then I'm good to merge this.
TL;DR - there are a bunch of corner cases that we need to address before releasing, but can happen as follow-up PR's. For this PR, I'm good to merge once we get rid of the duplicated <.place />'s?
| |> assign(:start_place, List.first(assigns.itinerary.legs).from) | ||
| |> assign(:start_time, List.first(assigns.itinerary.legs).start) | ||
| |> assign(:end_place, List.last(assigns.itinerary.legs).to) | ||
| |> assign(:end_time, List.last(assigns.itinerary.legs).stop) |
There was a problem hiding this comment.
This whole blob is kind of gross, but @thecristen and I talked offline, and we're going to remove it in a fast-follow PR, so that this one doesn't get bloated.
There was a problem hiding this comment.
Side note, at least the variable name lengths decrement by one letter per line, so they form a nice shape.
Summary of changes
Asana Ticket: Trip Planner Preview | Details: Transit segment and Trip Planner Preview | Details: Location Item
Somewhat rough around the edges and still missing some acceptance criteria, but wanted to get a chance for review before leaving for a couple days. Notable items remaining:
OpenTripPlannerClientOpenTripPlannerClientto give us the stop'slocationType.. then we could read that value to decide which icon to render.