Skip to content

Commit

Permalink
Add map-projectionchange event. Delete map projection attrChgCallbk
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
prushfor authored and prushfor committed Nov 10, 2023
1 parent fde273c commit e372cc4
Show file tree
Hide file tree
Showing 10 changed files with 107 additions and 62 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
5 changes: 4 additions & 1 deletion src/map-extent.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,11 @@ export class MapExtent extends HTMLElement {
);
this._changeHandler = this._handleChange.bind(this);
this.parentLayer.addEventListener('map-change', this._changeHandler);
this.mapEl = this.parentLayer.closest('mapml-viewer,map[is=web-map]');
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 +524,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>
38 changes: 38 additions & 0 deletions test/e2e/api/events/map-projectionchange.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
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());
await page.goto('events/map-projectionchange.html');
});

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

test('do something and test it', 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);

});
});
2 changes: 1 addition & 1 deletion test/e2e/layers/multipleExtents.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ test.describe('Multiple Extents Bounds Tests', () => {
const alabamaExtentItem = page.getByText('alabama_feature');
await expect(alabamaExtentItem).toHaveCount(1);
await expect(alabamaExtentItem).toHaveCSS('font-style', 'normal');

const alabamaMapExtent = page.locator('map-extent[label=alabama_feature]');
await expect(alabamaMapExtent).toHaveCount(1);
await expect(alabamaMapExtent).not.toHaveAttribute('disabled');
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/mapml-viewer/customTCRS.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ test.describe('Playwright Custom TCRS Tests', () => {
});

test('Simple Custom TCRS, tiles load, mismatched layer disabled', async () => {
await page.waitForTimeout(100);
await page.waitForTimeout(500);
const misMatchedLayerDisabled = await page.$eval(
'body > mapml-viewer:nth-child(1)',
(map) => map.querySelectorAll('layer-')[0].hasAttribute('disabled')
Expand Down

0 comments on commit e372cc4

Please sign in to comment.