Skip to content

Commit

Permalink
Fix client hangs when hovering over a <map-a> that is too long issue. (
Browse files Browse the repository at this point in the history
…#972)

Closes #971
  • Loading branch information
prushforth authored Aug 14, 2024
1 parent 9694e4f commit 4577f57
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 15 deletions.
6 changes: 5 additions & 1 deletion src/mapml.css
Original file line number Diff line number Diff line change
Expand Up @@ -683,12 +683,16 @@ svg.leaflet-image-layer.leaflet-interactive g {
bottom: 0;
background-color: rgb(222,225,230);
border-radius: 0 5px 0 0 ;
z-index: 1050;
z-index: 1050;
max-width: 60%;
}

.mapml-link-preview > p {
margin: 3px 5px 2px 3px;
color: rgb(60,64,67);
text-overflow: ellipsis;
overflow-x: hidden;
white-space: nowrap;
}

/**
Expand Down
13 changes: 4 additions & 9 deletions src/mapml/features/path.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,20 +116,15 @@ export var Path = L.Path.extend({
(e) => {
if (e.target !== e.currentTarget) return;
hovered = true;
let resolver = document.createElement('a'),
mapWidth = this._map.getContainer().clientWidth;
let resolver = document.createElement('a');
resolver.href = link.url;
p.innerHTML = resolver.href;

this._map.getContainer().appendChild(container);

while (p.clientWidth > mapWidth / 2) {
p.innerHTML =
p.innerHTML.substring(0, p.innerHTML.length - 5) + '...';
}
setTimeout(() => {
if (hovered) p.innerHTML = resolver.href;
}, 1000);
// setTimeout(() => {
// if (hovered) p.innerHTML = resolver.href;
// }, 1000);
},
this
);
Expand Down
2 changes: 1 addition & 1 deletion src/mapml/layers/FeatureLayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ export var FeatureLayer = L.FeatureGroup.extend({
);
}
let cs =
feature.getElementsByTagName('map-geometry')[0]?.getAttribute('cs') ||
feature.getElementsByTagName('map-geometry')[0]?.getAttribute('cs') ??
fallbackCS;
// options.layerBounds and options.zoomBounds are set by TemplatedTileLayer._createFeatures
// each geometry needs bounds so that it can be a good community member of this._layers
Expand Down
1 change: 1 addition & 0 deletions src/mapml/layers/TemplatedFeaturesLayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ export var TemplatedFeaturesLayer = L.Layer.extend({
}
});
L.extend(this._features.options, { _leafletLayer: this._features });
this._features._layerEl = this._linkEl.getLayerEl();
} else {
this._features.eachLayer((layer) => layer.addTo(map));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,16 @@ test.describe('map-a loaded inline or remote, directly or via templated map-link
);
});
}
test('Long map-a href is actionable, does not cause client to hang', async () => {
const longUrl = "http://localhost:8080/geoserver/tiger/wms?service=WMS&version=1.1.0&request=GetMap&layers=tiger%3Atiger_roads&bbox=-74.02722%2C40.684221%2C-73.907005%2C40.878178&width=476&height=768&srs=EPSG%3A4326&styles=&format=text%2Fmapml";
// remove the top three layers (the bottom layer has a long href)
await page.getByTestId('remote-templated').evaluate(layer => layer.remove());
await page.getByTestId('inline-templated').evaluate(layer => layer.remove());
await page.getByTestId('remote-features').evaluate(layer => layer.remove());
const aWithLongHref = page.getByRole('link', { name: 'Long href' });
await aWithLongHref.hover();
const toast = await page.locator('.mapml-link-preview > p').evaluate((p) => p.innerText );
expect(toast).toEqual(longUrl);

});
});
5 changes: 3 additions & 2 deletions test/e2e/elements/map-a/map-a.html
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,10 @@
</table>
</map-properties>
</map-feature>
<map-feature id="poi.1" class="poi-r1-s1 poi-r1-s2">
<map-feature data-testid="poi-1" id="poi.1" class="poi-r1-s1 poi-r1-s2">
<map-featurecaption>Long href</map-featurecaption>
<map-geometry>
<map-a href="https://example.org/" target="_top">
<map-a type="text/mapml" target="_blank" href="http://localhost:8080/geoserver/tiger/wms?service=WMS&version=1.1.0&request=GetMap&layers=tiger%3Atiger_roads&bbox=-74.02722%2C40.684221%2C-73.907005%2C40.878178&width=476&height=768&srs=EPSG%3A4326&styles=&format=text%2Fmapml">
<map-point>
<map-coordinates>-8238806.8429565085 4969306.111096254</map-coordinates>
</map-point>
Expand Down
8 changes: 6 additions & 2 deletions test/e2e/layers/multipleExtents.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -261,9 +261,11 @@ test.describe('Multiple Extents Bounds Tests', () => {
await expect(
page.locator('.mapml-debug-vectors.projection-centre ')
).toHaveCount(1);
// templated features formerly did not keep track of their layer- element,
// which prevented them from being added to the debug layer. That is fixed.
await expect(
page.locator('.mapml-debug-vectors.multiple-extents')
).toHaveCount(2);
).toHaveCount(6);
await expect(
page.locator('.mapml-debug-vectors.single-extent')
).toHaveCount(1);
Expand Down Expand Up @@ -312,9 +314,11 @@ test.describe('Multiple Extents Bounds Tests', () => {
await expect(
page.locator('.mapml-debug-vectors.projection-centre ')
).toHaveCount(1);
// templated features formerly did not keep track of their layer- element,
// which prevented them from being added to the debug layer. That is fixed.
await expect(
page.locator('.mapml-debug-vectors.multiple-extents')
).toHaveCount(2);
).toHaveCount(6);
await expect(
page.locator('.mapml-debug-vectors.single-extent')
).toHaveCount(1);
Expand Down

0 comments on commit 4577f57

Please sign in to comment.