From 5d4b45475de2bca140194860c215c93df5207529 Mon Sep 17 00:00:00 2001 From: Shan He Date: Fri, 1 Jan 2021 15:56:47 +0800 Subject: [PATCH] [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 --- src/components/bottom-widget.js | 6 ++++-- src/components/common/data-table/index.js | 5 +++++ src/layers/arc-layer/arc-layer.js | 6 +++--- src/layers/base-layer.js | 9 ++++++--- src/layers/cluster-layer/cluster-layer.js | 5 +++-- src/layers/geojson-layer/geojson-layer.js | 6 ++++-- src/layers/grid-layer/grid-layer.js | 5 +++-- src/layers/h3-hexagon-layer/h3-hexagon-layer.js | 5 +++-- src/layers/h3-hexagon-layer/h3-utils.js | 2 +- src/layers/hexagon-layer/hexagon-layer.js | 5 +++-- src/layers/hexagon-layer/hexagon-utils.js | 2 +- src/layers/icon-layer/icon-layer.js | 5 +++-- src/layers/index.d.ts | 1 + src/layers/line-layer/line-layer.js | 6 ++++-- src/layers/point-layer/point-layer.js | 5 +++-- src/layers/trip-layer/trip-layer.js | 17 ++++++++++++++--- src/processors/data-processor.js | 2 +- src/reducers/vis-state-updaters.js | 4 ++++ src/styles/base.js | 16 ++++++++-------- test/browser/components/injector-test.js | 2 -- 20 files changed, 74 insertions(+), 40 deletions(-) diff --git a/src/components/bottom-widget.js b/src/components/bottom-widget.js index 99fdf07a0b..91a1d7bc0c 100644 --- 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 && ( -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 index d2f7611af6..1db27b159a 100644 --- 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 index b543030ce3..6df98d3662 100644 --- 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 index b59afe75e6..05a7c04801 100644 --- 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 index 9e97acd2cf..12b53b4654 100644 --- 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 index d2d74dba2d..e7eec8119f 100644 --- 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 index 9f03947526..27501cdfe3 100644 --- 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 index a027ca487a..4188966694 100644 --- 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 index d8530a96dd..772614e725 100644 --- 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 index 1b11340158..0b9ac64130 100644 --- 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 index 1f171ad301..5d52533d98 100644 --- 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 index 3c0cc0fba5..4134aee5c6 100644 --- 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 index 7a766fd17d..078683e0b5 100644 --- 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 index 77f9ab976b..3f808d2aae 100644 --- 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 index 071f226a12..65c305560c 100644 --- 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 index 4213fe4b77..f178274f63 100644 --- 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 index 80b769d4dd..1918abf783 100644 --- 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 index 457150d9a6..650f115ac3 100644 --- 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 index b9eaa4c889..23d7118eaa 100644 --- 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` diff --git a/test/browser/components/injector-test.js b/test/browser/components/injector-test.js index 6be4a79d9e..8e09e85f8f 100644 --- a/test/browser/components/injector-test.js +++ b/test/browser/components/injector-test.js @@ -61,7 +61,6 @@ test('Components -> injector -> injectComponents', t => { test('Components -> injector -> missing deps', t => { const spy = sinon.spy(Console, 'error'); - // eslint-disable-next-line react/display-name const myCustomNameFactory = () => () =>
name
; // eslint-disable-next-line react/display-name @@ -98,7 +97,6 @@ test('Components -> injector -> missing deps', t => { test('Components -> injector -> wrong factory type', t => { const spy = sinon.spy(Console, 'error'); - // const spy = sinon.spy(Console, 'error'); // eslint-disable-next-line react/display-name const myCustomHeaderFactory = Name => () => (