Merge pull request metabase#8552 from metabase/pivot-table-improvements

Pivot table improvements

frontend/src/metabase/components/ColorRangePicker.jsx

@@ -1,3 +1,5 @@
+/* @flow */
+
 import React from "react";
 
 import PopoverWithTrigger from "metabase/components/PopoverWithTrigger";
@@ -7,6 +9,19 @@ import { getColorScale } from "metabase/lib/colors";
 import d3 from "d3";
 import cx from "classnames";
 
+import type { ColorString } from "metabase/lib/colors";
+
+type Props = {
+  value: ColorString[],
+  onChange: (ColorString[]) => void,
+  ranges: ColorString[][],
+  className?: string,
+  style?: { [key: string]: any },
+  sections?: number,
+  quantile?: boolean,
+  columns?: number,
+};
+
 const ColorRangePicker = ({
   value,
   onChange,
@@ -16,7 +31,7 @@ const ColorRangePicker = ({
   sections = 5,
   quantile = false,
   columns = 2,
-}) => (
+}: Props) => (
   <PopoverWithTrigger
     triggerElement={
       <ColorRangePreview
@@ -53,13 +68,20 @@ const ColorRangePicker = ({
   </PopoverWithTrigger>
 );
 
+type ColorRangePreviewProps = {
+  colors: ColorString[],
+  sections?: number,
+  quantile?: boolean,
+  className?: string,
+};
+
 export const ColorRangePreview = ({
   colors = [],
   sections = 5,
   quantile = false,
   className,
   ...props
-}) => {
+}: ColorRangePreviewProps) => {
   const scale = getColorScale([0, sections - 1], colors, quantile);
   return (
     <div className={cx(className, "flex")} {...props}>

frontend/src/metabase/lib/colors.js

@@ -6,9 +6,9 @@ import { Harmonizer } from "color-harmony";
 
 import { deterministicAssign } from "./deterministic";
 
-type ColorName = string;
-type ColorString = string;
-type ColorFamily = { [name: ColorName]: ColorString };
+export type ColorName = string;
+export type ColorString = string;
+export type ColorFamily = { [name: ColorName]: ColorString };
 
 // NOTE: DO NOT ADD COLORS WITHOUT EXTREMELY GOOD REASON AND DESIGN REVIEW
 // NOTE: KEEP SYNCRONIZED WITH COLORS.CSS

frontend/src/metabase/lib/data_grid.js

@@ -1,66 +1,37 @@
-import * as SchemaMetadata from "metabase/lib/schema_metadata";
 import { formatValue } from "metabase/lib/formatting";
 
-function compareNumbers(a, b) {
-  return a - b;
-}
-
-export function pivot(data) {
-  // find the lowest cardinality dimension and make it our "pivoted" column
-  // TODO: we assume dimensions are in the first 2 columns, which is less than ideal
-  let pivotCol = 0,
-    normalCol = 1,
-    cellCol = 2,
-    pivotColValues = distinctValues(data, pivotCol),
-    normalColValues = distinctValues(data, normalCol);
-  if (normalColValues.length <= pivotColValues.length) {
-    pivotCol = 1;
-    normalCol = 0;
-
-    let tmp = pivotColValues;
-    pivotColValues = normalColValues;
-    normalColValues = tmp;
-  }
-
-  // sort the column values sensibly
-  if (SchemaMetadata.isNumeric(data.cols[pivotCol])) {
-    pivotColValues.sort(compareNumbers);
-  } else {
-    pivotColValues.sort();
-  }
-
-  if (SchemaMetadata.isNumeric(data.cols[normalCol])) {
-    normalColValues.sort(compareNumbers);
-  } else {
-    normalColValues.sort();
-  }
+export function pivot(data, normalCol, pivotCol, cellCol) {
+  const { pivotValues, normalValues } = distinctValuesSorted(
+    data.rows,
+    pivotCol,
+    normalCol,
+  );
 
   // make sure that the first element in the pivoted column list is null which makes room for the label of the other column
-  pivotColValues.unshift(data.cols[normalCol].display_name);
+  pivotValues.unshift(data.cols[normalCol].display_name);
 
   // start with an empty grid that we'll fill with the appropriate values
-  const pivotedRows = normalColValues.map((normalColValues, index) => {
-    const row = pivotColValues.map(() => null);
+  const pivotedRows = normalValues.map((normalValues, index) => {
+    const row = pivotValues.map(() => null);
     // for onVisualizationClick:
     row._dimension = {
-      value: normalColValues,
+      value: normalValues,
       column: data.cols[normalCol],
     };
     return row;
   });
 
   // fill it up with the data
   for (let j = 0; j < data.rows.length; j++) {
-    let normalColIdx = normalColValues.lastIndexOf(data.rows[j][normalCol]);
-    let pivotColIdx = pivotColValues.lastIndexOf(data.rows[j][pivotCol]);
+    let normalColIdx = normalValues.lastIndexOf(data.rows[j][normalCol]);
+    let pivotColIdx = pivotValues.lastIndexOf(data.rows[j][pivotCol]);
 
     pivotedRows[normalColIdx][0] = data.rows[j][normalCol];
-    // NOTE: we are hard coding the expectation that the metric is in the 3rd column
-    pivotedRows[normalColIdx][pivotColIdx] = data.rows[j][2];
+    pivotedRows[normalColIdx][pivotColIdx] = data.rows[j][cellCol];
   }
 
   // provide some column metadata to maintain consistency
-  const cols = pivotColValues.map(function(value, idx) {
+  const cols = pivotValues.map(function(value, idx) {
     if (idx === 0) {
       // first column is always the coldef of the normal column
       return data.cols[normalCol];
@@ -81,21 +52,88 @@ export function pivot(data) {
 
   return {
     cols: cols,
-    columns: pivotColValues,
+    columns: pivotValues,
     rows: pivotedRows,
   };
 }
 
-export function distinctValues(data, colIdx) {
-  let vals = data.rows.map(function(r) {
-    return r[colIdx];
-  });
+export function distinctValuesSorted(rows, pivotColIdx, normalColIdx) {
+  const normalSet = new Set();
+  const pivotSet = new Set();
 
-  return vals.filter(function(v, i) {
-    return i == vals.lastIndexOf(v);
-  });
+  const normalSortState = new SortState();
+  const pivotSortState = new SortState();
+
+  for (const row of rows) {
+    const pivotValue = row[pivotColIdx];
+    const normalValue = row[normalColIdx];
+
+    normalSet.add(normalValue);
+    pivotSet.add(pivotValue);
+
+    normalSortState.update(normalValue, pivotValue);
+    pivotSortState.update(pivotValue, normalValue);
+  }
+
+  const normalValues = Array.from(normalSet);
+  const pivotValues = Array.from(pivotSet);
+
+  normalSortState.sort(normalValues);
+  pivotSortState.sort(pivotValues);
+
+  return { normalValues, pivotValues };
 }
 
-export function cardinality(data, colIdx) {
-  return distinctValues(data, colIdx).length;
+// This should work for both strings and numbers
+const DEFAULT_COMPARE = (a, b) => (a < b ? -1 : a > b ? 1 : 0);
+
+class SortState {
+  constructor(compare = DEFAULT_COMPARE) {
+    this.compare = compare;
+
+    this.asc = true;
+    this.desc = true;
+    this.lastValue = undefined;
+
+    this.groupAsc = true;
+    this.groupDesc = true;
+    this.lastGroupKey = undefined;
+    this.isGrouped = false;
+  }
+  update(value, groupKey) {
+    // skip the first value since there's nothing to compare it to
+    if (this.lastValue !== undefined) {
+      // compare the current value with the previous value
+      const result = this.compare(value, this.lastValue);
+      // update global sort state
+      this.asc = this.asc && result >= 0;
+      this.desc = this.desc && result <= 0;
+      if (
+        // if current and last values are different
+        result !== 0 &&
+        // and current and last group are same
+        this.lastGroupKey !== undefined &&
+        this.lastGroupKey === groupKey
+      ) {
+        // update grouped sort state
+        this.groupAsc = this.groupAsc && result >= 0;
+        this.groupDesc = this.groupDesc && result <= 0;
+        this.isGrouped = true;
+      }
+    }
+    // update last value and group key
+    this.lastValue = value;
+    this.lastGroupKey = groupKey;
+  }
+  sort(array) {
+    if (this.isGrouped) {
+      if (this.groupAsc && this.groupDesc) {
+        console.warn("This shouldn't happen");
+      } else if (this.groupAsc && !this.asc) {
+        array.sort(this.compare);
+      } else if (this.groupDesc && !this.desc) {
+        array.sort((a, b) => this.compare(b, a));
+      }
+    }
+  }
 }

frontend/src/metabase/query_builder/components/FieldList.jsx

@@ -37,6 +37,7 @@ type Props = {
 
   alwaysExpanded?: boolean,
   enableSubDimensions?: boolean,
+  useOriginalDimension?: boolean,
 
   hideSectionHeader?: boolean,
 };
@@ -213,6 +214,7 @@ export default class FieldList extends Component {
     const {
       field,
       enableSubDimensions,
+      useOriginalDimension,
       onFilterChange,
       onFieldChange,
     } = this.props;
@@ -222,9 +224,13 @@ export default class FieldList extends Component {
       // ensure if we select the same item we don't reset datetime-field's unit
       onFieldChange(field);
     } else {
-      const dimension = item.dimension.defaultDimension() || item.dimension;
+      const dimension = useOriginalDimension
+        ? item.dimension
+        : item.dimension.defaultDimension() || item.dimension;
       const shouldExcludeBinning =
-        !enableSubDimensions && dimension instanceof BinnedDimension;
+        !enableSubDimensions &&
+        !useOriginalDimension &&
+        dimension instanceof BinnedDimension;
 
       if (shouldExcludeBinning) {
         // If we don't let user choose the sub-dimension, we don't want to treat the field

frontend/src/metabase/query_builder/components/FieldWidget.jsx

@@ -30,11 +30,13 @@ export default class FieldWidget extends Component {
     isInitiallyOpen: PropTypes.bool,
     tableMetadata: PropTypes.object.isRequired,
     enableSubDimensions: PropTypes.bool,
+    useOriginalDimension: PropTypes.bool,
   };
 
   static defaultProps = {
     color: "brand",
     enableSubDimensions: true,
+    useOriginalDimension: false,
   };
 
   setField(value) {
@@ -60,6 +62,7 @@ export default class FieldWidget extends Component {
             customFieldOptions={this.props.customFieldOptions}
             onFieldChange={this.setField}
             enableSubDimensions={this.props.enableSubDimensions}
+            useOriginalDimension={this.props.useOriginalDimension}
           />
         </Popover>
       );

frontend/src/metabase/query_builder/components/SortWidget.jsx

@@ -76,6 +76,7 @@ export default class SortWidget extends Component {
           setField={this.setField}
           isInitiallyOpen={this.state.field === null}
           enableSubDimensions={false}
+          useOriginalDimension={true}
         />
 
         <SelectionModule

frontend/src/metabase/visualizations/components/TableInteractive.jsx

@@ -3,12 +3,11 @@
 import React, { Component } from "react";
 import PropTypes from "prop-types";
 import ReactDOM from "react-dom";
-import { t } from "c-3po";
 import "./TableInteractive.css";
 
 import Icon from "metabase/components/Icon.jsx";
 
-import { formatValue, formatColumn } from "metabase/lib/formatting";
+import { formatValue } from "metabase/lib/formatting";
 import { isID, isFK } from "metabase/lib/schema_metadata";
 import {
   getTableCellClickedObject,
@@ -447,16 +446,11 @@ export default class TableInteractive extends Component {
   }
 
   tableHeaderRenderer = ({ key, style, columnIndex }: CellRendererProps) => {
-    const { sort, isPivoted, settings } = this.props;
+    const { sort, isPivoted, getColumnTitle } = this.props;
     const { cols } = this.props.data;
     const column = cols[columnIndex];
 
-    let columnTitle =
-      settings.column(column)["_column_title_full"] || formatColumn(column);
-
-    if (!columnTitle && this.props.isPivoted && columnIndex !== 0) {
-      columnTitle = t`Unset`;
-    }
+    const columnTitle = getColumnTitle(columnIndex);
 
     let clicked;
     if (isPivoted) {

frontend/src/metabase/visualizations/components/TableSimple.jsx

@@ -11,7 +11,7 @@ import Ellipsified from "metabase/components/Ellipsified.jsx";
 import Icon from "metabase/components/Icon.jsx";
 import MiniBar from "./MiniBar";
 
-import { formatValue, formatColumn } from "metabase/lib/formatting";
+import { formatValue } from "metabase/lib/formatting";
 import {
   getTableCellClickedObject,
   isColumnRightAligned,
@@ -97,6 +97,7 @@ export default class TableSimple extends Component {
       visualizationIsClickable,
       isPivoted,
       settings,
+      getColumnTitle,
     } = this.props;
     const { rows, cols } = data;
     const getCellBackgroundColor = settings["table._cell_background_getter"];
@@ -155,10 +156,7 @@ export default class TableSimple extends Component {
                             marginRight: 3,
                           }}
                         />
-                        <Ellipsified>
-                          {settings.column(col)["_column_title_full"] ||
-                            formatColumn(col)}
-                        </Ellipsified>
+                        <Ellipsified>{getColumnTitle(colIndex)}</Ellipsified>
                       </div>
                     </th>
                   ))}

frontend/src/metabase/visualizations/components/settings/ChartSettingsTableFormatting.jsx

@@ -389,7 +389,7 @@ const RuleEditor = ({ rule, cols, isNew, onChange, onDone, onRemove }) => {
         <div>
           <h3 className="mt3 mb1">{t`Colors`}</h3>
           <ColorRangePicker
-            colors={rule.colors}
+            value={rule.colors}
             onChange={colors => {
               MetabaseAnalytics.trackEvent(
                 "Chart Settings",
@@ -399,6 +399,7 @@ const RuleEditor = ({ rule, cols, isNew, onChange, onDone, onRemove }) => {
               );
               onChange({ ...rule, colors });
             }}
+            ranges={COLOR_RANGES}
           />
           <h3 className="mt3 mb1">{t`Start the range at`}</h3>
           <Radio