Fix popup issue (#1368)

## Launch Checklist

When clicking the map to create a popup and then clicking on the map
again without closing the popup something breaks in react.
This PR fixes it.

Steps to reproduce:
1. Click on the map
2. Click on the map but not on the popup

The following malformed popup appears:
Before:
<img width="610" height="626" alt="image"
src="https://github.com/user-attachments/assets/99cbdf9a-0b3c-436d-ab13-f219ee3c6e75"
/>

After - popup looks good.
<img width="610" height="626" alt="image"
src="https://github.com/user-attachments/assets/6b379c0d-cbbf-40ed-916d-3cc10eaf4410"
/>

I couldn't make cypress reproduce the exact scenario, but I added some
basic tests for popups.

 - [x] Briefly describe the changes in this PR.
- [x] Include before/after visuals or gifs if this PR includes visual
changes.
 - [x] Write tests for all new functionality.
 - [x] Add an entry to `CHANGELOG.md` under the `## main` section.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
This commit is contained in:
Harel M
2025-09-10 14:44:38 +03:00
committed by GitHub
parent 9c85883b8a
commit c3c6118df1
8 changed files with 146 additions and 26 deletions
+1
View File
@@ -6,6 +6,7 @@
### 🐞 Bug fixes ### 🐞 Bug fixes
- Fixed an issue when clicking on a popup and then clicking on the map again
- Fix modal close button possition - Fix modal close button possition
- Fixed an issue with the generation of tranlations - Fixed an issue with the generation of tranlations
- _...Add new stuff here..._ - _...Add new stuff here..._
+19
View File
@@ -29,4 +29,23 @@ describe("map", () => {
then(get.searchControl()).shouldBeVisible(); then(get.searchControl()).shouldBeVisible();
}); });
}); });
describe("popup", () => {
beforeEach(() => {
when.setStyle("rectangles");
})
it("should open on feature click", () => {
when.clickCenter("maplibre:map");
then(get.elementByTestId("feature-layer-popup")).shouldBeVisible();
});
it("should open a second feature after closing popup", () => {
when.clickCenter("maplibre:map");
then(get.elementByTestId("feature-layer-popup")).shouldBeVisible();
when.closePopup();
then(get.elementByTestId("feature-layer-popup")).shouldNotExist();
when.clickCenter("maplibre:map");
then(get.elementByTestId("feature-layer-popup")).shouldBeVisible();
});
});
}); });
+5
View File
@@ -21,6 +21,11 @@ export default class MaputnikCypressHelper {
this.helper.when.wait(1); this.helper.when.wait(1);
this.helper.get.elementByTestId(targetElement).realMouseUp(); this.helper.get.elementByTestId(targetElement).realMouseUp();
}, },
clickCenter: (element: string) => {
this.helper.get.elementByTestId(element).realMouseDown({ button: "left", position: "center" });
this.helper.when.wait(200);
this.helper.get.elementByTestId(element).realMouseUp();
},
...this.helper.when, ...this.helper.when,
}; };
+16 -1
View File
@@ -78,6 +78,13 @@ export class MaputnikDriver {
fixture: "geojson-raster-style.json", fixture: "geojson-raster-style.json",
}, },
}); });
this.helper.given.interceptAndMockResponse({
method: "GET",
url: baseUrl + "rectangles-style.json",
response: {
fixture: "rectangles-style.json",
},
});
this.helper.given.interceptAndMockResponse({ this.helper.given.interceptAndMockResponse({
method: "GET", method: "GET",
url: "*example.local/*", url: "*example.local/*",
@@ -107,7 +114,7 @@ export class MaputnikDriver {
.selectFile("cypress/fixtures/example-style.json", { force: true }); .selectFile("cypress/fixtures/example-style.json", { force: true });
}, },
setStyle: ( setStyle: (
styleProperties: "geojson" | "raster" | "both" | "layer" | "", styleProperties: "geojson" | "raster" | "both" | "layer" | "rectangles" | "",
zoom?: number zoom?: number
) => { ) => {
const url = new URL(baseUrl); const url = new URL(baseUrl);
@@ -124,7 +131,11 @@ export class MaputnikDriver {
case "layer": case "layer":
url.searchParams.set("style", baseUrl + "example-layer-style.json"); url.searchParams.set("style", baseUrl + "example-layer-style.json");
break; break;
case "rectangles":
url.searchParams.set("style", baseUrl + "rectangles-style.json");
break;
} }
if (zoom) { if (zoom) {
url.hash = `${zoom}/41.3805/2.1635`; url.hash = `${zoom}/41.3805/2.1635`;
} }
@@ -164,6 +175,10 @@ export class MaputnikDriver {
.clear() .clear()
.type(text, { parseSpecialCharSequences: false }); .type(text, { parseSpecialCharSequences: false });
}, },
closePopup: () => {
this.helper.get.element(".maplibregl-popup-close-button").click();
}
}; };
public get = { public get = {
+92
View File
@@ -0,0 +1,92 @@
{
"version": 8,
"sources": {
"rectangles": {
"type": "geojson",
"data": {
"type": "FeatureCollection",
"features": [
{
"type": "Feature",
"properties": {},
"geometry": {
"type": "Polygon",
"coordinates": [
[
[
-130.78125,
-33.13755119234615
],
[
-130.78125,
63.548552232036414
],
[
15.468749999999998,
63.548552232036414
],
[
15.468749999999998,
-33.13755119234615
],
[
-130.78125,
-33.13755119234615
]
]
]
}
},
{
"type": "Feature",
"properties": {},
"geometry": {
"type": "Polygon",
"coordinates": [
[
[
-48.515625,
-54.97761367069625
],
[
-48.515625,
36.5978891330702
],
[
169.45312499999997,
36.5978891330702
],
[
169.45312499999997,
-54.97761367069625
],
[
-48.515625,
-54.97761367069625
]
]
]
}
}
]
}
}
},
"layers": [
{
"id": "background",
"type": "background",
"paint": {
"background-color": "white"
}
},
{
"id": "rectangles",
"type": "fill",
"source": "rectangles",
"paint": {
"fill-opacity": 0.3
}
}
]
}
+11 -23
View File
@@ -1,4 +1,4 @@
import React, {type JSX} from 'react' import React from 'react'
import {createRoot} from 'react-dom/client' import {createRoot} from 'react-dom/client'
import MapLibreGl, {LayerSpecification, LngLat, Map, MapOptions, SourceSpecification, StyleSpecification} from 'maplibre-gl' import MapLibreGl, {LayerSpecification, LngLat, Map, MapOptions, SourceSpecification, StyleSpecification} from 'maplibre-gl'
import MaplibreInspect from '@maplibre/maplibre-gl-inspect' import MaplibreInspect from '@maplibre/maplibre-gl-inspect'
@@ -17,17 +17,6 @@ import { withTranslation, WithTranslation } from 'react-i18next'
import i18next from 'i18next' import i18next from 'i18next'
import { Protocol } from "pmtiles"; import { Protocol } from "pmtiles";
function renderPopup(
popupElement: JSX.Element,
mountNode: HTMLElement,
popup: MapLibreGl.Popup
): HTMLElement {
const root = createRoot(mountNode);
popup.once('close', () => root.unmount());
root.render(popupElement);
return mountNode as HTMLElement;
}
function buildInspectStyle(originalMapStyle: StyleSpecification, coloredLayers: HighlightedLayer[], highlightedLayer?: HighlightedLayer) { function buildInspectStyle(originalMapStyle: StyleSpecification, coloredLayers: HighlightedLayer[], highlightedLayer?: HighlightedLayer) {
const backgroundLayer = { const backgroundLayer = {
"id": "background", "id": "background",
@@ -179,6 +168,7 @@ class MapMaplibreGlInternal extends React.Component<MapMaplibreGlInternalProps,
map.addControl(nav, 'top-right'); map.addControl(nav, 'top-right');
const tmpNode = document.createElement('div'); const tmpNode = document.createElement('div');
const root = createRoot(tmpNode);
const inspectPopup = new MapLibreGl.Popup({ const inspectPopup = new MapLibreGl.Popup({
closeOnClick: false closeOnClick: false
@@ -197,21 +187,19 @@ class MapMaplibreGlInternal extends React.Component<MapMaplibreGlInternalProps,
buildInspectStyle: (originalMapStyle: StyleSpecification, coloredLayers: HighlightedLayer[]) => buildInspectStyle(originalMapStyle, coloredLayers, this.props.highlightedLayer), buildInspectStyle: (originalMapStyle: StyleSpecification, coloredLayers: HighlightedLayer[]) => buildInspectStyle(originalMapStyle, coloredLayers, this.props.highlightedLayer),
renderPopup: (features: InspectFeature[]) => { renderPopup: (features: InspectFeature[]) => {
if(this.props.inspectModeEnabled) { if(this.props.inspectModeEnabled) {
return renderPopup( inspectPopup.once('open', () => {
<MapMaplibreGlFeaturePropertyPopup features={features} />, root.render(<MapMaplibreGlFeaturePropertyPopup features={features} />);
tmpNode, });
inspectPopup return tmpNode;
);
} else { } else {
return renderPopup( inspectPopup.once('open', () => {
<MapMaplibreGlLayerPopup root.render(<MapMaplibreGlLayerPopup
features={features} features={features}
onLayerSelect={this.onLayerSelectById} onLayerSelect={this.onLayerSelectById}
zoom={this.state.zoom} zoom={this.state.zoom}
/>, />,);
tmpNode, });
inspectPopup return tmpNode;
);
} }
} }
}) })
@@ -66,7 +66,7 @@ type FeaturePropertyPopupProps = {
class FeaturePropertyPopup extends React.Component<FeaturePropertyPopupProps> { class FeaturePropertyPopup extends React.Component<FeaturePropertyPopupProps> {
render() { render() {
const features = removeDuplicatedFeatures(this.props.features) const features = removeDuplicatedFeatures(this.props.features)
return <div className="maputnik-feature-property-popup"> return <div className="maputnik-feature-property-popup" dir="ltr" data-wd-key="feature-property-popup">
<table className="maputnik-popup-table"> <table className="maputnik-popup-table">
<tbody> <tbody>
{features.map(renderFeature)} {features.map(renderFeature)}
+1 -1
View File
@@ -104,7 +104,7 @@ class FeatureLayerPopup extends React.Component<FeatureLayerPopupProps> {
</div> </div>
}) })
return <div className="maputnik-feature-layer-popup"> return <div className="maputnik-feature-layer-popup" data-wd-key="feature-layer-popup" dir="ltr">
{items} {items}
</div> </div>
} }