From e3801a5eb92fdc78a62cf008840f68ebc73a9f72 Mon Sep 17 00:00:00 2001
From: Cyprien P <cyprien.pindat@gmail.com>
Date: Thu, 14 Oct 2021 17:21:46 +0200
Subject: [PATCH] (core) Fix pie sectors order according to section sort spec

Summary:
Plotly sorts pie charts sectors by default and that is overiding the
section ordering. This diff fixes that by passing setting .sort to
false (thus disabling reordering) when there is a sort spec going on.

Issue was reported by user: https://gristlabs.getgrist.com/k1f3bMzUvitZ/User-Feedback#a1.s3.r333.c19

Test Plan: Added nbrowser test

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D3075
---
 app/client/components/ChartView.ts | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/app/client/components/ChartView.ts b/app/client/components/ChartView.ts
index 81ab27e0..2d2eb708 100644
--- a/app/client/components/ChartView.ts
+++ b/app/client/components/ChartView.ts
@@ -78,8 +78,16 @@ interface PlotData {
   config?: Partial<Config>;
 }
 
+// Data options to pass to chart functions.
+interface DataOptions {
+
+  // Allows to set the pie sort option (see: https://plotly.com/javascript/reference/pie/#pie-sort).
+  // Supports pie charts only.
+  sort?: boolean;
+}
+
 // Convert a list of Series into a set of Plotly traces.
-type ChartFunc = (series: Series[], options: ChartOptions) => PlotData;
+type ChartFunc = (series: Series[], options: ChartOptions, dataOptions?: DataOptions) => PlotData;
 
 
 // Helper for converting numeric Date/DateTime values (seconds since Epoch) to JS Date objects for
@@ -195,18 +203,24 @@ export class ChartView extends Disposable {
       }
     }
 
+    const dataOptions: DataOptions = {};
     const options: ChartOptions = this._options.peek() || {};
     let plotData: PlotData = {data: []};
 
+    const sortSpec = this.viewSection.activeSortSpec.peek();
+    if (this._chartType.peek() === 'pie' && sortSpec?.length) {
+      dataOptions.sort = false;
+    }
+
     if (!options.multiseries) {
-      plotData = chartFunc(series, options);
+      plotData = chartFunc(series, options, dataOptions);
     } else if (series.length > 1) {
       // We need to group all series by the first column.
       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.
       for (const gSeries of nseries.values()) {
-        const part = chartFunc(gSeries, options);
+        const part = chartFunc(gSeries, options, dataOptions);
         part.data = plotData.data.concat(part.data);
         plotData = part;
       }
@@ -672,7 +686,7 @@ export const chartTypes: {[name: string]: ChartFunc} = {
     });
   },
 
-  pie(series: Series[]): PlotData {
+  pie(series: Series[], _options: ChartOptions, dataOptions: DataOptions = {}): PlotData {
     let line: Series;
     if (series.length === 0) {
       return {data: []};
@@ -692,6 +706,7 @@ export const chartTypes: {[name: string]: ChartFunc} = {
         // (a falsy value would cause plotly to show its index, like "2" which is more confusing).
         labels: series[0].values.map(v => (v == null || v === "") ? "-" : v),
         values: line.values,
+        ...dataOptions,
       }]
     };
   },