apply cr feedback (#29077)

getsentry · Oct 7, 2021 · 2bb85c6 · 2bb85c6 · evanpurkhiser · Oct 7, 2021
1 parent b0cdf67
commit 2bb85c6
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 16 deletions.
diff --git a/static/app/components/charts/barChart.tsx b/static/app/components/charts/barChart.tsx
@@ -11,8 +11,8 @@ type ChartProps = React.ComponentProps<typeof BaseChart>;
 export type BarChartSeries = Series & Omit<EChartOption.SeriesBar, 'data' | 'name'>;
 
 type Props = Omit<ChartProps, 'series'> & {
-  stacked?: boolean;
   series: BarChartSeries[];
+  stacked?: boolean;
 };
 
 class BarChart extends React.Component<Props> {

diff --git a/static/app/components/charts/baseChart.tsx b/static/app/components/charts/baseChart.tsx
@@ -109,6 +109,10 @@ type Props = {
       ) => string;
       valueFormatter?: (value: number, label?: string) => string | number;
       nameFormatter?: (name: string) => string;
+      /**
+       * Array containing seriesNames that need to be indented
+       */
+      indentLabels?: string[];
     };
   /**
    * DataZoom (allows for zooming of chart)
@@ -456,6 +460,9 @@ const ChartContainer = styled('div')`
     font-weight: normal;
     color: ${p => p.theme.white};
   }
+  .tooltip-label-indent {
+    margin-left: ${space(3)};
+  }
   .tooltip-series > div {
     display: flex;
     justify-content: space-between;

diff --git a/static/app/components/charts/components/tooltip.tsx b/static/app/components/charts/components/tooltip.tsx
@@ -81,7 +81,12 @@ type TooltipFormatters =
   | 'nameFormatter';
 
 type FormatterOptions = Pick<NonNullable<ChartProps['tooltip']>, TooltipFormatters> &
-  Pick<ChartProps, NeededChartProps>;
+  Pick<ChartProps, NeededChartProps> & {
+    /**
+     * Array containing seriesNames that need to be indented
+     */
+    indentLabels?: string[];
+  };
 
 function getFormatter({
   filter,
@@ -93,6 +98,7 @@ function getFormatter({
   bucketSize,
   valueFormatter = defaultValueFormatter,
   nameFormatter = defaultNameFormatter,
+  indentLabels = [],
 }: FormatterOptions) {
   const getFilter = (seriesParam: EChartOption.Tooltip.Format) => {
     // Series do not necessarily have `data` defined, e.g. releases don't have `data`, but rather
@@ -140,10 +146,14 @@ function getFormatter({
         seriesParamsOrParam.name
       );
 
+      const className = indentLabels.includes(seriesParamsOrParam.name ?? '')
+        ? 'tooltip-label tooltip-label-indent'
+        : 'tooltip-label';
+
       return [
         '<div class="tooltip-series">',
         `<div>
-          <span class="tooltip-label"><strong>${seriesParamsOrParam.name}</strong></span>
+          <span class="${className}"><strong>${seriesParamsOrParam.name}</strong></span>
           ${truncatedName}: ${formattedValue}
         </div>`,
         '</div>',
@@ -163,7 +173,7 @@ function getFormatter({
       ? seriesParams[0].axisValue
       : getSeriesValue(seriesParams[0], 0);
 
-    const label =
+    const date =
       seriesParams.length &&
       axisFormatterOrDefault(
         timestamp,
@@ -182,11 +192,16 @@ function getFormatter({
             truncationFormatter(s.seriesName ?? '', truncate)
           );
           const value = valueFormatter(getSeriesValue(s, 1), s.seriesName);
-          return `<div><span class="tooltip-label">${s.marker} <strong>${formattedLabel}</strong></span> ${value}</div>`;
+
+          const className = indentLabels.includes(formattedLabel)
+            ? 'tooltip-label tooltip-label-indent'
+            : 'tooltip-label';
+
+          return `<div><span class="${className}">${s.marker} <strong>${formattedLabel}</strong></span> ${value}</div>`;
         })
         .join(''),
       '</div>',
-      `<div class="tooltip-date">${label}</div>`,
+      `<div class="tooltip-date">${date}</div>`,
       `<div class="tooltip-arrow"></div>`,
     ].join('');
   };
@@ -208,6 +223,7 @@ export default function Tooltip({
   valueFormatter,
   nameFormatter,
   hideDelay,
+  indentLabels,
   ...props
 }: Props = {}): EChartOption.Tooltip {
   formatter =
@@ -222,6 +238,7 @@ export default function Tooltip({
       formatAxisLabel,
       valueFormatter,
       nameFormatter,
+      indentLabels,
     });
 
   return {

diff --git a/static/app/components/charts/miniBarChart.tsx b/static/app/components/charts/miniBarChart.tsx
@@ -94,7 +94,7 @@ class MiniBarChart extends React.Component<Props> {
     let chartSeries: BarChartSeries[] = [];
 
     // Ensure bars overlap and that empty values display as we're disabling the axis lines.
-    if (series && series.length) {
+    if (!!series?.length) {
       chartSeries = series.map((original, i: number) => {
         const updated = {
           ...original,
@@ -222,6 +222,7 @@ class MiniBarChart extends React.Component<Props> {
         animation: false,
       },
     };
+
     return <BarChart series={chartSeries} {...chartOptions} {...barChartProps} />;
   }
 }

diff --git a/static/app/components/charts/series/barSeries.tsx b/static/app/components/charts/series/barSeries.tsx
@@ -10,18 +10,19 @@ type BarChartDataObject = Omit<EChartOption.SeriesBar.DataObject, 'value'> & {
   value: (string | number) | (string | number)[];
 };
 
-export default function barSeries(
-  props: Omit<EChartOption.SeriesBar, 'data'> & {
-    data?:
-      | (string | number | void | BarChartDataObject | (string | number)[])[]
-      | (string | number | void | BarChartDataObject)[][]
-      | undefined;
-  } = {}
-): EChartOption.SeriesBar {
-  const {data, ...rest} = props;
+type Props = Omit<EChartOption.SeriesBar, 'data'> & {
+  data?:
+    | (string | number | void | BarChartDataObject | (string | number)[])[]
+    | (string | number | void | BarChartDataObject)[][]
+    | undefined;
+};
+
+function barSeries({data, ...rest}: Props): EChartOption.SeriesBar {
   return {
     ...rest,
     data: data as EChartOption.SeriesBar['data'],
     type: 'bar',
   };
 }
+
+export default barSeries;