refactor(address code review comments): address code review comments

apache-superset · Aug 13, 2019 · 338d2cd · 338d2cd
1 parent ebc0774
commit 338d2cd
Show file tree

Hide file tree

Showing 6 changed files with 26 additions and 33 deletions.
diff --git a/packages/superset-ui-plugin-chart-table/src/Table.tsx b/packages/superset-ui-plugin-chart-table/src/Table.tsx
@@ -195,6 +195,7 @@ class TableVis extends React.PureComponent<InternalTableProps, TableState> {
         )}
         <DataTable
           data={dataToRender}
+          keys={dataToRender && dataToRender.length > 0 ? Object.keys(dataToRender[0].data) : []}
           columnMetadata={columnMetadata}
           zebra
           rowHeight={heightType}

diff --git a/packages/superset-ui-plugin-chart-table/src/processColumns.ts b/packages/superset-ui-plugin-chart-table/src/processColumns.ts
@@ -70,7 +70,7 @@ export default function processColumns({
 
     // Handle verbose names for percents
     if (!label) {
-      if (key[0] === '%') {
+      if (key.length > 0 && key[0] === '%') {
         const cleanedKey = key.substring(1);
         label = `% ${verboseMap[cleanedKey] || cleanedKey}`;
         formatFunction = formatPercent;

diff --git a/packages/superset-ui-plugin-chart-table/src/processData.ts b/packages/superset-ui-plugin-chart-table/src/processData.ts
@@ -16,32 +16,24 @@ export default function processData({
     timeseriesLimitMetric &&
     ((timeseriesLimitMetric as AdhocMetric).label || (timeseriesLimitMetric as string));
 
-  let processedData: {
-    data: PlainObject;
-  }[] = records.map((row: PlainObject) => ({
-    data: row,
-  }));
+  let processedRecords = records;
 
   if (sortByKey) {
-    processedData = processedData.sort((a, b) => {
-      const delta = a.data[sortByKey] - b.data[sortByKey];
-      if (orderDesc) {
-        return -delta;
-      }
-
-      return delta;
-    });
-    if (metrics.indexOf(sortByKey) < 0) {
-      processedData = processedData.map(row => {
-        const data = { ...row.data };
-        delete data[sortByKey];
-
-        return {
-          data,
-        };
-      });
-    }
+    processedRecords = records
+      .slice()
+      .sort(
+        orderDesc ? (a, b) => b[sortByKey] - a[sortByKey] : (a, b) => a[sortByKey] - b[sortByKey],
+      );
   }
 
-  return processedData;
+  return processedRecords.map(
+    sortByKey && metrics.indexOf(sortByKey) < 0
+      ? row => {
+          const data = { ...row };
+          delete data[sortByKey];
+
+          return { data };
+        }
+      : row => ({ data: row }),
+  );
 }
diff --git a/packages/superset-ui-plugin-chart-table/src/processMetrics.ts b/packages/superset-ui-plugin-chart-table/src/processMetrics.ts
@@ -18,5 +18,5 @@ export default function processMetrics({
 
   return processedMetrics
     .concat(processedPercentMetrics)
-    .filter(m => typeof records[0][m as string] === 'number');
+    .filter(m => typeof records[0][m] === 'number');
 }
diff --git a/packages/superset-ui-plugin-chart-table/src/renderer.tsx b/packages/superset-ui-plugin-chart-table/src/renderer.tsx
@@ -25,6 +25,12 @@ export type Cell = {
   value: any;
 };
 
+const numberStyle: CSSProperties = {
+  marginLeft: 'auto',
+  marginRight: '4px',
+  zIndex: 10,
+};
+
 export const getRenderer = ({
   column,
   alignPositiveNegative,
@@ -83,12 +89,6 @@ export const getRenderer = ({
         width: `${width}%`,
       };
 
-      const numberStyle: CSSProperties = {
-        marginLeft: 'auto',
-        marginRight: '4px',
-        zIndex: 10,
-      };
-
       return (
         <div style={boxStyle}>
           <div style={boxContainerStyle}>

diff --git a/packages/superset-ui-plugins-demo/storybook/stories/plugin-chart-table/Stories.jsx b/packages/superset-ui-plugins-demo/storybook/stories/plugin-chart-table/Stories.jsx
@@ -51,7 +51,7 @@ export default [
           alignPn: false,
           colorPn: true,
           includeSearch: true,
-          metrics: ['sum__num', 'trend'],
+          metrics: ['sum__num'],
           orderDesc: true,
           pageLength: 0,
           percentMetrics: [],