ref(dashboards): Use BigNumberWidgetVisualization in Dashboards (#7…

…9246) This is Step 1 of using `BigNumberWidget` inside Dashboards. This change uses the _innards_ of `BigNumberWidget` as the innards of `WidgetCardChart`. `BigNumberWidgetVisualization` is the "innards", that component renders the actual text, the thresholds, etc. Dropping it straight into the place it was extracted from. A few notes: - in `BigNumberWidget`, the thresholds indicator is next to the value, not next the heading. In this PR, I make that change in Dashboards, too - in `BigNumberWidget` "expanding" numbers isn't supported, but isn't needed because all numbers _always_ show the expanded value on hover. In this new version, modal, regular, and mobile numbers all show up the same. Let me know if this causes a problem! - I added a bit more error checking in a few places, and some small layout improvements
getsentry · Oct 23, 2024 · de8b3c0 · de8b3c0
1 parent 8c06d49
commit de8b3c0
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 107 deletions.
diff --git a/static/app/components/modals/widgetViewerModal.tsx b/static/app/components/modals/widgetViewerModal.tsx
@@ -121,6 +121,7 @@ interface Props extends ModalRenderProps, WidgetViewerModalOptions {
 const FULL_TABLE_ITEM_LIMIT = 20;
 const HALF_TABLE_ITEM_LIMIT = 10;
 const HALF_CONTAINER_HEIGHT = 300;
+const BIG_NUMBER_HEIGHT = 160;
 const EMPTY_QUERY_NAME = '(Empty Query Condition)';
 
 const shouldWidgetCardChartMemo = (prevProps, props) => {
@@ -854,7 +855,7 @@ function WidgetViewerModal(props: Props) {
                   ].includes(widget.displayType)
                     ? SLIDER_HEIGHT
                     : 0)
-                : null
+                : BIG_NUMBER_HEIGHT
             }
           >
             {(!!seriesData || !!tableData) && chartUnmodified ? (
@@ -1197,6 +1198,7 @@ export const modalCss = css`
 `;
 
 const Container = styled('div')<{height?: number | null}>`
+  display: flex;
   height: ${p => (p.height ? `${p.height}px` : 'auto')};
   position: relative;
   padding-bottom: ${space(3)};

diff --git a/static/app/views/dashboards/widgetCard/chart.tsx b/static/app/views/dashboards/widgetCard/chart.tsx
@@ -21,7 +21,6 @@ import {getSeriesSelection, isChartHovered} from 'sentry/components/charts/utils
 import LoadingIndicator from 'sentry/components/loadingIndicator';
 import type {PlaceholderProps} from 'sentry/components/placeholder';
 import Placeholder from 'sentry/components/placeholder';
-import {Tooltip} from 'sentry/components/tooltip';
 import {IconWarning} from 'sentry/icons';
 import {space} from 'sentry/styles/space';
 import type {PageFilters} from 'sentry/types/core';
@@ -39,7 +38,7 @@ import {
   getDurationUnit,
   tooltipFormatter,
 } from 'sentry/utils/discover/charts';
-import {getFieldFormatter} from 'sentry/utils/discover/fieldRenderers';
+import type {EventsMetaType} from 'sentry/utils/discover/eventView';
 import type {AggregationOutputType} from 'sentry/utils/discover/fields';
 import {
   aggregateOutputType,
@@ -53,14 +52,14 @@ import {
 } from 'sentry/utils/discover/fields';
 import getDynamicText from 'sentry/utils/getDynamicText';
 import {eventViewFromWidget} from 'sentry/views/dashboards/utils';
-import {AutoSizedText} from 'sentry/views/dashboards/widgetCard/autoSizedText';
 import WidgetLegendNameEncoderDecoder from 'sentry/views/dashboards/widgetLegendNameEncoderDecoder';
 
 import {getFormatter} from '../../../components/charts/components/tooltip';
 import {getDatasetConfig} from '../datasetConfig/base';
 import type {Widget} from '../types';
 import {DisplayType} from '../types';
 import type WidgetLegendSelectionState from '../widgetLegendSelectionState';
+import {BigNumberWidgetVisualization} from '../widgets/bigNumberWidget/bigNumberWidgetVisualization';
 
 import type {GenericWidgetQueriesChildrenProps} from './genericWidgetQueries';
 
@@ -203,9 +202,9 @@ class WidgetCardChart extends Component<WidgetCardChartProps> {
       return <BigNumber>{'\u2014'}</BigNumber>;
     }
 
-    const {location, organization, widget, isMobile, expandNumbers} = this.props;
+    const {widget} = this.props;
 
-    return tableResults.map(result => {
+    return tableResults.map((result, i) => {
       const tableMeta = {...result.meta};
       const fields = Object.keys(tableMeta);
 
@@ -220,47 +219,31 @@ class WidgetCardChart extends Component<WidgetCardChartProps> {
         }
       }
 
-      // Change tableMeta for the field from integer to string since we will be rendering with toLocaleString
-      const shouldExpandInteger = !!expandNumbers && tableMeta[field] === 'integer';
-      if (shouldExpandInteger) {
-        tableMeta[field] = 'string';
-      }
+      const data = result?.data;
+      const meta = result?.meta as EventsMetaType;
+      const value = data?.[0]?.[selectedField];
 
       if (
         !field ||
         !result.data?.length ||
         selectedField === 'equation|' ||
-        selectedField === ''
+        selectedField === '' ||
+        !defined(value) ||
+        !Number.isFinite(value) ||
+        Number.isNaN(value)
       ) {
         return <BigNumber key={`big_number:${result.title}`}>{'\u2014'}</BigNumber>;
       }
 
-      const dataRow = result.data[0];
-      const fieldRenderer = getFieldFormatter(field, tableMeta, false);
-
-      const unit = tableMeta.units?.[field];
-      const rendered = fieldRenderer(
-        shouldExpandInteger ? {[field]: dataRow[field].toLocaleString()} : dataRow,
-        {location, organization, unit}
-      );
-
-      const isModalWidget = !(widget.id || widget.tempId);
-      if (isModalWidget || isMobile) {
-        return <BigNumber key={`big_number:${result.title}`}>{rendered}</BigNumber>;
-      }
-
-      return expandNumbers ? (
-        <BigText>{rendered}</BigText>
-      ) : (
-        <AutoResizeParent key={`big_number:${result.title}`}>
-          <AutoSizedText>
-            <NumberContainerOverride>
-              <Tooltip title={rendered} showOnlyOnOverflow>
-                {rendered}
-              </Tooltip>
-            </NumberContainerOverride>
-          </AutoSizedText>
-        </AutoResizeParent>
+      return (
+        <BigNumberWidgetVisualization
+          key={i}
+          field={field}
+          value={value}
+          meta={meta}
+          thresholds={widget.thresholds ?? undefined}
+          preferredPolarity="-"
+        />
       );
     });
   }
@@ -673,46 +656,6 @@ const BigNumber = styled('div')`
   }
 `;
 
-const AutoResizeParent = styled('div')`
-  position: absolute;
-  color: ${p => p.theme.headingColor};
-  inset: 0;
-
-  * {
-    line-height: 1;
-    text-align: left !important;
-  }
-`;
-
-const BigText = styled('div')`
-  display: block;
-  width: 100%;
-  color: ${p => p.theme.headingColor};
-  font-size: max(min(8vw, 90px), 30px);
-  padding: ${space(1)} ${space(3)} 0 ${space(3)};
-  white-space: nowrap;
-
-  * {
-    text-align: left !important;
-  }
-`;
-
-/**
- * This component overrides the default behavior of `NumberContainer`,
- * which wraps every single number in big widgets. This override forces
- * `NumberContainer` to never truncate its values, which makes it possible
- * to auto-size them.
- */
-const NumberContainerOverride = styled('div')`
-  display: inline-block;
-
-  * {
-    text-overflow: clip !important;
-    display: inline;
-    white-space: nowrap;
-  }
-`;
-
 const ChartWrapper = styled('div')<{autoHeightResize: boolean; noPadding?: boolean}>`
   ${p => p.autoHeightResize && 'height: 100%;'}
   padding: ${p => (p.noPadding ? `0` : `0 ${space(3)} ${space(3)}`)};

diff --git a/static/app/views/dashboards/widgetCard/index.tsx b/static/app/views/dashboards/widgetCard/index.tsx
@@ -47,7 +47,6 @@ import {Toolbar} from 'sentry/views/dashboards/widgetCard/toolbar';
 
 import type {DashboardFilters, Widget} from '../types';
 import {DisplayType, OnDemandExtractionState, WidgetType} from '../types';
-import {getColoredWidgetIndicator, hasThresholdMaxValue} from '../utils';
 import {DEFAULT_RESULTS_LIMIT} from '../widgetBuilder/utils';
 import type WidgetLegendSelectionState from '../widgetLegendSelectionState';
 
@@ -234,10 +233,6 @@ function WidgetCard(props: Props) {
                     >
                       <WidgetTitle>{widget.title}</WidgetTitle>
                     </Tooltip>
-                    {widget.thresholds &&
-                      hasThresholdMaxValue(widget.thresholds) &&
-                      data?.tableResults &&
-                      getColoredWidgetIndicator(widget.thresholds, data?.tableResults)}
                     <ExtractedMetricsTag queryKey={widget} />
                     <DisplayOnDemandWarnings widget={widget} />
                     <DiscoverSplitAlert widget={widget} />

diff --git a/static/app/views/dashboards/widgets/bigNumberWidget/bigNumberWidgetVisualization.tsx b/static/app/views/dashboards/widgets/bigNumberWidget/bigNumberWidgetVisualization.tsx
@@ -79,15 +79,22 @@ export function BigNumberWidgetVisualization(props: BigNumberWidgetVisualization
   return (
     <Wrapper>
       <NumberAndDifferenceContainer>
-        {props.thresholds && (
-          <ThresholdsIndicator
-            preferredPolarity={props.preferredPolarity}
-            thresholds={props.thresholds}
-            unit={unit ?? ''}
-            value={clampedValue}
-            type={type ?? 'integer'}
-          />
-        )}
+        {defined(props.thresholds?.max_values.max1) &&
+          defined(props.thresholds?.max_values.max2) && (
+            <ThresholdsIndicator
+              preferredPolarity={props.preferredPolarity}
+              thresholds={{
+                unit: props.thresholds.unit ?? undefined,
+                max_values: {
+                  max1: props.thresholds.max_values.max1,
+                  max2: props.thresholds.max_values.max2,
+                },
+              }}
+              unit={unit ?? ''}
+              value={clampedValue}
+              type={type ?? 'integer'}
+            />
+          )}
 
         <NumberContainerOverride>
           <Tooltip

diff --git a/static/app/views/dashboards/widgets/bigNumberWidget/thresholdsIndicator.tsx b/static/app/views/dashboards/widgets/bigNumberWidget/thresholdsIndicator.tsx
@@ -5,10 +5,18 @@ import type {Polarity} from 'sentry/components/percentChange';
 
 import {normalizeUnit} from '../../utils';
 import {ThresholdsHoverWrapper} from '../../widgetBuilder/buildSteps/thresholdsStep/thresholdsHoverWrapper';
-import type {Thresholds} from '../common/types';
+import type {ThresholdsConfig} from '../../widgetBuilder/buildSteps/thresholdsStep/thresholdsStep';
+
+type ValidThresholds = {
+  max_values: {
+    max1: number;
+    max2: number;
+  };
+  unit?: string;
+};
 
 interface ThresholdsIndicatorProps {
-  thresholds: Thresholds;
+  thresholds: ValidThresholds;
   type: string;
   unit: string;
   value: number;
@@ -28,8 +36,8 @@ export function ThresholdsIndicator({
   const {max1, max2} = max_values;
 
   const normalizedValue = normalizeUnit(value, valueUnit, type);
-  const normalizedMax1 = normalizeUnit(max1, thresholdUnit, type);
-  const normalizedMax2 = normalizeUnit(max2, thresholdUnit, type);
+  const normalizedMax1 = thresholdUnit ? normalizeUnit(max1, thresholdUnit, type) : max1;
+  const normalizedMax2 = thresholdUnit ? normalizeUnit(max2, thresholdUnit, type) : max2;
 
   const state = getThresholdState(
     normalizedValue,
@@ -40,8 +48,16 @@ export function ThresholdsIndicator({
 
   const colorName = COLOR_NAME_FOR_STATE[state];
 
+  const thresholdsConfig: ThresholdsConfig = {
+    unit: thresholdUnit ?? null,
+    max_values: {
+      max1: max1 ?? null,
+      max2: max2 ?? null,
+    },
+  };
+
   return (
-    <ThresholdsHoverWrapper thresholds={thresholds} type={type}>
+    <ThresholdsHoverWrapper thresholds={thresholdsConfig} type={type}>
       <Circle role="status" aria-label={state} color={theme[colorName]} />
     </ThresholdsHoverWrapper>
   );

diff --git a/static/app/views/dashboards/widgets/common/types.tsx b/static/app/views/dashboards/widgets/common/types.tsx
@@ -1,3 +1,5 @@
+import type {ThresholdsConfig} from '../../widgetBuilder/buildSteps/thresholdsStep/thresholdsStep';
+
 export type Meta = {
   fields: Record<string, string>;
   units?: Record<string, string | null>;
@@ -19,13 +21,4 @@ export interface StateProps {
   onRetry?: () => void;
 }
 
-export type MaxValues = {
-  max1: number;
-  max2: number;
-};
-
-// `max_values` is Snake Case to preserve compatibility with the current widget serializer. We _do_ want to change it to Camel Case!
-export interface Thresholds {
-  max_values: MaxValues;
-  unit: string;
-}
+export type Thresholds = ThresholdsConfig;