(core) Fix chart when x axis is set to a choice list column

Summary:
Bug reported by user: https://gristlabs.getgrist.com/doc/check-ins/p/3#a1.s7.r1183.c19p

Setting x axis to a column of type ChoiceList was breaking chart.

This diff fixes that by splitting the record into several records: one for each choice.

`test/nbrowser/ChartView1.ts` was becoming too big and long to run, so this diff introduces `test/nbrowser/ChartView2.ts` to add more test and `test/nbrowser/chartViewTestUtils.ts` to put all utilities or testing charts.

Test Plan: Adds new test.

Reviewers: georgegevoian

Reviewed By: georgegevoian

Differential Revision: https://phab.getgrist.com/D3041
This commit is contained in:
Cyprien P 2021-09-30 09:41:36 +02:00
parent d4ea5b3761
commit a0c53f2b61
2 changed files with 51 additions and 2 deletions

View File

@ -1,6 +1,6 @@
import * as BaseView from 'app/client/components/BaseView'; import * as BaseView from 'app/client/components/BaseView';
import {GristDoc} from 'app/client/components/GristDoc'; import {GristDoc} from 'app/client/components/GristDoc';
import {sortByXValues, uniqXValues} from 'app/client/lib/chartUtil'; import {sortByXValues, splitValuesByIndex, uniqXValues} from 'app/client/lib/chartUtil';
import {Delay} from 'app/client/lib/Delay'; import {Delay} from 'app/client/lib/Delay';
import {Disposable} from 'app/client/lib/dispose'; import {Disposable} from 'app/client/lib/dispose';
import {fromKoSave} from 'app/client/lib/fromKoSave'; import {fromKoSave} from 'app/client/lib/fromKoSave';
@ -18,6 +18,7 @@ import {cssDragger} from 'app/client/ui2018/draggableList';
import {icon} from 'app/client/ui2018/icons'; import {icon} from 'app/client/ui2018/icons';
import {linkSelect, menu, menuItem, select} from 'app/client/ui2018/menus'; import {linkSelect, menu, menuItem, select} from 'app/client/ui2018/menus';
import {nativeCompare} from 'app/common/gutil'; import {nativeCompare} from 'app/common/gutil';
import {decodeObject} from 'app/plugin/objtypes';
import {Events as BackboneEvents} from 'backbone'; import {Events as BackboneEvents} from 'backbone';
import {Computed, dom, DomElementArg, fromKo, Disposable as GrainJSDisposable, IOption, import {Computed, dom, DomElementArg, fromKo, Disposable as GrainJSDisposable, IOption,
makeTestId, Observable, styled} from 'grainjs'; makeTestId, Observable, styled} from 'grainjs';
@ -94,6 +95,11 @@ function dateGetter(getter: RowPropGetter): RowPropGetter {
}; };
} }
// List of column types whose values are encoded has list, ie: ['L', 'foo', ...]. Such values
// require special treatment to show correctly in charts.
const LIST_TYPES = ['ChoiceList', 'RefList'];
/** /**
* ChartView component displays created charts. * ChartView component displays created charts.
*/ */
@ -163,7 +169,7 @@ export class ChartView extends Disposable {
const fields: ViewFieldRec[] = this.viewSection.viewFields().all(); const fields: ViewFieldRec[] = this.viewSection.viewFields().all();
const rowIds: number[] = this.sortedRows.getKoArray().peek() as number[]; const rowIds: number[] = this.sortedRows.getKoArray().peek() as number[];
const series: Series[] = fields.map((field) => { let series: Series[] = fields.map((field) => {
// Use the colId of the displayCol, which may be different in case of Reference columns. // Use the colId of the displayCol, which may be different in case of Reference columns.
const colId: string = field.displayColModel.peek().colId.peek(); const colId: string = field.displayColModel.peek().colId.peek();
const getter = this.tableModel.tableData.getRowPropFunc(colId) as RowPropGetter; const getter = this.tableModel.tableData.getRowPropFunc(colId) as RowPropGetter;
@ -175,6 +181,20 @@ export class ChartView extends Disposable {
}; };
}); });
const startIndexForYAxis = this._options.prop('multiseries') ? 2 : 1;
for (let i = 0; i < series.length; ++i) {
if (i < fields.length && LIST_TYPES.includes(fields[i].column.peek().pureType.peek())) {
if (i < startIndexForYAxis) {
// For x-axis and group column data, split series we should split records.
series = splitValuesByIndex(series, i);
} else {
// For all y-axis, it's not sure what would be a sensible representation for choice list,
// simply stringify choice list values seems reasonable.
series[i].values = series[i].values.map((v) => String(decodeObject(v as any)));
}
}
}
const options: ChartOptions = this._options.peek() || {}; const options: ChartOptions = this._options.peek() || {};
let plotData: PlotData = {data: []}; let plotData: PlotData = {data: []};

View File

@ -1,7 +1,9 @@
import {typedCompare} from 'app/common/SortFunc'; import {typedCompare} from 'app/common/SortFunc';
import {decodeObject} from 'app/plugin/objtypes';
import {Datum} from 'plotly.js'; import {Datum} from 'plotly.js';
import range = require('lodash/range'); import range = require('lodash/range');
import uniqBy = require('lodash/uniqBy'); import uniqBy = require('lodash/uniqBy');
import flatten = require('lodash/flatten');
/** /**
* Sort all values in a list of series according to the values in the first one. * Sort all values in a list of series according to the values in the first one.
@ -31,3 +33,30 @@ export function uniqXValues<T extends {values: Datum[]}>(series: Array<T>): Arra
values: line.values.filter((_val, i) => indexToKeep.has(i)) values: line.values.filter((_val, i) => indexToKeep.has(i))
})); }));
} }
// Creates new version of series that split any entry whose value in the first series is a list into
// multiple entries, one entry for each list's item. For all other series, newly created entries have
// the same value as the original.
export function splitValues<T extends {values: Datum[]}>(series: Array<T>): Array<T> {
return splitValuesByIndex(series, 0);
}
// This method is like splitValues except it splits according to the values of the series at position index.
export function splitValuesByIndex<T extends {values: Datum[]}>(series: Array<T>, index: number): Array<T> {
const decoded = (series[index].values as any[]).map(decodeObject);
return series.map((s, si) => {
if (si === index) {
return {...series[index], values: flatten(decoded)};
}
let values: Datum[] = [];
for (const [i, splitByValue] of decoded.entries()) {
if (Array.isArray(splitByValue)) {
values = values.concat(Array(splitByValue.length).fill(s.values[i]));
} else {
values.push(s.values[i]);
}
}
return {...s, values};
});
}