Skip to content

Enable layer name change #865

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

Merged
merged 1 commit into from
Jun 28, 2023
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
6 changes: 1 addition & 5 deletions src/layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,7 @@ export class MapLayer extends HTMLElement {
attributeChangedCallback(name, oldValue, newValue) {
switch (name) {
case 'label':
if (oldValue !== newValue) {
this.dispatchEvent(
new CustomEvent('labelchanged', { detail: { target: this } })
);
}
this?._layer?.setName(newValue);
break;
case 'checked':
if (this._layer) {
Expand Down
16 changes: 15 additions & 1 deletion src/mapml/layers/MapMLLayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,19 @@ export var MapMLLayer = L.Layer.extend({
this._setLayerElExtent();
}
},
setName(newName) {
// a layer's accessible name is set by the <map-title>, if present
// if it's not available the <layer- label="accessible-name"> attribute
// can be used
if (!this._titleIsReadOnly) {
this._title = newName;
this._mapmlLayerItem.querySelector('.mapml-layer-item-name').innerHTML =
newName;
}
},
getName() {
return this._title;
},

onAdd: function (map) {
if (this._extent && !this._validProjection(map)) {
Expand Down Expand Up @@ -1615,6 +1628,7 @@ export var MapMLLayer = L.Layer.extend({

if (mapml.querySelector('map-title')) {
layer._title = mapml.querySelector('map-title').textContent.trim();
layer._titleIsReadOnly = true;
} else if (mapml instanceof Element && mapml.hasAttribute('label')) {
layer._title = mapml.getAttribute('label').trim();
}
Expand Down Expand Up @@ -1642,7 +1656,7 @@ export var MapMLLayer = L.Layer.extend({
layer._layerEl.parentElement._toggleControls();
}
layer._layerEl.dispatchEvent(
new CustomEvent('extentload', { detail: layer })
new CustomEvent('extentload', { detail: layer, bubbles: true })
);
}

Expand Down
81 changes: 81 additions & 0 deletions test/e2e/api/domApi-HTMLLayerElement.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
<!DOCTYPE html>
<html lang="en">
<head>
<title>domApi-HTMLLayerElement.html</title>
<meta charset="UTF-8">
<script type="module" src="mapml-viewer.js"></script>
</head>
<body>
<template>
<layer- id="remote-with-title" label="This should not appear in layer control" src="with-title.mapml" checked></layer->
<layer- id="remote-no-title" label="Layer with name settable via HTMLLayerElement.label" src="no-title.mapml" checked></layer->
<layer- id="local-no-title" label="Another Layer with name settable via HTMLLayerElement.label" checked>
<map-meta name="projection" content="OSMTILE"></map-meta>
<map-feature id="hg" zoom="3" min="0" max="22">
<map-featurecaption>Statue of a guy with a hat</map-featurecaption>
<map-geometry cs="gcrs">
<map-point>
<map-coordinates>-75.70628 45.39878</map-coordinates>
</map-point>
</map-geometry>
<map-properties>
<h1>A Man With Two Hats</h1>
</map-properties>
</map-feature>
</layer->
<layer- id="local-with-title" label="This should not appear in layer control" checked>
<map-title>Layer name set via local map-title element - unsettable via HTMLLayerelement.label</map-title>
<map-meta name="projection" content="OSMTILE"></map-meta>
<map-feature id="hg" zoom="3" min="0" max="22">
<map-featurecaption>Statue of a guy with a hat</map-featurecaption>
<map-geometry cs="gcrs">
<map-point>
<map-coordinates>-75.70528 45.39778</map-coordinates>
</map-point>
</map-geometry>
<map-properties>
<h1>A Man With Two Hats</h1>
</map-properties>
</map-feature>
</layer->
</template>
<mapml-viewer projection="OSMTILE" controls zoom="15" lat="45.39778" lon="-75.70528" width="500" height="500">
</mapml-viewer>
</body>
<script>

let remoteWithTitle = document.querySelector('template').content.cloneNode(true).querySelector('#remote-with-title');
remoteWithTitle = document.querySelector('mapml-viewer').appendChild(remoteWithTitle);
let remoteNoTitle = document.querySelector('template').content.cloneNode(true).querySelector('#remote-no-title');
remoteNoTitle = document.querySelector('mapml-viewer').appendChild(remoteNoTitle);
let localNoTitle = document.querySelector('template').content.cloneNode(true).querySelector('#local-no-title');
localNoTitle = document.querySelector('mapml-viewer').appendChild(localNoTitle);
let localWithTitle = document.querySelector('template').content.cloneNode(true).querySelector('#local-with-title');
localWithTitle = document.querySelector('mapml-viewer').appendChild(localWithTitle);


addEventListener("extentload", (e) => {
if (e.target === remoteWithTitle) {
// setting label should not change the layer name in layer control
remoteWithTitle.label = "Unforsettable in every way";
} else if (e.target === remoteNoTitle) {
// setting label should set the layer name in layer control
remoteNoTitle.label = "Bedtime For Bonzo";
} else if (e.target === localWithTitle) {
// setting label should not set the layer name in layer control
localWithTitle.label = "No dice, buddy!";
} else if (e.target === localNoTitle) {
// setting label should set the layer name in layer control
// but because the layer is not fully initialized, need to wait unit
// it triggers the createmap event, which eventually creates layer-._layer
// but if we run this too early, before createmap, then the setter
// for label does not have any (desired) side effects, namely running
// layer-._layer.setName(labelValue). hence setTimeout.
setTimeout(()=>{
localNoTitle.label = "Go ahead, make my day!";
}, 500);
}
});

</script>
</html>
68 changes: 68 additions & 0 deletions test/e2e/api/domApi-HTMLLayerElement.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import { test, expect, chromium } from '@playwright/test';

test.describe('HTMLLayerElement DOM API Tests', () => {
let page;
let context;
test.beforeAll(async () => {
context = await chromium.launchPersistentContext('');
page =
context.pages().find((page) => page.url() === 'about:blank') ||
(await context.newPage());
page = await context.newPage();
await page.goto('domApi-HTMLLayerElement.html');
});

test.afterAll(async function () {
await context.close();
});
test('Setting HTMLLayerElement.label sets the layer name per spec', async () => {
let remoteWithTitleLabel = await page.evaluate(() => {
return document.querySelector('#remote-with-title').label;
});
expect(remoteWithTitleLabel).toEqual('Unforsettable in every way');
let remoteWithTitleName = await page.evaluate(() => {
let layer = document.querySelector('#remote-with-title');
return layer._layer.getName();
});
expect(remoteWithTitleName).toEqual(
'MapML author-controlled name - unsettable'
);

let remoteNoTitleLabel = await page.evaluate(() => {
return document.querySelector('#remote-no-title').label;
});
expect(remoteNoTitleLabel).toEqual('Bedtime For Bonzo');
let remoteNoTitleName = await page.evaluate(() => {
let layer = document.querySelector('#remote-no-title');
return layer._layer.getName();
});
expect(remoteNoTitleName).toEqual(remoteNoTitleLabel);

let localWithTitleLabel = await page.evaluate(() => {
return document.querySelector('#local-with-title').label;
});
expect(localWithTitleLabel).toEqual('No dice, buddy!');
let localWithTitleName = await page.evaluate(() => {
let layer = document.querySelector('#local-with-title');
return layer._layer.getName();
});
expect(localWithTitleName).not.toEqual(localWithTitleLabel);

// THIS SHOULD NOT BE NECESSARY, BUT IT IS see comment below
await page.waitForTimeout(500);
let localNoTitleLabel = await page.evaluate(() => {
return document.querySelector('#local-no-title').label;
});
expect(localNoTitleLabel).toEqual('Go ahead, make my day!');
let localNoTitleName = await page.evaluate(() => {
let layer = document.querySelector('#local-no-title');
return layer._layer.getName();
});
// this isn't working, because waiting for the createmap event means
// that the layer-._layer doesn't exist, so attributeChangeCallback on label
// shortcircuits (does / can not setName on _layer) unless you wait for it.
// need to ditch the createmap event!!
expect(localNoTitleName).toEqual(localNoTitleLabel);
});
// add other tests here for HTMLLayerElement
});
26 changes: 26 additions & 0 deletions test/e2e/api/no-title.mapml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<mapml- xmlns="http://www.w3.org/1999/xhtml">
<map-head>
<!-- no map-title provided means the HTML <layer- src> that references
this file may set its name via the HTMLLayerElement.label setter -->
<map-meta charset="utf-8" ></map-meta>
<map-meta content="text/mapml" http-equiv="Content-Type" ></map-meta>
<map-link rel="license" href="http://opendatacommons.org/licenses/odbl/1-0/" title="© OpenStreetMap and contributors" />
<map-meta name="projection" content="OSMTILE"></map-meta>
<map-meta name="zoom" content="min=0,max=22,value=3" ></map-meta>
<map-meta name="cs" content="gcrs" ></map-meta>
<map-meta name="extent" content="top-left-easting=-8433179, top-left-northing=5689316, bottom-right-easting=-8420968, bottom-right-northing=5683139"></map-meta>
</map-head>
<map-body>
<map-feature id="hg" zoom="3" min="0" max="22">
<map-featurecaption>Statue of a guy with a hat</map-featurecaption>
<map-geometry cs="gcrs">
<map-point>
<map-coordinates>-75.70528 45.39878</map-coordinates>
</map-point>
</map-geometry>
<map-properties>
<h1>A Man With Two Hats</h1>
</map-properties>
</map-feature>
</map-body>
</mapml->
25 changes: 25 additions & 0 deletions test/e2e/api/with-title.mapml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<mapml- xmlns="http://www.w3.org/1999/xhtml">
<map-head>
<map-title>MapML author-controlled name - unsettable</map-title>
<map-meta charset="utf-8" ></map-meta>
<map-meta content="text/mapml" http-equiv="Content-Type" ></map-meta>
<map-link rel="license" href="http://opendatacommons.org/licenses/odbl/1-0/" title="© OpenStreetMap and contributors" />
<map-meta name="projection" content="OSMTILE"></map-meta>
<map-meta name="zoom" content="min=0,max=22,value=3" ></map-meta>
<map-meta name="cs" content="gcrs" ></map-meta>
<map-meta name="extent" content="top-left-easting=-8433179, top-left-northing=5689316, bottom-right-easting=-8420968, bottom-right-northing=5683139"></map-meta>
</map-head>
<map-body>
<map-feature id="hg" zoom="3" min="0" max="22">
<map-featurecaption>Statue of a guy with a hat</map-featurecaption>
<map-geometry cs="gcrs">
<map-point>
<map-coordinates>-75.70628 45.39778</map-coordinates>
</map-point>
</map-geometry>
<map-properties>
<h1>A Man With Two Hats</h1>
</map-properties>
</map-feature>
</map-body>
</mapml->