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.

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

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

Merged
merged 2 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 24 additions & 0 deletions umap/static/umap/js/modules/rendering/ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions umap/tests/fixtures/test_upload_data.csv
Original file line number Diff line number Diff line change
@@ -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
14 changes: 7 additions & 7 deletions umap/tests/integration/test_browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion umap/tests/integration/test_datalayer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)")
Expand Down
3 changes: 1 addition & 2 deletions umap/tests/integration/test_edit_datalayer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand All @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion umap/tests/integration/test_edit_marker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
6 changes: 3 additions & 3 deletions umap/tests/integration/test_facets_browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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)
Expand All @@ -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")
Expand Down
63 changes: 63 additions & 0 deletions umap/tests/integration/test_view_marker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)