Adds tests

apache · Apr 30, 2024 · 3c3c3b1 · 3c3c3b1
1 parent ae46232
commit 3c3c3b1
Show file tree

Hide file tree

Showing 19 changed files with 146 additions and 138 deletions.
diff --git a/superset-frontend/packages/superset-ui-core/src/utils/tooltip.ts b/superset-frontend/packages/superset-ui-core/src/utils/tooltip.ts
@@ -24,11 +24,8 @@ const TRUNCATION_STYLE = `
   text-overflow: ellipsis;
 `;
 
-// TODO Check if there's any column alignment different than ['left', 'right', 'right']
-// TODO: Remove colored border from all chart types?
 export function tooltipHtml(
   data: string[][],
-  columnAlignments: ('left' | 'right' | 'middle')[],
   title?: string,
   focusedRow?: number,
 ) {
@@ -43,12 +40,11 @@ export function tooltipHtml(
           ${data
             .map((row, i) => {
               const rowStyle =
-                i === focusedRow ? 'font-weight: 700;' : 'opacity: 0.7;';
+                i === focusedRow ? 'font-weight: 700;' : 'opacity: 0.8;';
               let padding = 0;
               const cells = row.map((cell, j) => {
-                // TODO: Maybe make the font a little darker
                 const cellStyle = `
-                  text-align: ${columnAlignments[j]};
+                  text-align: ${j > 0 ? 'right' : 'left'};
                   padding-left: ${padding}px;
                   ${TRUNCATION_STYLE}
                 `;

diff --git a/superset-frontend/packages/superset-ui-core/test/utils/tooltip.test.ts b/superset-frontend/packages/superset-ui-core/test/utils/tooltip.test.ts
@@ -0,0 +1,115 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { tooltipHtml } from '@superset-ui/core';
+
+const TITLE_STYLE =
+  'style="font-weight: 700;max-width:300px;overflow:hidden;text-overflow:ellipsis;"';
+const TR_STYLE = 'style="opacity:0.8;"';
+const TR_FOCUS_STYLE = 'style="font-weight:700;"';
+const TD_TEXT_STYLE =
+  'style="text-align:left;padding-left:0px;max-width:300px;overflow:hidden;text-overflow:ellipsis;"';
+const TD_NUMBER_STYLE =
+  'style="text-align:right;padding-left:16px;max-width:300px;overflow:hidden;text-overflow:ellipsis;"';
+
+const data = [
+  ['a', 'b', 'c'],
+  ['1', '2', '3'],
+];
+
+function removeWhitespaces(text: string) {
+  return text.replace(/\s/g, '');
+}
+
+test('should return a table with the given data', () => {
+  const title = 'Title';
+  const html = removeWhitespaces(tooltipHtml(data, title));
+  const expectedHtml = removeWhitespaces(`
+    <div>
+        <span ${TITLE_STYLE}>Title</span>
+        <table>
+            <tr ${TR_STYLE}>
+                <td ${TD_TEXT_STYLE}>a</td>
+                <td ${TD_NUMBER_STYLE}>b</td>
+                <td ${TD_NUMBER_STYLE}>c</td>
+            </tr>
+            <tr ${TR_STYLE}>
+                <td ${TD_TEXT_STYLE}>1</td>
+                <td ${TD_NUMBER_STYLE}>2</td>
+                <td ${TD_NUMBER_STYLE}>3</td>
+            </tr>
+        </table>
+    </div>`);
+  expect(html).toMatch(expectedHtml);
+});
+
+test('should return a table with the given data and a focused row', () => {
+  const title = 'Title';
+  const focusedRow = 1;
+  const html = removeWhitespaces(tooltipHtml(data, title, focusedRow));
+  const expectedHtml = removeWhitespaces(`
+    <div>
+        <span ${TITLE_STYLE}>Title</span>
+        <table>
+            <tr ${TR_STYLE}>
+                <td ${TD_TEXT_STYLE}>a</td>
+                <td ${TD_NUMBER_STYLE}>b</td>
+                <td ${TD_NUMBER_STYLE}>c</td>
+            </tr>
+            <tr ${TR_FOCUS_STYLE}>
+                <td ${TD_TEXT_STYLE}>1</td>
+                <td ${TD_NUMBER_STYLE}>2</td>
+                <td ${TD_NUMBER_STYLE}>3</td>
+            </tr>
+        </table>
+    </div>`);
+  expect(html).toMatch(expectedHtml);
+});
+
+test('should return a table with no data', () => {
+  const title = 'Title';
+  const html = removeWhitespaces(tooltipHtml([], title));
+  const expectedHtml = removeWhitespaces(`
+    <div>
+        <span ${TITLE_STYLE}>Title</span>
+        <table>
+            <tr><td>No data</td></tr>
+        </table>
+    </div>`);
+  expect(html).toMatch(expectedHtml);
+});
+
+test('should return a table with the given data and no title', () => {
+  const html = removeWhitespaces(tooltipHtml(data));
+  const expectedHtml = removeWhitespaces(`
+    <div>
+        <table>
+            <tr ${TR_STYLE}>
+                <td ${TD_TEXT_STYLE}>a</td>
+                <td ${TD_NUMBER_STYLE}>b</td>
+                <td ${TD_NUMBER_STYLE}>c</td>
+            </tr>
+            <tr ${TR_STYLE}>
+                <td ${TD_TEXT_STYLE}>1</td>
+                <td ${TD_NUMBER_STYLE}>2</td>
+                <td ${TD_NUMBER_STYLE}>3</td>
+            </tr>
+        </table>
+    </div>`);
+  expect(html).toMatch(expectedHtml);
+});
diff --git a/...ntend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/transformProps.ts b/...ntend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/transformProps.ts
@@ -239,7 +239,6 @@ export default function transformProps(
                     : headerFormatter.format(params[0].data[1]),
                 ],
               ],
-              ['left', 'right'],
               formatTime(params[0].data[0]),
             ),
         },

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Bubble/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Bubble/transformProps.ts
@@ -70,7 +70,6 @@ export function formatTooltip(
       [yAxisLabel, yAxisFormatter(params.data[1])],
       [sizeLabel, tooltipSizeFormatter(params.data[2])],
     ],
-    ['left', 'right'],
     title,
   );
 }

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Funnel/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Funnel/transformProps.ts
@@ -283,7 +283,7 @@ export default function transformProps(
         if (enumName.includes('Percent')) {
           row.push(formattedPercent);
         }
-        return tooltipHtml([row], ['left', 'right', 'right'], title);
+        return tooltipHtml([row], title);
       },
     },
     legend: {

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Gauge/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Gauge/transformProps.ts
@@ -288,11 +288,7 @@ export default function transformProps(
     ...getDefaultTooltip(refs),
     formatter: (params: CallbackDataParams) => {
       const { name, value } = params;
-      return tooltipHtml(
-        [[metricLabel, formatValue(value as number)]],
-        ['left', 'right'],
-        name,
-      );
+      return tooltipHtml([[metricLabel, formatValue(value as number)]], name);
     },
   };
 

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Graph/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Graph/transformProps.ts
@@ -317,11 +317,7 @@ export default function transformProps(
           getKeyByValue(nodes, Number(params.data.target)),
         );
         const title = `${source} > ${target}`;
-        return tooltipHtml(
-          [[metricLabel, `${params.value}`]],
-          ['left', 'right'],
-          title,
-        );
+        return tooltipHtml([[metricLabel, `${params.value}`]], title);
       },
     },
     legend: {

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Heatmap/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Heatmap/transformProps.ts
@@ -195,7 +195,7 @@ export default function transformProps(
         if (showPercentage) {
           row.push(`${percentFormatter(percentage)} (${suffix})`);
         }
-        return tooltipHtml([row], ['left', 'right', 'right'], title);
+        return tooltipHtml([row], title);
       },
     },
     visualMap: {

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
@@ -648,7 +648,6 @@ export default function transformProps(
         }
         return tooltipHtml(
           rows,
-          ['left', 'right', 'right'],
           tooltipFormatter(xValue),
           keys.findIndex(key => key === focusedSeries),
         );

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts
@@ -326,7 +326,6 @@ export default function transformProps(
         });
         return tooltipHtml(
           [[metricLabel, formattedValue, formattedPercent]],
-          ['left', 'right', 'right'],
           name,
         );
       },

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/transformProps.ts
@@ -152,7 +152,7 @@ export function formatTooltip({
       compareValuePercentage,
     ]);
   }
-  return tooltipHtml(rows, ['left', 'right'], title);
+  return tooltipHtml(rows, title);
 }
 
 export default function transformProps(

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -579,7 +579,6 @@ export default function transformProps(
         }
         return tooltipHtml(
           rows,
-          ['left', 'right', 'right'],
           tooltipFormatter(xValue),
           keys.findIndex(key => key === focusedSeries),
         );

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Tree/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Tree/transformProps.ts
@@ -49,7 +49,7 @@ export function formatTooltip({
     .map(pathInfo => pathInfo?.name || '')
     .filter(path => path !== '');
   const row = value ? [metricLabel, String(value)] : [];
-  return tooltipHtml([row], ['left', 'right'], treePath.join(' ▸ '));
+  return tooltipHtml([row], treePath.join(' ▸ '));
 }
 
 export default function transformProps(

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/transformProps.ts
@@ -101,7 +101,7 @@ export function formatTooltip({
   if (formattedPercent) {
     row.push(formattedPercent);
   }
-  return tooltipHtml([row], ['left', 'right', 'right'], treePath.join(' ▸ '));
+  return tooltipHtml([row], treePath.join(' ▸ '));
 }
 
 export default function transformProps(

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Waterfall/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Waterfall/transformProps.ts
@@ -80,7 +80,7 @@ function formatTooltip({
     ]);
   }
   rows.push([TOTAL_MARK, defaultFormatter(series.data.totalSum)]);
-  return tooltipHtml(rows, ['left', 'right'], title);
+  return tooltipHtml(rows, title);
 }
 
 function transformer({

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts
@@ -17,12 +17,7 @@
  * under the License.
  */
 import { isNumber } from 'lodash';
-import {
-  DataRecord,
-  DataRecordValue,
-  DTTM_ALIAS,
-  ValueFormatter,
-} from '@superset-ui/core';
+import { DataRecord, DTTM_ALIAS, ValueFormatter } from '@superset-ui/core';
 import { OptionName } from 'echarts/types/src/util/types';
 import { TooltipMarker } from 'echarts/types/src/util/format';
 import {

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/tooltip.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/tooltip.ts
@@ -24,6 +24,7 @@ import { Refs } from '../types';
 export function getDefaultTooltip(refs: Refs) {
   return {
     appendToBody: true,
+    borderColor: 'transparent',
     position: (
       canvasMousePos: [number, number],
       params: CallbackDataParams,

diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Funnel/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Funnel/transformProps.test.ts
@@ -21,12 +21,9 @@ import {
   getNumberFormatter,
   supersetTheme,
 } from '@superset-ui/core';
-import transformProps, {
-  formatFunnelLabel,
-} from '../../src/Funnel/transformProps';
+import transformProps, { parseParams } from '../../src/Funnel/transformProps';
 import {
   EchartsFunnelChartProps,
-  EchartsFunnelLabelTypeType,
   PercentCalcType,
 } from '../../src/Funnel/types';
 
@@ -89,85 +86,40 @@ describe('formatFunnelLabel', () => {
       data: { firstStepPercent: 0.5, prevStepPercent: 0.85 },
     };
     expect(
-      formatFunnelLabel({
+      parseParams({
         params,
         numberFormatter,
-        labelType: EchartsFunnelLabelTypeType.Key,
         percentCalculationType: PercentCalcType.Total,
       }),
-    ).toEqual('My Label');
+    ).toEqual(['My Label', '1.23k', '12.34%']);
     expect(
-      formatFunnelLabel({
+      parseParams({
         params,
         numberFormatter,
-        labelType: EchartsFunnelLabelTypeType.Value,
-        percentCalculationType: PercentCalcType.Total,
-      }),
-    ).toEqual('1.23k');
-    expect(
-      formatFunnelLabel({
-        params,
-        numberFormatter,
-        labelType: EchartsFunnelLabelTypeType.Percent,
-        percentCalculationType: PercentCalcType.Total,
-      }),
-    ).toEqual('12.34%');
-    expect(
-      formatFunnelLabel({
-        params,
-        numberFormatter,
-        labelType: EchartsFunnelLabelTypeType.Percent,
         percentCalculationType: PercentCalcType.FirstStep,
       }),
-    ).toEqual('50.00%');
+    ).toEqual(['My Label', '1.23k', '50.00%']);
     expect(
-      formatFunnelLabel({
+      parseParams({
         params,
         numberFormatter,
-        labelType: EchartsFunnelLabelTypeType.Percent,
         percentCalculationType: PercentCalcType.PreviousStep,
       }),
-    ).toEqual('85.00%');
-    expect(
-      formatFunnelLabel({
-        params,
-        numberFormatter,
-        labelType: EchartsFunnelLabelTypeType.KeyValue,
-        percentCalculationType: PercentCalcType.Total,
-      }),
-    ).toEqual('My Label: 1.23k');
-    expect(
-      formatFunnelLabel({
-        params,
-        numberFormatter,
-        labelType: EchartsFunnelLabelTypeType.KeyPercent,
-        percentCalculationType: PercentCalcType.Total,
-      }),
-    ).toEqual('My Label: 12.34%');
-    expect(
-      formatFunnelLabel({
-        params,
-        numberFormatter,
-        labelType: EchartsFunnelLabelTypeType.KeyValuePercent,
-        percentCalculationType: PercentCalcType.Total,
-      }),
-    ).toEqual('My Label: 1.23k (12.34%)');
+    ).toEqual(['My Label', '1.23k', '85.00%']);
     expect(
-      formatFunnelLabel({
+      parseParams({
         params: { ...params, name: '<NULL>' },
         numberFormatter,
-        labelType: EchartsFunnelLabelTypeType.Key,
         percentCalculationType: PercentCalcType.Total,
       }),
-    ).toEqual('<NULL>');
+    ).toEqual(['<NULL>', '1.23k', '12.34%']);
     expect(
-      formatFunnelLabel({
+      parseParams({
         params: { ...params, name: '<NULL>' },
         numberFormatter,
-        labelType: EchartsFunnelLabelTypeType.Key,
         percentCalculationType: PercentCalcType.Total,
         sanitizeName: true,
       }),
-    ).toEqual('&lt;NULL&gt;');
+    ).toEqual(['&lt;NULL&gt;', '1.23k', '12.34%']);
   });
 });