(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
This commit is contained in:
Cyprien P 2021-10-14 17:21:46 +02:00
parent c8da5c7356
commit e3801a5eb9

View File

@ -78,8 +78,16 @@ interface PlotData {
config?: Partial<Config>; 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. // 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 // 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() || {}; const options: ChartOptions = this._options.peek() || {};
let plotData: PlotData = {data: []}; let plotData: PlotData = {data: []};
const sortSpec = this.viewSection.activeSortSpec.peek();
if (this._chartType.peek() === 'pie' && sortSpec?.length) {
dataOptions.sort = false;
}
if (!options.multiseries) { if (!options.multiseries) {
plotData = chartFunc(series, options); plotData = chartFunc(series, options, dataOptions);
} else if (series.length > 1) { } else if (series.length > 1) {
// We need to group all series by the first column. // We need to group all series by the first column.
const nseries = groupSeries(series[0].values, series.slice(1)); 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. // This will be in the order in which nseries Map was created; concat() flattens the arrays.
for (const gSeries of nseries.values()) { for (const gSeries of nseries.values()) {
const part = chartFunc(gSeries, options); const part = chartFunc(gSeries, options, dataOptions);
part.data = plotData.data.concat(part.data); part.data = plotData.data.concat(part.data);
plotData = part; 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; let line: Series;
if (series.length === 0) { if (series.length === 0) {
return {data: []}; 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). // (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), labels: series[0].values.map(v => (v == null || v === "") ? "-" : v),
values: line.values, values: line.values,
...dataOptions,
}] }]
}; };
}, },