Fix how QueryHandler manages multiple fetches using Promise.allSettled

Remove timeout from multipleHeterogeneousQueryExtents test Remove windows/linux system dependency from test Fix system dependency for copy-pasting text in test
Maps4HTML · Sep 25, 2023 · 1b452fa · 1b452fa
1 parent 31b5b99
commit 1b452fa
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 21 deletions.
diff --git a/src/mapml/handlers/QueryHandler.js b/src/mapml/handlers/QueryHandler.js
@@ -102,25 +102,23 @@ export var QueryHandler = L.Handler.extend({
           }
         })
         .then((response) => {
-          if (!layer._mapmlFeatures) layer._mapmlFeatures = [];
+          let features = [];
           if (response.contenttype.startsWith('text/mapml')) {
             // the mapmldoc could have <map-meta> elements that are important, perhaps
             // also, the mapmldoc can have many features
             let mapmldoc = parser.parseFromString(
-                response.text,
-                'application/xml'
-              ),
-              features = Array.prototype.slice.call(
-                mapmldoc.querySelectorAll('map-feature')
-              );
+              response.text,
+              'application/xml'
+            );
+            features = Array.prototype.slice.call(
+              mapmldoc.querySelectorAll('map-feature')
+            );
             // <map-meta> elements
             layer.metas = Array.prototype.slice.call(
               mapmldoc.querySelectorAll(
                 'map-meta[name=cs], map-meta[name=zoom], map-meta[name=projection]'
               )
             );
-            if (features.length)
-              layer._mapmlFeatures = layer._mapmlFeatures.concat(features);
           } else {
             // synthesize a single feature from text or html content
             let geom =
@@ -139,9 +137,9 @@ export var QueryHandler = L.Handler.extend({
                   'text/html'
                 )
                 .querySelector('map-feature');
-            layer._mapmlFeatures.push(feature);
+            features.push(feature);
           }
-          return layer._mapmlFeatures;
+          return { features: features, template: template };
         })
         .catch((err) => {
           console.log('Looks like there was a problem. Status: ' + err.message);
@@ -248,14 +246,21 @@ export var QueryHandler = L.Handler.extend({
         fetches.push(fetchFeatures(template, obj));
       }
     }
-    Promise.allSettled(fetches).then(() => {
-      // create connection between queried <map-feature> and its parent <map-extent>
-      if (layer._mapmlFeatures) {
-        for (let feature of layer._mapmlFeatures) {
-          feature._extentEl = template._extentEl;
+    Promise.allSettled(fetches).then((results) => {
+      layer._mapmlFeatures = [];
+      // f is an array of {features[], template}
+
+      for (let f of results) {
+        if (f.status === 'fulfilled') {
+          // create connection between queried <map-feature> and its parent <map-extent>
+          for (let feature of f.value.features) {
+            feature._extentEl = f.value.template._extentEl;
+          }
+          layer._mapmlFeatures = layer._mapmlFeatures.concat(f.value.features);
         }
-        displayFeaturesPopup(layer._mapmlFeatures, e.latlng);
       }
+      if (layer._mapmlFeatures.length > 0)
+        displayFeaturesPopup(layer._mapmlFeatures, e.latlng);
     });
 
     function displayFeaturesPopup(features, loc) {

diff --git a/test/e2e/core/layerContextMenu.test.js b/test/e2e/core/layerContextMenu.test.js
@@ -45,6 +45,7 @@ test.describe('Playwright Layer Context Menu Tests', () => {
     ).jsonValue();
 
     expect(menuDisplay).toEqual('block');
+    await page.keyboard.press('Escape');
   });
 
   test('Layer context menu copy layer', async () => {
@@ -157,6 +158,7 @@ test.describe('Playwright Layer Context Menu Tests', () => {
   });
 
   test('Copy layer with relative src attribute', async () => {
+    await page.reload();
     await page.hover(
       'div > div.leaflet-control-container > div.leaflet-top.leaflet-right > div'
     );
@@ -167,8 +169,10 @@ test.describe('Playwright Layer Context Menu Tests', () => {
 
     await page.keyboard.press('l');
     await page.click('body > textarea#messageLayer');
-    await page.keyboard.press('Control+a');
-    await page.keyboard.press('Backspace');
+    // reload is better than deleting text, because of cross-platform issue
+    // with copy-pasting text on Windows/Linux
+    //    await page.keyboard.press('Control+a');
+    //    await page.keyboard.press('Backspace');
     await page.keyboard.press('Control+v');
     const copyLayer = await page.$eval(
       'body > textarea#messageLayer',

diff --git a/test/e2e/layers/multipleHeterogeneousQueryExtents.test.js b/test/e2e/layers/multipleHeterogeneousQueryExtents.test.js
@@ -92,8 +92,6 @@ test.describe('Multiple Extent Queries with heterogeneous response content types
       (span) => span.innerText.toLowerCase()
     );
     expect(thirdExtentInLayerControl).toEqual('html query response');
-
-    await page.waitForTimeout(1000);
 
     await page.click('div');
     await page.waitForSelector(