feat: only add visible markers (and tooltips) to DOM (#2204)

umap/static/umap/js/modules/rendering/ui.js

@@ -36,6 +36,12 @@ const FeatureMixin = {
     }
   },
 
+  _removeIcon: function () {
+    // It may not be in the DOM, and Leaflet does not deal with this
+    // situation
+    if (this._icon) Marker.prototype._removeIcon.call(this)
+  },
+
   addInteractions: function () {
     this.on('contextmenu editable:vertex:contextmenu', this.onContextMenu)
     this.on('click', this.onClick)
@@ -160,14 +166,32 @@ export const LeafletMarker = Marker.extend({
     return this.setLatLng(latlng)
   },
 
+  getEvents: function () {
+    const events = Marker.prototype.getEvents.call(this)
+    events.moveend = this.onMoveEnd
+    return events
+  },
+
   addInteractions() {
     PointMixin.addInteractions.call(this)
     this._popupHandlersAdded = true // prevent Leaflet from binding event on bindPopup
     this.on('popupopen', this.highlight)
     this.on('popupclose', this.resetHighlight)
   },
 
+  onMoveEnd: function () {
+    this._initIcon()
+    this.update()
+  },
+
   _initIcon: function () {
+    if (!this._map.getBounds().contains(this.getCenter())) {
+      if (this._icon) this._removeIcon()
+      if (this._tooltip && this.isTooltipOpen()) {
+        this.unbindTooltip()
+      }
+      return
+    }
     this.options.icon = this.getIcon()
     Marker.prototype._initIcon.call(this)
     // Allow to run code when icon is actually part of the DOM

umap/tests/fixtures/test_upload_data.csv

@@ -1,3 +1,3 @@
 Foo,Latitude,geo_Longitude,title,description
-bar,41.34,122.86,a point somewhere,the description of this point
-bar,43.34,121.86,a point somewhere else,the description of this other point
+bar,48.5,14.5,a point somewhere,the description of this point
+bar,45.7,14.7,a point somewhere else,the description of this other point

umap/tests/integration/test_browser.py

@@ -81,7 +81,7 @@ def test_data_browser_should_be_open(live_server, page, bootstrap, map):
 
 
 def test_data_browser_should_be_filterable(live_server, page, bootstrap, map):
-    page.goto(f"{live_server.url}{map.get_absolute_url()}")
+    page.goto(f"{live_server.url}{map.get_absolute_url()}#2/19/-2")
     expect(page.get_by_title("Features in this layer: 3")).to_be_visible()
     markers = page.locator(".leaflet-marker-icon")
     paths = page.locator(".leaflet-overlay-pane path")
@@ -115,7 +115,7 @@ def test_filter_uses_layer_setting_if_any(live_server, page, bootstrap, map):
     datalayer = map.datalayer_set.first()
     datalayer.settings["labelKey"] = "foo"
     datalayer.save()
-    page.goto(f"{live_server.url}{map.get_absolute_url()}")
+    page.goto(f"{live_server.url}{map.get_absolute_url()}#2/19/-2")
     expect(page.get_by_title("Features in this layer: 3")).to_be_visible()
     markers = page.locator(".leaflet-marker-icon")
     paths = page.locator(".leaflet-overlay-pane path")
@@ -154,7 +154,7 @@ def test_filter_works_with_variable_in_labelKey(live_server, page, map):
     data = deepcopy(DATALAYER_DATA)
     data["_umap_options"]["labelKey"] = "{name} ({bar})"
     DataLayerFactory(map=map, data=data)
-    page.goto(f"{live_server.url}{map.get_absolute_url()}")
+    page.goto(f"{live_server.url}{map.get_absolute_url()}#2/19/-2")
     expect(page.get_by_title("Features in this layer: 3")).to_be_visible()
     markers = page.locator(".leaflet-marker-icon")
     paths = page.locator(".leaflet-overlay-pane path")
@@ -182,7 +182,7 @@ def test_filter_works_with_missing_name(live_server, page, map):
     data = deepcopy(DATALAYER_DATA)
     del data["features"][0]["properties"]["name"]
     DataLayerFactory(map=map, data=data, name="foobar")
-    page.goto(f"{live_server.url}{map.get_absolute_url()}")
+    page.goto(f"{live_server.url}{map.get_absolute_url()}#2/19/-2")
     expect(page.get_by_title("Features in this layer: 3")).to_be_visible()
     markers = page.locator(".leaflet-marker-icon")
     paths = page.locator(".leaflet-overlay-pane path")
@@ -299,7 +299,7 @@ def test_data_browser_with_variable_in_name(live_server, page, bootstrap, map):
     # Include a variable
     map.settings["properties"]["labelKey"] = "{name} ({foo})"
     map.save()
-    page.goto(f"{live_server.url}{map.get_absolute_url()}")
+    page.goto(f"{live_server.url}{map.get_absolute_url()}#2/19/-2")
     expect(page.get_by_text("one point in france (point)")).to_be_visible()
     expect(page.get_by_text("one line in new zeland (line)")).to_be_visible()
     expect(page.get_by_text("one polygon in greenland (polygon)")).to_be_visible()
@@ -335,7 +335,7 @@ def test_should_sort_features_in_natural_order(live_server, map, page):
 
 
 def test_should_redraw_list_on_feature_delete(live_server, openmap, page, bootstrap):
-    page.goto(f"{live_server.url}{openmap.get_absolute_url()}")
+    page.goto(f"{live_server.url}{openmap.get_absolute_url()}#2/19/-2")
     # Enable edit
     page.get_by_role("button", name="Edit").click()
     buttons = page.locator(".umap-browser .datalayer li .icon-delete")
@@ -380,7 +380,7 @@ def test_should_use_color_variable(live_server, map, page):
 
 
 def test_should_allow_to_toggle_datalayer_visibility(live_server, map, page, bootstrap):
-    page.goto(f"{live_server.url}{map.get_absolute_url()}")
+    page.goto(f"{live_server.url}{map.get_absolute_url()}#2/19/-2")
     markers = page.locator(".leaflet-marker-icon")
     paths = page.locator(".leaflet-overlay-pane path")
     expect(markers).to_have_count(1)

umap/tests/integration/test_datalayer.py

@@ -107,7 +107,7 @@ def test_should_honour_color_variable(live_server, map, page):
         },
     }
     DataLayerFactory(map=map, data=data)
-    page.goto(f"{live_server.url}{map.get_absolute_url()}")
+    page.goto(f"{live_server.url}{map.get_absolute_url()}#6/47.5/2.5")
     expect(page.locator(".leaflet-overlay-pane path[fill='tomato']"))
     markers = page.locator(".leaflet-marker-icon .icon_container")
     expect(markers).to_have_css("background-color", "rgb(240, 248, 255)")

umap/tests/integration/test_edit_datalayer.py

@@ -86,7 +86,6 @@ def test_can_clone_datalayer(live_server, openmap, login, datalayer, page):
 
 
 def test_can_change_icon_class(live_server, openmap, page):
-    # Faster than doing a login
     data = {
         "type": "FeatureCollection",
         "features": [
@@ -98,7 +97,7 @@ def test_can_change_icon_class(live_server, openmap, page):
         ],
     }
     DataLayerFactory(map=openmap, data=data)
-    page.goto(f"{live_server.url}{openmap.get_absolute_url()}?edit")
+    page.goto(f"{live_server.url}{openmap.get_absolute_url()}?edit#6/45.3/1")
     expect(page.locator(".umap-div-icon")).to_be_visible()
     page.get_by_role("link", name="Manage layers").click()
     expect(page.locator(".umap-circle-icon")).to_be_hidden()

umap/tests/integration/test_edit_marker.py

@@ -40,7 +40,7 @@ def test_can_edit_on_shift_click(live_server, openmap, page, datalayer):
 
 
 def test_marker_style_should_have_precedence(live_server, openmap, page, bootstrap):
-    page.goto(f"{live_server.url}{openmap.get_absolute_url()}?edit")
+    page.goto(f"{live_server.url}{openmap.get_absolute_url()}?edit#6/48.5/19")
 
     # Change colour at layer level
     page.get_by_role("link", name="Manage layers").click()

umap/tests/integration/test_facets_browser.py

@@ -175,7 +175,7 @@ def test_date_facet_search(live_server, page, map):
     map.save()
     DataLayerFactory(map=map, data=DATALAYER_DATA1)
     DataLayerFactory(map=map, data=DATALAYER_DATA2)
-    page.goto(f"{live_server.url}{map.get_absolute_url()}")
+    page.goto(f"{live_server.url}{map.get_absolute_url()}#6/47.5/-1.5")
     markers = page.locator(".leaflet-marker-icon")
     expect(markers).to_have_count(4)
     expect(page.get_by_text("Date Filter")).to_be_visible()
@@ -196,7 +196,7 @@ def test_choice_with_empty_value(live_server, page, map):
     del data["features"][1]["properties"]["mytype"]
     DataLayerFactory(map=map, data=data)
     DataLayerFactory(map=map, data=DATALAYER_DATA2)
-    page.goto(f"{live_server.url}{map.get_absolute_url()}")
+    page.goto(f"{live_server.url}{map.get_absolute_url()}#6/47.5/-1.5")
     expect(page.get_by_text("<empty value>")).to_be_visible()
     markers = page.locator(".leaflet-marker-icon")
     expect(markers).to_have_count(4)
@@ -212,7 +212,7 @@ def test_number_with_zero_value(live_server, page, map):
     data["features"][0]["properties"]["mynumber"] = 0
     DataLayerFactory(map=map, data=data)
     DataLayerFactory(map=map, data=DATALAYER_DATA2)
-    page.goto(f"{live_server.url}{map.get_absolute_url()}")
+    page.goto(f"{live_server.url}{map.get_absolute_url()}#6/47.5/-1.5")
     expect(page.get_by_label("Min")).to_have_value("0")
     expect(page.get_by_label("Max")).to_have_value("14")
     page.get_by_label("Min").fill("1")

umap/tests/integration/test_view_marker.py

@@ -106,3 +106,66 @@ def test_extended_properties_in_popup(live_server, map, page, bootstrap):
     expect(page.get_by_text("Alt: 241")).to_be_visible()
     expect(page.get_by_text("Zoom: 7")).to_be_visible()
     expect(page.get_by_text("Layer: test datalayer")).to_be_visible()
+
+
+def test_only_visible_markers_are_added_to_dom(live_server, map, page):
+    data = {
+        "type": "FeatureCollection",
+        "features": [
+            {
+                "type": "Feature",
+                "properties": {
+                    "name": "marker 1",
+                    "description": "added to dom",
+                },
+                "geometry": {
+                    "type": "Point",
+                    "coordinates": [14.6, 48.5],
+                },
+            },
+            {
+                "type": "Feature",
+                "properties": {
+                    "name": "marker 2",
+                    "description": "not added to dom at load",
+                },
+                "geometry": {
+                    "type": "Point",
+                    "coordinates": [12.6, 44.5],
+                },
+            },
+        ],
+    }
+    DataLayerFactory(map=map, data=data)
+    map.settings["properties"]["showLabel"] = True
+    map.save()
+    page.goto(f"{live_server.url}{map.get_absolute_url()}")
+    markers = page.locator(".leaflet-marker-icon")
+    tooltips = page.locator(".leaflet-tooltip")
+    expect(markers).to_have_count(1)
+    expect(tooltips).to_have_count(1)
+
+    # Zoom in/out to show the other marker
+    page.get_by_label("Zoom out").click()
+    expect(markers).to_have_count(2)
+    expect(tooltips).to_have_count(2)
+    page.get_by_label("Zoom in").click()
+    expect(markers).to_have_count(1)
+    expect(tooltips).to_have_count(1)
+
+    # Drag map to show/hide the marker
+    map_el = page.locator("#map")
+    map_el.drag_to(
+        map_el,
+        source_position={"x": 100, "y": 600},
+        target_position={"x": 100, "y": 200},
+    )
+    expect(markers).to_have_count(2)
+    expect(tooltips).to_have_count(2)
+    map_el.drag_to(
+        map_el,
+        source_position={"x": 100, "y": 600},
+        target_position={"x": 100, "y": 200},
+    )
+    expect(markers).to_have_count(1)
+    expect(tooltips).to_have_count(1)