Skip to content

Commit

Permalink
feat: Show "Depart at" buttons and show the correct itinerary on click (
Browse files Browse the repository at this point in the history
#2238)

* prefactor: Convert <.itinerary_detail /> into a live component

And change its input from `itinerary` to `itineraries` so that it can
be responsible for choosing which itinerary to display

* feat: Show "Depart at" buttons and show the correct itinerary on click

* style: Style depart-at buttons correctly (entirely with Tailwind)

* refactor: Extract <.depart_at_button /> as a sub-component

* style: Format the "Depart at" text correctly

* cleanup: Remove some useless assigns

* tests: Add tests that validate itinerary-toggling behavior

* refactor/cleanup: Rename selected_trip_index to selected_itinerary_index

* fix: Remove TODO tag, which was causing Credo to fail

* refactor: Convert itinerary_details to a function component

* tests: Co-locate exlicit testing values

* cleanup: Make test helper functions private

* fix: Don't preserve selected_itinerary_detail_index when hiding and re-showing an itinerary

* tests: Use existing factory for otp_itineraries rather than calling limit_route_types each time

* cleanup: Use faker for trip times and trip ID's in tests
  • Loading branch information
joshlarson authored Dec 3, 2024
1 parent fc582ba commit 5ab1881
Show file tree
Hide file tree
Showing 5 changed files with 196 additions and 23 deletions.
3 changes: 2 additions & 1 deletion assets/tailwind.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ module.exports = {
},
"brand-primary": {
DEFAULT: "#165c96",
darkest: "#0b2f4c"
darkest: "#0b2f4c",
lightest: "#cee0f4"
},
"logan-express": {
BB: "#f16823",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ defmodule DotcomWeb.Components.LiveComponents.TripPlannerResultsSection do

@impl true
def mount(socket) do
{:ok, socket |> assign(:expanded_itinerary_index, nil)}
{:ok,
socket
|> assign(:expanded_itinerary_index, nil)
|> assign(:selected_itinerary_detail_index, 0)}
end

@impl true
Expand Down Expand Up @@ -64,6 +67,7 @@ defmodule DotcomWeb.Components.LiveComponents.TripPlannerResultsSection do
<.itinerary_panel
results={results}
details_index={@expanded_itinerary_index}
selected_itinerary_detail_index={@selected_itinerary_detail_index}
target={@myself}
/>
</div>
Expand Down Expand Up @@ -117,7 +121,11 @@ defmodule DotcomWeb.Components.LiveComponents.TripPlannerResultsSection do
<.itinerary_summary summary={@summary} />
</div>
<.itinerary_detail :for={itinerary <- @itineraries} itinerary={itinerary} />
<.itinerary_detail
itineraries={@itineraries}
selected_itinerary_detail_index={@selected_itinerary_detail_index}
target={@target}
/>
</div>
"""
end
Expand All @@ -141,7 +149,17 @@ defmodule DotcomWeb.Components.LiveComponents.TripPlannerResultsSection do
_ -> nil
end

{:noreply, socket |> assign(:expanded_itinerary_index, index)}
{:noreply,
socket
|> assign(:expanded_itinerary_index, index)
|> assign(:selected_itinerary_detail_index, 0)}
end

@impl true
def handle_event("set_selected_itinerary_detail_index", %{"trip-index" => index_str}, socket) do
{index, ""} = Integer.parse(index_str)

{:noreply, socket |> assign(:selected_itinerary_detail_index, index)}
end

defp format_datetime_short(datetime) do
Expand Down
64 changes: 58 additions & 6 deletions lib/dotcom_web/components/trip_planner/itinerary_detail.ex
Original file line number Diff line number Diff line change
@@ -1,14 +1,66 @@
defmodule DotcomWeb.Components.TripPlanner.ItineraryDetail do
@moduledoc """
A component to render an itinerary in detail..
The section of the trip planner page that shows the map and
the summary or details panel
"""

use DotcomWeb, :component

import DotcomWeb.Components.TripPlanner.Leg

alias Dotcom.TripPlan.PersonalDetail

def itinerary_detail(assigns) do
def itinerary_detail(
%{
itineraries: itineraries,
selected_itinerary_detail_index: selected_itinerary_detail_index
} = assigns
) do
assigns =
assign(assigns, :selected_itinerary, Enum.at(itineraries, selected_itinerary_detail_index))

~H"""
<div>
<p class="text-sm mb-2 mt-3">Depart at</p>
<div class="flex">
<.depart_at_button
:for={{itinerary, index} <- Enum.with_index(@itineraries)}
active={@selected_itinerary_detail_index == index}
phx-click="set_selected_itinerary_detail_index"
phx-value-trip-index={index}
phx-target={@target}
>
<%= Timex.format!(itinerary.start, "%-I:%M%p", :strftime) %>
</.depart_at_button>
</div>
<.specific_itinerary_detail itinerary={@selected_itinerary} />
</div>
"""
end

attr :active, :boolean
attr :rest, :global
slot :inner_block

defp depart_at_button(%{active: active} = assigns) do
background_class = if active, do: "bg-brand-primary-lightest", else: "bg-transparent"
assigns = assign(assigns, :background_class, background_class)

~H"""
<button
type="button"
class={[
"border border-brand-primary rounded px-2.5 py-1.5 mr-2 text-brand-primary text-lg",
"hover:bg-brand-primary-lightest #{@background_class}"
]}
{@rest}
>
<%= render_slot(@inner_block) %>
</button>
"""
end

defp specific_itinerary_detail(assigns) do
assigns =
assign(
assigns,
Expand All @@ -19,11 +71,11 @@ defmodule DotcomWeb.Components.TripPlanner.ItineraryDetail do
)

~H"""
<details class="mt-4">
<summary class="border border-2 border-brand-primary px-3 py-2 flex items-center hover:border-brand-primary-darkest hover:bg-gray-lighter">
<div class="mt-4">
<div>
Depart at <%= Timex.format!(@itinerary.start, "%-I:%M%p", :strftime) %>
<.route_symbol :for={route <- @all_routes} route={route} class="ml-2" />
</summary>
</div>
<div :for={leg <- @itinerary.legs}>
<.leg
start_time={leg.start}
Expand All @@ -35,7 +87,7 @@ defmodule DotcomWeb.Components.TripPlanner.ItineraryDetail do
realtime_state={leg.realtime_state}
/>
</div>
</details>
</div>
"""
end
end
123 changes: 110 additions & 13 deletions test/dotcom_web/live/trip_planner_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,33 @@ defmodule DotcomWeb.Live.TripPlannerTest do
import Mox
import Phoenix.LiveViewTest

alias Test.Support.Factories.TripPlanner.TripPlanner, as: TripPlannerFactory

setup :verify_on_exit!

defp stub_otp_results(itineraries) do
expect(OpenTripPlannerClient.Mock, :plan, fn _ ->
{:ok, %OpenTripPlannerClient.Plan{itineraries: itineraries}}
end)
end

defp stub_populated_otp_results() do
itineraries = TripPlannerFactory.build_list(3, :otp_itinerary)

stub_otp_results(itineraries)
end

defp update_trip_details(itinerary, trip_id: trip_id, start_time: start_time) do
updated_transit_leg =
itinerary.legs
|> Enum.at(1)
|> update_in([:trip, :gtfs_id], fn _ -> "ma_mbta_us:#{trip_id}" end)

itinerary
|> Map.update!(:legs, &List.replace_at(&1, 1, updated_transit_leg))
|> Map.put(:start, DateTime.new!(Date.utc_today(), start_time, "America/New_York"))
end

test "Preview version behind basic auth", %{conn: conn} do
conn = get(conn, ~p"/preview/trip-planner")

Expand Down Expand Up @@ -96,9 +121,7 @@ defmodule DotcomWeb.Live.TripPlannerTest do
}
}

expect(OpenTripPlannerClient.Mock, :plan, fn _ ->
{:ok, %OpenTripPlannerClient.Plan{itineraries: []}}
end)
stub_otp_results([])

{:ok, view, _html} = live(conn, ~p"/preview/trip-planner?#{params}")

Expand All @@ -115,16 +138,6 @@ defmodule DotcomWeb.Live.TripPlannerTest do
Test.Support.Factories.Stops.Stop.build(:stop)
end)

# Uhhh the OTP factory will generate with any route_type value but our
# parsing will break with unexpected route types
itineraries =
OpenTripPlannerClient.Test.Support.Factory.build_list(3, :itinerary)
|> Enum.map(&Test.Support.Factories.TripPlanner.TripPlanner.limit_route_types/1)

expect(OpenTripPlannerClient.Mock, :plan, fn _ ->
{:ok, %OpenTripPlannerClient.Plan{itineraries: itineraries}}
end)

%{
conn:
put_req_header(
Expand All @@ -144,12 +157,16 @@ defmodule DotcomWeb.Live.TripPlannerTest do
end

test "starts out with no 'View All Options' button", %{conn: conn, params: params} do
stub_populated_otp_results()

{:ok, view, _html} = live(conn, ~p"/preview/trip-planner?#{params}")

refute render_async(view) =~ "View All Options"
end

test "clicking 'Details' button opens details view", %{conn: conn, params: params} do
stub_populated_otp_results()

{:ok, view, _html} = live(conn, ~p"/preview/trip-planner?#{params}")

render_async(view)
Expand All @@ -162,6 +179,8 @@ defmodule DotcomWeb.Live.TripPlannerTest do
conn: conn,
params: params
} do
stub_populated_otp_results()

{:ok, view, _html} = live(conn, ~p"/preview/trip-planner?#{params}")

render_async(view)
Expand All @@ -171,5 +190,83 @@ defmodule DotcomWeb.Live.TripPlannerTest do

refute render_async(view) =~ "View All Options"
end

test "'Depart At' buttons toggle which itinerary to show", %{
conn: conn,
params: params
} do
trip_datetime_1 = Faker.DateTime.forward(2)
trip_time_1 = trip_datetime_1 |> DateTime.to_time()
trip_id_1 = Faker.UUID.v4()

trip_datetime_2 = trip_datetime_1 |> DateTime.shift(hour: 1)
trip_time_2 = trip_datetime_2 |> DateTime.to_time()
trip_time_display_2 = trip_time_2 |> Timex.format!("%-I:%M", :strftime)
trip_id_2 = Faker.UUID.v4()

base_itinerary = TripPlannerFactory.build(:otp_itinerary)

# Right now, the headsign (which is what we actually want to
# show) is not available from OTP client, but we're rendering
# the trip ID in its place. Once the headsign is available, we
# should update these updates and the assertions below to use
# the headsign instead of the trip ID.
stub_otp_results([
update_trip_details(base_itinerary, trip_id: trip_id_1, start_time: trip_time_1),
update_trip_details(base_itinerary, trip_id: trip_id_2, start_time: trip_time_2)
])

{:ok, view, _html} = live(conn, ~p"/preview/trip-planner?#{params}")

render_async(view)

view |> element("button", "Details") |> render_click()

assert render_async(view) =~ trip_id_1
refute render_async(view) =~ trip_id_2

view |> element("button", trip_time_display_2) |> render_click()

assert render_async(view) =~ trip_id_2
refute render_async(view) =~ trip_id_1
end

test "'Depart At' button state is not preserved when leaving details view", %{
conn: conn,
params: params
} do
trip_datetime_1 = Faker.DateTime.forward(2)
trip_time_1 = trip_datetime_1 |> DateTime.to_time()
trip_id_1 = Faker.UUID.v4()

trip_datetime_2 = trip_datetime_1 |> DateTime.shift(hour: 1)
trip_time_2 = trip_datetime_2 |> DateTime.to_time()
trip_time_display_2 = trip_time_2 |> Timex.format!("%-I:%M", :strftime)
trip_id_2 = Faker.UUID.v4()

base_itinerary = TripPlannerFactory.build(:otp_itinerary)

# Right now, the headsign (which is what we actually want to
# show) is not available from OTP client, but we're rendering
# the trip ID in its place. Once the headsign is available, we
# should update these updates and the assertions below to use
# the headsign instead of the trip ID.
stub_otp_results([
update_trip_details(base_itinerary, trip_id: trip_id_1, start_time: trip_time_1),
update_trip_details(base_itinerary, trip_id: trip_id_2, start_time: trip_time_2)
])

{:ok, view, _html} = live(conn, ~p"/preview/trip-planner?#{params}")

render_async(view)

view |> element("button", "Details") |> render_click()
view |> element("button", trip_time_display_2) |> render_click()
view |> element("button", "View All Options") |> render_click()
view |> element("button", "Details") |> render_click()

assert render_async(view) =~ trip_id_1
refute render_async(view) =~ trip_id_2
end
end
end
5 changes: 5 additions & 0 deletions test/support/factories/trip_planner/trip_planner.ex
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ defmodule Test.Support.Factories.TripPlanner.TripPlanner do
|> Parser.parse()
end

def otp_itinerary_factory do
Factory.build(:itinerary)
|> limit_route_types()
end

def leg_factory do
[:walking_leg, :transit_leg]
|> Faker.Util.pick()
Expand Down

0 comments on commit 5ab1881

Please sign in to comment.