From ab7af2b2ef3f37a93f175b520e5c38e25bb85689 Mon Sep 17 00:00:00 2001
From: Cyprien P <cyprien.pindat@gmail.com>
Date: Thu, 21 Oct 2021 10:02:51 +0200
Subject: [PATCH] (core) Fix bars order in chart bars order when multiseries is
 on.

Summary:
 - Grouping series may result in series with inconsistent number of values. This can result in inconsistent ordering of the bars displayed by plotly.
 - This diff fixes it by consolidating grouped series by adding unll values for each missing xvalues in the series.

Here a is a minimal example of that bug:
{F36639}

Test Plan: Includes new nbrowser test.

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D3085
---
 app/client/components/ChartView.ts | 13 ++++++++++++-
 app/client/lib/chartUtil.ts        | 25 +++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/app/client/components/ChartView.ts b/app/client/components/ChartView.ts
index 2d2eb708..ad6abd57 100644
--- a/app/client/components/ChartView.ts
+++ b/app/client/components/ChartView.ts
@@ -1,6 +1,6 @@
 import * as BaseView from 'app/client/components/BaseView';
 import {GristDoc} from 'app/client/components/GristDoc';
-import {sortByXValues, splitValuesByIndex, uniqXValues} from 'app/client/lib/chartUtil';
+import {consolidateValues, sortByXValues, splitValuesByIndex, uniqXValues} from 'app/client/lib/chartUtil';
 import {Delay} from 'app/client/lib/Delay';
 import {Disposable} from 'app/client/lib/dispose';
 import {fromKoSave} from 'app/client/lib/fromKoSave';
@@ -219,7 +219,18 @@ export class ChartView extends Disposable {
       const nseries = groupSeries(series[0].values, series.slice(1));
 
       // This will be in the order in which nseries Map was created; concat() flattens the arrays.
+      const xvalues = Array.from(new Set(series[1].values));
       for (const gSeries of nseries.values()) {
+
+        // All series have partial list of values, ie: if some may have Q1, Q2, Q3, Q4 as x values
+        // some others might only have Q1. This causes inconsistent result in regard of the order
+        // bars will be displayed by plotly (for bar charts). This eventually result in bars not
+        // following the sorting order. This line fixes that issue by consolidating all series to
+        // have at least on entry of each x values.
+        if (this._chartType.peek() === 'bar') {
+          if (sortSpec?.length) { consolidateValues(gSeries, xvalues); }
+        }
+
         const part = chartFunc(gSeries, options, dataOptions);
         part.data = plotData.data.concat(part.data);
         plotData = part;
diff --git a/app/client/lib/chartUtil.ts b/app/client/lib/chartUtil.ts
index b18c0ede..c5724d2e 100644
--- a/app/client/lib/chartUtil.ts
+++ b/app/client/lib/chartUtil.ts
@@ -60,3 +60,28 @@ export function splitValuesByIndex<T extends {values: Datum[]}>(series: Array<T>
     return {...s, values};
   });
 }
+
+/**
+ * Makes sure series[0].values includes all of the values in xvalues and that they appears in the
+ * same order. 0 is used to fill missing values in series[i].values for i > 1 (making function
+ * suited only for numeric series AND only to use with for bar charts). Function does mutate series.
+ *
+ * Note it would make more sense to pad missing values with `null`, but plotly handles null the same
+ * as missing values. Hence we're padding with 0.
+ */
+export function consolidateValues(series: Array<{values: Datum[]}>, xvalues: Datum[]) {
+  let i = 0;
+  for (const xval of xvalues) {
+    if (i < series[0].values.length && xval !== series[0].values[i]
+        || i > series[0].values.length - 1) {
+      series[0].values.splice(i, 0, xval);
+      for (let j = 1; j < series.length; ++j) {
+        series[j].values.splice(i, 0, 0);
+      }
+    }
+    while (xval === series[0].values[i] && i < series[0].values.length) {
+      i++;
+    }
+  }
+  return series;
+}