From 575357080f400be4eccbede98a24a8799405cec2 Mon Sep 17 00:00:00 2001 From: Igor Dykhta Date: Mon, 4 Mar 2024 20:30:15 +0200 Subject: [PATCH] [Bug] fix yaxis chat doesn't update Signed-off-by: Ihor Dykhta --- .../common/range-slider-timeline-panel.tsx | 23 +--- .../src/common/range-slider-timeline.tsx | 51 +++++--- src/components/src/common/range-slider.tsx | 4 +- .../filter-synced-dataset-panel.tsx | 7 +- src/reducers/src/vis-state-updaters.ts | 116 +++++++++++++----- src/styles/src/base.ts | 3 + src/types/reducers.d.ts | 9 +- src/utils/src/filter-utils.ts | 32 +++-- src/utils/src/plot.ts | 9 +- src/utils/src/time.ts | 1 - test/node/reducers/vis-state-test.js | 57 ++++++++- 11 files changed, 222 insertions(+), 90 deletions(-) diff --git a/src/components/src/common/range-slider-timeline-panel.tsx b/src/components/src/common/range-slider-timeline-panel.tsx index 2695533f1c..1faf077084 100644 --- a/src/components/src/common/range-slider-timeline-panel.tsx +++ b/src/components/src/common/range-slider-timeline-panel.tsx @@ -3,11 +3,9 @@ import React, {useMemo} from 'react'; import RangeSliderTimelineFactory from './range-slider-timeline'; -import {Layers} from '../common/icons'; -import {Tooltip} from '../common/styled-components'; function RangeSliderTimelinePanelFactory(RangeSliderTimeline) { - const RangeSliderTimelinePanel = ({timelines, scaledValue, timelineLabel, style}) => { + const RangeSliderTimelinePanel = ({timelines, scaledValue, style}) => { const containerStyle = useMemo( () => ({ display: 'flex', @@ -18,24 +16,11 @@ function RangeSliderTimelinePanelFactory(RangeSliderTimeline) { [style] ); - const iconWrapperStyle = { - marginRight: '8px', - cursor: 'pointer' - }; - return (
-
- - - {timelineLabel} - -
-
- {timelines.map((timeline, index) => ( - - ))} -
+ {timelines.map((timeline, index) => ( + + ))}
); }; diff --git a/src/components/src/common/range-slider-timeline.tsx b/src/components/src/common/range-slider-timeline.tsx index 4f10352eec..30ee6bbc43 100644 --- a/src/components/src/common/range-slider-timeline.tsx +++ b/src/components/src/common/range-slider-timeline.tsx @@ -5,6 +5,7 @@ import React, {useMemo} from 'react'; // eslint-disable-next-line no-unused-vars import {CSSProperties} from 'react'; import {ArrowDownFull, TimelineMarker} from '../common/icons'; +import {Tooltip} from '../common/styled-components'; const BACKGROUND_LINE_STYLE: CSSProperties = { height: '4px', @@ -21,6 +22,17 @@ const TIMELINE_MARKER_STYLE: CSSProperties = { color: '#3D4866' }; +const containerStyle = { + display: 'flex', + width: '100%', + height: '16px' +}; + +const iconWrapperStyle = { + marginRight: '8px', + cursor: 'pointer' +}; + const TIMELINE_INDICATOR_STYLE: CSSProperties = { position: 'absolute', top: '-14px', @@ -29,56 +41,67 @@ const TIMELINE_INDICATOR_STYLE: CSSProperties = { }; function RangeSliderTimelineFactory() { - const RangeSliderTimeline = ({line, scaledValue, style}) => { + const RangeSliderTimeline = ({timeline, scaledValue, style}) => { + const {startTime, endTime, syncMode, Icon, label} = timeline; + const progressStyle: CSSProperties = { - left: `${line[0]}%`, + left: `${startTime}%`, top: '0', - width: `${line[1] - line[0]}%`, + width: `${endTime - startTime}%`, height: '100%', position: 'absolute', backgroundColor: '#5558DB' }; - const containerStyle = useMemo( + const progressBarContainer = useMemo( () => ({ ...BACKGROUND_LINE_STYLE, + flex: 1, ...style }), [style] ); - const value = scaledValue[line[2]]; + const value = scaledValue[syncMode]; const leftMarketStyle = useMemo( () => ({ - left: `calc(${line[0]}% - 4px)`, + left: `calc(${startTime}% - 4px)`, ...TIMELINE_MARKER_STYLE }), - [line] + [startTime] ); const rightMarketStyle = useMemo( () => ({ - left: `calc(${line[1]}% - 4px)`, + left: `calc(${endTime}% - 4px)`, ...TIMELINE_MARKER_STYLE }), - [line] + [endTime] ); const indicatorStyle = useMemo( () => ({ ...TIMELINE_INDICATOR_STYLE, - left: `calc(${value}% - 2px)` + left: `calc(${value}% - 3px)` }), [value] ); return (
-
- - - +
+ + + {label} + +
+
+
+ + + +
); }; diff --git a/src/components/src/common/range-slider.tsx b/src/components/src/common/range-slider.tsx index b474bbdfca..ad43eede91 100644 --- a/src/components/src/common/range-slider.tsx +++ b/src/components/src/common/range-slider.tsx @@ -66,7 +66,7 @@ interface RangeSliderProps { step?: number; sliderHandleWidth?: number; xAxis?: ElementType; - timelines?: number[][]; + timelines?: any[]; timelineLabel?: string; timezone?: string | null; @@ -240,7 +240,6 @@ export default function RangeSliderFactory( playbackControlWidth, setFilterPlot, timelines, - timelineLabel, animationWindow, filter, datasets @@ -291,7 +290,6 @@ export default function RangeSliderFactory( {timelines?.length ? ( diff --git a/src/components/src/filters/filter-panels/filter-synced-dataset-panel.tsx b/src/components/src/filters/filter-panels/filter-synced-dataset-panel.tsx index d1bbfb6623..3420f593df 100644 --- a/src/components/src/filters/filter-panels/filter-synced-dataset-panel.tsx +++ b/src/components/src/filters/filter-panels/filter-synced-dataset-panel.tsx @@ -6,7 +6,7 @@ import styled, {withTheme} from 'styled-components'; import {FormattedMessage} from '@kepler.gl/localization'; import {ALL_FIELD_TYPES} from '@kepler.gl/constants'; -import {getAnimatableLayers} from '@kepler.gl/utils'; +import {getAnimatableVisibleLayers} from '@kepler.gl/utils'; import {Button} from '../../common/styled-components'; import {Add, Trash} from '../../common/icons'; @@ -264,10 +264,7 @@ function FilterSyncedDatasetPanelFactory( [syncTimeFilterWithLayerTimeline, idx] ); - const animatableLayers = useMemo( - () => getAnimatableLayers(layers).filter(l => l.type === 'trip'), - [layers] - ); + const animatableLayers = useMemo(() => getAnimatableVisibleLayers(layers), [layers]); const isLinkedWithLayerTimeline = useMemo(() => filter.syncedWithLayerTimeline, [filter]); diff --git a/src/reducers/src/vis-state-updaters.ts b/src/reducers/src/vis-state-updaters.ts index 2d9fc8b70d..69c1fe61d7 100644 --- a/src/reducers/src/vis-state-updaters.ts +++ b/src/reducers/src/vis-state-updaters.ts @@ -42,6 +42,7 @@ import { isPlainObject, isObject, addNewLayersToSplitMap, + snapToMarks, computeSplitMapLayers, removeLayerFromSplitMaps, isRgbColor, @@ -59,7 +60,8 @@ import { isInRange, mergeFilterDomainStep, updateFilterPlot, - removeFilterPlot + removeFilterPlot, + isLayerAnimatable } from '@kepler.gl/utils'; // Mergers @@ -123,7 +125,12 @@ import { import {findFieldsToShow} from './interaction-utils'; import {hasPropsToMerge, getPropValueToMerger} from './merger-handler'; import {mergeDatasetsByOrder} from './vis-state-merger'; -import {fixEffectOrder, getAnimatableLayers, mergeTimeDomains} from '@kepler.gl/utils'; +import { + fixEffectOrder, + getAnimatableVisibleLayers, + getAnimatableVisibleLayersByType, + mergeTimeDomains +} from '@kepler.gl/utils'; import {createEffect} from '@kepler.gl/effects'; // react-palm @@ -505,6 +512,17 @@ export function layerConfigChangeUpdater( let newState = state; if ('isVisible' in action.newConfig) { newState = updateStateOnLayerVisibilityChange(state, newLayer); + const filterIndex = filterSyncedWithTimeline(state); + if (isLayerAnimatable(newLayer) && filterIndex !== -1) { + // if layer is going to be visible we sync with filter otherwise we need to check whether other animatable layers exists and are visible + newState = syncTimeFilterWithLayerTimelineUpdater(newState, { + idx: filterIndex, + // @ts-expect-error why layers are assigned to enable? + enable: action.newConfig.isVisible + ? action.newConfig.isVisible + : getAnimatableVisibleLayers(state.layers) + }); + } } if ('columns' in action.newConfig && newLayer.config.animation.enabled) { @@ -535,6 +553,15 @@ export function layerAnimationChangeUpdater(state: S, action return updateStateWithLayerAndData(state, {layerData, layer, idx}); } +/** + * + * @param state + * @returns index of the filter synced to timeline or -1 + */ +function filterSyncedWithTimeline(state: VisState): number { + return state.filters.findIndex(f => (f as TimeRangeFilter).syncedWithLayerTimeline); +} + /** * Updates isValid flag of a layer. * Updates isVisible based on the value of isValid. @@ -905,14 +932,12 @@ export const setLayerAnimationTimeUpdater = ( } }; // update animation config for each layer - const result = state.layers.reduce((accu, l) => { + return state.layers.reduce((accu, l) => { if (l.config.animation.enabled && l.type !== 'trip') { return layerAnimationChangeUpdater(accu, {oldLayer: l, prop: 'currentTime', currentTime}); } return accu; }, nextState); - - return result; }; /** @@ -929,7 +954,10 @@ export function setFilterAnimationTimeUpdater( const filter = newState.filters[idx]; if ((filter as TimeRangeFilter).syncedWithLayerTimeline) { const timelineValue = getTimelineFromTrip(filter); - newState = setLayerAnimationTimeUpdater(newState, {value: timelineValue}); + const value = state.animationConfig.timeSteps + ? snapToMarks(timelineValue, state.animationConfig.timeSteps) + : timelineValue; + newState = setLayerAnimationTimeUpdater(newState, {value}); } return newState; @@ -960,7 +988,7 @@ export function setFilterAnimationWindowUpdater( filters: swap_(newFilter)(state.filters) }; - const newSyncTimelineMode = getSyncAnimationMode(newFilter); + const newSyncTimelineMode = getSyncAnimationMode(newFilter as TimeRangeFilter); return setTimeFilterTimelineModeUpdater(newState, {id, mode: newSyncTimelineMode}); } @@ -1033,8 +1061,13 @@ export function setFilterUpdater( // pass only the dataset we need to update newState = updateAllLayerDomainData(newState, datasetIdsToFilter, newFilter); + // if (newFilter.syncedWithLayerTimeline) { + // newState = syncTimeFilterWithLayerTimelineUpdater(state, {idx, enable: newFilter.syncedWithLayerTimeline}); + // } + return newState; } + function _updateFilterDataIdAtValueIndex(filter, valueIndex, value, datasets) { let newFilter = filter; if (filter.dataId[valueIndex]) { @@ -1135,6 +1168,7 @@ function _updateFilterProp(state, filter, prop, value, valueIndex, datasetIds?) // only filter the current dataset break; } + case FILTER_UPDATER_PROPS.layerId: { // We need to update only datasetId/s if we have added/removed layers // - check for layerId changes (XOR works because of string values) @@ -1175,6 +1209,7 @@ function _updateFilterProp(state, filter, prop, value, valueIndex, datasetIds?) }; break; } + default: filter = set([prop], value, filter); datasetIdsToFilter = [...filter.dataId]; @@ -2700,7 +2735,7 @@ export function updateAllLayerDomainData( export function updateAnimationDomain(state: S): S { // merge all animatable layer domain and update global config - const animatableLayers = getAnimatableLayers(state.layers); + const animatableLayers = getAnimatableVisibleLayers(state.layers); if (!animatableLayers.length) { return { @@ -3157,29 +3192,52 @@ export function syncTimeFilterWithLayerTimelineUpdater( const {idx: filterIdx, enable = false} = action; const filter = state.filters[filterIdx] as TimeRangeFilter; - const newAnimationConfig = {...state.animationConfig}; + const hasHexTileLayer = + enable && Boolean(getAnimatableVisibleLayersByType(state.layers, 'hexTile').length); - const newFilterDomain = enable - ? mergeTimeDomains([filter.domain, newAnimationConfig.domain as [number, number] | null]) - : filter.domain; + const newFilterDomain = + hasHexTileLayer && state.animationConfig.domain + ? mergeTimeDomains([filter.domain, state.animationConfig.domain as [number, number]]) + : filter.domain; const syncTimelineMode = getSyncAnimationMode(filter); - const newFilter = { - ...filter, - value: adjustValueToFilterDomain(newFilterDomain, filter), + let newState = setFilterAnimationWindowUpdater(state, { + id: filter.id, + animationWindow: hasHexTileLayer ? ANIMATION_WINDOW.interval : filter.animationWindow + }); + + let newFilter = newState.filters[filterIdx]; + + const newFilterValue = adjustValueToFilterDomain( + hasHexTileLayer ? [newFilterDomain[0], newFilterDomain[0]] : newFilter.value, + { + domain: newFilterDomain, + // check whether this is valid type here - required to pass ts checks. + type: newFilter.type + } + ); + + newState = setFilterUpdater(newState, { + idx: filterIdx, + prop: 'value', + value: newFilterValue + }); + + newFilter = { + ...newState.filters[filterIdx], syncedWithLayerTimeline: enable, syncTimelineMode - }; + } as TimeRangeFilter; const animationConfigCurrentTime = enable ? newFilterDomain[0] - : state.animationConfig.domain?.[0]; + : newState.animationConfig.domain?.[0] ?? 0; return setLayerAnimationTimeUpdater( { - ...state, - filters: swap_(newFilter)(state.filters) + ...newState, + filters: swap_(newFilter)(newState.filters) }, {value: animationConfigCurrentTime ?? null} ); @@ -3216,18 +3274,18 @@ function getTimelineFromTrip(filter) { return filter.value[filter.syncTimelineMode]; } -function getSyncAnimationMode(filter) { - return filter.animationWindow === ANIMATION_WINDOW.free - ? SYNC_TIMELINE_MODES.start - : SYNC_TIMELINE_MODES.end; -} - -function validateSyncAnimationMode(filter, newMode) { - if (filter.animationWindow !== ANIMATION_WINDOW.free && newMode === SYNC_TIMELINE_MODES.start) { - return false; +function getSyncAnimationMode(filter: TimeRangeFilter) { + if (filter.animationWindow === ANIMATION_WINDOW.free) { + return filter.syncTimelineMode ?? SYNC_TIMELINE_MODES.end; } - return true; + return SYNC_TIMELINE_MODES.end; +} + +function validateSyncAnimationMode(filter: TimeRangeFilter, newMode: number) { + return !( + filter.animationWindow !== ANIMATION_WINDOW.free && newMode === SYNC_TIMELINE_MODES.start + ); } // Find dataId from a saved visState property: diff --git a/src/styles/src/base.ts b/src/styles/src/base.ts index 6652a7d3c2..f24d75601f 100644 --- a/src/styles/src/base.ts +++ b/src/styles/src/base.ts @@ -1559,6 +1559,9 @@ export const theme = { fieldTokenHeight, fieldTokenWidth, + // COLORS + BLUE2: 'rgba(85, 88, 219, 0.2)', + // Effect panel effectPanelWidth, effectPanelHeight, diff --git a/src/types/reducers.d.ts b/src/types/reducers.d.ts index fad4c6919e..4eca19fede 100644 --- a/src/types/reducers.d.ts +++ b/src/types/reducers.d.ts @@ -74,6 +74,11 @@ export type LineChart = { series: {x: number; y: number}[]; yDomain: number[] | undefined[]; xDomain: number[]; + + // Is this a valid part of LineChart? + aggregation: string; + interval: string; + yAxis: string; }; type FilterViewType = 'side' | 'enlarged' | 'minified'; @@ -238,12 +243,12 @@ export type SplitMap = { export type AnimationConfigTimeFormat = 'L' | 'L LT' | 'L LTS'; export type AnimationConfig = { - domain: number[] | null; + domain: [number, number] | null; currentTime: number | null; speed: number; duration?: number | null; isAnimating?: boolean; - timeSteps?: null | number[]; + timeSteps: number[] | null; // auto generated based on time domain defaultTimeFormat: AnimationTimeFormat | null; // custom ui input diff --git a/src/utils/src/filter-utils.ts b/src/utils/src/filter-utils.ts index a25b46f2d2..b74e130b66 100644 --- a/src/utils/src/filter-utils.ts +++ b/src/utils/src/filter-utils.ts @@ -1229,19 +1229,27 @@ export function mergeTimeDomains(domains: ([number, number] | null)[]): [number, } /** - * - * @param layers {Layer[]} + * @param {Layer} layer + */ +export function isLayerAnimatable(layer: any): boolean { + return layer.config.animation?.enabled && Array.isArray(layer.config.animation.domain); +} + +/** + * @param {Layer[]} layers * @returns {Layer[]} */ -export function getAnimatableLayers(layers: any[]): any[] { - const animatableLayers = layers.filter( - l => - l.config.isVisible && - l.config.animation && - l.config.animation.enabled && - Array.isArray(l.config.animation.domain) - ); - return animatableLayers; +export function getAnimatableVisibleLayers(layers: any[]): any[] { + return layers.filter(l => isLayerAnimatable(l) && l.config.isVisible); +} + +/** + * @param {Layer[]} layers + * @param {string} type + * @returns {Layer[]} + */ +export function getAnimatableVisibleLayersByType(layers: any[], type: string): any[] { + return getAnimatableVisibleLayers(layers).filter(l => l.type === type); } export function mergeFilterWithTimeline( @@ -1285,7 +1293,7 @@ export function scaleSourceDomainToDestination( return [scaledOffset, scaledSourceDomainSize + scaledOffset]; } -export function getFilterScaledTimeline(filter, animationConfig): number[] { +export function getFilterScaledTimeline(filter, animationConfig): [number, number] | [] { if (!(filter.syncedWithLayerTimeline && animationConfig?.domain)) { return []; } diff --git a/src/utils/src/plot.ts b/src/utils/src/plot.ts index e72aa509f8..d60b2618db 100644 --- a/src/utils/src/plot.ts +++ b/src/utils/src/plot.ts @@ -240,7 +240,8 @@ export function validBin(b) { } /** - * Use in slider, given a number and an array of numbers, return the nears number from the array + * Use in slider, given a number and an array of numbers, return the nears number from the array. + * Takes a value, timesteps and return the actual step. * @param value * @param marks */ @@ -354,10 +355,9 @@ export function getLineChart(datasets: Datasets, filter: Filter): LineChart { if ( filter.lineChart && - // @ts-expect-error add aggregation to LineChart? filter.lineChart.aggregation === aggregation && - // @ts-expect-error add aggregation to LineChart? - filter.lineChart.interval === interval + filter.lineChart.interval === interval && + filter.lineChart.yAxis === yAxis?.name ) { // don't update lineChart if plotType hasn't change return filter.lineChart; @@ -399,6 +399,7 @@ export function getLineChart(datasets: Datasets, filter: Filter): LineChart { series: split, title: `${aggrName}${' of '}${yAxis ? yAxis.name : 'Count'}`, fieldType: yAxis ? yAxis.type : 'integer', + yAxis: yAxis ? yAxis.name : null, allTime: { title: `All Time Average`, value: aggregate(series, AGGREGATION_TYPES.average, d => d.y) diff --git a/src/utils/src/time.ts b/src/utils/src/time.ts index eaa8ce3c33..b50a35d72a 100644 --- a/src/utils/src/time.ts +++ b/src/utils/src/time.ts @@ -51,7 +51,6 @@ export const getTimelineFromAnimationConfig = (animationConfig: AnimationConfig) // @ts-expect-error value: toArray(currentTime), enableInteraction: true, - // @ts-expect-error domain, speed, isAnimating: isAnimating || false, diff --git a/test/node/reducers/vis-state-test.js b/test/node/reducers/vis-state-test.js index ebc2d94729..03c7380c65 100644 --- a/test/node/reducers/vis-state-test.js +++ b/test/node/reducers/vis-state-test.js @@ -3236,6 +3236,7 @@ test('#visStateReducer -> SET_FILTER_PLOT.yAxis', t => { ], markers: [] }, + yAxis: 'uid', title: 'Total of uid', fieldType: 'integer', allTime: {title: 'All Time Average', value: 580.5454545454545} @@ -3374,9 +3375,63 @@ test('#visStateReducer -> SET_FILTER_PLOT.yAxis', t => { // test filter cmpFilters(t, expectedFilterWName, stateWithFilterPlot.filters[0]); + // set filterPlot yAxis again + const yAxisField2 = stateWithFilterName.datasets.smoothie.fields.find( + f => f.name === 'gps_data.lat' + ); + const stateWithFilterPlot2 = reducer( + stateWithFilterPlot, + VisStateActions.setFilterPlot(0, {yAxis: yAxisField2}) + ); + const expectedFilterWName2 = { + ...expectedFilterWName, + yAxis: yAxisField2, + lineChart: { + yDomain: [29.9870074, 90.18377960000001], + xDomain: [1474070985000, 1474072215000], + interval: '15-second', + aggregation: 'sum', + series: { + lines: [ + [ + {x: 1474070985000, y: 29.9900937, delta: 'last', pct: null}, + {x: 1474071045000, y: 29.9927699, delta: 'last', pct: 0.00008923613333024682}, + {x: 1474071105000, y: 29.9907261, delta: 'last', pct: -0.00006814308937832221}, + {x: 1474071165000, y: 29.9870074, delta: 'last', pct: -0.0001239949972401734}, + {x: 1474071240000, y: 29.9923041, delta: 'last', pct: 0.00017663316413490536}, + {x: 1474071300000, y: 29.9968249, delta: 'last', pct: 0.00015073200061350596}, + {x: 1474071360000, y: 30.0037217, delta: 'last', pct: 0.0002299176670528158}, + {x: 1474071420000, y: 30.0116207, delta: 'last', pct: 0.00026326733993142846}, + {x: 1474071480000, y: 30.0208925, delta: 'last', pct: 0.0003089403299035078}, + {x: 1474071540000, y: 30.0218999, delta: 'last', pct: 0.000033556630603251856}, + {x: 1474071555000, y: 30.0229344, delta: 'last', pct: 0.00003445817897751978}, + {x: 1474071600000, y: 30.0264237, delta: 'last', pct: 0.0001162211512542309}, + {x: 1474071675000, y: 30.0292134, delta: 'last', pct: 0.00009290816741525582}, + {x: 1474071735000, y: 30.034391, delta: 'last', pct: 0.00017241876871805346}, + {x: 1474071795000, y: 30.0352752, delta: 'last', pct: 0.000029439584774718954}, + {x: 1474071855000, y: 30.0395918, delta: 'last', pct: 0.00014371767767252643}, + {x: 1474071915000, y: 30.0497387, delta: 'last', pct: 0.00033778421716099144}, + {x: 1474071975000, y: 30.0538936, delta: 'last', pct: 0.00013826742526714978}, + {x: 1474072050000, y: 30.060911, delta: 'last', pct: 0.0002334938724878657}, + {x: 1474072110000, y: 30.060334, delta: 'last', pct: -0.000019194361741060598}, + {x: 1474072170000, y: 30.0554663, delta: 'last', pct: -0.0001619310018312477}, + {x: 1474072200000, y: 90.18377960000001, delta: 'last', pct: 2.0005782874844305} + ] + ], + markers: [] + }, + yAxis: 'gps_data.lat', + title: 'Total of gps_data.lat', + fieldType: 'real', + allTime: {title: 'All Time Average', value: 32.757264254545454} + } + }; + // gps_data.lat + cmpFilters(t, expectedFilterWName2, stateWithFilterPlot2.filters[0]); + // set filterPlot type const stateWithFilterPlotHistogram = reducer( - stateWithFilterPlot, + stateWithFilterPlot2, VisStateActions.setFilterPlot(0, {plotType: {type: 'histogram'}}) ); t.deepEqual(