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

Add the new metro map component to the preview trip planner #2208

Merged
merged 15 commits into from
Nov 5, 2024
Merged

Add the new metro map component to the preview trip planner #2208

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
c7d9057
working locally
anthonyshull Oct 31, 2024
b14cf0b
reset configs for non local and add tw content so spinners show
anthonyshull Oct 31, 2024
24ce059
remove empty lines
anthonyshull Oct 31, 2024
2275a5b
remove test for map and add fallback function for pin update
anthonyshull Oct 31, 2024
bb43459
csp
anthonyshull Nov 1, 2024
2cce169
use atom instead of integer
anthonyshull Nov 1, 2024
7e78cbc
fix csp
anthonyshull Nov 1, 2024
0a9c8eb
another csp attempt
anthonyshull Nov 1, 2024
7d00259
another csp attempt
anthonyshull Nov 1, 2024
ded5eba
rename arg
anthonyshull Nov 1, 2024
c4b2b45
rename arg
anthonyshull Nov 1, 2024
a850675
add arcgis back in
anthonyshull Nov 4, 2024
7ad0dd0
pr feedback
anthonyshull Nov 4, 2024
fb4fdaf
default pin update functions
anthonyshull Nov 5, 2024
63626ad
lowering jest threshold since it is failing and i havent edited any j…
anthonyshull Nov 5, 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
1 change: 1 addition & 0 deletions assets/css/app.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
border-style: solid; /* 2 */
border-width: 0; /* 2 */
box-sizing: border-box; /* 1 */
--tw-content: '' !important;
}

@import '../../deps/mbta_metro/priv/static/assets/default.css';
Expand Down
8 changes: 4 additions & 4 deletions assets/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion assets/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
"leaflet": "^1.4.0",
"leaflet-rotatedmarker": "^0.2.0",
"lodash": "^4.17.21",
"mbta_metro": "^0.0.50",
"mbta_metro": "^0.0.51",
"mobile-detect": "^1.4.5",
"phoenix": "file:../deps/phoenix",
"phoenix_html": "file:../deps/phoenix_html",
Expand Down
26 changes: 26 additions & 0 deletions config/config.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,30 @@ for config_file <- Path.wildcard("config/{deps,dotcom}/*.exs") do
import_config("../#{config_file}")
end

config :mbta_metro, :map, %{
center: [-71.0589, 42.3601],
style: %{
"version" => 8,
"sources" => %{
"raster-tiles" => %{
"type" => "raster",
"tiles" => ["https://mbta-map-tiles-dev.s3.amazonaws.com/osm_tiles/{z}/{x}/{y}.png"],
"tileSize" => 256,
"attribution" =>
"&copy; <a href=\"https://www.openstreetmap.org/copyright\">OpenStreetMap</a>"
}
},
"layers" => [
%{
"id" => "mbta-tiles",
"type" => "raster",
"source" => "raster-tiles",
"minzoom" => 9,
"maxzoom" => 18
}
]
},
zoom: 14
}

import_config "#{config_env()}.exs"
5 changes: 3 additions & 2 deletions config/runtime.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,9 @@ case config_env() do
"style-src 'self' 'unsafe-inline' www.gstatic.com #{System.get_env("STATIC_HOST", "")} cdn.jsdelivr.net",
"script-src 'self' 'unsafe-eval' 'unsafe-inline' #{System.get_env("STATIC_HOST", "")} insitez.blob.core.windows.net snap.licdn.com connect.facebook.net www.instagram.com www.google-analytics.com *.google.com www.gstatic.com www.googletagmanager.com *.googleapis.com data.mbta.com *.arcgis.com",
"font-src 'self' #{System.get_env("STATIC_HOST", "")}",
"connect-src 'self' wss://#{host} #{sentry_dsn_host || ""} *.googleapis.com analytics.google.com www.google-analytics.com www.google.com px.ads.linkedin.com stats.g.doubleclick.net *.arcgis.com",
"frame-src 'self' data.mbta.com www.youtube.com www.google.com cdn.knightlab.com livestream.com www.instagram.com"
"connect-src 'self' wss://#{host} #{sentry_dsn_host || ""} *.googleapis.com analytics.google.com www.google-analytics.com www.google.com px.ads.linkedin.com stats.g.doubleclick.net *.arcgis.com *.s3.amazonaws.com",
"frame-src 'self' data.mbta.com www.youtube.com www.google.com cdn.knightlab.com livestream.com www.instagram.com *.arcgis.com",
"worker-src blob: ;"
],
"; "
)
Expand Down
7 changes: 5 additions & 2 deletions lib/dotcom_web/components/live_components/trip_planner_form.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ defmodule DotcomWeb.Components.LiveComponents.TripPlannerForm do
for={@form}
method="get"
phx-submit="save_form"
phx-change="validate"
phx-change="handle_change"
phx-target={@myself}
>
<div :for={field <- [:from, :to]} class="mb-1" id="trip-planner-locations" phx-update="ignore">
Expand Down Expand Up @@ -165,7 +165,9 @@ defmodule DotcomWeb.Components.LiveComponents.TripPlannerForm do
{:noreply, new_socket}
end

def handle_event("validate", %{"input_form" => params}, socket) do
def handle_event("handle_change", %{"input_form" => params}, socket) do
send(self(), {:changed_form, params})

form =
params
|> InputForm.validate_params()
Expand All @@ -181,6 +183,7 @@ defmodule DotcomWeb.Components.LiveComponents.TripPlannerForm do
|> case do
{:ok, data} ->
send(self(), {:updated_form, data})

{:noreply, socket}

{:error, changeset} ->
Expand Down
106 changes: 84 additions & 22 deletions lib/dotcom_web/live/trip_planner.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,17 @@ defmodule DotcomWeb.Live.TripPlanner do

@form_id "trip-planner-form"

@map_config Application.compile_env!(:mbta_metro, :map)

@impl true
def mount(_params, _session, socket) do
socket =
socket
|> assign(:error, nil)
|> assign(:form_name, @form_id)
|> assign(:map_config, @map_config)
|> assign(:pins, [])
|> assign(:submitted_values, nil)
|> assign(:error, nil)
|> assign_async(:groups, fn ->
{:ok, %{groups: nil}}
end)
Expand Down Expand Up @@ -67,22 +71,31 @@ defmodule DotcomWeb.Live.TripPlanner do
<.itinerary_group :for={group <- groups} group={group} />
</div>
</.async_result>
<div id="trip-planner-map-wrapper" class="w-full" phx-update="ignore">
<div style="min-height: 400px;" id="trip-planner-map" phx-hook="TripPlannerMap" />
</div>
<.live_component
module={MbtaMetro.Live.Map}
id="trip-planner-map"
class="h-96 w-full relative overflow-none"
config={@map_config}
pins={@pins}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pins={@pins}
pins={[@from, @to]}

This, along with changing update_pin_in_socket to simply:

defp update_pin_in_socket(socket, [longitude, latitude], direction)
     when longitude != "" and latitude != "" do
  socket
  |> assign(direction, [String.to_float(longitude), String.to_float(latitude)])
end

defp update_pin_in_socket(socket, [longitude, latitude], direction)
     when longitude == "" or latitude == "" do
  socket
  |> assign(direction, [])
end

And adding

|> assign(:from, [])
|> assign(:to, [])

To the socket initialization in mount...

Would allow removing all of the place_pin logic, and most of the more complicated guts of the update_pin_in_socket functions as well.

(Net reduction of 41 lines of code 😇)

I think it would also be possible to combine update_pin_in_socket with update_*_pin, potentially for an even greater LoC reduction (and maybe clearer code), but this feels like a start.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oooh actually, yes. If we replace all of the pin stuff with

 defp update_from_pin(socket, %{"from" => from}) do
   assign(socket, :from, to_geojson(from))
 end

 defp update_to_pin(socket, %{"to" => to}) do
   assign(socket, :to, to_geojson(to))
 end

 defp to_geojson(%{"longitude" => longitude, "latitude" => latitude})
      when longitude != "" and latitude != "" do
   [String.to_float(longitude), String.to_float(latitude)]
 end

 defp to_geojson(_coordinates) do
   []
 end

That gets us a net reduction of 53 lines of code (-54, but +1 because we have to set two assigns in the mount), and it resolves my concern with abruptly changing to geojson format in the middle of the live view, since this way we can keep the geojson format closer to the component where it's actually being used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

had to add in another 6 lines of code because you have to have defaults for update_from_pin and update_to_pin for when the pattern doesn't match.

/>
</section>
</div>
"""
end

@impl true
def handle_event("map_change", %{"id" => id} = params, socket) do
{:noreply, push_event(socket, id, location_props(params))}
def handle_event(_event, _params, socket) do
{:noreply, socket}
end

@impl true
def handle_event(_event, _params, socket) do
{:noreply, socket}
def handle_info({:changed_form, params}, socket) do
new_socket =
socket
|> update_from_pin(params)
|> update_to_pin(params)

{:noreply, new_socket}
end

@impl true
Expand All @@ -109,26 +122,75 @@ defmodule DotcomWeb.Live.TripPlanner do
{:noreply, socket}
end

# Selected from list of popular locations
defp location_props(%{"stop_id" => stop} = props) when is_binary(stop) do
Map.take(props, ["name", "latitude", "longitude", "stop_id"])
defp update_from_pin(socket, %{
"from" => %{"longitude" => from_longitude, "latitude" => from_latitude}
}) do
update_pin_in_socket(socket, [from_longitude, from_latitude], :from)
end

defp update_to_pin(socket, %{"to" => %{"longitude" => to_longitude, "latitude" => to_latitude}}) do
update_pin_in_socket(socket, [to_longitude, to_latitude], :to)
end

defp update_to_pin(socket, _params) do
socket
end

defp update_pin_in_socket(socket, [longitude, latitude], direction)
when longitude != "" and latitude != "" do
pins =
place_pin(
socket.assigns.pins,
[String.to_float(longitude), String.to_float(latitude)],
direction
)

socket |> assign(:pins, pins)
end

# GTFS stop
defp location_props(%{"stop" => stop}) when is_map(stop) do
Map.take(stop, ["name", "latitude", "longitude"])
|> Map.put("stop_id", stop["id"])
defp update_pin_in_socket(socket, [longitude, latitude], direction)
when longitude == "" or latitude == "" do
pins = remove_pin(socket.assigns.pins, direction)

socket |> assign(:pins, pins)
end

defp update_pin_in_socket(socket, _coordinates, _direction) do
socket
end

defp place_pin([], pin, :from) do
[pin]
end

defp place_pin([], pin, :to) do
[[], pin]
end

defp place_pin(pins, pin, :from) do
[pin | List.delete_at(pins, 0)]
end

defp place_pin(pins, pin, :to) do
[List.first(pins) | [pin]]
end

defp place_pin(pins, _pin, _direction) do
pins
end

defp remove_pin([], _direction), do: []

defp remove_pin(pins, :from) do
[[] | List.delete_at(pins, 0)]
end

# From AWS
defp location_props(%{"address" => address} = props) do
Map.take(props, ["latitude", "longitude"])
|> Map.put_new("name", address)
defp remove_pin(pins, :to) do
[List.first(pins)]
end

# Geolocated
defp location_props(props) do
Map.take(props, ["name", "latitude", "longitude"])
defp remove_pin(pins, _direction) do
pins
end

defp submission_summary(%{from: %{name: from_name}, to: %{name: to_name}, modes: modes}) do
Expand Down
2 changes: 1 addition & 1 deletion mix.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ defmodule DotCom.Mixfile do
{:jason, "1.4.4", override: true},
{:logster, "1.1.1"},
{:mail, "0.4.1"},
{:mbta_metro, "0.0.50"},
{:mbta_metro, "0.0.51"},
{:mock, "0.3.8", [only: :test]},
{:mox, "1.2.0", [only: :test]},
{:nebulex, "2.6.4"},
Expand Down
2 changes: 1 addition & 1 deletion mix.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
"makeup_elixir": {:hex, :makeup_elixir, "0.16.2", "627e84b8e8bf22e60a2579dad15067c755531fea049ae26ef1020cad58fe9578", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "41193978704763f6bbe6cc2758b84909e62984c7752b3784bd3c218bb341706b"},
"makeup_erlang": {:hex, :makeup_erlang, "1.0.1", "c7f58c120b2b5aa5fd80d540a89fdf866ed42f1f3994e4fe189abebeab610839", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "8a89a1eeccc2d798d6ea15496a6e4870b75e014d1af514b1b71fa33134f57814"},
"makeup_html": {:hex, :makeup_html, "0.1.1", "c3d4abd39d5f7e925faca72ada6e9cc5c6f5fa7cd5bc0158315832656cf14d7f", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "44f2a61bc5243645dd7fafeaa6cc28793cd22f3c76b861e066168f9a5b2c26a4"},
"mbta_metro": {:hex, :mbta_metro, "0.0.50", "26592935c4f2016d436ce277998e2ce67c0787cfd20b20e9bd5d5db1e57a2c10", [:mix], [{:heroicons, "0.5.6", [hex: :heroicons, repo: "hexpm", optional: true]}, {:jason, "1.4.4", [hex: :jason, repo: "hexpm", optional: false]}, {:phoenix, "1.7.14", [hex: :phoenix, repo: "hexpm", optional: false]}, {:phoenix_live_view, "1.0.0-rc.6", [hex: :phoenix_live_view, repo: "hexpm", optional: false]}, {:phoenix_storybook, "0.6.4", [hex: :phoenix_storybook, repo: "hexpm", optional: false]}, {:timex, "3.7.11", [hex: :timex, repo: "hexpm", optional: false]}], "hexpm", "ff5e9c3862556a3424f458a05e224fad7e634193bd75c1f1e24df9e01bdd2152"},
"mbta_metro": {:hex, :mbta_metro, "0.0.51", "039732742f3ad2c4747d310462958f944f8531608c74e634ef409ea8231f093f", [:mix], [{:heroicons, "0.5.6", [hex: :heroicons, repo: "hexpm", optional: true]}, {:jason, "1.4.4", [hex: :jason, repo: "hexpm", optional: false]}, {:phoenix, "1.7.14", [hex: :phoenix, repo: "hexpm", optional: false]}, {:phoenix_live_view, "1.0.0-rc.6", [hex: :phoenix_live_view, repo: "hexpm", optional: false]}, {:phoenix_storybook, "0.6.4", [hex: :phoenix_storybook, repo: "hexpm", optional: false]}, {:timex, "3.7.11", [hex: :timex, repo: "hexpm", optional: false]}], "hexpm", "75d3533e0844eb0030d50faa30d21d9b904c5ebecbce957f4f0cd1c8b6878b12"},
"meck": {:hex, :meck, "0.9.2", "85ccbab053f1db86c7ca240e9fc718170ee5bda03810a6292b5306bf31bae5f5", [:rebar3], [], "hexpm", "81344f561357dc40a8344afa53767c32669153355b626ea9fcbc8da6b3045826"},
"metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [:rebar3], [], "hexpm", "69b09adddc4f74a40716ae54d140f93beb0fb8978d8636eaded0c31b6f099f16"},
"mime": {:hex, :mime, "2.0.6", "8f18486773d9b15f95f4f4f1e39b710045fa1de891fada4516559967276e4dc2", [:mix], [], "hexpm", "c9945363a6b26d747389aac3643f8e0e09d30499a138ad64fe8fd1d13d9b153e"},
Expand Down
12 changes: 0 additions & 12 deletions test/dotcom_web/live/trip_planner_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,6 @@ defmodule DotcomWeb.Live.TripPlannerTest do
end

test "pushes updated location to the map", %{view: view} do
updated_location = %{
"latitude" => Faker.Address.latitude(),
"longitude" => Faker.Address.longitude(),
"name" => Faker.Company.name()
}

id = Faker.Internet.slug()

view
|> render_hook(:map_change, Map.put_new(updated_location, "id", id))

assert_push_event(view, ^id, ^updated_location)
end
end
end
Loading