From 87cc7d755d3112cd1aa93eb36b15952b1887df6a Mon Sep 17 00:00:00 2001 From: Cyprien P Date: Tue, 10 May 2022 17:35:42 +0200 Subject: [PATCH] (core) Fix chart mixing up axis Summary: Diff fixes couple edge cases: - When changing chart's groupby columns, the data-engine changes the view fields, which is not okay with charts, hence makes sure view fields stay the same as much as possible using `this.setSectionViewFieldsFromArray()` - Also there's a logic in `this.setSectionViewFieldsFromArray()` that handle what to do when some columns goes missing during updates. Diff fixes this logic two. This is really corner case. Test Plan: Both cases are not added to ChartAggregate.ts Reviewers: georgegevoian Reviewed By: georgegevoian Differential Revision: https://phab.getgrist.com/D3405 --- app/client/components/GristDoc.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/app/client/components/GristDoc.ts b/app/client/components/GristDoc.ts index 62460f41..96d40de5 100644 --- a/app/client/components/GristDoc.ts +++ b/app/client/components/GristDoc.ts @@ -595,6 +595,7 @@ export class GristDoc extends DisposableWithEvents { const docData = this.docModel.docData; const oldVal: IPageWidget = toPageWidget(section); const viewModel = section.view(); + const colIds = section.viewFields().all().map((f) => f.column().colId()); if (isEqual(oldVal, newVal)) { // nothing to be done @@ -621,6 +622,10 @@ export class GristDoc extends DisposableWithEvents { await docData.sendAction( ['UpdateSummaryViewSection', section.getRowId(), newVal.columns] ); + // Charts needs to keep view fields consistent across update. + if (newVal.type === 'chart' && oldVal.type === 'chart') { + await this.setSectionViewFieldsFromArray(section, colIds); + } } // update link @@ -650,13 +655,15 @@ export class GristDoc extends DisposableWithEvents { // If split series and/or x-axis do not exist any more in new table, update options to make them // undefined - if (!mapColIdToColumn.has(colIds[0])) { + if (colIds.length) { if (section.optionsObj.prop('multiseries')()) { - await section.optionsObj.prop('multiseries').saveOnly(false); if (!mapColIdToColumn.has(colIds[0])) { + await section.optionsObj.prop('multiseries').saveOnly(false); + } + if (colIds.length > 1 && !mapColIdToColumn.has(colIds[1])) { await section.optionsObj.prop('isXAxisUndefined').saveOnly(true); } - } else { + } else if (!mapColIdToColumn.has(colIds[0])) { await section.optionsObj.prop('isXAxisUndefined').saveOnly(true); } }