Adds tests

apache · Apr 30, 2024 · 10cfb22 · 10cfb22
1 parent ae46232
commit 10cfb22
Show file tree

Hide file tree

Showing 22 changed files with 192 additions and 205 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 {
@@ -112,8 +107,9 @@ export const formatForecastTooltipSeries = ({
       value += `ŷ = ${formatter(forecastTrend)}`;
     }
     if (forecastLower && forecastUpper) {
+      if (value) value += ' ';
       // the lower bound needs to be added to the upper bound
-      value += ` (${formatter(forecastLower)}, ${formatter(
+      value += `(${formatter(forecastLower)}, ${formatter(
         forecastLower + forecastUpper,
       )})`;
     }

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/Bubble/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Bubble/transformProps.test.ts
@@ -160,42 +160,44 @@ describe('Bubble formatTooltip', () => {
       data: [10000, 20000, 3, 'bubble title', 'bubble dimension'],
     };
 
-    expect(
-      formatTooltip(
-        params,
-        'x-axis-label',
-        'y-axis-label',
-        'size-label',
-        dollerFormatter,
-        dollerFormatter,
-        percentFormatter,
-      ),
-    ).toEqual(
-      `<p>bubble title </br> bubble dimension</p>
-        x-axis-label: $10,000.00 <br/>
-        y-axis-label: $20,000.00 <br/>
-        size-label: 300.0%`,
+    const html = formatTooltip(
+      params,
+      'x-axis-label',
+      'y-axis-label',
+      'size-label',
+      dollerFormatter,
+      dollerFormatter,
+      percentFormatter,
     );
+    expect(html).toContain('bubble title');
+    expect(html).toContain('bubble dimension');
+    expect(html).toContain('x-axis-label');
+    expect(html).toContain('y-axis-label');
+    expect(html).toContain('size-label');
+    expect(html).toContain('$10,000.00');
+    expect(html).toContain('$20,000.00');
+    expect(html).toContain('300.0%');
   });
   it('Should generate correct bubble label content without dimension', () => {
     const params = {
       data: [10000, 25000, 3, 'bubble title', null],
     };
-    expect(
-      formatTooltip(
-        params,
-        'x-axis-label',
-        'y-axis-label',
-        'size-label',
-        dollerFormatter,
-        dollerFormatter,
-        percentFormatter,
-      ),
-    ).toEqual(
-      `<p>bubble title</p>
-        x-axis-label: $10,000.00 <br/>
-        y-axis-label: $25,000.00 <br/>
-        size-label: 300.0%`,
+    const html = formatTooltip(
+      params,
+      'x-axis-label',
+      'y-axis-label',
+      'size-label',
+      dollerFormatter,
+      dollerFormatter,
+      percentFormatter,
     );
+    expect(html).toContain('bubble title');
+    expect(html).not.toContain('bubble dimension');
+    expect(html).toContain('x-axis-label');
+    expect(html).toContain('y-axis-label');
+    expect(html).toContain('size-label');
+    expect(html).toContain('$10,000.00');
+    expect(html).toContain('$25,000.00');
+    expect(html).toContain('300.0%');
   });
 });