fix: Ordering of subplots

- User is expecting the plots to be ordered 0 at top, increasing downward - Plotlys bounds coordinate system has 0,0 in the bottom left, increasing upward - Invert the coordinates so that it appears in plotly correctly - Add unit tests
deephaven · Feb 27, 2023 · 9650d26 · 9650d26
1 parent 144605a
commit 9650d26
Show file tree

Hide file tree

Showing 3 changed files with 100 additions and 6 deletions.
diff --git a/packages/chart/src/ChartTestUtils.ts b/packages/chart/src/ChartTestUtils.ts
@@ -83,22 +83,41 @@ class ChartTestUtils {
     series = [ChartTestUtils.makeSeries()],
     axes = ChartTestUtils.makeDefaultAxes(),
     showLegend = null,
+    rowspan = 1,
+    colspan = 1,
+    row = 0,
+    column = 0,
   }: {
     title?: string;
     series?: Series[];
     axes?: Axis[];
     showLegend?: boolean | null;
+    rowspan?: number;
+    colspan?: number;
+    row?: number;
+    column?: number;
   } = {}): Chart {
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    return new (dh as any).Chart({ title, series, axes, showLegend });
+    return new (dh as any).Chart({
+      title,
+      series,
+      axes,
+      showLegend,
+      row,
+      column,
+      rowspan,
+      colspan,
+    });
   }
 
   static makeFigure({
     title = 'Figure',
     charts = [ChartTestUtils.makeChart()],
+    rows = 1,
+    cols = 1,
   } = {}): Figure {
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    return new (dh as any).plot.Figure({ title, charts });
+    return new (dh as any).plot.Figure({ title, charts, rows, cols });
   }
 }
 

diff --git a/packages/chart/src/ChartUtils.test.ts b/packages/chart/src/ChartUtils.test.ts
@@ -315,6 +315,79 @@ describe('updating layout axes', () => {
   });
 });
 
+describe('handles subplots and columns/rows correctly', () => {
+  const width = ChartUtils.AXIS_SIZE_PX * 5;
+  const height = ChartUtils.AXIS_SIZE_PX * 10;
+
+  it('handles row location correctly', () => {
+    const axes = ChartTestUtils.makeDefaultAxes();
+    const charts = [
+      ChartTestUtils.makeChart({ axes, row: 0 }),
+      ChartTestUtils.makeChart({ axes, row: 1 }),
+    ];
+    const figure = ChartTestUtils.makeFigure({ charts, rows: 2 });
+    expect(
+      ChartUtils.getChartBounds(figure, charts[0], width, height)
+    ).toEqual({ bottom: 0.55, top: 1, left: 0, right: 1 });
+    expect(
+      ChartUtils.getChartBounds(figure, charts[1], width, height)
+    ).toEqual({ bottom: 0, top: 0.45, left: 0, right: 1 });
+  });
+
+  it('handles column location correctly', () => {
+    const axes = ChartTestUtils.makeDefaultAxes();
+    const charts = [
+      ChartTestUtils.makeChart({ axes, column: 0 }),
+      ChartTestUtils.makeChart({ axes, column: 1 }),
+    ];
+    const figure = ChartTestUtils.makeFigure({ charts, cols: 2 });
+    expect(
+      ChartUtils.getChartBounds(figure, charts[0], width, height)
+    ).toEqual({ bottom: 0, top: 1, left: 0, right: 0.4 });
+    expect(
+      ChartUtils.getChartBounds(figure, charts[1], width, height)
+    ).toEqual({ bottom: 0, top: 1, left: 0.6, right: 1 });
+  });
+
+  it('handles colspan', () => {
+    const axes = ChartTestUtils.makeDefaultAxes();
+    const charts = [
+      ChartTestUtils.makeChart({ axes, column: 0 }),
+      ChartTestUtils.makeChart({ axes, column: 1 }),
+      ChartTestUtils.makeChart({ axes, row: 1, colspan: 2 }),
+    ];
+    const figure = ChartTestUtils.makeFigure({ charts, cols: 2, rows: 2 });
+    expect(
+      ChartUtils.getChartBounds(figure, charts[0], width, height)
+    ).toEqual({ bottom: 0.55, top: 1, left: 0, right: 0.4 });
+    expect(
+      ChartUtils.getChartBounds(figure, charts[1], width, height)
+    ).toEqual({ bottom: 0.55, top: 1, left: 0.6, right: 1 });
+    expect(
+      ChartUtils.getChartBounds(figure, charts[2], width, height)
+    ).toEqual({ bottom: 0, top: 0.45, left: 0, right: 1 });
+  });
+
+  it('handles rowspan', () => {
+    const axes = ChartTestUtils.makeDefaultAxes();
+    const charts = [
+      ChartTestUtils.makeChart({ axes, row: 0 }),
+      ChartTestUtils.makeChart({ axes, row: 1 }),
+      ChartTestUtils.makeChart({ axes, column: 1, rowspan: 2 }),
+    ];
+    const figure = ChartTestUtils.makeFigure({ charts, cols: 2, rows: 2 });
+    expect(
+      ChartUtils.getChartBounds(figure, charts[0], width, height)
+    ).toEqual({ bottom: 0.55, top: 1, left: 0, right: 0.4 });
+    expect(
+      ChartUtils.getChartBounds(figure, charts[1], width, height)
+    ).toEqual({ bottom: 0, top: 0.45, left: 0, right: 0.4 });
+    expect(
+      ChartUtils.getChartBounds(figure, charts[2], width, height)
+    ).toEqual({ bottom: 0, top: 1, left: 0.6, right: 1 });
+  });
+});
+
 describe('returns the axis layout ranges properly', () => {
   function makeLayout(layout) {
     return {

diff --git a/packages/chart/src/ChartUtils.ts b/packages/chart/src/ChartUtils.ts
@@ -1140,8 +1140,8 @@ class ChartUtils {
   static getChartBounds(
     figure: Figure,
     chart: Chart,
-    plotWidth = 0,
-    plotHeight = 0
+    plotWidth: number,
+    plotHeight: number
   ): ChartBounds {
     const { cols, rows } = figure;
     const { column, colspan, row, rowspan } = chart;
@@ -1154,9 +1154,11 @@ class ChartUtils {
     const yMarginSize = ChartUtils.AXIS_SIZE_PX / plotHeight;
 
     const bounds: ChartBounds = {
+      // Need to invert the row positioning so the first one defined shows up on top instead of the bottom, since coordinates start in bottom left
+      bottom: (rows - endRow) * rowSize + (endRow < rows ? yMarginSize / 2 : 0),
+      top: (rows - row) * rowSize - (row > 0 ? yMarginSize / 2 : 0),
+
       left: column * columnSize + (column > 0 ? xMarginSize / 2 : 0),
-      bottom: row * rowSize + (row > 0 ? yMarginSize / 2 : 0),
-      top: endRow * rowSize - (endRow < rows ? yMarginSize / 2 : 0),
       right: endColumn * columnSize - (endColumn < cols ? xMarginSize / 2 : 0),
     };