diff --git a/CHANGELOG.md b/CHANGELOG.md index 47e9ee3918..763e1def44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## main ### ✨ Features and improvements +- Hide Popup when its parent Marker is behind terrain ([#3865](https://github.com/maplibre/maplibre-gl-js/pull/3865)) - _...Add new stuff here..._ ### 🐞 Bug fixes diff --git a/src/ui/marker.test.ts b/src/ui/marker.test.ts index a061ea465e..a930dbc80b 100644 --- a/src/ui/marker.test.ts +++ b/src/ui/marker.test.ts @@ -157,13 +157,15 @@ describe('marker', () => { expect(!marker.getPopup()).toBeTruthy(); }); - test('Marker#togglePopup opens a popup that was closed', () => { + test('Marker#togglePopup opens a popup that was closed', async () => { const map = createMap(); const marker = new Marker() .setLngLat([0, 0]) .addTo(map) - .setPopup(new Popup()) - .togglePopup(); + .setPopup(new Popup()); + + await sleep(500); + marker.togglePopup(); expect(marker.getPopup().isOpen()).toBeTruthy(); @@ -184,13 +186,15 @@ describe('marker', () => { map.remove(); }); - test('Enter key on Marker opens a popup that was closed', () => { + test('Enter key on Marker opens a popup that was closed', async () => { const map = createMap(); const marker = new Marker() .setLngLat([0, 0]) .addTo(map) .setPopup(new Popup()); + await sleep(500); + // popup not initially open expect(marker.getPopup().isOpen()).toBeFalsy(); @@ -202,13 +206,15 @@ describe('marker', () => { map.remove(); }); - test('Space key on Marker opens a popup that was closed', () => { + test('Space key on Marker opens a popup that was closed', async () => { const map = createMap(); const marker = new Marker() .setLngLat([0, 0]) .addTo(map) .setPopup(new Popup()); + await sleep(500); + // popup not initially open expect(marker.getPopup().isOpen()).toBeFalsy(); @@ -292,7 +298,7 @@ describe('marker', () => { map.remove(); }); - test('Popup anchors around default Marker', () => { + test('Popup anchors around default Marker', async () => { const map = createMap(); const marker = new Marker() @@ -300,6 +306,8 @@ describe('marker', () => { .setPopup(new Popup().setText('Test')) .addTo(map); + await sleep(500); + // open the popup marker.togglePopup(); @@ -361,7 +369,7 @@ describe('marker', () => { map.remove(); }); - test('Popup is opened at its marker position after marker is moved to another globe', () => { + test('Popup is opened at its marker position after marker is moved to another globe', async () => { const map = createMap({width: 3000}); const marker = new Marker() @@ -369,6 +377,8 @@ describe('marker', () => { .setPopup(new Popup().setText('Test')) .addTo(map); + await sleep(500); + marker._pos = new Point(2999, 242); marker._lngLat = map.unproject(marker._pos); marker.togglePopup(); @@ -377,7 +387,7 @@ describe('marker', () => { map.remove(); }); - test('Popup is re-opened at its marker position after marker is moved to another globe', () => { + test('Popup is re-opened at its marker position after marker is moved to another globe', async () => { const map = createMap({width: 3000}); const marker = new Marker() @@ -387,6 +397,7 @@ describe('marker', () => { .togglePopup() .togglePopup(); + await sleep(500); marker._pos = new Point(2999, 242); marker._lngLat = map.unproject(marker._pos); marker.togglePopup(); @@ -1029,6 +1040,56 @@ describe('marker', () => { map.remove(); }); + test('Removes an open popup when going behind 3d terrain', async () => { + const map = createMap(); + const marker = new Marker() + .setLngLat([0, 0]) + .addTo(map) + .setPopup(new Popup()); + + await sleep(500); + marker.togglePopup(); + + expect(marker._popup.isOpen()).toBeTruthy(); + + map.transform.lngLatToCameraDepth = () => .95; // Mocking distance to marker + + map.terrain = { + getElevationForLngLatZoom: () => 0, + depthAtPoint: () => .92 + } as any as Terrain; + map.fire('terrain'); + + await sleep(500); + + expect(marker._popup?.isOpen()).toBeFalsy(); + map.remove(); + }); + + test('Does not open a popup when behind 3d terrain', async () => { + const map = createMap(); + const marker = new Marker() + .setLngLat([0, 0]) + .addTo(map) + .setPopup(new Popup()); + + map.transform.lngLatToCameraDepth = () => .95; + + map.terrain = { + getElevationForLngLatZoom: () => 0, + depthAtPoint: () => .92 + } as any as Terrain; + map.fire('terrain'); + + await sleep(500); + + marker.togglePopup(); + + expect(marker._popup.isOpen()).toBeFalsy(); + + map.remove(); + }); + test('Marker\'s lng is wrapped when slightly crossing 180 with {renderWorldCopies: false}', () => { const map = createMap({width: 1024, renderWorldCopies: false}); const marker = new Marker() diff --git a/src/ui/marker.ts b/src/ui/marker.ts index 30b7690ff2..8a6dc05b84 100644 --- a/src/ui/marker.ts +++ b/src/ui/marker.ts @@ -515,6 +515,8 @@ export class Marker extends Evented { togglePopup(): this { const popup = this._popup; + if (this._element.style.opacity === this._opacityWhenCovered) return this; + if (!popup) return this; else if (popup.isOpen()) popup.remove(); else { @@ -559,6 +561,8 @@ export class Marker extends Evented { const markerDistanceCenter = map.transform.lngLatToCameraDepth(this._lngLat, elevation + elevationToCenter); // Display at full opacity if center is visible. const centerIsInvisible = markerDistanceCenter - terrainDistanceCenter > forgiveness; + + if (this._popup?.isOpen() && centerIsInvisible) this._popup.remove(); this._element.style.opacity = centerIsInvisible ? this._opacityWhenCovered : this._opacity; }