[Bug] Bug fixes (#1388)

- Passing the correct animationFilterIdx to bottom time widget - Fixed data table memory warning - fix input hover style overwrite focus|active style - fix animationconfig.domain is null causing tirp layer to crash - check objectInfo.object in isLayerHovered - check filter idx before setting filter property - fix cannot read property 'group' of null when copy layer config - fix(csv): Parse TRUE boolean values properly Signed-off-by: Shan He <heshan0131@gmail.com>
keplergl · Jan 1, 2021 · 5d4b454 · 5d4b454
1 parent 35bf90a
commit 5d4b454
Show file tree

Hide file tree

Showing 20 changed files with 74 additions and 40 deletions.
diff --git a/src/components/bottom-widget.js b/src/components/bottom-widget.js
@@ -149,7 +149,9 @@ export default function BottomWidgetFactory(
       () => filters.findIndex(f => f.enlarged && f.type === FILTER_TYPES.timeRange),
       [filters]
     );
-    const animatedFilter = useMemo(() => filters.find(f => f.isAnimating), [filters]);
+    const animatedFilterIdx = useMemo(() => filters.findIndex(f => f.isAnimating), [filters]);
+    const animatedFilter = animatedFilterIdx > -1 ? filters[animatedFilterIdx] : null;
+
     const enlargedFilterWidth = isOpen ? containerW - sidePanelWidth : containerW;
 
     // show playback control if layers contain trip layer & at least one trip layer is visible
@@ -196,7 +198,7 @@ export default function BottomWidgetFactory(
         {filter && (
           <FilterAnimationController
             filter={filter}
-            filterIdx={enlargedFilterIdx}
+            filterIdx={animatedFilterIdx > -1 ? animatedFilterIdx : enlargedFilterIdx}
             setFilterAnimationTime={visStateActions.setFilterAnimationTime}
           >
             {animationControlProps =>

diff --git a/src/components/common/data-table/index.js b/src/components/common/data-table/index.js
@@ -331,7 +331,12 @@ function DataTableFactory(FieldToken) {
 
     componentWillUnmount() {
       window.removeEventListener('resize', this.scaleCellsToWidth);
+      // fix Warning: Can't perform a React state update on an unmounted component
+      this.setState = () => {
+        return;
+      };
     }
+
     root = createRef();
     columns = props => props.columns;
     pinnedColumns = props => props.pinnedColumns;

diff --git a/src/layers/arc-layer/arc-layer.js b/src/layers/arc-layer/arc-layer.js
@@ -194,7 +194,7 @@ export default class ArcLayer extends Layer {
       ...this.getVisualChannelUpdateTriggers()
     };
     const defaultLayerProps = this.getDefaultDeckLayerProps(opts);
-
+    const hoveredObject = this.hasHoveredObject(objectHovered);
     return [
       new DeckArcLayer({
         ...defaultLayerProps,
@@ -205,11 +205,11 @@ export default class ArcLayer extends Layer {
         extensions: [...defaultLayerProps.extensions, new BrushingExtension()]
       }),
       // hover layer
-      ...(this.isLayerHovered(objectHovered)
+      ...(hoveredObject
         ? [
             new DeckArcLayer({
               ...this.getDefaultHoverLayerProps(),
-              data: [objectHovered.object],
+              data: [hoveredObject],
               widthScale: this.config.visConfig.thickness,
               getSourceColor: this.config.highlightColor,
               getTargetColor: this.config.highlightColor,

diff --git a/src/layers/base-layer.js b/src/layers/base-layer.js
@@ -450,6 +450,7 @@ export default class Layer {
     Object.values(this.visualChannels).forEach(v => {
       if (
         configToCopy.visConfig[v.range] &&
+        this.visConfigSettings[v.range] &&
         visConfigSettings[v.range].group !== this.visConfigSettings[v.range].group
       ) {
         notToCopy.push(v.range);
@@ -1054,10 +1055,12 @@ export default class Layer {
     }
   }
 
+  hasHoveredObject(objectInfo) {
+    return this.isLayerHovered(objectInfo) && objectInfo.object ? objectInfo.object : null;
+  }
+
   isLayerHovered(objectInfo) {
-    return (
-      objectInfo && objectInfo.layer && objectInfo.picked && objectInfo.layer.props.id === this.id
-    );
+    return objectInfo?.picked && objectInfo?.layer?.props?.id === this.id;
   }
 
   getRadiusScaleByZoom(mapState, fixedRadius) {

diff --git a/src/layers/cluster-layer/cluster-layer.js b/src/layers/cluster-layer/cluster-layer.js
@@ -78,6 +78,7 @@ export default class ClusterLayer extends AggregationLayer {
       }
     };
     const {_filterData: filterData, ...clusterData} = data;
+    const hoveredObject = this.hasHoveredObject(objectHovered);
 
     return [
       new DeckGLClusterLayer({
@@ -106,11 +107,11 @@ export default class ClusterLayer extends AggregationLayer {
         onSetColorDomain: layerCallbacks.onSetLayerDomain
       }),
       // hover layer
-      ...(this.isLayerHovered(objectHovered)
+      ...(hoveredObject
         ? [
             new ScatterplotLayer({
               id: `${this.id}-hovered`,
-              data: [objectHovered.object],
+              data: [hoveredObject],
               getFillColor: this.config.highlightColor,
               getRadius: d => d.radius,
               radiusScale: 1,

diff --git a/src/layers/geojson-layer/geojson-layer.js b/src/layers/geojson-layer/geojson-layer.js
@@ -291,6 +291,8 @@ export default class GeoJsonLayer extends Layer {
     };
 
     const pickable = interactionConfig.tooltip.enabled;
+    const hoveredObject = this.hasHoveredObject(objectHovered);
+
     return [
       new DeckGLGeoJsonLayer({
         ...defaultLayerProps,
@@ -319,13 +321,13 @@ export default class GeoJsonLayer extends Layer {
             : {})
         }
       }),
-      ...(this.isLayerHovered(objectHovered) && !visConfig.enable3d
+      ...(hoveredObject && !visConfig.enable3d
         ? [
             new DeckGLGeoJsonLayer({
               ...this.getDefaultHoverLayerProps(),
               ...layerProps,
               wrapLongitude: false,
-              data: [objectHovered.object],
+              data: [hoveredObject],
               getLineWidth: data.getLineWidth,
               getRadius: data.getRadius,
               getElevation: data.getElevation,

diff --git a/src/layers/grid-layer/grid-layer.js b/src/layers/grid-layer/grid-layer.js
@@ -60,6 +60,7 @@ export default class GridLayer extends AggregationLayer {
     const zoomFactor = this.getZoomFactor(mapState);
     const {visConfig} = this.config;
     const cellSize = visConfig.worldUnitSize * 1000;
+    const hoveredObject = this.hasHoveredObject(objectHovered);
 
     return [
       new EnhancedGridLayer({
@@ -70,14 +71,14 @@ export default class GridLayer extends AggregationLayer {
       }),
 
       // render an outline of each cell if not extruded
-      ...(this.isLayerHovered(objectHovered) && !visConfig.enable3d
+      ...(hoveredObject && !visConfig.enable3d
         ? [
             new GeoJsonLayer({
               ...this.getDefaultHoverLayerProps(),
               wrapLongitude: false,
               data: [
                 pointToPolygonGeo({
-                  object: objectHovered.object,
+                  object: hoveredObject,
                   cellSize,
                   coverage: visConfig.coverage,
                   mapState

diff --git a/src/layers/h3-hexagon-layer/h3-hexagon-layer.js b/src/layers/h3-hexagon-layer/h3-hexagon-layer.js
@@ -217,6 +217,7 @@ export default class HexagonIdLayer extends Layer {
     };
 
     const defaultLayerProps = this.getDefaultDeckLayerProps(opts);
+    const hoveredObject = this.hasHoveredObject(objectHovered);
 
     return [
       new H3HexagonLayer({
@@ -247,11 +248,11 @@ export default class HexagonIdLayer extends Layer {
           }
         }
       }),
-      ...(this.isLayerHovered(objectHovered) && !config.sizeField
+      ...(hoveredObject && !config.sizeField
         ? [
             new GeoJsonLayer({
               ...this.getDefaultHoverLayerProps(),
-              data: [idToPolygonGeo(objectHovered)],
+              data: [idToPolygonGeo(hoveredObject)],
               getLineColor: config.highlightColor,
               lineWidthScale: DEFAULT_LINE_SCALE_VALUE * zoomFactor,
               wrapLongitude: false

diff --git a/src/layers/h3-hexagon-layer/h3-utils.js b/src/layers/h3-hexagon-layer/h3-utils.js
@@ -36,7 +36,7 @@ export function getCentroid({id}) {
   return h3ToGeo(id).reverse();
 }
 
-export function idToPolygonGeo({object}, properties) {
+export function idToPolygonGeo(object, properties) {
   if (!object || !object.id) {
     return null;
   }

diff --git a/src/layers/hexagon-layer/hexagon-layer.js b/src/layers/hexagon-layer/hexagon-layer.js
@@ -65,6 +65,7 @@ export default class HexagonLayer extends AggregationLayer {
     const zoomFactor = this.getZoomFactor(mapState);
     const {visConfig} = this.config;
     const radius = visConfig.worldUnitSize * 1000;
+    const hoveredObject = this.hasHoveredObject(objectHovered);
 
     return [
       new EnhancedHexagonLayer({
@@ -75,13 +76,13 @@ export default class HexagonLayer extends AggregationLayer {
       }),
 
       // render an outline of each hexagon if not extruded
-      ...(this.isLayerHovered(objectHovered) && !visConfig.enable3d
+      ...(hoveredObject && !visConfig.enable3d
         ? [
             new GeoJsonLayer({
               ...this.getDefaultHoverLayerProps(),
               wrapLongitude: false,
               data: [
-                hexagonToPolygonGeo(objectHovered, {}, radius * visConfig.coverage, mapState)
+                hexagonToPolygonGeo(hoveredObject, {}, radius * visConfig.coverage, mapState)
               ].filter(d => d),
               getLineColor: this.config.highlightColor,
               lineWidthScale: clamp([1, 100], radius * 0.1 * zoomFactor)

diff --git a/src/layers/hexagon-layer/hexagon-utils.js b/src/layers/hexagon-layer/hexagon-utils.js
@@ -21,7 +21,7 @@
 import {WebMercatorViewport} from '@deck.gl/core';
 import Console from 'global/console';
 
-export function hexagonToPolygonGeo({object}, properties, radius, mapState) {
+export function hexagonToPolygonGeo(object, properties, radius, mapState) {
   const viewport = new WebMercatorViewport(mapState);
   if (!Array.isArray(object.position)) {
     return null;

diff --git a/src/layers/icon-layer/icon-layer.js b/src/layers/icon-layer/icon-layer.js
@@ -272,6 +272,7 @@ export default class IconLayer extends Layer {
         opts
       )
     ];
+    const hoveredObject = this.hasHoveredObject(objectHovered);
 
     return !this.iconGeometry
       ? []
@@ -288,12 +289,12 @@ export default class IconLayer extends Layer {
             extensions
           }),
 
-          ...(this.isLayerHovered(objectHovered)
+          ...(hoveredObject
             ? [
                 new SvgIconLayer({
                   ...this.getDefaultHoverLayerProps(),
                   ...layerProps,
-                  data: [objectHovered.object],
+                  data: [hoveredObject],
                   getPosition: data.getPosition,
                   getRadius: data.getRadius,
                   getFillColor: this.config.highlightColor,

diff --git a/src/layers/index.d.ts b/src/layers/index.d.ts
@@ -88,6 +88,7 @@ export class Layer {
   isValidToSave(): boolean;
   getVisualChannelDescription(key: string): VisualChannelDescription;
   isLayerHovered(objectInfo: any): boolean;
+  hasHoveredObject(objectInfo: any): any | null;
   getHoverData(object: any, allData?: Dataset['allData'], fields?: Dataset['fields']): any;
 }
 

diff --git a/src/layers/line-layer/line-layer.js b/src/layers/line-layer/line-layer.js
@@ -75,6 +75,8 @@ export default class LineLayer extends ArcLayer {
       ...this.getVisualChannelUpdateTriggers()
     };
     const defaultLayerProps = this.getDefaultDeckLayerProps(opts);
+    const hoveredObject = this.hasHoveredObject(objectHovered);
+
     return [
       // base layer
       new EnhancedLineLayer({
@@ -86,12 +88,12 @@ export default class LineLayer extends ArcLayer {
         extensions: [...defaultLayerProps.extensions, new BrushingExtension()]
       }),
       // hover layer
-      ...(this.isLayerHovered(objectHovered)
+      ...(hoveredObject
         ? [
             new EnhancedLineLayer({
               ...this.getDefaultHoverLayerProps(),
               ...layerProps,
-              data: [objectHovered.object],
+              data: [hoveredObject],
               getColor: this.config.highlightColor,
               getTargetColor: this.config.highlightColor,
               getWidth: data.getWidth

diff --git a/src/layers/point-layer/point-layer.js b/src/layers/point-layer/point-layer.js
@@ -272,6 +272,7 @@ export default class PointLayer extends Layer {
       filterRange: defaultLayerProps.filterRange,
       ...brushingProps
     };
+    const hoveredObject = this.hasHoveredObject(objectHovered);
 
     return [
       new ScatterplotLayer({
@@ -288,12 +289,12 @@ export default class PointLayer extends Layer {
         extensions
       }),
       // hover layer
-      ...(this.isLayerHovered(objectHovered)
+      ...(hoveredObject
         ? [
             new ScatterplotLayer({
               ...this.getDefaultHoverLayerProps(),
               ...layerProps,
-              data: [objectHovered.object],
+              data: [hoveredObject],
               getLineColor: this.config.highlightColor,
               getFillColor: this.config.highlightColor,
               getRadius: data.getRadius,

diff --git a/src/layers/trip-layer/trip-layer.js b/src/layers/trip-layer/trip-layer.js
@@ -244,12 +244,23 @@ export default class TripLayer extends Layer {
     const {data, gpuFilter, mapState, animationConfig} = opts;
     const {visConfig} = this.config;
     const zoomFactor = this.getZoomFactor(mapState);
+    const isValidTime =
+      animationConfig &&
+      Array.isArray(animationConfig.domain) &&
+      animationConfig.domain.every(Number.isFinite) &&
+      Number.isFinite(animationConfig.currentTime);
+
+    if (!isValidTime) {
+      return [];
+    }
+
+    const domain0 = animationConfig.domain?.[0];
 
     const updateTriggers = {
       ...this.getVisualChannelUpdateTriggers(),
       getTimestamps: {
         columns: this.config.columns,
-        domain0: animationConfig.domain[0]
+        domain0
       },
       getFilterValue: gpuFilter.filterValueUpdateTriggers
     };
@@ -259,7 +270,7 @@ export default class TripLayer extends Layer {
       new DeckGLTripsLayer({
         ...defaultLayerProps,
         ...data,
-        getTimestamps: d => data.getTimestamps(d).map(ts => ts - animationConfig.domain[0]),
+        getTimestamps: d => data.getTimestamps(d).map(ts => ts - domain0),
         widthScale: this.config.visConfig.thickness * zoomFactor * zoomFactorValue,
         rounded: true,
         wrapLongitude: false,
@@ -268,7 +279,7 @@ export default class TripLayer extends Layer {
           depthMask: false
         },
         trailLength: visConfig.trailLength * 1000,
-        currentTime: animationConfig.currentTime - animationConfig.domain[0],
+        currentTime: animationConfig.currentTime - domain0,
         updateTriggers
       })
     ];

diff --git a/src/processors/data-processor.js b/src/processors/data-processor.js
@@ -59,7 +59,7 @@ const IGNORE_DATA_TYPES = Object.keys(AnalyzerDATA_TYPES).filter(
 export const PARSE_FIELD_VALUE_FROM_STRING = {
   [ALL_FIELD_TYPES.boolean]: {
     valid: d => typeof d === 'boolean',
-    parse: d => d === 'true' || d === 'True' || d === '1'
+    parse: d => d === 'true' || d === 'True' || d === 'TRUE' || d === '1'
   },
   [ALL_FIELD_TYPES.integer]: {
     valid: d => parseInt(d, 10) === d,

diff --git a/src/reducers/vis-state-updaters.js b/src/reducers/vis-state-updaters.js
@@ -559,6 +559,10 @@ export function setFilterUpdater(state, action) {
   const {idx, prop, value, valueIndex = 0} = action;
 
   const oldFilter = state.filters[idx];
+  if (!oldFilter) {
+    Console.error(`filters.${idx} is undefined`);
+    return state;
+  }
   let newFilter = set([prop], value, oldFilter);
   let newState = state;
 

diff --git a/src/styles/base.js b/src/styles/base.js
@@ -496,6 +496,14 @@ const input = css`
   opacity: ${props => (props.disabled ? 0.5 : 1)};
   box-shadow: ${props => props.theme.inputBoxShadow};
 
+  :hover {
+    cursor: ${props => (props.type === 'number' || props.type === 'text' ? 'text' : 'pointer')};
+    background-color: ${props =>
+      props.active ? props.theme.inputBgdActive : props.theme.inputBgdHover};
+    border-color: ${props =>
+      props.active ? props.theme.inputBorderActiveColor : props.theme.inputBorderHoverColor};
+  }
+
   :active,
   :focus,
   &.focus,
@@ -504,14 +512,6 @@ const input = css`
     border-color: ${props => props.theme.inputBorderActiveColor};
     box-shadow: ${props => props.theme.inputBoxShadowActive};
   }
-
-  :hover {
-    cursor: ${props => (props.type === 'number' || props.type === 'text' ? 'text' : 'pointer')};
-    background-color: ${props =>
-      props.active ? props.theme.inputBgdActive : props.theme.inputBgdHover};
-    border-color: ${props =>
-      props.active ? props.theme.inputBorderActiveColor : props.theme.inputBorderHoverColor};
-  }
 `;
 
 const inputLT = css`