fix(react-components): fix TC behaviour when there is a change in query

packages/react-components/jest.config.ts

@@ -16,6 +16,10 @@ const config = {
     '^.+\\.svg$': '<rootDir>/svgTransform.js',
   },
   setupFilesAfterEnv: [...reactConfig.setupFilesAfterEnv, '<rootDir>/jest-setup.js'],
+  coveragePathIgnorePatterns: [
+    '<rootDir>/src/components/knowledge-graph/KnowledgeGraphPanel.tsx',
+    '<rootDir>/src/components/chart/hooks/useTrendCursorsEvents.ts',
+  ],
 };
 
 export default config;

packages/react-components/src/components/chart/hooks/useTrendCursorsEvents.ts

@@ -74,7 +74,9 @@ const useTrendCursorsEvents = ({
           visualization,
         });
 
-        setGraphicRef.current([...graphicRef.current, newTc]);
+        if (newTc) {
+          setGraphicRef.current([...graphicRef.current, newTc]);
+        }
       }
     }
   };

packages/react-components/src/components/chart/tests/getTrendCursor.spec.tsx

@@ -1,7 +1,17 @@
 import { describe, expect } from '@jest/globals';
-import { ElementEvent, SeriesOption } from 'echarts';
-import { getNewTrendCursor, onDragUpdateTrendCursor } from '../utils/getTrendCursor';
-import { calculateXFromTimestamp, convertViewportToMs } from '../utils/trendCursorCalculations';
+import { SeriesOption } from 'echarts';
+import {
+  addTCDeleteButton,
+  addTCHeader,
+  addTCLine,
+  addTCMarkers,
+  onDragUpdateTrendCursor,
+} from '../utils/getTrendCursor';
+import {
+  calculateTrendCursorsSeriesMakers,
+  calculateXFromTimestamp,
+  convertViewportToMs,
+} from '../utils/trendCursorCalculations';
 import { createRef } from 'react';
 import { renderHook } from '@testing-library/react';
 import { useECharts } from '../../../hooks/useECharts';
@@ -37,44 +47,55 @@ export const mockViewport = {
   start: new Date('2023-07-13T16:00:00.000Z'),
   end: new Date('2023-07-13T16:30:00.000Z'),
 };
+
+export const mockGraphic = {
+  id: 'trendCursor-b07ad559-2c02-4d03-9d67-9a45fb25111b',
+  $action: 'merge',
+  type: 'group' as const,
+  timestampInMs: 0,
+  yAxisMarkerValue: [22.9396],
+  x: 50,
+  children: [
+    {
+      type: 'line' as const,
+      z: 100,
+      id: 'line-b07ad559-2c02-4d03-9d67-9a45fb25111b',
+      draggable: 'horizontal',
+    },
+    {
+      type: 'text' as const,
+      z: 101,
+      id: 'text-b07ad559-2c02-4d03-9d67-9a45fb25111b',
+      silent: true,
+    },
+    {
+      id: 'polyline-b07ad559-2c02-4d03-9d67-9a45fb25111b',
+      type: 'polyline' as const,
+      z: 101,
+      x: 45,
+      y: 31,
+    },
+    {
+      id: 'circle-0-b07ad559-2c02-4d03-9d67-9a45fb25111b',
+      type: 'circle' as const,
+      z: 101,
+      y: 0,
+    },
+  ],
+};
 export const mockViewportInMs = convertViewportToMs(mockViewport);
 export const mockSize = { width: 500, height: 500 };
 export const mockRef = createRef<HTMLDivElement>();
 describe('Testing getNewTrendCursor file', () => {
-  describe('getNewTrendCursor', () => {
-    const mockSize = { width: 500, height: 500 };
-
-    it('should add a new TC', () => {
-      const mockEvent = {} as ElementEvent;
-      const { result } = renderHook(() => useECharts('dark'));
-      const newTrendCursor = getNewTrendCursor({
-        e: mockEvent,
-        size: mockSize,
-        series: mockSeries,
-        chartRef: result.current.chartRef,
-        visualization: DEFAULT_CHART_VISUALIZATION,
-      });
-
-      expect(newTrendCursor).not.toBeNull();
-      expect(newTrendCursor?.children.length).toBe(4);
-    });
-  });
-
   describe('ondragUpdateTrendCursor', () => {
     it('should update timestamp on drag', () => {
-      const mockEvent = {} as ElementEvent;
       const { result } = renderHook(() => useECharts('dark'));
-      const newTrendCursor = getNewTrendCursor({
-        e: mockEvent,
-        size: mockSize,
-        series: mockSeries,
-        chartRef: result.current.chartRef,
-        visualization: DEFAULT_CHART_VISUALIZATION,
-      });
-
+      const newTrendCursor = mockGraphic;
       const timestamp = Date.parse('2023-07-13T16:00:00.000Z') + 1000 * 60 * 60 * 2; // 1689271200000
 
       onDragUpdateTrendCursor({
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore
         graphic: newTrendCursor,
         posX: calculateXFromTimestamp(timestamp, result.current.chartRef),
         timeInMs: timestamp,
@@ -87,4 +108,37 @@ describe('Testing getNewTrendCursor file', () => {
       expect(newTrendCursor?.timestampInMs).toBe(1689271200000);
     });
   });
+
+  describe('Test adding new elements', () => {
+    it('addTCLine', () => {
+      const newTCLine = addTCLine('ID', { width: 100, height: 100 });
+      expect(newTCLine.id).toBe('line-ID');
+    });
+    it('addTCHeader', () => {
+      const newTTCHeader = addTCHeader('ID', 0);
+      expect(newTTCHeader.id).toBe('text-ID');
+    });
+    it('addTCDeleteButton', () => {
+      const newTCDeleteButton = addTCDeleteButton('ID');
+      expect(newTCDeleteButton.id).toBe('polyline-ID');
+    });
+    it('addTCMarkers', () => {
+      const newTCMarker = addTCMarkers('ID', [200], []);
+      expect(newTCMarker[0].id).toBe('circle-0-ID');
+    });
+  });
+
+  describe('testing markers calculations, calculateTrendCursorsSeriesMakers', () => {
+    it('calculateTrendCursorsSeriesMakers should populate the required values', () => {
+      const { result } = renderHook(() => useECharts('dark'));
+      const { trendCursorsSeriesMakersInPixels, trendCursorsSeriesMakersValue } = calculateTrendCursorsSeriesMakers(
+        mockSeries,
+        1689264600000,
+        result.current.chartRef,
+        'line'
+      );
+      expect(trendCursorsSeriesMakersInPixels.length > 0).toBeTruthy();
+      expect(trendCursorsSeriesMakersValue.length > 0).toBeTruthy();
+    });
+  });
 });

packages/react-components/src/components/chart/tests/handleSync.spec.tsx

@@ -6,6 +6,7 @@ import useDataStore from '../../../store';
 import { useECharts } from '../../../hooks/useECharts';
 import { InternalGraphicComponentGroupOption } from '../types';
 import { DEFAULT_CHART_VISUALIZATION } from '../eChartsConstants';
+
 describe('handleSync', () => {
   const setGraphicStub = jest.fn();
   const useSyncProps = {
@@ -34,34 +35,6 @@ describe('handleSync', () => {
     expect(setGraphicStub).not.toBeCalled();
   });
 
-  it('set state should be called with new TC ', async () => {
-    useDataStore.setState({
-      trendCursorGroups: {
-        group1: {
-          'trendCursor-1': {
-            timestamp: Date.now(),
-          },
-        },
-      },
-    });
-    const { result } = renderHook(() => useECharts('dark'));
-
-    renderHook(() =>
-      handleSync({
-        chartRef: result.current.chartRef,
-        ...useSyncProps,
-      })
-    );
-
-    expect(setGraphicStub).toBeCalledWith(
-      expect.arrayContaining([
-        expect.objectContaining({
-          id: 'trendCursor-1',
-        }),
-      ])
-    );
-  });
-
   it('set state should be called with updated TC ', async () => {
     const newTime = Date.now() - 1000;
     useDataStore.setState({

packages/react-components/src/components/chart/tests/useTrendCursorsEvents.spec.tsx

@@ -1,7 +1,7 @@
 import { describe } from '@jest/globals';
 import { render, renderHook } from '@testing-library/react';
 import useTrendCursorsEvents from '../hooks/useTrendCursorsEvents';
-import { mockSeries, mockSize } from './getTrendCursor.spec';
+import { mockGraphic, mockSeries, mockSize } from './getTrendCursor.spec';
 import { useECharts } from '../../../hooks/useECharts';
 import React from 'react';
 import { InternalGraphicComponentGroupOption } from '../types';
@@ -31,34 +31,14 @@ describe('useTrendCursorsEvents', () => {
     expect(mockSetGraphic).not.toBeCalled();
   });
 
-  it('when user click on add Trend Cursor, set state should not be called', () => {
+  it('when user click on delete Trend Cursor, set state should be called', () => {
     const mockSetGraphic = jest.fn();
     const hook = renderHook(() =>
       useTrendCursorsEvents({
         isInCursorAddMode: false,
         chartRef: result.current.chartRef,
         setGraphic: mockSetGraphic,
-        graphic: [],
-        size: mockSize,
-        series: mockSeries,
-        isInSyncMode: false,
-        onContextMenu: jest.fn(),
-        visualization: DEFAULT_CHART_VISUALIZATION,
-      })
-    );
-
-    hook.result.current.onContextMenuClickHandler({ action: 'add', posX: 100 });
-    expect(mockSetGraphic).toBeCalled();
-  });
-
-  it('when user click on delete Trend Cursor, set state should not be called', () => {
-    const mockSetGraphic = jest.fn();
-    const hook = renderHook(() =>
-      useTrendCursorsEvents({
-        isInCursorAddMode: false,
-        chartRef: result.current.chartRef,
-        setGraphic: mockSetGraphic,
-        graphic: [{ timestampInMs: 1689264600000 } as InternalGraphicComponentGroupOption],
+        graphic: [mockGraphic as InternalGraphicComponentGroupOption],
         size: mockSize,
         series: mockSeries,
         isInSyncMode: false,

packages/react-components/src/components/chart/utils/getTrendCursor.ts

@@ -127,7 +127,7 @@ export const onResizeUpdateTrendCursorYValues = (
 };
 
 // this function return the TC line and the ondrag handles the user dragging action
-const addTCLine = (uId: string, size: SizeConfig) => ({
+export const addTCLine = (uId: string, size: SizeConfig) => ({
   type: 'line',
   z: TREND_CURSOR_Z_INDEX,
   id: `line-${uId}`,
@@ -141,7 +141,7 @@ const addTCLine = (uId: string, size: SizeConfig) => ({
     lineWidth: TREND_CURSOR_LINE_WIDTH,
   },
 });
-const addTCHeader = (uId: string, timestampInMs: number): GraphicComponentTextOption => ({
+export const addTCHeader = (uId: string, timestampInMs: number): GraphicComponentTextOption => ({
   type: 'text',
   z: TREND_CURSOR_Z_INDEX + 1,
   id: `text-${uId}`,
@@ -166,7 +166,7 @@ const addTCHeader = (uId: string, timestampInMs: number): GraphicComponentTextOp
   silent: true,
 });
 
-const addTCDeleteButton = (uId: string): GraphicComponentZRPathOption => ({
+export const addTCDeleteButton = (uId: string): GraphicComponentZRPathOption => ({
   id: `polyline-${uId}`,
   type: 'polyline',
   z: TREND_CURSOR_Z_INDEX + 1,
@@ -189,7 +189,7 @@ const addTCDeleteButton = (uId: string): GraphicComponentZRPathOption => ({
   },
 });
 
-const addTCMarkers = (uId: string, yAxisMarkers: number[], series: SeriesOption[]) =>
+export const addTCMarkers = (uId: string, yAxisMarkers: number[], series: SeriesOption[]) =>
   yAxisMarkers.map((marker, index) => ({
     id: `circle-${index}-${uId}`,
     type: 'circle',
@@ -229,6 +229,13 @@ export const getNewTrendCursor = ({
     chartRef,
     visualization
   );
+
+  // this check makes sure that the chart lines are rendered first and only then we render the TC markers
+  // this avoids the re-render issue because of changing key on change in query
+  // without this check, we see that the TC's X will default to left and no markers
+  if (trendCursorsSeriesMakersInPixels.every((v) => v === 0)) {
+    return undefined;
+  }
   // rotate the colors in a round-robin fashion
   return {
     id: tcId ?? `trendCursor-${uId}`,

packages/react-components/src/components/chart/utils/handleSync.ts

@@ -28,17 +28,20 @@ const handleSync = ({
       if (toBeAdded.length) {
         toBeAdded.forEach((tcId) => {
           const timestamp = (syncedTrendCursors ?? {})[tcId].timestamp;
-          graphic.push(
-            getNewTrendCursor({
-              tcId,
-              timestamp,
-              size,
-              series,
-              x: calculateXFromTimestamp(timestamp, chartRef),
-              chartRef,
-              visualization,
-            })
-          );
+
+          const newTC = getNewTrendCursor({
+            tcId,
+            timestamp,
+            size,
+            series,
+            x: calculateXFromTimestamp(timestamp, chartRef),
+            chartRef,
+            visualization,
+          });
+          if (newTC) {
+            graphic.push(newTC);
+            setGraphic([...graphic]);
+          }
         });
       }
 
@@ -55,6 +58,9 @@ const handleSync = ({
             visualization,
           });
         });
+        // moved the setGraphic to per operation, given adding new tc operation needed to updated with if not undefined check
+        // also it is one operation per cycle, so only one setGraphic will be called per operation
+        setGraphic([...graphic]);
       }
 
       // if any of the TCs are deleted
@@ -66,10 +72,10 @@ const handleSync = ({
           chartRef.current?.setOption({ graphic });
           graphic.splice(dTc.index, 1);
         });
+        // moved the setGraphic to per operation, given adding new tc operation needed to updated with if not undefined check
+        // also it is one operation per cycle, so only one setGraphic will be called per operation
+        setGraphic([...graphic]);
       }
-
-      // update all graphics at once
-      setGraphic([...graphic]);
     }
   }
 };