Skip to content

Commit 24ed56f

Browse files
author
Peter Rushforth
committed
If this is the last feature or tile to be removed from a static layer
context, i.e. as child of a <map-layer>, the container for the MapFeatureLayer or the MapTileLayer should be removed from the layer rendered dom; includes basic tests of this for both types of static content. Update ToDo file, index.html to remind me to fix problems with TemplatedFeaturesOrTilesLayer
1 parent 98d9c10 commit 24ed56f

File tree

9 files changed

+198
-401
lines changed

9 files changed

+198
-401
lines changed

ToDo

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,3 +94,22 @@ sibling's MapTileLayer GridLayer. This behaviour is implemented by <map-feature
9494
when being added to the shadow root of a <map-link> element, with the
9595
TemplatedFeaturesOrTilesLayer being the analog of MapLayer in this case.
9696

97+
TemplatedFeaturesOrTilesLayer; the query selector on line 210 should restrict itself
98+
by content type and it should specifically exclude map-extent.
99+
100+
TemplatedFeaturesOrTilesLayer is effectively the operation behind
101+
<map-link rel="features">
102+
103+
on moveend, it fetches from the filled-in template and appends the content it finds to
104+
the <map-link> shadowRoot. The <map-feature> and <map-tile> built in behaviours
105+
take over from there. The trouble is now that there is different built-in behaviour,
106+
especially for <map-feature>, that the _container is left with empty <svg> elements
107+
after each move or zoom, and these build up. Figure out how to make them go
108+
away after the last <map-feature> is removed. This is something to do with
109+
the parameterization of the MapFeatureLayer (via options), and the associated
110+
renderer possibly. Anyway, fix it and add a test.
111+
112+
I updated index.html to access the GeoServer canada provinces layer which includes
113+
a <map-link rel="features"> that can be used to debug. Will have to figure out
114+
how to test it without a dynamic server, but it does need a test.
115+

index.html

Lines changed: 3 additions & 388 deletions
Large diffs are not rendered by default.

src/map-feature.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ export class HTMLFeatureElement extends HTMLElement {
1212
}
1313

1414
/* jshint ignore:start */
15-
#hasConnected;
15+
#hasConnected; // prevents attributeChangedCallback before connectedCallback
1616
/* jshint ignore:end */
1717
get zoom() {
1818
// for templated or queried features ** native zoom is only used for zoomTo() **
@@ -223,6 +223,12 @@ export class HTMLFeatureElement extends HTMLElement {
223223
this._observer.disconnect();
224224
if (this._featureLayer) {
225225
this.removeFeature(this._featureLayer);
226+
// If this was the last feature in the layer, clean up the layer
227+
if (this._featureLayer.getLayers().length === 0) {
228+
this._featureLayer.remove();
229+
this._featureLayer = null;
230+
delete this._featureLayer;
231+
}
226232
}
227233
}
228234

src/map-tile.js

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export class HTMLTileElement extends HTMLElement {
1111
return ['row', 'col', 'zoom', 'src'];
1212
}
1313
/* jshint ignore:start */
14-
#hasConnected;
14+
#hasConnected; // prevents attributeChangedCallback before connectedCallback
1515
#initialRow;
1616
#initialCol;
1717
#initialZoom;
@@ -119,7 +119,7 @@ export class HTMLTileElement extends HTMLElement {
119119
/* jshint ignore:end */
120120
// Get parent element to determine how to handle the tile
121121
// Need to handle shadow DOM correctly like map-feature does
122-
this._parentElement =
122+
this._parentEl =
123123
this.parentNode.nodeName.toUpperCase() === 'MAP-LAYER' ||
124124
this.parentNode.nodeName.toUpperCase() === 'LAYER-' ||
125125
this.parentNode.nodeName.toUpperCase() === 'MAP-LINK'
@@ -141,6 +141,7 @@ export class HTMLTileElement extends HTMLElement {
141141

142142
// If this was the last tile in the layer, clean up the layer
143143
if (this._tileLayer._mapTiles && this._tileLayer._mapTiles.length === 0) {
144+
this._tileLayer.remove();
144145
this._tileLayer = null;
145146
delete this._tileLayer;
146147
}
@@ -217,24 +218,24 @@ export class HTMLTileElement extends HTMLElement {
217218
getMeta(metaName) {
218219
let name = metaName.toLowerCase();
219220
if (name !== 'cs' && name !== 'zoom' && name !== 'projection') return;
220-
let sdMeta = this._parentElement.shadowRoot.querySelector(
221+
let sdMeta = this._parentEl.shadowRoot.querySelector(
221222
`map-meta[name=${name}][content]`
222223
);
223-
if (this._parentElement.nodeName === 'MAP-LINK') {
224+
if (this._parentEl.nodeName === 'MAP-LINK') {
224225
// sd.map-meta || map-extent meta || layer meta
225-
return sdMeta || this._parentElement.parentElement.getMeta(metaName);
226+
return sdMeta || this._parentEl.parentElement.getMeta(metaName);
226227
} else {
227-
return this._parentElement.src
228-
? this._parentElement.shadowRoot.querySelector(
228+
return this._parentEl.src
229+
? this._parentEl.shadowRoot.querySelector(
229230
`map-meta[name=${name}][content]`
230231
)
231-
: this._parentElement.querySelector(`map-meta[name=${name}][content]`);
232+
: this._parentEl.querySelector(`map-meta[name=${name}][content]`);
232233
}
233234
}
234235
async _createOrGetTileLayer() {
235-
await this._parentElement.whenReady();
236+
await this._parentEl.whenReady();
236237
if (this.isFirst()) {
237-
const parentElement = this._parentElement;
238+
const parentElement = this._parentEl;
238239

239240
// Create a new MapTileLayer
240241
this._tileLayer = mapTileLayer({

src/mapml/layers/MapFeatureLayer.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,9 @@ export var MapFeatureLayer = FeatureGroup.extend({
222222
delete this._queryFeatures;
223223
DomUtil.remove(this._container);
224224
}
225+
if (this._context === 'static') {
226+
DomUtil.remove(this._container);
227+
}
225228
FeatureGroup.prototype.onRemove.call(this, map);
226229
this._map.featureIndex.cleanIndex();
227230
},

test/e2e/elements/map-feature/map-feature-rendering.test.js

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,63 @@ test.describe('map-feature rendering tests', () => {
1616
maxDiffPixels: 100
1717
});
1818
});
19+
test('removing a map-feature from DOM removes its rendering', async ({
20+
page
21+
}) => {
22+
await page.goto('static-features.html');
23+
// Wait for initial rendering
24+
await page.waitForTimeout(1000);
25+
const viewer = page.getByTestId('viewer');
26+
let nFeatures = await viewer.evaluate(
27+
(v) => v.querySelectorAll('map-feature').length
28+
);
29+
expect(nFeatures).toEqual(5);
30+
const f = page.locator('map-feature').first();
31+
const rendered = await f.evaluate((f) => {
32+
f._featureLayer._container.setAttribute(
33+
'data-testid',
34+
'test-feature-container'
35+
);
36+
f._groupEl.setAttribute('data-testid', 'test-feature-rendering');
37+
return f._groupEl.isConnected;
38+
});
39+
expect(rendered).toBe(true);
40+
await expect(page.getByTestId('test-feature-rendering')).toHaveCount(1);
41+
await expect(page.getByTestId('test-feature-container')).toHaveCount(1);
42+
await f.evaluate((f) => f.remove());
43+
await expect(page.getByTestId('test-feature-rendering')).toHaveCount(0);
44+
await expect(
45+
page
46+
.getByTestId('test-feature-container')
47+
.locator('g[aria-label="Feature"]')
48+
).toHaveCount(4);
49+
});
50+
51+
test('removing last map-feature in a sequence removes rendering container', async ({
52+
page
53+
}) => {
54+
await page.goto('static-features.html');
55+
// Wait for initial rendering
56+
await page.waitForTimeout(1000);
57+
const viewer = page.getByTestId('viewer');
58+
let nFeatures = await viewer.evaluate(
59+
(v) => v.querySelectorAll('map-feature').length
60+
);
61+
expect(nFeatures).toEqual(5);
62+
const containerConnected = await viewer.evaluate((v) => {
63+
v.querySelector('map-feature')._featureLayer._container.setAttribute(
64+
'data-testid',
65+
'test-feature-container'
66+
);
67+
return v.querySelector('map-feature')._featureLayer._container
68+
.isConnected;
69+
});
70+
expect(containerConnected).toBe(true);
71+
nFeatures = await viewer.evaluate((v) => {
72+
v.querySelectorAll('map-feature').forEach((el) => el.remove());
73+
return v.querySelectorAll('map-feature').length;
74+
});
75+
expect(nFeatures).toEqual(0);
76+
await expect(page.getByTestId('test-feature-container')).toHaveCount(0);
77+
});
1978
});
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
<!DOCTYPE html>
2+
<html lang="en">
3+
4+
<head>
5+
<title>mixedLayer.html</title>
6+
<meta charset="UTF-8">
7+
<meta name="viewport" content="width=device-width, initial-scale=1.0">
8+
<script type="module" src="mapml.js"></script>
9+
<style>
10+
map-layer {
11+
display: none;
12+
}
13+
</style>
14+
</head>
15+
16+
<body>
17+
<mapml-viewer data-testid="viewer" width="500" height="500" projection="CBMTILE" zoom="2" lat="45.5052040" lon="-75.2202344" controls>
18+
19+
<map-layer label="Static inline content" checked>
20+
<map-meta name="zoom" content="min=0,max=10"></map-meta>
21+
<map-meta name="projection" content="CBMTILE"></map-meta>
22+
<!-- for features, zoom is advisory (min/max control rendering) -->
23+
<!-- these features should be in one MapFeatureLayer -->
24+
25+
<!-- test the z-index of these features -->
26+
<map-feature zoom="2" min="2" max="2"> col="10" row="10"
27+
<map-geometry cs="tilematrix">
28+
<map-polygon>
29+
<map-coordinates>10 10 11 10 11 11 10 11 10 10</map-coordinates>
30+
</map-polygon>
31+
</map-geometry>
32+
</map-feature>
33+
<map-feature zoom="2" min="2" max="2"> col="10" row="11"
34+
<map-geometry cs="tilematrix">
35+
<map-polygon>
36+
<map-coordinates>10 11 10 12 11 12 11 11 10 11</map-coordinates>
37+
</map-polygon>
38+
</map-geometry>
39+
</map-feature>
40+
<map-feature zoom="2" min="2" max="2"> col="9" row="11"
41+
<map-geometry cs="tilematrix">
42+
<map-polygon>
43+
<map-coordinates>9 11 9 12 10 12 10 11 9 11</map-coordinates>
44+
</map-polygon>
45+
</map-geometry>
46+
</map-feature>
47+
<map-feature zoom="2" min="2" max="2"> col="9" row="10"
48+
<map-geometry cs="tilematrix">
49+
<map-polygon>
50+
<map-coordinates>9 10 9 11 10 11 10 10 9 10</map-coordinates>
51+
</map-polygon>
52+
</map-geometry>
53+
</map-feature>
54+
<map-feature zoom="2" min="2" max="2"> col="11" row="11"
55+
<map-geometry cs="tilematrix">
56+
<map-polygon>
57+
<map-coordinates>11 11 11 12 12 12 12 11 11 11</map-coordinates>
58+
</map-polygon>
59+
</map-geometry>
60+
</map-feature>
61+
</map-layer>
62+
63+
<map-layer id="US" label="static remote features" src="data/us_pop_density.mapml" ></map-layer>
64+
</mapml-viewer>
65+
66+
</body>
67+
68+
</html>

test/e2e/elements/map-tile/map-tile-test.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
</head>
1515

1616
<body>
17-
<mapml-viewer width="500" height="500" projection="CBMTILE" zoom="2" lat="45.5052040" lon="-75.2202344" controls>
17+
<mapml-viewer data-testid="viewer" width="500" height="500" projection="CBMTILE" zoom="2" lat="45.5052040" lon="-75.2202344" controls>
1818

1919
<map-layer label="Static Tile Test Layer" checked>
2020
<map-meta name="zoom" content="min=0,max=10"></map-meta>
@@ -46,4 +46,4 @@
4646

4747
</body>
4848

49-
</html>
49+
</html>

test/e2e/elements/map-tile/map-tile.test.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,32 @@ test.describe('Map Tile Dynamic Updates Tests', () => {
110110
expect(removedTiles.removedCount).toBeGreaterThan(0);
111111
});
112112

113+
test('removing the last map-tile in a sequence removes MapTileLayer container', async () => {
114+
// Reset by reloading the page
115+
await page.reload();
116+
await page.waitForTimeout(1000);
117+
118+
const viewer = page.getByTestId('viewer');
119+
let nTiles = await viewer.evaluate(
120+
(v) => v.querySelectorAll('map-tile').length
121+
);
122+
expect(nTiles).toEqual(15);
123+
const containerConnected = await viewer.evaluate((v) => {
124+
v.querySelector('map-tile')._tileLayer._container.setAttribute(
125+
'data-testid',
126+
'test-tile-container'
127+
);
128+
return v.querySelector('map-tile')._tileLayer._container.isConnected;
129+
});
130+
expect(containerConnected).toBe(true);
131+
nTiles = await viewer.evaluate((v) => {
132+
v.querySelectorAll('map-tile').forEach((el) => el.remove());
133+
return v.querySelectorAll('map-tile').length;
134+
});
135+
expect(nTiles).toEqual(0);
136+
await expect(page.getByTestId('test-tile-container')).toHaveCount(0);
137+
});
138+
113139
test('adding map-tile to DOM renders it on map', async () => {
114140
// Reset by reloading the page
115141
await page.reload();

0 commit comments

Comments
 (0)