From 365f3c7ae235c8585bfb76b2170f56a3241c6367 Mon Sep 17 00:00:00 2001
From: Cyprien P <cyprien.pindat@gmail.com>
Date: Wed, 27 Apr 2022 13:59:19 +0200
Subject: [PATCH] (core) Auto-check aggregate values option as soon as table is
 a summary
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Summary:
Auto-check makes sure that the “Aggregate Values” option is checked as
soon as the chart table is summarized. Before, user had to check it
explicitly.

More on this:
https://grist.slack.com/archives/C04AYS9JF/p1649400119930389?thread_ts=1649338496.915759&cid=C04AYS9JF

Test Plan: Updated tests

Reviewers: jarek

Reviewed By: jarek

Differential Revision: https://phab.getgrist.com/D3422
---
 app/client/components/ChartView.ts  | 14 +++++++-------
 app/client/components/ViewLayout.ts |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/app/client/components/ChartView.ts b/app/client/components/ChartView.ts
index bb4d78ff..092be88e 100644
--- a/app/client/components/ChartView.ts
+++ b/app/client/components/ChartView.ts
@@ -491,11 +491,15 @@ export class ChartConfig extends GrainJSDisposable {
     })
     .onWrite((colId) => this._setXAxis(colId));
 
+  // Whether value is aggregated or not
+  private _isValueAggregated = Computed.create(this, (use) => this._isSummaryTable(use))
+    .onWrite((val) => this._setAggregation(val));
+
   // Columns options
   private _columnsOptions: Computed<Array<IOptionFull<string>>> = Computed.create(
     this, this._freezeXAxis, (use, freeze) => {
       if (freeze) { return this._columnsOptions.get(); }
-      const columns = use(this._optionsObj.prop('aggregate')) ?
+      const columns = use(this._isValueAggregated) ?
         this._getSummarySourceColumns(use) :
         this._getColumns(use);
       return columns
@@ -548,9 +552,6 @@ export class ChartConfig extends GrainJSDisposable {
       });
     });
 
-  private _isValueAggregated = Computed.create(this, (use) => use(this._optionsObj.prop('aggregate')))
-    .onWrite((val) => this._setAggregation(val));
-
   constructor(private _gristDoc: GristDoc, private _section: ViewSectionRec) {
     super();
     ChartConfig._instanceMap.set(_section, this);
@@ -711,7 +712,7 @@ export class ChartConfig extends GrainJSDisposable {
         // 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')()) {
+        if (this._isValueAggregated.get()) {
           const splitColId = this._groupDataColId.get();
           const cols = splitColId === colId ? [colId] : [splitColId, colId];
           await this._setGroupByColumns(cols);
@@ -753,7 +754,7 @@ export class ChartConfig extends GrainJSDisposable {
         // 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')()) {
+        if (this._isValueAggregated.get()) {
           const xAxisColId = this._xAxis.get();
           const cols = xAxisColId === colId ? [colId] : [colId, xAxisColId];
           await this._setGroupByColumns(cols);
@@ -840,7 +841,6 @@ export class ChartConfig extends GrainJSDisposable {
     try {
       this._freezeXAxis.set(true);
       await this._gristDoc.docData.bundleActions(`Toggle chart aggregation`, async () => {
-        await this._optionsObj.prop('aggregate').saveOnly(val);
         if (val) {
           await this._doAggregation();
         } else {
diff --git a/app/client/components/ViewLayout.ts b/app/client/components/ViewLayout.ts
index a8723676..19b21295 100644
--- a/app/client/components/ViewLayout.ts
+++ b/app/client/components/ViewLayout.ts
@@ -329,7 +329,7 @@ export function buildViewSectionDom(options: {
 // With new widgetPopup it is hard to click on viewSection without a activating it, hence we
 // add a little blank space to use in test.
 const cssTestClick = styled(`div`, `
-  min-width: 1px;
+  min-width: 2px;
 `);
 
 const cssSigmaIcon = styled(icon, `