Skip to content

load geojson in worker#805

Merged
digitaltom merged 7 commits into
mainfrom
load_geojson_in_worker
Jul 4, 2026
Merged

load geojson in worker#805
digitaltom merged 7 commits into
mainfrom
load_geojson_in_worker

Conversation

@digitaltom

Copy link
Copy Markdown
Collaborator
  • fix sort order of overlayed features on click
  • load geojson in maplibre web worker out of main thread

Copilot AI review requested due to automatic review settings July 2, 2026 22:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the map data flow so GeoJSON feature payloads are no longer embedded in the default /m/:id.json response, and are instead streamed per-layer via a new /m/:id/layer/:layer_id.geojson endpoint to enable MapLibre to fetch + parse in its web worker. It also adjusts click-selection behavior to respect visual z-order for overlaid features and updates the “Map export” flow to request a self-contained export explicitly.

Changes:

  • Add a per-layer GeoJSON endpoint and client-side layer streaming via source.setData(url) + getData() (worker parsing).
  • Change default map JSON to return layer summaries only; gate embedded GeoJSON behind ?export=true.
  • Fix click selection ordering for stacked/overlaid rendered features.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
spec/requests/maps_controller_spec.rb Adds request specs for the new layer GeoJSON endpoint (success/404/permissions).
spec/models/map_spec.rb Verifies default map JSON excludes embedded geojson and export mode includes it.
spec/features/map_share_spec.rb Updates share UI expectation to use ?export=true for map export.
config/routes.rb Adds a new route for /m/:id/layer/:layer_id.geojson.
app/views/maps/modals/_share.haml Updates “Map export” link to include export=true.
app/views/maps/maplibre.haml Removes the inline early-fetch script for map JSON.
app/models/map.rb Changes to_json to optionally include embedded layer geojson via include_features:.
app/models/feature.rb Ensures properties.id is populated for MapLibre promoteId:'id'.
app/javascript/maplibre/map.js Avoids upsert diffs caused solely by properties.id and tweaks source log message.
app/javascript/maplibre/layers/layers.js Reworks layer-definition loading to use gon on initial load + adds hidden-layer data loading.
app/javascript/maplibre/layers/layer.js Adjusts click stack selection to prefer visual top feature, with stable cycling.
app/javascript/maplibre/layers/geojson.js Streams geojson via URL so MapLibre parses in worker; reads back via getData() and avoids redundant re-parse when possible.
app/javascript/channels/map_channel.js Forces refetch + resets memoization on reconnect to rebuild layers reliably.
app/controllers/maps_controller.rb Adds #layer action, includes gon.map_layers, and toggles export embedding via params[:export].

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app/models/map.rb
Comment thread app/javascript/maplibre/layers/layers.js
* origin/main:
  surgical feature updates instead of full layer render
  extract surgical applyFeatureUpdate
  extract buildLineExtrusion(), avoid full render on bringToFront
  avoid unneccessary full map reloads on channel reconnect when nothing changed.
  safeguard for non existing layer

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Comment thread app/controllers/maps_controller.rb
Comment thread app/javascript/maplibre/layers/layers.js
@digitaltom
digitaltom enabled auto-merge July 4, 2026 12:14
@digitaltom
digitaltom merged commit fc5c024 into main Jul 4, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants