Skip to content
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

Fix issue #935 #954

Merged
merged 5 commits into from
Apr 10, 2024
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
59 changes: 37 additions & 22 deletions src/map-extent.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,13 @@ export class MapExtent extends HTMLElement {
let extentsRootFieldset =
this.parentLayer._propertiesGroupAnatomy;
let position = Array.from(
this.parentNode.querySelectorAll('map-extent:not([hidden])')
this.parentLayer.src
? this.parentLayer.shadowRoot.querySelectorAll(
':host > map-extent:not([hidden])'
)
: this.parentLayer.querySelectorAll(
':scope > map-extent:not([hidden])'
)
).indexOf(this);
if (newValue !== null) {
// remove from layer control (hide from user)
Expand All @@ -190,12 +196,18 @@ export class MapExtent extends HTMLElement {
this._layerControlHTML
);
} else if (position > 0) {
this.parentNode
.querySelectorAll('map-extent:not([hidden])')
[position - 1]._layerControlHTML.insertAdjacentElement(
'afterend',
this._layerControlHTML
);
Array.from(
this.parentLayer.src
? this.parentLayer.shadowRoot.querySelectorAll(
':host > map-extent:not([hidden])'
)
: this.parentLayer.querySelectorAll(
':scope > map-extent:not([hidden])'
)
)[position - 1]._layerControlHTML.insertAdjacentElement(
'afterend',
this._layerControlHTML
);
}
}
this._validateLayerControlContainerHidden();
Expand All @@ -220,16 +232,13 @@ export class MapExtent extends HTMLElement {
async connectedCallback() {
// this.parentNode.host returns the layer- element when parentNode is
// the shadow root
this.parentLayer =
this.parentNode.nodeName.toUpperCase() === 'LAYER-'
? this.parentNode
: this.parentNode.host;
this.parentLayer = this.getLayerEl();
if (
this.hasAttribute('data-moving') ||
this.parentLayer.hasAttribute('data-moving')
)
return;
this.mapEl = this.parentLayer.closest('mapml-viewer,map[is=web-map]');
this.mapEl = this.getMapEl();
await this.mapEl.whenProjectionDefined(this.units).catch(() => {
throw new Error('Undefined projection:' + this.units);
});
Expand All @@ -252,7 +261,9 @@ export class MapExtent extends HTMLElement {
opacity: this.opacity,
crs: M[this.units],
extentZIndex: Array.from(
this.parentLayer.querySelectorAll('map-extent')
this.parentLayer.src
? this.parentLayer.shadowRoot.querySelectorAll(':host > map-extent')
: this.parentLayer.querySelectorAll(':scope > map-extent')
).indexOf(this),
extentEl: this
});
Expand Down Expand Up @@ -418,9 +429,11 @@ export class MapExtent extends HTMLElement {
// can be added to mapmllayer layerGroup no matter layer- is checked or not
this._extentLayer.addTo(this.parentLayer._layer);
this._extentLayer.setZIndex(
Array.from(this.parentLayer.querySelectorAll('map-extent')).indexOf(
this
)
Array.from(
this.parentLayer.src
? this.parentLayer.shadowRoot.querySelectorAll(':host > map-extent')
: this.parentLayer.querySelectorAll(':scope > map-extent')
).indexOf(this)
);
} else {
this.parentLayer._layer?.removeLayer(this._extentLayer);
Expand All @@ -430,13 +443,15 @@ export class MapExtent extends HTMLElement {
}
_validateLayerControlContainerHidden() {
let extentsFieldset = this.parentLayer._propertiesGroupAnatomy;
let nodeToSearch = this.parentLayer.src
? this.parentLayer.shadowRoot
: this.parentLayer;
if (!extentsFieldset) return;
if (
nodeToSearch.querySelectorAll('map-extent:not([hidden])').length === 0
) {
const numberOfVisibleSublayers = (
this.parentLayer.src
? this.parentLayer.shadowRoot.querySelectorAll(
':host > map-extent:not([hidden])'
)
: this.parentLayer.querySelectorAll(':scope > map-extent:not([hidden])')
).length;
if (numberOfVisibleSublayers === 0) {
extentsFieldset.setAttribute('hidden', '');
} else {
extentsFieldset.removeAttribute('hidden');
Expand Down
47 changes: 45 additions & 2 deletions src/mapml/control/LayerControl.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,52 @@ export var LayerControl = L.Control.Layers.extend({
}
},

_withinZoomBounds: function (zoom, range) {
return range.min <= zoom && zoom <= range.max;
// imported from leaflet with slight modifications
// for layerControl ordering based on zIndex
_update: function () {
if (!this._container) {
return this;
}

L.DomUtil.empty(this._baseLayersList);
L.DomUtil.empty(this._overlaysList);

this._layerControlInputs = [];
var baseLayersPresent,
overlaysPresent,
i,
obj,
baseLayersCount = 0;

// <----------- MODIFICATION from the default _update method
// sort the layercontrol layers object based on the zIndex
// provided by MapMLLayer
if (this.options.sortLayers) {
this._layers.sort((a, b) =>
this.options.sortFunction(a.layer, b.layer, a.name, b.name)
);
}
// -------------------------------------------------->
for (i = 0; i < this._layers.length; i++) {
obj = this._layers[i];
this._addItem(obj);
overlaysPresent = overlaysPresent || obj.overlay;
baseLayersPresent = baseLayersPresent || !obj.overlay;
baseLayersCount += !obj.overlay ? 1 : 0;
}

// Hide base layers section if there's only one layer.
if (this.options.hideSingleBase) {
baseLayersPresent = baseLayersPresent && baseLayersCount > 1;
this._baseLayersList.style.display = baseLayersPresent ? '' : 'none';
}

this._separator.style.display =
overlaysPresent && baseLayersPresent ? '' : 'none';

return this;
},

_addItem: function (obj) {
var layercontrols = obj.layer._layerEl._layerControlHTML;
// the input is required by Leaflet...
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ export var createLayerControlExtentHTML = function () {
extentSettings
),
extentOpacitySummary = L.DomUtil.create('summary', '', opacityControl),
mapEl = this.parentLayer.parentNode,
layerEl = this.parentLayer,
mapEl = this.getMapEl(),
layerEl = this.getLayerEl(),
opacity = L.DomUtil.create('input', '', opacityControl);
extentSettings.hidden = true;
extent.setAttribute('aria-grabbed', 'false');
Expand Down Expand Up @@ -227,7 +227,8 @@ export var createLayerControlExtentHTML = function () {
let extentEl = c.querySelector('span').extent;

extentEl.setAttribute('data-moving', '');
layerEl.insertAdjacentElement('beforeend', extentEl);
const node = layerEl.src ? layerEl.shadowRoot : layerEl;
node.append(extentEl);
extentEl.removeAttribute('data-moving');

extentEl.extentZIndex = zIndex;
Expand Down
80 changes: 80 additions & 0 deletions test/e2e/core/drag.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,4 +128,84 @@ test.describe('UI Drag&Drop Test', () => {
expect(layerIndex).toEqual('1');
expect(controlText).toBe('Static MapML with tiles');
});
test('Re-order checked bug (#955) test', async () => {
await page.waitForTimeout(500);
const layerControl = page.locator('.leaflet-top.leaflet-right');
await layerControl.hover();
const overlaysList = page.locator('.leaflet-control-layers-overlays');
const startingLowestLayer = overlaysList
.getByRole('group', { name: 'Canada Base Map - Transportation (CBMT)' })
.first();

// assert that CBMT is first of three layers
const cbmtIsFirstInLayerControl = await overlaysList.evaluate((l) => {
return (
l.firstElementChild.querySelector('.mapml-layer-item-name')
.textContent === 'Canada Base Map - Transportation (CBMT)'
);
});
expect(cbmtIsFirstInLayerControl).toBe(true);

const fromBox = await startingLowestLayer.boundingBox();
const fromPos = {
x: fromBox.x + fromBox.width / 2,
y: fromBox.y + fromBox.height / 3
};
let toPos = { x: fromPos.x, y: fromPos.y + fromBox.height * 1.1 };

// move (drag/drop) CBMT to the top of the layer stack
await page.mouse.move(fromPos.x, fromPos.y);
await page.mouse.down();
await page.mouse.move(toPos.x, toPos.y);
await page.mouse.up();
await page.mouse.move(toPos.x, toPos.y);
toPos = { x: toPos.x, y: toPos.y + fromBox.height * 1.1 };
await page.mouse.down();
await page.mouse.move(toPos.x, toPos.y);
await page.mouse.up();

let cbmtIsTopOfLayerControl = await overlaysList.evaluate((l) => {
return (
l.querySelectorAll(':scope > fieldset').length === 3 &&
l.lastElementChild.querySelector('.mapml-layer-item-name')
.textContent === 'Canada Base Map - Transportation (CBMT)'
);
});
// assert that CBMT is third of three layers
expect(cbmtIsTopOfLayerControl).toBe(true);

const cbmtCheckbox = overlaysList
.getByRole('checkbox', {
name: 'Canada Base Map - Transportation (CBMT)'
})
.first();
let cbmtIsChecked = await cbmtCheckbox.isChecked();
expect(cbmtIsChecked).toBe(true);
await page.pause();
await cbmtCheckbox.click();
cbmtIsChecked = await cbmtCheckbox.isChecked();
expect(cbmtIsChecked).toBe(false);
// cbmt layer should still be on top of layer control despite that it's unchecked
cbmtIsTopOfLayerControl = await overlaysList.evaluate((l) => {
return (
l.querySelectorAll(':scope > fieldset').length === 3 &&
l.lastElementChild.querySelector('.mapml-layer-item-name')
.textContent === 'Canada Base Map - Transportation (CBMT)'
);
});
// assert that CBMT is still third of three layers
expect(cbmtIsTopOfLayerControl).toBe(true);
await cbmtCheckbox.click();
cbmtIsChecked = await cbmtCheckbox.isChecked();
expect(cbmtIsChecked).toBe(true);
cbmtIsTopOfLayerControl = await overlaysList.evaluate((l) => {
return (
l.querySelectorAll(':scope > fieldset').length === 3 &&
l.lastElementChild.querySelector('.mapml-layer-item-name')
.textContent === 'Canada Base Map - Transportation (CBMT)'
);
});
// assert that CBMT is still third of three layers
expect(cbmtIsTopOfLayerControl).toBe(true);
});
});
Binary file added test/e2e/elements/map-extent/Img_Sample.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
92 changes: 92 additions & 0 deletions test/e2e/elements/map-extent/map-extent-checked-ordering.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import { test, expect, chromium } from '@playwright/test';

test.describe('map-extent checked order tests', () => {
let page;
let context;
test.beforeAll(async function () {
context = await chromium.launchPersistentContext('', { slowMo: 500 });
page =
context.pages().find((page) => page.url() === 'about:blank') ||
(await context.newPage());
await page.goto('map-extent-checked.html');
});
test('map-extent layer control order correct when cycling checked state', async () => {
// Fixed #935 https://github.com/Maps4HTML/Web-Map-Custom-Element/issues/935
/*
Go to this map map-extent-checked.html

Open the layer control for the layer settings.

Un-check the imagery layer <map-extent>
Check the imagery layer <map-extent>

What should happen:
The imagery layer <map-extent> should draw underneath the states layer.

What actually happens:
The imagery layer <map-extent> draws on top of the states layer.
* */
const layerControl = page.locator('.leaflet-top.leaflet-right');
await layerControl.hover();
const layerSettings = layerControl.getByTitle('Layer Settings');
await layerSettings.click();
const imageryExtentCheckbox = layerControl.getByRole('checkbox', {
name: 'Extent One'
});
await imageryExtentCheckbox.click(); // turn it off
await imageryExtentCheckbox.click(); // turn it on
const ext1 = page.getByTestId('ext1');
let imageryZIndex = await ext1.evaluate((e) => {
return +e._extentLayer._container.style.zIndex;
});
expect(imageryZIndex).toEqual(0);
const ext2 = page.getByTestId('ext2');
let statesZIndex = await ext2.evaluate((e) => {
return +e._extentLayer._container.style.zIndex;
});
expect(statesZIndex).toEqual(1);
// re-order them via the layer control
const imageryFieldset = layerControl.getByRole('group', {
name: 'Extent One'
});
let controlBBox = await imageryFieldset.boundingBox();
let from = {
x: controlBBox.x + controlBBox.width / 2,
y: controlBBox.y + controlBBox.height / 2
},
to = { x: from.x, y: from.y + controlBBox.height * 1.1 };

await page.mouse.move(from.x, from.y);
await page.mouse.down();
await page.mouse.move(to.x, to.y);
await page.mouse.up();
imageryZIndex = await ext1.evaluate((e) => {
return +e._extentLayer._container.style.zIndex;
});
expect(imageryZIndex).toEqual(1);
statesZIndex = await ext2.evaluate((e) => {
return +e._extentLayer._container.style.zIndex;
});
expect(statesZIndex).toEqual(0);

await page.mouse.move(from.x, from.y);
await page.mouse.down();
await page.mouse.move(to.x, to.y);
await page.mouse.up();
await imageryExtentCheckbox.click(); // turn it off
await imageryExtentCheckbox.click(); // turn it on
imageryZIndex = await ext1.evaluate((e) => {
return +e._extentLayer._container.style.zIndex;
});
expect(imageryZIndex).toEqual(0);
statesZIndex = await ext2.evaluate((e) => {
return +e._extentLayer._container.style.zIndex;
});
expect(statesZIndex).toEqual(1);
// TO DO re-order them via the DOM (insertAdjacentHTML),
// ensure that
// a) render order/z-index is correct
// b) render order is reflected in layer control order as well
// see https://github.com/Maps4HTML/Web-Map-Custom-Element/issues/956
});
});
Loading