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 all 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
64 changes: 60 additions & 4 deletions 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 All @@ -48,6 +95,8 @@ defmodule Dotcom.TripPlan.Map do
end

@spec extend_to_endpoints(String.t(), Leg.t()) :: String.t()
defp extend_to_endpoints(polyline, _leg) when not is_binary(polyline), do: ""

defp extend_to_endpoints(polyline, %{from: from, to: to})
when is_map(from) and is_map(to) do
from = {Position.longitude(from), Position.latitude(from)}
Expand All @@ -59,9 +108,7 @@ defmodule Dotcom.TripPlan.Map do
|> Polyline.encode()
end

defp extend_to_endpoints(_polyline, _leg) do
""
end
defp extend_to_endpoints(_polyline, _leg), do: ""

@spec markers_for_legs([Leg.t()]) :: [Marker.t()]
defp markers_for_legs(legs) do
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("#00843D"), do: "green-500"
defp tailwind_color("#ED8B00"), do: "orange-500"
defp tailwind_color("#80276C"), do: "purple-500"
defp tailwind_color("#DA291C"), do: "red-500"
defp tailwind_color("#7C878E"), do: "silver-500"

defp tailwind_color(_), do: "black"
end
27 changes: 26 additions & 1 deletion 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,7 +8,30 @@ defmodule DotcomWeb.Components.TripPlanner.TripPlannerResultsSection do

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

def trip_planner_results_section(assigns) do
alias Dotcom.TripPlan

def trip_planner_results_section(
%{itinerary_selection: itinerary_selection, results: itinerary_results} = assigns
) do
assigns =
if itinerary_results.result && itinerary_selection != :summary do
{:detail,
%{itinerary_group_index: itinerary_group_index, itinerary_index: itinerary_index}} =
itinerary_selection

itinerary =
Enum.at(itinerary_results.result, 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)

assign(assigns, %{lines: lines, points: points})
else
assign(assigns, %{lines: [], points: []})
end

~H"""
<section class={[
"flex flex-col",
Expand Down Expand Up @@ -49,7 +72,9 @@ defmodule DotcomWeb.Components.TripPlanner.TripPlannerResultsSection do
@itinerary_selection == :summary && "hidden md:block"
]}
config={@map_config}
lines={@lines}
pins={[@from, @to]}
points={@points}
/>

<.async_result :let={results} assign={@results}>
Expand Down
Loading