Skip to content

Commit

Permalink
fix filtering when changing data
Browse files Browse the repository at this point in the history
  • Loading branch information
@zbigg
zbigg committed Sep 29, 2023
1 parent 1065f9f commit 2fcc04c
Show file tree
Hide file tree
Showing 8 changed files with 236 additions and 174 deletions.
43 changes: 2 additions & 41 deletions packages/react-ui/__tests__/widgets/TimeSeriesWidgetUI.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ describe('TimeSeriesWidgetUI', () => {
expect(onTimelineUpdate).toBeCalled();
});

test('renders with initial timeline position', () => {
test('renders with initial timeline position', async () => {
const onTimelineUpdate = jest.fn();

render(
Expand All @@ -96,7 +96,7 @@ describe('TimeSeriesWidgetUI', () => {
expect(screen.queryByTestId('pause-icon')).not.toBeInTheDocument();
expect(screen.queryByTestId('stop')).toBeInTheDocument();
expect(screen.queryByTestId('play-icon')).toBeInTheDocument();
expect(onTimelineUpdate).toHaveBeenCalledWith(2);
await waitFor(() => expect(onTimelineUpdate).toHaveBeenCalledWith(2));
});

test('plays when play button is fired', () => {
Expand Down Expand Up @@ -129,45 +129,6 @@ describe('TimeSeriesWidgetUI', () => {
expect(onPause).toBeCalled();
});

test('updates data cause reset component', () => {
const NEW_DATA = [
{ name: 1514761200000, value: 310 },
{ name: 1515366000000, value: 406 },
{ name: 1515970800000, value: 387 },
{ name: 1516575600000, value: 471 }
];

const onTimelineUpdate = jest.fn();
const onStop = jest.fn();

const { rerender } = render(
<Widget
isPlaying={false}
isPaused={true}
onStop={onStop}
onTimelineUpdate={onTimelineUpdate}
/>
);

rerender(
<Widget
data={NEW_DATA}
isPlaying={false}
isPaused={true}
onStop={onStop}
onTimelineUpdate={onTimelineUpdate}
/>
);

expect(screen.queryByTestId('pause-icon')).not.toBeInTheDocument();
expect(screen.queryByTestId('stop')).toBeInTheDocument();
expect(screen.queryByTestId('play-icon')).toBeInTheDocument();
expect(onTimelineUpdate).toHaveBeenCalledWith(0);
// Wait a second, because onStop is called with a certain delay
setTimeout(() => expect(onStop).toBeCalled());
jest.runOnlyPendingTimers();
});

test('updates internal state from outside correctly', () => {
const { rerender } = render(<Widget isPlaying={true} isPaused={false} />);
expect(screen.queryByTestId('pause-icon')).toBeInTheDocument();
Expand Down
104 changes: 79 additions & 25 deletions packages/react-ui/src/widgets/TimeSeriesWidgetUI/TimeSeriesWidgetUI.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useMemo, useCallback } from 'react';
import React, { useMemo, useCallback, useEffect, useRef } from 'react';
import { Box, Link, useTheme } from '@mui/material';
import PropTypes from 'prop-types';

Expand All @@ -15,6 +15,7 @@ import { commonPalette } from '../../theme/sections/palette';
import { TimeSeriesControls } from './components/TimeSeriesControls';
import TimeSeriesLayout from './components/TimeSeriesLayout';
import ChartLegend from '../ChartLegend';
import { findNearestPointInData, getDate } from './utils/utilities';

function TimeSeriesWidgetUI({
data,
Expand All @@ -30,9 +31,9 @@ function TimeSeriesWidgetUI({
fitHeight,
showControls,
animation,
timelinePosition,
onTimelineUpdate,
timeWindow,
timelinePosition,
onTimeWindowUpdate,
selectedCategories,
onSelectedCategoriesChange,
Expand All @@ -45,6 +46,35 @@ function TimeSeriesWidgetUI({
palette,
showLegend
}) {
let prevEmittedTimeWindow = useRef();
const handleTimeWindowUpdate = useCallback(
(timeWindow) => {
if (timeWindow.length === 2) {
if (prevEmittedTimeWindow.current?.length === 1) {
onTimelineUpdate?.(undefined);
}
const sorted = timeWindow
.sort((timeA, timeB) => (timeA < timeB ? -1 : 1))
.map(getDate);
onTimeWindowUpdate?.(sorted);
}

if (timeWindow.length === 1) {
if (prevEmittedTimeWindow.current?.length === 2) {
onTimeWindowUpdate?.([]);
}
const position = findNearestPointInData(timeWindow[0], data);
onTimelineUpdate?.(position);
}

prevEmittedTimeWindow.current = timeWindow;

// Only executed when timeWindow changes
// eslint-disable-next-line react-hooks/exhaustive-deps
},
[onTimeWindowUpdate, onTimelineUpdate, data]
);

const content = isLoading ? (
<TimeSeriesSkeleton
fitHeight={fitHeight}
Expand All @@ -70,6 +100,7 @@ function TimeSeriesWidgetUI({
palette={palette}
showLegend={showLegend}
selectedCategories={selectedCategories}
timelinePosition={timelinePosition}
onSelectedCategoriesChange={onSelectedCategoriesChange}
/>
);
Expand All @@ -81,10 +112,8 @@ function TimeSeriesWidgetUI({
isPaused={isPaused}
onPause={onPause}
onStop={onStop}
timelinePosition={timelinePosition}
onTimelineUpdate={onTimelineUpdate}
timeWindow={timeWindow}
onTimeWindowUpdate={onTimeWindowUpdate}
onTimeWindowUpdate={handleTimeWindowUpdate}
>
{content}
</TimeSeriesProvider>
Expand Down Expand Up @@ -134,8 +163,6 @@ TimeSeriesWidgetUI.defaultProps = {
animation: true,
isPlaying: false,
isPaused: false,
timelinePosition: 0,
timeWindow: [],
showControls: true,
isLoading: false,
palette: Object.values(commonPalette.qualitative.bold)
Expand All @@ -162,13 +189,41 @@ function TimeSeriesWidgetUIContent({
palette,
selectedCategories,
onSelectedCategoriesChange,
showLegend
showLegend,
timelinePosition
}) {
const theme = useTheme();
const fallbackColor = theme.palette.secondary.main;

const { isPlaying, isPaused, timeWindow, timelinePosition, stop } =
useTimeSeriesContext();
const { isPlaying, isPaused, timeWindow, stop, setTimeWindow } = useTimeSeriesContext();

useEffect(() => {
if (timelinePosition !== undefined) {
if (timelinePosition < 0 || timelinePosition >= data.length) return;

const timeAtSelectedPosition = data[timelinePosition].name;
setTimeWindow([timeAtSelectedPosition]);
} else if (data.length > 0 && timeWindow.length === 0) {
setTimeWindow([data[0].name]);
}
// ignore timeWindow, as we're only expecting to change when external data changes
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [timelinePosition, data]);

useEffect(() => {
const start = data[0].name;
const end = data[data.length - 1].name;
if (
timeWindow[0] < start ||
timeWindow[1] > end ||
timeWindow[1] < start ||
timeWindow[1] > end
) {
setTimeWindow([]);
}
// only run on data updates to cross-check that time-window isn't out-of bounds
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [data]);

const series = useMemo(() => {
const colorMapping = {};
Expand Down Expand Up @@ -207,7 +262,7 @@ function TimeSeriesWidgetUIContent({
}

// If timeWindow is activated
if (timeWindow.length) {
if (timeWindow.length === 2) {
const [start, end] = timeWindow.map((time) => new Date(time));
return formatTimeRange({ start, end, stepSize });
}
Expand All @@ -220,21 +275,21 @@ function TimeSeriesWidgetUIContent({
return formatTimeRange({ start, end, stepSize });
}

// If animation is active
if (timelinePosition >= 0 && data[timelinePosition]) {
const currentDate = new Date(data[timelinePosition].name);
return formatBucketRange({ date: currentDate, stepSize, stepMultiplier });
if (timeWindow.length === 1) {
const date = new Date(timeWindow[0]);
return formatBucketRange({ date, stepSize, stepMultiplier });
}
}, [data, timeWindow, isPlaying, isPaused, timelinePosition, stepSize, stepMultiplier]);
}, [data, timeWindow, isPlaying, isPaused, stepSize, stepMultiplier]);

const showClearButton = useMemo(() => {
return (
isPlaying || isPaused || timeWindow.length > 0 || selectedCategories?.length > 0
);
}, [isPaused, isPlaying, selectedCategories?.length, timeWindow.length]);

const handleStop = () => {
const handleClear = () => {
stop();
setTimeWindow([]);
onSelectedCategoriesChange?.([]);
};

Expand Down Expand Up @@ -262,18 +317,17 @@ function TimeSeriesWidgetUIContent({

const header = (
<>
{!!currentDate && (
<Box>
<Typography color='textSecondary' variant='caption'>
{currentDate}
</Typography>
</Box>
)}
<Box>
<Typography color='textSecondary' variant='caption'>
{currentDate || '-'}
</Typography>
</Box>

{showClearButton && (
<Link
variant='caption'
style={{ cursor: 'pointer' }}
onClick={handleStop}
onClick={handleClear}
underline='hover'
>
Clear
Expand Down
42 changes: 23 additions & 19 deletions packages/react-ui/src/widgets/TimeSeriesWidgetUI/components/TimeSeriesControls.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Box, Menu, IconButton, MenuItem, SvgIcon } from '@mui/material';
import { GroupDateTypes } from '@carto/react-core';
import Typography from '../../../components/atoms/Typography';
import { useTimeSeriesContext } from '../hooks/TimeSeriesContext';
import { countDistinctTimePoints, findNearestPointInData } from '../utils/utilities';

// TimeWindow step is the amount of time (in seconds) that pass in every iteration during the animation.
// It depends on step size for a better animation speed adjustment.
Expand All @@ -24,16 +25,8 @@ export function TimeSeriesControls({ data, stepSize }) {
const [speed, setSpeed] = useState(1);
const animationRef = useRef({ animationFrameId: null, timeoutId: null });

const {
isPlaying,
isPaused,
timeWindow,
timelinePosition,
setTimelinePosition,
setTimeWindow,
stop,
togglePlay
} = useTimeSeriesContext();
const { isPlaying, isPaused, timeWindow, setTimeWindow, stop, togglePlay } =
useTimeSeriesContext();

// If data changes, stop animation. useDidMountEffect is used to avoid
// being executed in the initial rendering because that cause
Expand Down Expand Up @@ -92,13 +85,13 @@ export function TimeSeriesControls({ data, stepSize }) {

// Running timeline
useEffect(() => {
if (isPlaying && !timeWindow.length && data.length) {
if (isPlaying && timeWindow.length === 1 && data.length) {
animateTimeline({
speed,
timelinePosition,
timelinePosition: timeWindow[0],
data,
drawFrame: (newTimelinePosition) => {
setTimelinePosition(newTimelinePosition);
setTimeWindow([newTimelinePosition]);
},
onEnd: () => {
setTimeout(handleStop, 250);
Expand All @@ -107,9 +100,11 @@ export function TimeSeriesControls({ data, stepSize }) {
});

return () => stopAnimation();
} else if (isPlaying && timeWindow.length === 0) {
setTimeWindow([data[0].name]);
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [data, isPlaying, speed, timeWindow.length, handleStop, setTimelinePosition]);
}, [data, isPlaying, speed, timeWindow.length, handleStop]);

const handleOpenSpeedMenu = (e) => {
if (e?.currentTarget) {
Expand Down Expand Up @@ -282,11 +277,13 @@ function animateTimeline({
onEnd,
animationRef
}) {
let currentTimeline = timelinePosition;
let currentDataIndex = findNearestPointInData(timelinePosition, data) || 0;
let currentTime = data[currentDataIndex].name;

const numberOfTimePoints = countDistinctTimePoints(data);
const fpsToUse =
Math.max(
Math.round(Math.sqrt(data.length) / 2), // FPS based on data length
Math.round(Math.sqrt(numberOfTimePoints) / 2), // FPS based on number of unique time points
MIN_FPS // Min FPS
) * speed;

Expand All @@ -297,11 +294,18 @@ function animateTimeline({
};

const animate = () => {
currentTimeline = Math.min(data.length, currentTimeline + 1);
if (currentTimeline === data.length) {
// // search for next time different than previous one
for (; currentDataIndex < data.length; currentDataIndex++) {
const itemTime = data[currentDataIndex].name;
if (itemTime !== currentTime) {
currentTime = itemTime;
break;
}
}
if (currentDataIndex === data.length) {
onEnd();
} else {
drawFrame(currentTimeline);
drawFrame(currentTime);
fireAnimation();
}
};
Expand Down
Loading

0 comments on commit 2fcc04c

Please sign in to comment.