feat(SF2.0/UpcomingDepartures): Fancy labels for Commuter Rail predictions #2853
Conversation
joshlarson
force-pushed
the
jdl/sf2.0/ud/cr-fancy-labels
branch
from
7d5b675 to
cc2d9a7
Compare
joshlarson
changed the base branch from
jdl/sf2.0/ud/more-bus-and-cr-trips
to
jdl/sf2.0/ud/bus-interleave-schedule-and-prediction-data
joshlarson
force-pushed
the
jdl/sf2.0/ud/cr-fancy-labels
branch
from
a55dfbf to
8b0489a
Compare
| |> case do | ||
| nil -> nil | ||
| "Commuter Rail" -> nil | ||
| name -> name |> String.trim("Commuter Rail - ") | ||
| end |
There was a problem hiding this comment.
I have a similar function in Dotcom.ScheduleFinder, I wonder if we should start sharing here
There was a problem hiding this comment.
Yeah I was thinking the same thing.
There was a problem hiding this comment.
...however, could we treat that as a follow-up? The two implementations aren't exactly compatible with each other, and I'm not sure about blocking this (and the various PR's that have stacked up behind this) on reconciling them.
| defp arrival_substatus(%{ | ||
| predicted_schedule: %PredictedSchedule{schedule: nil} | ||
| }), | ||
| do: :on_time |
There was a problem hiding this comment.
Shouldn't this compare the time? Otherwise wouldn't we risk rendering "on time" when showing a status-less added trip that doesn't happen to be on time?
There was a problem hiding this comment.
Hmm. What should it say when a trip has been added? I guess "Added" would be a reasonable choice... I defaulted to "on time", because when a trip doesn't have a schedule, then it definitely hasn't gotten off that schedule.
Can ask Chase, and/or add this to the list of assumptions in Notion.
There was a problem hiding this comment.
Not sure, I recommend asking. IMO I don't think "Added" would be meaningful, and maybe this is a case for no status
There was a problem hiding this comment.
Chase is out of office until next week - shall we come up with a stop-gap guess until he comes back?
| }), | ||
| headsign: trip.headsign, | ||
| platform_name: platform_name(predicted_schedule), | ||
| status: predicted_schedule |> PredictedSchedule.status(), |
There was a problem hiding this comment.
Do you need this field? I'm not sure if you're using upcoming_departure.status directly (you read the same value into arrival_substatus)
There was a problem hiding this comment.
Oh oops - I added this for debugging and forgot to remove it.
Before
After
Other Examples
Off-Schedule
Status Lifted from the Prediction
Asana Ticket: [SF/UD] CR: Show train number, track number, and status when available, and HH:MM instead of N min