Repository navigation
refactor: Replace line page's map implementation with MaplibreX - #3627
Open
joshlarson wants to merge 27 commits into
Open
joshlarson wants to merge 27 commits into
joshlarson wants to merge 27 commits into
Conversation
Map the maplibrex import to a minimal local declaration so tsc doesn't compile the dependency's TypeScript sources. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolve imports from maplibrex's sources against assets/node_modules, add the deck.gl type packages it needs, and drop the hand-written maplibrex shim. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
lvachon1
reviewed
Nov 4, 2026
| <%!-- Receives the "fit-bounds" event pushed when the direction or variant changes --%> | ||
| <div id="new-map-fit" phx-hook="FitBounds" /> | ||
|
|
||
| <MaplibreX.Components.geojson_layer |
Contributor
There was a problem hiding this comment.
Is there a reason one is a geojson_layer with type=line and one is a line_layer? They seem to take the same params and do the same thing?
Collaborator
Author
There was a problem hiding this comment.
Yes, actually, although I don't know if there's a better way.
The geojson_layer adds the shape data as a "source", which the line_layer can then use. That's why the data isn't being passed into the line_layer again.
lvachon1
reviewed
Nov 4, 2026
| "lodash": "^4.17.21", | ||
| "maplibre-gl": "4.7.0", | ||
| "maplibre-gl": "5.24.0", | ||
| "maplibrex": "file:../deps/maplibrex", |
Contributor
There was a problem hiding this comment.
This path/file doesn't exist on my machine, and npm install doesn't seem to do it either.
Collaborator
Author
There was a problem hiding this comment.
Does running mix deps.get first do it?
Replace the mutating updatePopup method with a syncPopup helper that returns a new entry, so the no-param-reassign disable is no longer needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MaplibreX compares the map style by reference, so a style object made every patch call map.setStyle and wipe the route shape layers. Wrap the map hook to skip setStyle when the style is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Scope
Asana Ticket: 💈🗺️🛠️ Investigate replacing custom map component implementation with MapLibreX
Implementation
Note
🤖 I used Copilot to generate almost all of this PR, with a decent amount of back and forth. I have looked over and vetted all of the code.
You may find it easier to review this PR commit-by-commit, since each commit accomplishes an isolated bit of functionality and/or fixes a bug. Almost but not quite all of the commits were AI-generated - the first two were just me fiddling with dependencies - the ones with
robotin the prefix were Copilot.I reviewed each diff before committing it.
Screenshots
Things look pretty much the same. Here's a screenshot demonstrating that fact.
How to test
Visit a new line diagram page (1, 39, 66, Red Line) and observe the following subtle changes:
Everything else should be the same!
Note
One feature that is intentionally preserved is that the map zoom/center resets when you change the variant. That's an intentional design decision, since the route shape can vary pretty dramatically between variants, and we don't want the shape to just disappear if you change the variant (if the newly-selected one is off-screen).