Fix onHover behavior (#2925)

visgl · Apr 9, 2019 · 27c9659 · 27c9659
1 parent 8dd638d
commit 27c9659
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 29 deletions.
diff --git a/docs/upgrade-guide.md b/docs/upgrade-guide.md
@@ -16,7 +16,7 @@
 
 **Breaking Changes**
 
-- `onLayerHover` and `onLayerClick` props are replaced with `onHover` and `onClick`. The first argument passed to the callback will always be an [picking info](/docs/developer-guide/interactivity.md#the-picking-info-object) object, and the second argument is the pointer event. This change makes these two events consistent with other event callbacks.
+- `onLayerHover` and `onLayerClick` props are replaced with `onHover` and `onClick`. The first argument passed to the callback will always be a valid [picking info](/docs/developer-guide/interactivity.md#the-picking-info-object) object, and the second argument is the pointer event. This change makes these two events behave consistently with other event callbacks.
 
 #### Layers
 

diff --git a/examples/layer-browser/src/components/layer-info.js b/examples/layer-browser/src/components/layer-info.js
@@ -15,22 +15,24 @@ export default class LayerInfo extends PureComponent {
 
     return (
       <div id="layer-info">
-        {hovered && (
-          <div>
-            <h4>Hover</h4>
-            <span>
-              Layer: {hovered.layer.id} Object: {this._infoToString(hovered)}
-            </span>
-          </div>
-        )}
-        {clicked && (
-          <div>
-            <h4>Click</h4>
-            <span>
-              Layer: {clicked.layer.id} Object: {this._infoToString(clicked)}
-            </span>
-          </div>
-        )}
+        {hovered &&
+          hovered.object && (
+            <div>
+              <h4>Hover</h4>
+              <span>
+                Layer: {hovered.layer.id} Object: {this._infoToString(hovered)}
+              </span>
+            </div>
+          )}
+        {clicked &&
+          clicked.object && (
+            <div>
+              <h4>Click</h4>
+              <span>
+                Layer: {clicked.layer.id} Object: {this._infoToString(clicked)}
+              </span>
+            </div>
+          )}
         {queried && (
           <div>
             <h4>Pick Result</h4>

diff --git a/modules/core/src/lib/deck-picker.js b/modules/core/src/lib/deck-picker.js
@@ -136,6 +136,7 @@ export default class DeckPicker {
   }
 
   // Pick the closest object at the given (x,y) coordinate
+  // eslint-disable-next-line max-statements
   pickClosestObject({layers, viewports, x, y, radius, depth = 1, mode, onViewportActive}) {
     this.updatePickingBuffer();
     // Convert from canvas top-left to WebGL bottom-left coordinates
@@ -153,6 +154,7 @@ export default class DeckPicker {
       deviceHeight: height
     });
 
+    let infos;
     const result = [];
     const affectedLayers = {};
 
@@ -188,7 +190,7 @@ export default class DeckPicker {
       }
 
       // This logic needs to run even if no object is picked.
-      const infos = processPickInfo({
+      infos = processPickInfo({
         pickInfo,
         lastPickedInfo: this.lastPickedInfo,
         mode,
@@ -218,7 +220,7 @@ export default class DeckPicker {
       layers[layerId].restorePickingColors(affectedLayers[layerId])
     );
 
-    return result;
+    return {result, emptyInfo: infos && infos.get(null)};
   }
 
   // Pick all objects within the given bounding box
@@ -348,11 +350,12 @@ export default class DeckPicker {
     const pickingEvent = this.pickingEvent;
 
     infos.forEach(info => {
+      if (!info.layer) {
+        return;
+      }
+
       let handled = false;
       switch (mode) {
-        case 'click':
-          handled = info.layer.onClick(info, pickingEvent);
-          break;
         case 'hover':
           handled = info.layer.onHover(info, pickingEvent);
           break;

diff --git a/modules/core/src/lib/deck.js b/modules/core/src/lib/deck.js
@@ -314,7 +314,7 @@ export default class Deck {
       activateViewport,
       mode: 'query',
       depth: 1
-    });
+    }).result;
     this.stats.get('pickObject Time').timeEnd();
     return selectedInfos.length ? selectedInfos[0] : null;
   }
@@ -332,7 +332,7 @@ export default class Deck {
       activateViewport,
       mode: 'query',
       depth
-    });
+    }).result;
     this.stats.get('pickMultipleObjects Time').timeEnd();
     return selectedInfos;
   }
@@ -497,7 +497,7 @@ export default class Deck {
 
     if (_pickRequest.mode) {
       // perform picking
-      const selectedInfos = this.deckPicker.pickObject(
+      const {result, emptyInfo} = this.deckPicker.pickObject(
         Object.assign(
           {
             layers: this.layerManager.getLayers(),
@@ -508,10 +508,9 @@ export default class Deck {
           _pickRequest
         )
       );
-      if (_pickRequest.callback && selectedInfos) {
-        const firstInfo = selectedInfos.find(info => info.index >= 0) || selectedInfos[0];
-        // As per documentation, send null value when no valid object is picked.
-        _pickRequest.callback(firstInfo, _pickRequest.event);
+      if (_pickRequest.callback) {
+        const pickedInfo = result.find(info => info.index >= 0) || emptyInfo;
+        _pickRequest.callback(pickedInfo, _pickRequest.event);
       }
       _pickRequest.mode = null;
     }

diff --git a/modules/core/src/lib/picking/pick-info.js b/modules/core/src/lib/picking/pick-info.js
@@ -87,6 +87,9 @@ export function processPickInfo({
   // Please be very careful when changing this pattern
   const infos = new Map();
 
+  // Make sure infos always contain something even if no layer is affected
+  infos.set(null, baseInfo);
+
   affectedLayers.forEach(layer => {
     let info = Object.assign({}, baseInfo);