Browse Source

fix(starfish): Fixes chart axis sync sometimes not working and slideoverpanel initial state (#49688)

Fixes an issue with synced y axis sometimes not syncing all charts
(especially in api module) and fixes slideOverPanel sometimes displaying
initially incorrectly
edwardgou-sentry 1 year ago
parent
commit
6034b9bb85

+ 15 - 9
static/app/views/starfish/components/chart.tsx

@@ -1,4 +1,4 @@
-import {useEffect, useRef, useState} from 'react';
+import {RefObject, useEffect, useRef, useState} from 'react';
 import {useTheme} from '@emotion/react';
 import {LineSeriesOption} from 'echarts';
 import * as echarts from 'echarts/core';
@@ -39,8 +39,10 @@ type Props = {
   utc: boolean;
   aggregateOutputFormat?: 'number' | 'percentage' | 'duration';
   chartColors?: string[];
+  chartGroup?: string;
   definedAxisTicks?: number;
   disableXAxis?: boolean;
+  forwardedRef?: RefObject<ReactEchartsRef>;
   grid?: AreaChartProps['grid'];
   height?: number;
   hideYAxisSplitLine?: boolean;
@@ -118,14 +120,18 @@ function Chart({
   throughput,
   aggregateOutputFormat,
   onClick,
+  forwardedRef,
+  chartGroup,
 }: Props) {
   const router = useRouter();
   const theme = useTheme();
-  const chart = useRef<ReactEchartsRef>(null);
 
-  const echartsInstance = chart.current?.getEchartsInstance();
+  const defaultRef = useRef<ReactEchartsRef>(null);
+  const chartRef = forwardedRef || defaultRef;
+
+  const echartsInstance = chartRef?.current?.getEchartsInstance();
   if (echartsInstance && !echartsInstance.group) {
-    echartsInstance.group = STARFISH_CHART_GROUP;
+    echartsInstance.group = chartGroup ?? STARFISH_CHART_GROUP;
   }
 
   if (!data || data.length <= 0) {
@@ -215,7 +221,7 @@ function Chart({
         return element.classList.contains('echarts-for-react');
       }
     );
-    if (hoveredEchartElement === chart.current?.ele) {
+    if (hoveredEchartElement === chartRef?.current?.ele) {
       // Return undefined to use default formatter
       return getFormatter({
         isGroupedByDate: true,
@@ -292,7 +298,7 @@ function Chart({
           return (
             <BaseChart
               {...zoomRenderProps}
-              ref={chart}
+              ref={chartRef}
               height={height}
               previousPeriod={previousData}
               additionalSeries={transformedThroughput}
@@ -345,7 +351,7 @@ function Chart({
 
         return (
           <AreaChart
-            forwardedRef={chart}
+            forwardedRef={chartRef}
             height={height}
             {...zoomRenderProps}
             series={series}
@@ -363,10 +369,10 @@ function Chart({
 
 export default Chart;
 
-export function useSynchronizeCharts(deps: boolean[]) {
+export function useSynchronizeCharts(deps: boolean[] = []) {
   const [synchronized, setSynchronized] = useState<boolean>(false);
   useEffect(() => {
-    if (deps.every(dep => dep) && !synchronized) {
+    if (deps.every(Boolean)) {
       echarts.connect(STARFISH_CHART_GROUP);
       setSynchronized(true);
     }

+ 1 - 1
static/app/views/starfish/components/slideOverPanel.tsx

@@ -35,7 +35,7 @@ function SlideOverPanel(
 
 const _SlideOverPanel = styled(motion.div, {
   shouldForwardProp: prop =>
-    ['animate', 'transition'].includes(prop) ||
+    ['animate', 'transition', 'initial'].includes(prop) ||
     (prop !== 'collapsed' && isPropValid(prop)),
 })<{
   collapsed: boolean;

+ 15 - 13
static/app/views/starfish/modules/APIModule/APIModuleView.tsx

@@ -10,7 +10,7 @@ import DatePageFilter from 'sentry/components/datePageFilter';
 import {normalizeDateTimeParams} from 'sentry/components/organizations/pageFilters/parse';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
-import {Series} from 'sentry/types/echarts';
+import {ReactEchartsRef, Series} from 'sentry/types/echarts';
 import {useApiQuery} from 'sentry/utils/queryClient';
 import useOrganization from 'sentry/utils/useOrganization';
 import usePageFilters from 'sentry/utils/usePageFilters';
@@ -210,7 +210,9 @@ export default function APIModuleView({location, onSelect}: Props) {
     'p75(transaction.duration)'
   );
 
-  useSynchronizeCharts([!isGraphLoading]);
+  const loading = isGraphLoading || isTopTransactionDataLoading;
+
+  useSynchronizeCharts([!loading]);
 
   return (
     <Fragment>
@@ -226,37 +228,31 @@ export default function APIModuleView({location, onSelect}: Props) {
       <ChartsContainer>
         <ChartsContainerItem>
           <ChartPanel title={t('Top Transactions Throughput')}>
-            <APIModuleChart
-              data={tpmTransactionSeries}
-              loading={isTopTransactionDataLoading}
-            />
+            <APIModuleChart data={tpmTransactionSeries} loading={loading} />
           </ChartPanel>
         </ChartsContainerItem>
         <ChartsContainerItem>
           <ChartPanel title={t('Top Transactions p75')}>
-            <APIModuleChart
-              data={p75TransactionSeries}
-              loading={isTopTransactionDataLoading}
-            />
+            <APIModuleChart data={p75TransactionSeries} loading={loading} />
           </ChartPanel>
         </ChartsContainerItem>
       </ChartsContainer>
       <ChartsContainer>
         <ChartsContainerItem>
           <ChartPanel title={t('Throughput')}>
-            <APIModuleChart data={zeroFilledCounts} loading={isGraphLoading} />
+            <APIModuleChart data={zeroFilledCounts} loading={loading} />
           </ChartPanel>
         </ChartsContainerItem>
         <ChartsContainerItem>
           <ChartPanel title={t('Response Time')}>
-            <APIModuleChart data={zeroFilledQuantiles} loading={isGraphLoading} />
+            <APIModuleChart data={zeroFilledQuantiles} loading={loading} />
           </ChartPanel>
         </ChartsContainerItem>
         <ChartsContainerItem>
           <ChartPanel title={t('Error Rate')}>
             <APIModuleChart
               data={zeroFilledFailureRate}
-              loading={isGraphLoading}
+              loading={loading}
               chartColors={[themes.charts.getColorPalette(2)[2]]}
             />
           </ChartPanel>
@@ -306,10 +302,14 @@ function APIModuleChart({
   data,
   loading,
   chartColors,
+  forwardedRef,
+  chartGroup,
 }: {
   data: Series[];
   loading: boolean;
   chartColors?: string[];
+  chartGroup?: string;
+  forwardedRef?: React.RefObject<ReactEchartsRef>;
 }) {
   return (
     <Chart
@@ -331,6 +331,8 @@ function APIModuleChart({
       isLineChart
       chartColors={chartColors}
       disableXAxis
+      forwardedRef={forwardedRef}
+      chartGroup={chartGroup}
     />
   );
 }

+ 3 - 1
static/app/views/starfish/views/webServiceView/starfishView.tsx

@@ -20,7 +20,7 @@ import {useApiQuery} from 'sentry/utils/queryClient';
 import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import usePageFilters from 'sentry/utils/usePageFilters';
 import withApi from 'sentry/utils/withApi';
-import Chart from 'sentry/views/starfish/components/chart';
+import Chart, {useSynchronizeCharts} from 'sentry/views/starfish/components/chart';
 import MiniChartPanel from 'sentry/views/starfish/components/miniChartPanel';
 import {insertClickableAreasIntoSeries} from 'sentry/views/starfish/utils/insertClickableAreasIntoSeries';
 import {EndpointDataRow} from 'sentry/views/starfish/views/webServiceView/endpointDetails';
@@ -264,6 +264,8 @@ export function StarfishView(props: BasePerformanceViewProps) {
     );
   }
 
+  useSynchronizeCharts();
+
   return (
     <div data-test-id="starfish-view">
       <StyledRow minSize={200}>