Skip to content
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.

Sign up for GitHub

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

Itinerary detail map #2258

Merged
merged 6 commits into from
Dec 11, 2024
Merged

Itinerary detail map #2258

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4a0d006
map logic
anthonyshull Dec 10, 2024
e1b2b80
maps showing lines and points in colors
anthonyshull Dec 10, 2024
d827e4b
reorder colors
anthonyshull Dec 10, 2024
841ba9c
format
anthonyshull Dec 10, 2024
f06445c
handle case where polyline is charlist
anthonyshull Dec 10, 2024
6e93a85
refactor: Move TPRS logic out of the HEEx into &render/1
joshlarson Dec 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 57 additions & 1 deletion lib/dotcom/trip_plan/map.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ defmodule Dotcom.TripPlan.Map do
@moduledoc """
Handles generating the maps displayed within the TripPlan Controller
"""

alias Dotcom.TripPlan.{Leg, NamedPosition, TransitDetail}
alias Leaflet.{MapData, MapData.Marker}
alias Leaflet.MapData.Polyline, as: LeafletPolyline
Expand All @@ -15,7 +16,6 @@ defmodule Dotcom.TripPlan.Map do
|> MapData.new(14)
end

# Maps for results
@doc """
Returns the static map data and source URL
Accepts a function that will return either a
Expand All @@ -37,6 +37,53 @@ defmodule Dotcom.TripPlan.Map do
|> MapData.add_polylines(paths)
end

@doc """
Given an `itinerary_map`, convert the polylines to lines (for maplibre-gl-js).

This involves changing some key names and inverting lat/long to long/lat.
"""
def get_lines(itinerary_map) do
itinerary_map
|> Map.get(:polylines, [])
|> Enum.map(&polyline_to_line/1)
end

@doc """
Given an `itinerary_map`, convert the markers to points (for maplibre-gl-js).

This just gets the longitude and latitude from each marker.
"""
def get_points(itinerary_map) do
itinerary_map
|> Map.get(:markers, [])
|> Enum.map(&marker_to_point/1)
|> Enum.reject(&is_nil/1)
end

defp invert_coordinates(coordinates) do
coordinates
|> Enum.map(&invert_coordinate/1)
|> Enum.reject(&is_nil/1)
end

defp invert_coordinate([a, b]), do: [b, a]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit (non-blocking): This kind of thing is exactly why I would prefer to see us standardize on %{latitude: lat, longitude: long} (or %{lat: lat, long: long} or whatever), rather than having to keep track of which intermediate objects use [long, lat] versus which ones use [lat, long].

My ideal would be that any intermediate, dotcom-controlled data is always stored as a struct, and is only translated to [lat, long] or [long, lat] at the boundary, like right before being passed into MapLibreGL, or right after being parsed out of GTFS, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is right before it is being passed into maplibre gl.


defp invert_coordinate(_), do: nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we ever get a coordinate that isn't a tuple it will crash the process otherwise.


defp marker_to_point(%{longitude: longitude, latitude: latitude}) do
[longitude, latitude]
end

defp marker_to_point(_), do: nil

defp polyline_to_line(polyline) do
%{
color: Map.get(polyline, :color) |> tailwind_color(),
coordinates: Map.get(polyline, :positions, []) |> invert_coordinates(),
width: Map.get(polyline, :weight, 4)
}
end

@spec build_leg_path(Leg.t()) :: LeafletPolyline.t()
defp build_leg_path(leg) do
color = leg_color(leg)
Expand Down Expand Up @@ -150,4 +197,13 @@ defmodule Dotcom.TripPlan.Map do
def z_index(%{current: idx, start: idx}), do: 100
def z_index(%{current: idx, end: idx}), do: 100
def z_index(%{}), do: 0

defp tailwind_color("#003DA5"), do: "blue-500"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm torn about this. Most of the other places where we translate things from GTFS to color, we do it based on the route, but here we're pulling the color out of the polyline directly.

If it turns out that we can get this to work, should we use the colors directly in otther contexts too?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The color that's in the polyline came from the associated route, it's just parsed roundaboutly because legacy....

defp tailwind_color("#80276C"), do: "purple-500"
defp tailwind_color("#00843D"), do: "green-500"
defp tailwind_color("#ED8B00"), do: "orange-500"
defp tailwind_color("#DA291C"), do: "red-500"
defp tailwind_color("#7C878E"), do: "silver-500"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📓 For now, we have to use the Metro colors because it will only accept those. We can change Metro to pass in an arbitrary hex code.


defp tailwind_color(_), do: "black"
end
48 changes: 36 additions & 12 deletions lib/dotcom_web/components/trip_planner/trip_planner_results_section.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ defmodule DotcomWeb.Components.TripPlanner.TripPlannerResultsSection do

import DotcomWeb.Components.TripPlanner.{ItineraryDetail, ItinerarySummary}

alias Dotcom.TripPlan

def trip_planner_results_section(assigns) do
~H"""
<section class={[
Expand Down Expand Up @@ -39,19 +41,41 @@ defmodule DotcomWeb.Components.TripPlanner.TripPlannerResultsSection do
</button>
</div>
</.async_result>
<.async_result :let={results} assign={@results}>
<%= if results && @itinerary_selection != :summary do %>
<% {:detail, %{itinerary_group_index: itinerary_group_index, itinerary_index: itinerary_index}} = @itinerary_selection %>
<% itinerary = Enum.at(results, itinerary_group_index).itineraries |> Enum.at(itinerary_index) %>
<% itinerary_map = TripPlan.Map.itinerary_map(itinerary) %>
<% lines = TripPlan.Map.get_lines(itinerary_map) %>
<% points = TripPlan.Map.get_points(itinerary_map) %>
<.live_component
module={MbtaMetro.Live.Map}
id="trip-planner-map"
class={[
"h-64 md:h-96 w-full",
"relative overflow-none row-span-2",
@itinerary_selection == :summary && "hidden md:block"
]}
config={@map_config}
lines={lines}
pins={[@from, @to]}
points={points}
/>
<% else %>
<.live_component
module={MbtaMetro.Live.Map}
id="trip-planner-map"
class={[
"h-64 md:h-96 w-full",
"relative overflow-none row-span-2",
@itinerary_selection == :summary && "hidden md:block"
]}
config={@map_config}
pins={[@from, @to]}
/>
<% end %>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📓 None of this will be here after we change the state management. It must be embedded like this because the results component includes the map.


<.live_component
module={MbtaMetro.Live.Map}
id="trip-planner-map"
class={[
"h-64 md:h-96 w-full",
"relative overflow-none row-span-2",
@itinerary_selection == :summary && "hidden md:block"
]}
config={@map_config}
pins={[@from, @to]}
/>

</.async_result>
<.async_result :let={results} assign={@results}>
<div :if={results} class="w-full p-4 row-start-2 col-start-1">
<.itinerary_panel results={results} itinerary_selection={@itinerary_selection} />
Expand Down
Loading