Fix stacked charts breaking waterfall charts (#18093)

frontend/src/metabase/visualizations/components/LineAreaBarChart.css

@@ -64,6 +64,19 @@
   display: inherit;
 }
 
+/* restyle grid-line for 0 to look like X axis */
+.LineAreaBarChart .dc-chart .stacked line.zero {
+  stroke: var(--color-text-light);
+  opacity: 1;
+  stroke-dasharray: none;
+}
+
+/* restyle X axis for stacked charts to look like a grid line */
+.LineAreaBarChart .dc-chart .stacked .domain {
+  stroke: color(var(--color-text-medium) alpha(-80%));
+  stroke-dasharray: 5, 5;
+}
+
 /* gridline at 0 overlaps with X axis */
 .LineAreaBarChart .dc-chart .grid-line.horizontal line:first-child {
   display: none;

frontend/src/metabase/visualizations/lib/LineAreaBarPostRender.js

@@ -368,6 +368,22 @@ function onRenderAddExtraClickHandlers(chart) {
   }
 }
 
+function onRenderSetZeroGridLineClassName(chart) {
+  const yAxis = chart.y();
+  if (!yAxis) {
+    return;
+  }
+
+  const yZero = yAxis(0).toString();
+  chart
+    .select(".grid-line.horizontal")
+    .selectAll("line")
+    .filter(function() {
+      return d3.select(this).attr("y1") === yZero;
+    })
+    .attr("class", "zero");
+}
+
 // the various steps that get called
 function onRender(
   chart,
@@ -398,6 +414,7 @@ function onRender(
   onRenderSetClassName(chart, isStacked);
   onRenderRotateAxis(chart);
   onRenderAddExtraClickHandlers(chart);
+  onRenderSetZeroGridLineClassName(chart);
 }
 
 // +-------------------------------------------------------------------------------------------------------------------+

frontend/src/metabase/visualizations/lib/LineAreaBarRenderer.js

@@ -67,6 +67,7 @@ import {
 
 import { lineAddons } from "./graph/addons";
 import { initBrush } from "./graph/brush";
+import { stack, stackOffsetDiverging } from "./graph/stack";
 
 import type { VisualizationProps } from "metabase-types/types/Visualization";
 
@@ -439,6 +440,11 @@ function applyChartLineBarSettings(
         forceCenterBar || settings["graph.x_axis.scale"] !== "ordinal",
       );
   }
+
+  // AREA/BAR:
+  if (settings["stackable.stack_type"] === "stacked") {
+    chart.stackLayout(stack().offset(stackOffsetDiverging));
+  }
 }
 
 // TODO - give this a good name when I figure out what it does

frontend/src/metabase/visualizations/lib/chart_values.js

@@ -30,6 +30,7 @@ export function onRenderValueLabels(
   );
 
   let displays = seriesSettings.map(settings => settings.display);
+  const isStacked = chart.settings["stackable.stack_type"] === "stacked";
 
   if (
     showSeries.every(s => s === false) || // every series setting is off
@@ -38,7 +39,7 @@ export function onRenderValueLabels(
     return;
   }
 
-  if (chart.settings["stackable.stack_type"] === "stacked") {
+  if (isStacked) {
     // When stacked, flatten datas into one series. We'll sum values on the same x point later.
     datas = [datas.flat()];
 
@@ -82,23 +83,37 @@ export function onRenderValueLabels(
     const display = displays[seriesIndex];
 
     // Sum duplicate x values in the same series.
+    // Positive and negative values are stacked separately, unless it is a waterfall chart
     data = _.chain(data)
       .groupBy(([x]) => xScale(x))
       .values()
       .map(data => {
         const [[x]] = data;
-        const y = data.reduce((sum, [, y]) => sum + y, 0);
-        return [x, y];
+        const yp = data
+          .filter(([, y]) => y >= 0)
+          .reduce((sum, [, y]) => sum + y, 0);
+        const yn = data
+          .filter(([, y]) => y < 0)
+          .reduce((sum, [, y]) => sum + y, 0);
+
+        if (!isStacked) {
+          return [[x, yp + yn, 1]];
+        } else if (yp !== yn) {
+          return [[x, yp, 2], [x, yn, 2]];
+        } else {
+          return [[x, yp, 1]];
+        }
       })
+      .flatten(1)
       .value();
 
     data = data
-      .map(([x, y], i) => {
+      .map(([x, y, step], i) => {
         const isLocalMin =
           // first point or prior is greater than y
-          (i === 0 || data[i - 1][1] > y) &&
+          (i < step || data[i - step][1] > y) &&
           // last point point or next is greater than y
-          (i === data.length - 1 || data[i + 1][1] > y);
+          (i >= data.length - step || data[i + step][1] > y);
         const showLabelBelow = isLocalMin && display === "line";
         const rotated = barCount > 1 && isBarLike(display) && barWidth < 40;
         const hidden =

frontend/src/metabase/visualizations/lib/graph/stack.js

+import d3 from "d3";
+
+// d3.layout.stack applies offsets only to the first value within a group
+// this wrapper does that to each value to stack positive and negative series separately
+
+export function stack() {
+  const inner = d3.layout.stack();
+
+  let values = inner.values();
+  let order = inner.order();
+  let x = inner.x();
+  let y = inner.y();
+  let out = inner.out();
+  let offset = stackOffsetZero;
+
+  function stack(data, index) {
+    const n = data.length;
+
+    if (!n) {
+      return data;
+    }
+
+    // convert series to canonical two-dimensional representation
+    let series = data.map(function(d, i) {
+      return values.call(stack, d, i);
+    });
+
+    // convert each series to canonical [[x,y]] representation
+    let points = series.map(function(d) {
+      return d.map(function(v, i) {
+        return [x.call(stack, v, i), y.call(stack, v, i)];
+      });
+    });
+
+    // compute the order of series, and permute them
+    const orders = order.call(stack, points, index);
+    series = d3.permute(series, orders);
+    points = d3.permute(points, orders);
+
+    // compute the baseline
+    const offsets = offset.call(stack, points, index);
+
+    // propagate it to other series
+    const m = series[0].length;
+    for (let j = 0; j < m; j++) {
+      for (let i = 0; i < n; i++) {
+        out.call(stack, series[i][j], offsets[i][j], points[i][j][1]);
+      }
+    }
+
+    return data;
+  }
+
+  stack.values = function(x) {
+    if (!arguments.length) {
+      return values;
+    }
+
+    values = x;
+    return stack;
+  };
+
+  stack.order = function(x) {
+    if (!arguments.length) {
+      return order;
+    }
+
+    order = x;
+    return stack;
+  };
+
+  stack.offset = function(x) {
+    if (!arguments.length) {
+      return offset;
+    }
+
+    offset = x;
+    return stack;
+  };
+
+  stack.x = function(z) {
+    if (!arguments.length) {
+      return x;
+    }
+
+    x = z;
+    return stack;
+  };
+
+  stack.y = function(z) {
+    if (!arguments.length) {
+      return y;
+    }
+
+    y = z;
+    return stack;
+  };
+
+  stack.out = function(z) {
+    if (!arguments.length) {
+      return out;
+    }
+
+    out = z;
+    return stack;
+  };
+
+  return stack;
+}
+
+// series are stacked on top of each other, starting from zero
+export function stackOffsetZero(data) {
+  const n = data.length;
+  const m = data[0].length;
+  const y0 = [];
+
+  for (let i = 0; i < n; i++) {
+    y0[i] = [];
+  }
+
+  for (let j = 0; j < m; j++) {
+    for (let i = 0, d = 0; i < n; i++) {
+      y0[i][j] = d;
+      d += data[i][j][1];
+    }
+  }
+
+  return y0;
+}
+
+// series are stacked with separate tracks for positive and negative values
+export function stackOffsetDiverging(data) {
+  const n = data.length;
+  const m = data[0].length;
+  const y0 = [];
+
+  for (let i = 0; i < n; i++) {
+    y0[i] = [];
+  }
+
+  for (let j = 0; j < m; j++) {
+    for (let i = 0, dp = 0, dn = 0; i < n; i++) {
+      if (data[i][j][1] >= 0) {
+        y0[i][j] = dp;
+        dp += data[i][j][1];
+      } else {
+        y0[i][j] = dn;
+        dn += data[i][j][1];
+      }
+    }
+  }
+
+  return y0;
+}

frontend/src/metabase/visualizations/lib/graph/stack.unit.spec.js

+import { stack, stackOffsetDiverging } from "./stack";
+
+describe("stack", () => {
+  const data = [
+    [{ x: 1, y: 100 }, { x: 2, y: 100 }],
+    [{ x: 1, y: 200 }, { x: 2, y: -200 }],
+    [{ x: 1, y: 300 }, { x: 2, y: 300 }],
+  ];
+
+  it("should stack series by default", () => {
+    stack()(data);
+
+    expect(data).toEqual([
+      [{ x: 1, y: 100, y0: 0 }, { x: 2, y: 100, y0: 0 }],
+      [{ x: 1, y: 200, y0: 100 }, { x: 2, y: -200, y0: 100 }],
+      [{ x: 1, y: 300, y0: 300 }, { x: 2, y: 300, y0: -100 }],
+    ]);
+  });
+
+  it("should stack series with separate positive and negative tracks", () => {
+    stack().offset(stackOffsetDiverging)(data);
+
+    expect(data).toEqual([
+      [{ x: 1, y: 100, y0: 0 }, { x: 2, y: 100, y0: 0 }],
+      [{ x: 1, y: 200, y0: 100 }, { x: 2, y: -200, y0: 0 }],
+      [{ x: 1, y: 300, y0: 300 }, { x: 2, y: 300, y0: 100 }],
+    ]);
+  });
+});

frontend/test/metabase-visual/visualizations/bar.cy.spec.js

+import { restore, visitQuestionAdhoc } from "__support__/e2e/cypress";
+
+const testQuery = {
+  type: "native",
+  native: {
+    query:
+      "SELECT X, A, B, C " +
+      "FROM (VALUES (1,20,30,30),(2,10,-40,-20),(3,20,10,30)) T (X, A, B, C)",
+  },
+  database: 1,
+};
+
+describe("visual tests > visualizations > bar", () => {
+  beforeEach(() => {
+    restore();
+    cy.signInAsNormalUser();
+    cy.server();
+    cy.route("POST", "/api/dataset").as("dataset");
+  });
+
+  it("with stacked series", () => {
+    visitQuestionAdhoc({
+      dataset_query: testQuery,
+      display: "bar",
+      visualization_settings: {
+        "graph.dimensions": ["X"],
+        "graph.metrics": ["A", "B", "C"],
+        "stackable.stack_type": "stacked",
+      },
+    });
+
+    cy.wait("@dataset");
+
+    cy.percySnapshot();
+  });
+});

frontend/test/metabase-visual/visualizations/waterfall.cy.spec.js

+import { restore, visitQuestionAdhoc } from "__support__/e2e/cypress";
+import { SAMPLE_DATASET } from "__support__/e2e/cypress_sample_dataset";
+
+const { ORDERS, ORDERS_ID } = SAMPLE_DATASET;
+
+const testQuery = {
+  type: "query",
+  query: {
+    "source-table": ORDERS_ID,
+    aggregation: [["count"]],
+    breakout: [
+      [
+        "field",
+        ORDERS.CREATED_AT,
+        {
+          "temporal-unit": "year",
+        },
+      ],
+    ],
+  },
+  database: 1,
+};
+
+describe("visual tests > visualizations > waterfall", () => {
+  beforeEach(() => {
+    restore();
+    cy.signInAsNormalUser();
+    cy.server();
+    cy.route("POST", "/api/dataset").as("dataset");
+  });
+
+  it("with positive and negative series", () => {
+    visitQuestionAdhoc({
+      dataset_query: testQuery,
+      display: "waterfall",
+      visualization_settings: {
+        "graph.show_values": true,
+        "graph.dimensions": ["CREATED_AT"],
+        "graph.metrics": ["count"],
+      },
+    });
+
+    cy.wait("@dataset");
+
+    cy.percySnapshot();
+  });
+});