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 setting xaxis when both chart aggregation and split series

Browse Source 
Summary:
 - Symptoms where that Split Series could end up being turned off for
   no good reason. Also both x axis and split series could be mixed
   up.

 - Problems was caused by call to `setGroupByColumns` which modifies
   the sections viewFields. Diff fixes it by adjustin slightly the
   ordering of function call in `_setXAxis()`.

 - Problem of mixing up x axis and split series was fixed by being
   careful on the order of columns passed to the `setGroupByColumns`
   which then determine the ordering of the view fields.

Test Plan: Includes new test cases

Reviewers: jarek

Reviewed By: jarek

Differential Revision: https://phab.getgrist.com/D3365
This commit is contained in:
Cyprien P 2022-04-08 15:47:08 +02:00
parent 09da815c0c
commit 25e40bfa9b
1 changed files with 11 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -692,15 +692,6 @@ export class ChartConfig extends GrainJSDisposable {
this._freezeXAxis.set(true);
try {
// if values aggregation is 'on' update the grouped by columns first. This will make sure
// that colId is not missing from the summary table's columns (as could happen if it were a
// non-numeric for instance).
if (this._optionsObj.prop('aggregate')()) {
const splitColId = this._groupDataColId.get();
const cols = splitColId === colId ? [colId] : [colId, splitColId];
await this._setGroupByColumns(cols);
}
// first remove the current field
if (this._xAxisFieldIndex.get() !== -1 && this._xAxisFieldIndex.get() < viewFields.peek().length) {
await this._configFieldsHelper.removeField(viewFields.peek()[this._xAxisFieldIndex.get()]);
@ -709,13 +700,22 @@ export class ChartConfig extends GrainJSDisposable {
// if x axis was undefined, set option to false
await setSaveValue(this._optionsObj.prop('isXAxisUndefined'), false);
// if new field was used to group by column series, disable multiseries
// if new field was used to split series, disable multiseries
const fieldIndex = viewFields.peek().findIndex((f) => f.column.peek().colId() === colId);
if (fieldIndex === 0 && optionsObj.prop('multiseries').peek()) {
await optionsObj.prop('multiseries').setAndSave(false);
return;
}
// if values aggregation is 'on' update the grouped by columns before findColumn()
// call. This will make sure that colId is not missing from the summary table's columns (as
// could happen if it were a non-numeric for instance).
if (this._optionsObj.prop('aggregate')()) {
const splitColId = this._groupDataColId.get();
const cols = splitColId === colId ? [colId] : [splitColId, colId];
await this._setGroupByColumns(cols);
}
// if the new column for the x axis is already visible, make it the first visible column,
// else add it as the first visible field. The field will be first because it will be
// inserted before current xAxis column (which is already first (or second if we have
@ -754,7 +754,7 @@ export class ChartConfig extends GrainJSDisposable {
// non-numeric for instance).
if (this._optionsObj.prop('aggregate')()) {
const xAxisColId = this._xAxis.get();
const cols = xAxisColId === colId ? [colId] : [xAxisColId, colId];
const cols = xAxisColId === colId ? [colId] : [colId, xAxisColId];
await this._setGroupByColumns(cols);
}