Skip to content

Commit c0cdfd0

Browse files
author
Peter Rushforth
committed
Update map-feature.disconnectedCallback to clean up when it's the last
feature in a MapFeatureLayer being disconnected, make it remove the <svg> (_container) element created by the renderer. Because Renderer inherits from Layer, we can run its remove() method to do this. Because `disabled` is an observed attribute on <map-linK>, purge the shadowRoot contents when the link is disabled. Add test for <svg> count when panning and zooming the map to prevent regressions hopefully.
1 parent dc5bebd commit c0cdfd0

5 files changed

Lines changed: 72 additions & 3 deletions

File tree

‎ToDo‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ especially for <map-feature>, that the _container is left with empty <svg> eleme
107107
after each move or zoom, and these build up. Figure out how to make them go
108108
away after the last <map-feature> is removed. This is something to do with
109109
the parameterization of the MapFeatureLayer (via options), and the associated
110-
renderer possibly. Anyway, fix it and add a test.
110+
renderer possibly. Anyway, fix it and add a test. DONE
111111

112112
I updated index.html to access the GeoServer canada provinces layer which includes
113113
a <map-link rel="features"> that can be used to debug. Will have to figure out

‎src/map-feature.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,11 @@ export class HTMLFeatureElement extends HTMLElement {
225225
this.removeFeature(this._featureLayer);
226226
// If this was the last feature in the layer, clean up the layer
227227
if (this._featureLayer.getLayers().length === 0) {
228+
if (this._featureLayer.options.renderer) {
229+
// since this is the last reference to the MapFeatureLayer, we need to also
230+
// manually remove the shared renderer
231+
this._featureLayer.options.renderer.remove();
232+
}
228233
this._featureLayer.remove();
229234
this._featureLayer = null;
230235
delete this._featureLayer;
@@ -324,6 +329,9 @@ export class HTMLFeatureElement extends HTMLElement {
324329
this._featureLayer = new MapFeatureLayer(null, {
325330
// pass the vector layer a renderer of its own, otherwise leaflet
326331
// puts everything into the overlayPane
332+
// with this feature creating its own MapFeatureLayer for each
333+
// sub-sequence of features, it means that there may be > 1 <svg>
334+
// container (one per renderer) in the pane...
327335
renderer: featureRenderer(),
328336
// pass the vector layer the container for the parent into which
329337
// it will append its own container for rendering into

‎src/map-link.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,7 @@ export class HTMLLinkElement extends HTMLElement {
343343
) {
344344
this.parentExtent._extentLayer.removeLayer(this._templatedLayer);
345345
delete this._templatedLayer;
346+
this.shadowRoot.innerHTML = '';
346347
this.getLayerEl()._validateDisabled();
347348
}
348349
break;
@@ -559,8 +560,7 @@ export class HTMLLinkElement extends HTMLElement {
559560
zIndex: this.zIndex,
560561
pane: this.parentExtent._extentLayer.getContainer(),
561562
linkEl: this,
562-
projection: this.mapEl._map.options.projection,
563-
renderer: this.mapEl._map.options.renderer
563+
projection: this.mapEl._map.options.projection
564564
}).addTo(this.parentExtent._extentLayer);
565565
} else if (this.rel === 'query') {
566566
if (!this.shadowRoot) {

‎test/e2e/layers/renderer.html‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<!DOCTYPE html>
2+
<html lang="en">
3+
<head>
4+
<title>renderer.html</title>
5+
<meta charset="UTF-8">
6+
<script type="module" src="mapml.js"></script>
7+
</head>
8+
<body>
9+
<mapml-viewer data-testid="map" style="width: 500px;height: 500px;" projection="CBMTILE" zoom="0" lat="45.5052040" lon="-75.2202344" controls>
10+
<map-layer data-testid="test-layer" checked >
11+
<map-extent data-test-id="test-extent" units="CBMTILE" checked="checked" >
12+
<map-meta name="extent" content="top-left-easting=-6093972, top-left-northing=5350661, bottom-right-easting=5150842, bottom-right-northing=-5894153"></map-meta>
13+
<map-input name="z" type="zoom" min="0" max="18" value="0" ></map-input>
14+
<map-input name="xmin" type="location" units="gcrs" axis="longitude" position="top-left" ></map-input>
15+
<map-input name="ymin" type="location" units="gcrs" axis="latitude" position="bottom-right" ></map-input>
16+
<map-input name="xmax" type="location" units="gcrs" axis="longitude" position="bottom-right" ></map-input>
17+
<map-input name="ymax" type="location" units="gcrs" axis="latitude" position="top-left" ></map-input>
18+
<map-link data-testid="test-link" rel="features" tref="data/us_pop_density.mapml?{xmin}{ymin}{xmax}{ymax}{z}" ></map-link>
19+
</map-extent>
20+
</map-layer>
21+
</mapml-viewer>
22+
</body>
23+
</html>

‎test/e2e/layers/templatedFeaturesCleanup.test.js‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { test, expect, chromium } from '@playwright/test';
2+
3+
test.describe('Templated features cleanup test' , () => {
4+
let page;
5+
let context;
6+
test.beforeAll(async function () {
7+
context = await chromium.launchPersistentContext('');
8+
page =
9+
context.pages().find((page) => page.url() === 'about:blank') ||
10+
(await context.newPage());
11+
await page.goto('renderer.html');
12+
});
13+
14+
test.afterAll(async function () {
15+
await context.close();
16+
});
17+
18+
test('Templated <map-link rel="features"> renderer cleans up after itself', async () => {
19+
await page.waitForTimeout(1000);
20+
const link = page.getByTestId('test-link');
21+
let svgCount = await link.evaluate((l) => l._templatedLayer._container.querySelectorAll('svg').length);
22+
const viewer = page.getByTestId('map');
23+
await page.getByLabel('Zoom in').click();
24+
await page.getByLabel('Zoom in').click();
25+
await page.getByLabel('Zoom in').click();
26+
await page.getByLabel('Zoom in').click();
27+
await page.waitForTimeout(1000);
28+
svgCount = await link.evaluate((l) => l._templatedLayer._container.querySelectorAll('svg').length);
29+
expect(svgCount).toEqual(1);
30+
await page.getByLabel('Interactive map').press('ArrowLeft');
31+
await page.getByLabel('Interactive map').press('ArrowLeft');
32+
await page.getByLabel('Interactive map').press('ArrowLeft');
33+
await page.getByLabel('Reload').click();
34+
await page.waitForTimeout(1000);
35+
svgCount = await link.evaluate((l) => l._templatedLayer._container.querySelectorAll('svg').length);
36+
expect(svgCount).toEqual(1);
37+
});
38+
});

0 commit comments

Comments
 (0)