-
Notifications
You must be signed in to change notification settings - Fork 16
feat(SF2.0/UpcomingDepartures): Add vehicle status to the top of the trip details section #2870
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
base: main
Are you sure you want to change the base?
Conversation
dfe1ad4 to
c3757b1
Compare
| defstruct [ | ||
| :stops | ||
| ] | ||
|
|
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 recommend adding a @type spec here and to TripStop, as it'll make it clearer that stops is actually a list of TripStop
@type t :: %__MODULE__{
stops: [__MODULE__.TripStop.t()]
}
# in TripStop
@type t :: %__MODULE__{
cancelled?: boolean(),
# and the rest
}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.
| @@ -0,0 +1,43 @@ | |||
| defmodule Dotcom.ScheduleFinder.TripDetails do | |||
| @moduledoc """ | |||
| A struct representing trip details, including a list of stops visited before and after | |||
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.
You technically took away the stops before/after distinction here, so this comment is a little confusing 😅
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.
| time: PredictedSchedule.display_time(ps) | ||
| } | ||
| end) | ||
| |> Enum.sort_by(& &1.time) |
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.
|> Enum.sort_by(& &1.time, DateTime)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.
do we need to reject timeless predictions here? 🤔
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.
Fixed the sorting, and added a test that I think shows that we don't need to reject timeless predictions
| do: "Now at #{stop_name}" | ||
|
|
||
| defp vehicle_message(nil), | ||
| do: "Finishing another trip" |
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.
don't forget translation here!
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.
Too late - I forgot!
In both cases, before on the left; after on the right.
Things to note:
Asana Ticket: [SF/UD] All Modes: Show vehicle status at the top of trip details