Skip to content

Commit

Permalink
Add map-projectionchange event. (#911)
Browse files Browse the repository at this point in the history
* Add map-projectionchange event. Delete map projection attrChgCallbk
from triggering map-change event. Allow map-extent to react to 
map-projectionchange event, enable/disable according to projection match.

* Change initialization logic for opacity, so that layer- and map-extent
opacity value is maintained through map-projectionchange event.

* Remove MapMLLayer.validProjection attribute and flawed logic.

* Prettier formatting change only in multipleExtents.test.js

* Remove use of validProjection by layer context menu, was wrong anyway(?)

* Add waitfortimeout of 500ms in customTCRS.test.js (100ms was not enough)

* Add test for simple projection change.

* Add test for empty map history after map-projectionchange event

* Add test for opacity on layer and map-extent after map-projectionchange

* Bump timeout after map-projectionchange as opacity value wasn't
updating in time. Skip test of history after map-projectionchange, pending
full rewrite of history api tbd.

* Add await for mapEl.whenProjectionDefined(this.units) to map-extent,
so that invalid / undefined projections e.g. 'foo' will throw.

* Add test of map-extent units attribute set to undefined projection

* Optimize map-extent.test.js to speed it up a little

---------

Co-authored by: aliyanh <@AliyanH>
Co-authored by: Hanyu Y <@yhy0217>
Co-authored-by: prushfor <[email protected]>
  • Loading branch information
prushforth authored Nov 15, 2023
1 parent fde273c commit 5150ba3
Show file tree
Hide file tree
Showing 12 changed files with 277 additions and 152 deletions.
27 changes: 11 additions & 16 deletions src/layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export class MapLayer extends HTMLElement {
this._createLayerControlHTML = M._createLayerControlHTML.bind(this);
// this._opacity is used to record the current opacity value (with or without updates),
// the initial value of this._opacity should be set as opacity attribute value, if exists, or the default value 1.0
this._opacity = +(this.getAttribute('opacity') || 1.0);
this._opacity = this.opacity || 1.0;
const doConnected = this._onAdd.bind(this);
this.parentElement
.whenReady()
Expand Down Expand Up @@ -345,25 +345,20 @@ export class MapLayer extends HTMLElement {
'_mapmlvectors',
'_templatedLayer'
];
if (layer.validProjection) {
for (let j = 0; j < layerTypes.length; j++) {
let type = layerTypes[j];
if (this.checked) {
if (type === '_templatedLayer' && mapExtents.length > 0) {
for (let i = 0; i < mapExtents.length; i++) {
totalExtentCount++;
if (mapExtents[i]._validateDisabled()) disabledExtentCount++;
}
} else if (layer[type]) {
// not a templated layer
for (let j = 0; j < layerTypes.length; j++) {
let type = layerTypes[j];
if (this.checked) {
if (type === '_templatedLayer' && mapExtents.length > 0) {
for (let i = 0; i < mapExtents.length; i++) {
totalExtentCount++;
if (!layer[type].isVisible) disabledExtentCount++;
if (mapExtents[i]._validateDisabled()) disabledExtentCount++;
}
} else if (layer[type]) {
// not a templated layer
totalExtentCount++;
if (!layer[type].isVisible) disabledExtentCount++;
}
}
} else {
disabledExtentCount = 1;
totalExtentCount = 1;
}
// if all extents are not visible / disabled, set layer to disabled
if (
Expand Down
8 changes: 7 additions & 1 deletion src/map-extent.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,10 @@ export class MapExtent extends HTMLElement {
this.attachShadow({ mode: 'open' });
}
await this.parentLayer.whenReady();
this.mapEl = this.parentLayer.closest('mapml-viewer,map[is=web-map]');
await this.mapEl.whenProjectionDefined(this.units).catch(() => {
throw new Error('Undefined projection:' + this.units);
});
// when projection is changed, the parent layer-._layer is created (so whenReady is fulfilled) but then removed,
// then the map-extent disconnectedCallback will be triggered by layer-._onRemove() (clear the shadowRoot)
// even before connectedCallback is finished
Expand Down Expand Up @@ -168,9 +172,10 @@ export class MapExtent extends HTMLElement {
);
this._changeHandler = this._handleChange.bind(this);
this.parentLayer.addEventListener('map-change', this._changeHandler);
this.mapEl.addEventListener('map-projectionchange', this._changeHandler);
// this._opacity is used to record the current opacity value (with or without updates),
// the initial value of this._opacity should be set as opacity attribute value, if exists, or the default value 1.0
this._opacity = +(this.getAttribute('opacity') || 1.0);
this._opacity = this.opacity || 1.0;
this._templatedLayer = M.templatedLayer(this._templateVars, {
pane: this._layer._container,
opacity: this.opacity,
Expand Down Expand Up @@ -522,6 +527,7 @@ export class MapExtent extends HTMLElement {
this._layerControlHTML.remove();
this._map.removeLayer(this._templatedLayer);
this.parentLayer.removeEventListener('map-change', this._changeHandler);
this.mapEl.removeEventListener('map-projectionchange', this._changeHandler);
delete this._templatedLayer;
delete this.parentLayer.bounds;
}
Expand Down
10 changes: 7 additions & 3 deletions src/mapml-viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -377,9 +377,13 @@ export class MapViewer extends HTMLElement {
this.zoomTo(lat, lon, zoom);
if (M.options.announceMovement)
this._map.announceMovement.enable();
this.querySelectorAll('layer-').forEach((layer) => {
layer.dispatchEvent(new CustomEvent('map-change'));
});
// required to delay until map-extent.disabled is correctly set
// which happens as a result of layer-._validateDisabled()
// which happens so much we have to delay until they calls are
// completed
setTimeout(() => {
this.dispatchEvent(new CustomEvent('map-projectionchange'));
}, 0);
});
}
};
Expand Down
1 change: 0 additions & 1 deletion src/mapml/handlers/ContextMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,6 @@ export var ContextMenu = L.Handler.extend({
.closest('fieldset')
.parentNode.parentNode.parentNode.querySelector('span')
: elem.querySelector('span');
if (!elem.layer.validProjection) return;
this._layerClicked = elem;
this._layerMenu.removeAttribute('hidden');
this._showAtPoint(e.containerPoint, e, this._layerMenu);
Expand Down
36 changes: 0 additions & 36 deletions src/mapml/layers/MapMLLayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,6 @@ export var MapMLLayer = L.Layer.extend({
// OR use the extent of the content provided

this._initialize(local ? layerEl : mapml);

// a default extent can't be correctly set without the map to provide
// its bounds , projection, zoom range etc, so if that stuff's not
// established by metadata in the content, we should use map properties
// to set the extent, but the map won't be available until the <layer>
// element is attached to the <map> element, wait for that to happen.
// weirdness. options is actually undefined here, despite the hardcoded
// options above. If you use this.options, you see the options defined
// above. Not going to change this, but failing to understand ATM.
// may revisit some time.
this.validProjection = true;
},
setZIndex: function (zIndex) {
this.options.zIndex = zIndex;
Expand Down Expand Up @@ -98,11 +87,6 @@ export var MapMLLayer = L.Layer.extend({
},

onAdd: function (map) {
// probably don't need it except for layer context menu usage
if (this._properties && !this._validProjection(map)) {
this.validProjection = false;
return;
}
this._map = map;
if (this._mapmlvectors) map.addLayer(this._mapmlvectors);

Expand Down Expand Up @@ -218,26 +202,6 @@ export var MapMLLayer = L.Layer.extend({
}
},

_validProjection: function (map) {
const mapExtents = this._layerEl.querySelectorAll('map-extent');
let noLayer = false;
if (this._properties && mapExtents.length > 0) {
for (let i = 0; i < mapExtents.length; i++) {
if (mapExtents[i]._templateVars) {
for (let template of mapExtents[i]._templateVars)
if (
!template.projectionMatch &&
template.projection !== map.options.projection
) {
noLayer = true; // if there's a single template where projections don't match, set noLayer to true
break;
}
}
}
}
return !(noLayer || this.getProjection() !== map.options.projection);
},

addTo: function (map) {
map.addLayer(this);
return this;
Expand Down
10 changes: 7 additions & 3 deletions src/web-map.js
Original file line number Diff line number Diff line change
Expand Up @@ -422,9 +422,13 @@ export class WebMap extends HTMLMapElement {
this.zoomTo(lat, lon, zoom);
if (M.options.announceMovement)
this._map.announceMovement.enable();
this.querySelectorAll('layer-').forEach((layer) => {
layer.dispatchEvent(new CustomEvent('map-change'));
});
// required to delay until map-extent.disabled is correctly set
// which happens as a result of layer-._validateDisabled()
// which happens so much we have to delay until they calls are
// completed
setTimeout(() => {
this.dispatchEvent(new CustomEvent('map-projectionchange'));
}, 0);
});
}
};
Expand Down
38 changes: 38 additions & 0 deletions test/e2e/api/events/map-projectionchange.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>map-projectionchange event</title>
<!-- the layer in this map should continue to be visible when you change
the viewer projection from OSMTILE to CBMTILE. -->
<script type="module" src="/mapml-viewer.js"></script>
</head>
<body>
<mapml-viewer zoom="2" lon="-75.703611" lat="45.411105" width="500" height="500" controls projection="OSMTILE">
<layer- label="Projection changer" checked>
<map-extent label="National Geographic" units="OSMTILE" checked >
<map-input name="TileMatrix" type="zoom" value="18" min="0" max="18"></map-input>
<map-input name="TileCol" type="location" units="tilematrix" axis="column" min="0" max="262144"></map-input>
<map-input name="TileRow" type="location" units="tilematrix" axis="row" min="0" max="262144"></map-input>
<map-link rel="tile" tref="https://server.arcgisonline.com/arcgis/rest/services/NatGeo_World_Map/MapServer/WMTS/tile/1.0.0/NatGeo_World_Map/default/default028mm/{TileMatrix}/{TileRow}/{TileCol}.jpg"></map-link>
</map-extent>
<map-extent label="Canada Base Map - Transportation" units="CBMTILE" checked >
<map-input name="z" type="zoom" min="0" max="18"></map-input>
<map-input name="y" type="location" units="tilematrix" axis="row"></map-input>
<map-input name="x" type="location" units="tilematrix" axis="column"></map-input>
<map-link rel="tile" tref="/tiles/cbmt/{z}/c{x}_r{y}.png" ></map-link>
</map-extent>
</layer->
</mapml-viewer>
<script>
function changeProjection() {
const prj = document.body.querySelector('mapml-viewer').projection;
if (document.body.querySelector('mapml-viewer').projection === "OSMTILE") {
document.body.querySelector('mapml-viewer').projection = "CBMTILE";
} else {
document.body.querySelector('mapml-viewer').projection = "OSMTILE";
}
}
</script>
</body>
</html>
84 changes: 84 additions & 0 deletions test/e2e/api/events/map-projectionchange.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import { test, expect, chromium } from '@playwright/test';

test.describe('map-projectionchange test ', () => {
let page;
let context;
test.beforeAll(async () => {
context = await chromium.launchPersistentContext('');
page =
context.pages().find((page) => page.url() === 'about:blank') ||
(await context.newPage());
});

test.beforeEach(async function () {
await page.goto('events/map-projectionchange.html');
await page.waitForTimeout(1000);
});

test.afterAll(async function () {
await context.close();
});

test('Multiple map-extents in different projections adapt to map-projectionchange', async () => {
const viewer = await page.locator('mapml-viewer');
expect(await viewer.evaluate((v) => v.projection)).toEqual('OSMTILE');
expect(
await viewer.evaluate((v) => {
return v.querySelector('map-extent[units=OSMTILE]').disabled;
})
).toBe(false);
expect(
await viewer.evaluate((v) => {
return v.querySelector('map-extent[units=CBMTILE]').disabled;
})
).toBe(true);
await viewer.evaluate(() => changeProjection());
await page.waitForTimeout(500);
expect(await viewer.evaluate((v) => v.projection)).toEqual('CBMTILE');
expect(
await viewer.evaluate((v) => {
return v.querySelector('map-extent[units=OSMTILE]').disabled;
})
).toBe(true);
expect(
await viewer.evaluate((v) => {
return v.querySelector('map-extent[units=CBMTILE]').disabled;
})
).toBe(false);
});
test.skip('History is empty after map-projectionchange', async () => {
// history api needs a complete review; test can't pass without many
// odd hacks, so skip for now.
const viewer = await page.locator('mapml-viewer');
expect(await viewer.evaluate((v) => v.projection)).toEqual('OSMTILE');
await viewer.evaluate(() => changeProjection());
await page.waitForTimeout(500);
expect(await viewer.evaluate((v) => v.projection)).toEqual('CBMTILE');
const reload = await page.getByLabel('Reload');
expect(await reload.evaluate((button) => button.ariaDisabled)).toBe('true');
});
test('Opacity is maintained on layer- and map-extent after map-projectionchange', async () => {
const viewer = await page.locator('mapml-viewer');
const layer = await page.locator('layer-');
await page.pause();
await layer.evaluate((layer) => (layer.opacity = 0.5));
expect(
await layer.evaluate((layer) => {
return layer.opacity;
})
).toBe(0.5);
const osmtileExtent = await page.locator('map-extent[units=OSMTILE]');
await osmtileExtent.evaluate((e) => (e.opacity = 0.4));
const cbmtileExtent = await page.locator('map-extent[units=CBMTILE]');
await cbmtileExtent.evaluate((e) => (e.opacity = 0.3));
await viewer.evaluate(() => changeProjection());
await page.waitForTimeout(1000);
expect(await osmtileExtent.evaluate((e) => e.opacity)).toBe(0.4);
expect(await cbmtileExtent.evaluate((e) => e.opacity)).toBe(0.3);
expect(
await layer.evaluate((layer) => {
return layer.opacity;
})
).toBe(0.5);
});
});
6 changes: 6 additions & 0 deletions test/e2e/elements/map-extent/map-extent.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@
<map-input name="col" type="location" axis="column" units="tilematrix" min="14" max="19"></map-input>
<map-link rel='tile' tref='/data/cbmt/{zoomLevel}/c{col}_r{row}.png'></map-link>
</map-extent>
<map-extent data-testid="ext4" label="User-generated label - 4" units="foo" checked>
<map-input name="zoomLevel" type="zoom" value="3" min="0" max="3"></map-input>
<map-input name="row" type="location" axis="row" units="tilematrix" min="14" max="21"></map-input>
<map-input name="col" type="location" axis="column" units="tilematrix" min="14" max="19"></map-input>
<map-link rel='tile' tref='/data/cbmt/{zoomLevel}/c{col}_r{row}.png'></map-link>
</map-extent>
</template>
</head>

Expand Down
Loading

0 comments on commit 5150ba3

Please sign in to comment.