Archives/gristlabs_grist-core
1
0
Fork 0
mirror of https://github.com/gristlabs/grist-core.git synced 2024-10-27 20:44:07 +00:00
Code Issues Projects Releases Wiki Activity

(core) Fix bars order in chart bars order when multiseries is on.

Browse Source 
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
This commit is contained in:
Cyprien P 2021-10-21 10:02:51 +02:00
parent d63da496a8
commit ab7af2b2ef
2 changed files with 37 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
app/client/components/ChartView.ts
View File

@ -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;

25
app/client/lib/chartUtil.ts
View File

@ -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;
}