Fix #25637: SmartScalars should compactify like Scalars (#30337)

* copy compact logic from scalar

* re-organize compactify logic

* move duplicated logic into utils file

* move scalarUtils

* convert scalar_utils to typescript

* add smartscalar unit  tests for compactify

* add unit tests for compactifyValue

* refactor optional parameter

* default to compact = false for fullScalarValue

* refactor scalar_utils unit tests

frontend/src/metabase/visualizations/lib/scalar_utils.ts

+import { formatValue } from "metabase/lib/formatting";
+import { OptionsType } from "metabase/lib/formatting/types";
+
+// used below to determine whether we show compact formatting
+export const COMPACT_MAX_WIDTH = 250;
+export const COMPACT_WIDTH_PER_DIGIT = 25;
+export const COMPACT_MIN_LENGTH = 6;
+
+export function compactifyValue(
+  value: number,
+  width: number,
+  formatOptions: OptionsType = {},
+) {
+  const fullScalarValue = formatValue(value, {
+    ...formatOptions,
+    compact: false,
+  });
+  const compactScalarValue = formatValue(value, {
+    ...formatOptions,
+    compact: true,
+  });
+
+  // use the compact version of formatting if the component is narrower than
+  // the cutoff and the formatted value is longer than the cutoff
+  // also if the width is less than a certain multiplier of the number of digits
+  const displayCompact =
+    fullScalarValue !== null &&
+    typeof fullScalarValue === "string" &&
+    fullScalarValue.length > COMPACT_MIN_LENGTH &&
+    (width < COMPACT_MAX_WIDTH ||
+      width < COMPACT_WIDTH_PER_DIGIT * fullScalarValue.length);
+  const displayValue = displayCompact ? compactScalarValue : fullScalarValue;
+
+  return { displayValue, fullScalarValue };
+}

frontend/src/metabase/visualizations/lib/scalar_utils.unit.spec.ts

+import { TYPE } from "metabase-lib/types/constants";
+
+import {
+  compactifyValue,
+  COMPACT_WIDTH_PER_DIGIT,
+  COMPACT_MAX_WIDTH,
+  COMPACT_MIN_LENGTH,
+} from "./scalar_utils";
+
+describe("scalar utils", () => {
+  describe("compactifyValue", () => {
+    const formatOptions = {
+      column: {
+        base_type: TYPE.Number,
+        semantic_type: TYPE.Number,
+      },
+    };
+
+    it("displayValue is fullScalarValue when fullScalarValue.length <= COMPACT_MIN_LENGTH", () => {
+      const value = 45000;
+      const width = 200;
+
+      const { displayValue, fullScalarValue } = compactifyValue(
+        value,
+        width,
+        formatOptions,
+      ) as { displayValue: string; fullScalarValue: string };
+
+      expect(fullScalarValue.length).toBeLessThanOrEqual(COMPACT_MIN_LENGTH);
+
+      expect(displayValue).toBe(fullScalarValue);
+      expect(fullScalarValue).toBe("45,000");
+    });
+
+    it("displayValue is compact when fullScalarValue.length > COMPACT_MIN_LENGTH and width < COMPACT_MAX_WIDTH", () => {
+      const value = 45000.1343;
+      const width = 200;
+
+      const { displayValue, fullScalarValue } = compactifyValue(
+        value,
+        width,
+        formatOptions,
+      ) as { displayValue: string; fullScalarValue: string };
+
+      expect(fullScalarValue.length).toBeGreaterThan(COMPACT_MIN_LENGTH);
+      expect(width).toBeLessThan(COMPACT_MAX_WIDTH);
+
+      expect(displayValue).not.toBe(fullScalarValue);
+      expect(displayValue).toBe("45.0k");
+    });
+
+    it("displayValue is compact when fullScalarValue.length > COMPACT_MIN_LENGTH & width >= COMPACT_MAX_WIDTH & width < COMPACT_WIDTH_PER_DIGIT * fullScalarValue.length", () => {
+      const value = 100100100100;
+      const width = 350;
+
+      const { displayValue, fullScalarValue } = compactifyValue(
+        value,
+        width,
+        formatOptions,
+      ) as { displayValue: string; fullScalarValue: string };
+
+      expect(fullScalarValue.length).toBeGreaterThan(COMPACT_MIN_LENGTH);
+      expect(width).toBeGreaterThanOrEqual(COMPACT_MAX_WIDTH);
+      expect(width).toBeLessThan(
+        fullScalarValue.length * COMPACT_WIDTH_PER_DIGIT,
+      );
+
+      expect(displayValue).not.toBe(fullScalarValue);
+      expect(displayValue).toBe("100.1B");
+    });
+
+    it("displayValue is not compact when fullScalarValue.length > COMPACT_MIN_LENGTH & width >= COMPACT_MAX_WIDTH & width >= COMPACT_WIDTH_PER_DIGIT * fullScalarValue.length", () => {
+      const value = 10010010010;
+      const width = 350;
+
+      const { displayValue, fullScalarValue } = compactifyValue(
+        value,
+        width,
+        formatOptions,
+      ) as { displayValue: string; fullScalarValue: string };
+
+      expect(fullScalarValue.length).toBeGreaterThan(COMPACT_MIN_LENGTH);
+      expect(width).toBeGreaterThanOrEqual(COMPACT_MAX_WIDTH);
+      expect(width).toBeGreaterThanOrEqual(
+        fullScalarValue.length * COMPACT_WIDTH_PER_DIGIT,
+      );
+
+      expect(displayValue).toBe(fullScalarValue);
+      expect(displayValue).toBe("10,010,010,010");
+    });
+  });
+});

frontend/src/metabase/visualizations/visualizations/Scalar.jsx

@@ -3,10 +3,10 @@ import React, { Component } from "react";
 import { t } from "ttag";
 
 import _ from "underscore";
-import { formatValue } from "metabase/lib/formatting";
 
 import { fieldSetting } from "metabase/visualizations/lib/settings/utils";
 import { columnSettings } from "metabase/visualizations/lib/settings/column";
+import { compactifyValue } from "metabase/visualizations/lib/scalar_utils";
 
 import ScalarValue, {
   ScalarWrapper,
@@ -25,11 +25,6 @@ function legacyScalarSettingsToFormatOptions(settings) {
     .value();
 }
 
-// used below to determine whether we show compact formatting
-const COMPACT_MAX_WIDTH = 250;
-const COMPACT_WIDTH_PER_DIGIT = 25;
-const COMPACT_MIN_LENGTH = 6;
-
 // Scalar visualization shows a single number
 // Multiseries Scalar is transformed to a Funnel
 export default class Scalar extends Component {
@@ -191,21 +186,11 @@ export default class Scalar extends Component {
       jsx: true,
     };
 
-    const fullScalarValue = formatValue(value, formatOptions);
-    const compactScalarValue = formatValue(value, {
-      ...formatOptions,
-      compact: true,
-    });
-
-    // use the compact version of formatting if the component is narrower than
-    // the cutoff and the formatted value is longer than the cutoff
-    // also if the width is less than a certain multiplier of the number of digits
-    const displayCompact =
-      fullScalarValue !== null &&
-      fullScalarValue.length > COMPACT_MIN_LENGTH &&
-      (width < COMPACT_MAX_WIDTH ||
-        width < COMPACT_WIDTH_PER_DIGIT * fullScalarValue.length);
-    const displayValue = displayCompact ? compactScalarValue : fullScalarValue;
+    const { displayValue, fullScalarValue } = compactifyValue(
+      value,
+      width,
+      formatOptions,
+    );
 
     const clicked = {
       value,

frontend/src/metabase/visualizations/visualizations/SmartScalar.jsx

@@ -10,6 +10,7 @@ import Icon from "metabase/components/Icon";
 
 import { columnSettings } from "metabase/visualizations/lib/settings/column";
 import { NoBreakoutError } from "metabase/visualizations/lib/errors";
+import { compactifyValue } from "metabase/visualizations/lib/scalar_utils";
 
 import ScalarValue, {
   ScalarWrapper,
@@ -17,6 +18,7 @@ import ScalarValue, {
 } from "metabase/visualizations/components/ScalarValue";
 import * as Lib from "metabase-lib";
 import { isDate } from "metabase-lib/types/utils/isa";
+import { ScalarContainer } from "./Scalar.styled";
 
 import {
   PreviousValueContainer,
@@ -116,6 +118,15 @@ export default class Smart extends React.Component {
       return null;
     }
 
+    const lastValue = insight["last-value"];
+    const formatOptions = settings.column(column);
+
+    const { displayValue, fullScalarValue } = compactifyValue(
+      lastValue,
+      width,
+      formatOptions,
+    );
+
     const granularity = Lib.describeTemporalUnit(insight["unit"]).toLowerCase();
 
     const lastChange = insight["last-change"];
@@ -164,23 +175,30 @@ export default class Smart extends React.Component {
         <div className="Card-title absolute top right p1 px2">
           {actionButtons}
         </div>
-        <span
-          onClick={
-            isClickable &&
-            (() =>
-              this._scalar &&
-              onVisualizationClick({ ...clicked, element: this._scalar }))
-          }
-          ref={scalar => (this._scalar = scalar)}
+        <ScalarContainer
+          className="fullscreen-normal-text fullscreen-night-text"
+          tooltip={fullScalarValue}
+          alwaysShowTooltip={fullScalarValue !== displayValue}
+          isClickable={isClickable}
         >
-          <ScalarValue
-            gridSize={gridSize}
-            width={width}
-            totalNumGridCols={totalNumGridCols}
-            fontFamily={fontFamily}
-            value={formatValue(insight["last-value"], settings.column(column))}
-          />
-        </span>
+          <span
+            onClick={
+              isClickable &&
+              (() =>
+                this._scalar &&
+                onVisualizationClick({ ...clicked, element: this._scalar }))
+            }
+            ref={scalar => (this._scalar = scalar)}
+          >
+            <ScalarValue
+              gridSize={gridSize}
+              width={width}
+              totalNumGridCols={totalNumGridCols}
+              fontFamily={fontFamily}
+              value={displayValue}
+            />
+          </span>
+        </ScalarContainer>
         {isDashboard && (
           <ScalarTitle
             title={settings["card.title"]}

frontend/src/metabase/visualizations/visualizations/SmartScalar.unit.spec.js

@@ -5,8 +5,8 @@ import Visualization from "metabase/visualizations/components/Visualization";
 import { getSettingsWidgetsForSeries } from "metabase/visualizations/lib/settings/visualization";
 import { NumberColumn, DateTimeColumn } from "__support__/visualizations";
 
-const setup = series =>
-  renderWithProviders(<Visualization rawSeries={series} />);
+const setup = (series, width) =>
+  renderWithProviders(<Visualization rawSeries={series} width={width} />);
 
 const series = ({ rows, insights }) => {
   const cols = [
@@ -100,4 +100,46 @@ describe("SmartScalar", () => {
     const data = { cols: [NumberColumn({ name: "Count" })], rows: [[100]] };
     expect(() => getSettingsWidgetsForSeries([{ card, data }])).not.toThrow();
   });
+
+  it("shouldn't render compact if normal formatting is <=6 characters", () => {
+    const width = 200;
+    const rows = [
+      ["2019-10-01T00:00:00", 100],
+      [("2019-11-01T00:00:00", 81005)],
+    ];
+    const insights = [
+      {
+        "last-value": 81005,
+        "last-change": 80,
+        "previous-value": 100,
+        unit: "month",
+        col: "Count",
+      },
+    ];
+
+    setup(series({ rows, insights }), width);
+
+    expect(screen.getByText("81,005")).toBeInTheDocument();
+  });
+
+  it("should render compact if normal formatting is >6 characters and width <250", () => {
+    const width = 200;
+    const rows = [
+      ["2019-10-01T00:00:00", 100],
+      [("2019-11-01T00:00:00", 810750.54)],
+    ];
+    const insights = [
+      {
+        "last-value": 810750.54,
+        "last-change": 80,
+        "previous-value": 100,
+        unit: "month",
+        col: "Count",
+      },
+    ];
+
+    setup(series({ rows, insights }), width);
+
+    expect(screen.getByText("810.8k")).toBeInTheDocument();
+  });
 });