[Profiling] Fix Diff TopN bug

elastic · Oct 23, 2023 · 6b7f2d3 · 6b7f2d3
1 parent 3f908ea
commit 6b7f2d3
Showing 1 changed file with 66 additions and 34 deletions.
diff --git a/x-pack/plugins/profiling/public/components/topn_functions/function_row.tsx b/x-pack/plugins/profiling/public/components/topn_functions/function_row.tsx
@@ -15,8 +15,8 @@ import {
   useEuiTheme,
 } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
-import React from 'react';
 import { TopNFunctionSortField } from '@kbn/profiling-utils';
+import React, { useEffect } from 'react';
 import { asCost } from '../../utils/formatters/as_cost';
 import { asWeight } from '../../utils/formatters/as_weight';
 import { StackFrameSummary } from '../stack_frame_summary';
@@ -39,38 +39,8 @@ export function FunctionRow({
   onFrameClick,
   setCellProps,
 }: Props) {
-  const theme = useEuiTheme();
-  const successColor = useEuiBackgroundColor('success');
-  const dangerColor = useEuiBackgroundColor('danger');
-
   if (columnId === TopNFunctionSortField.Diff) {
-    if (!functionRow.diff) {
-      return (
-        <EuiText size="xs" color={theme.euiTheme.colors.primaryText}>
-          {i18n.translate('xpack.profiling.functionsView.newLabel', {
-            defaultMessage: 'New',
-          })}
-        </EuiText>
-      );
-    }
-
-    if (functionRow.diff.rank === 0) {
-      return null;
-    }
-
-    const color = functionRow.diff.rank > 0 ? 'success' : 'danger';
-    setCellProps({ style: { backgroundColor: color === 'success' ? successColor : dangerColor } });
-
-    return (
-      <EuiFlexGroup direction="row" gutterSize="xs">
-        <EuiFlexItem grow={false}>
-          <EuiIcon type={functionRow.diff.rank > 0 ? 'sortUp' : 'sortDown'} color={color} />
-        </EuiFlexItem>
-        <EuiFlexItem grow={false}>
-          <EuiText size="s">{Math.abs(functionRow.diff.rank)}</EuiText>
-        </EuiFlexItem>
-      </EuiFlexGroup>
-    );
+    return <DiffColumn diff={functionRow.diff} setCellProps={setCellProps} />;
   }
 
   if (columnId === TopNFunctionSortField.Rank) {
@@ -82,12 +52,12 @@ export function FunctionRow({
   }
 
   if (columnId === TopNFunctionSortField.Samples) {
-    setCellProps({ css: { textAlign: 'right' } });
     return (
-      <SampleStat
+      <SamplesColumn
         samples={functionRow.samples}
         diffSamples={functionRow.diff?.samples}
         totalSamples={totalCount}
+        setCellProps={setCellProps}
       />
     );
   }
@@ -116,3 +86,65 @@ export function FunctionRow({
 
   return null;
 }
+
+interface SamplesColumnProps {
+  samples: number;
+  diffSamples?: number;
+  totalSamples: number;
+  setCellProps: EuiDataGridCellValueElementProps['setCellProps'];
+}
+
+function SamplesColumn({ samples, totalSamples, diffSamples, setCellProps }: SamplesColumnProps) {
+  useEffect(() => {
+    setCellProps({ css: { textAlign: 'right' } });
+  }, [setCellProps]);
+  return <SampleStat samples={samples} diffSamples={diffSamples} totalSamples={totalSamples} />;
+}
+
+interface DiffColumnProps {
+  diff: IFunctionRow['diff'];
+  setCellProps: EuiDataGridCellValueElementProps['setCellProps'];
+}
+
+function DiffColumn({ diff, setCellProps }: DiffColumnProps) {
+  const theme = useEuiTheme();
+  const successColor = useEuiBackgroundColor('success');
+  const dangerColor = useEuiBackgroundColor('danger');
+
+  useEffect(() => {
+    if (diff && diff.rank > 0) {
+      const color = diff.rank > 0 ? 'success' : 'danger';
+      setCellProps({
+        style: { backgroundColor: color === 'success' ? successColor : dangerColor },
+      });
+    }
+  }, [dangerColor, diff, setCellProps, successColor]);
+
+  if (!diff) {
+    return (
+      <EuiText size="xs" color={theme.euiTheme.colors.primaryText}>
+        {i18n.translate('xpack.profiling.functionsView.newLabel', {
+          defaultMessage: 'New',
+        })}
+      </EuiText>
+    );
+  }
+
+  if (diff.rank === 0) {
+    return null;
+  }
+
+  return (
+    <EuiFlexGroup direction="row" gutterSize="xs">
+      <EuiFlexItem grow={false}>
+        <EuiIcon
+          type={diff.rank > 0 ? 'sortUp' : 'sortDown'}
+          color={diff.rank > 0 ? 'success' : 'danger'}
+        />
+      </EuiFlexItem>
+      <EuiFlexItem grow={false}>
+        <EuiText size="s">{Math.abs(diff.rank)}</EuiText>
+      </EuiFlexItem>
+    </EuiFlexGroup>
+  );
+}