From 8162a6d9596beec15ff7951ff34395f005979c8b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jaros=C5=82aw=20Sadzi=C5=84ski?=
 <jaroslaw.sadzinski@gmail.com>
Date: Tue, 23 Jul 2024 14:50:00 +0200
Subject: [PATCH] (core) Treating x axis as category for bar chart

Summary: Forcing category xaxis type for bar chart when labels are not numerical.

Test Plan: Added new and updated existing

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D4297
---
 app/client/components/ChartView.ts | 14 ++++++++--
 test/nbrowser/ChartView1.ts        | 43 +++++++++++++++++++++++++++++-
 2 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/app/client/components/ChartView.ts b/app/client/components/ChartView.ts
index 3c49e2dd..37ac7574 100644
--- a/app/client/components/ChartView.ts
+++ b/app/client/components/ChartView.ts
@@ -106,8 +106,9 @@ type RowPropGetter = (rowId: number) => Datum;
 // We convert Grist data to a list of Series first, from which we then construct Plotly traces.
 interface Series {
   label: string;          // Corresponds to the column name.
-  group?: Datum;          // The group value, when grouped.
   values: Datum[];
+  pureType?: string;      // The pure type of the column.
+  group?: Datum;          // The group value, when grouped.
   isInSortSpec?: boolean; // Whether this series is present in sort spec for this chart.
 }
 
@@ -273,6 +274,7 @@ export class ChartView extends Disposable {
         const pureType = field.displayColModel().pureType();
         const fullGetter = (pureType === 'Date' || pureType === 'DateTime') ? dateGetter(getter) : getter;
         return {
+          pureType,
           label: field.label(),
           values: rowIds.map(fullGetter),
           isInSortSpec: Boolean(Sort.findCol(this._sortSpec, field.colRef.peek())),
@@ -1121,7 +1123,15 @@ function basicPlot(series: Series[], options: ChartOptions, dataOptions: Data):
 export const chartTypes: {[name: string]: ChartFunc} = {
   // TODO There is a lot of code duplication across chart types. Some refactoring is in order.
   bar(series: Series[], options: ChartOptions): PlotData {
-    return basicPlot(series, options, {type: 'bar'});
+    // If the X axis is not from numerical column, treat it as category.
+    const data = basicPlot(series, options, {type: 'bar'});
+    const useCategory = series[0]?.pureType && !['Numeric', 'Int', 'Any'].includes(series[0].pureType);
+    const xaxisName = options.orientation === 'h' ? 'yaxis' : 'xaxis';
+    if (useCategory && data.layout && data.layout[xaxisName]) {
+      const axisConfig = data.layout[xaxisName]!;
+      axisConfig.type = 'category';
+    }
+    return data;
   },
   line(series: Series[], options: ChartOptions): PlotData {
     sortByXValues(series);
diff --git a/test/nbrowser/ChartView1.ts b/test/nbrowser/ChartView1.ts
index 337431e1..b78e02f9 100644
--- a/test/nbrowser/ChartView1.ts
+++ b/test/nbrowser/ChartView1.ts
@@ -21,6 +21,47 @@ describe('ChartView1', function() {
   gu.bigScreen();
   afterEach(() => gu.checkForErrors());
 
+  it('should treat text as category', async function() {
+    const revert = await gu.begin();
+    await gu.sendActions([
+      ['AddTable', 'Text', [
+        {id: 'X', type: 'Int'},
+        {id: 'Y', type: 'Int'}
+      ]],
+      ['AddRecord', 'Text', null, {X: 1, Y: 1}],
+      ['AddRecord', 'Text', null, {X: 100, Y: 2}],
+      ['AddRecord', 'Text', null, {X: 101, Y: 3}],
+      ['AddRecord', 'Text', null, {X: 102, Y: 4}],
+    ]);
+    await gu.openPage('Text');
+    await gu.addNewSection('Chart', 'Text');
+
+    const layout = () => getChartData().then(d => d.layout);
+
+    await gu.waitToPass(async () => {
+      // Check to make sure initial values are correct.
+      assert.deepEqual((await layout()).xaxis.type, 'linear');
+    });
+
+    // Now convert X to text.
+    await gu.sendActions([
+      ['ModifyColumn', 'Text', 'X', {type: 'Text'}],
+    ]);
+    assert.deepEqual((await layout()).xaxis.type, 'category');
+
+    // Invert the chart.
+    await gu.selectSectionByTitle('Text Chart');
+    await gu.toggleSidePanel('right', 'open');
+    await driver.find('.test-chart-orientation .test-select-open').click();
+    await driver.findContent('.test-select-menu', 'Horizontal').click();
+    await gu.waitForServer();
+    assert.deepEqual((await layout()).yaxis.type, 'category');
+    assert.deepEqual((await layout()).xaxis.type, 'linear');
+
+    await revert();
+    await gu.toggleSidePanel('right', 'close');
+  });
+
   it('should allow adding and removing chart viewsections', async function() {
     // Starting out with one section
     assert.lengthOf(await driver.findAll('.test-gristdoc .view_leaf'), 1);
@@ -133,7 +174,7 @@ describe('ChartView1', function() {
 
   it('should update chart when new columns are included', async function() {
     const chartDom = await driver.find('.test-chart-container');
-    // Check to make sure intial values are correct.
+    // Check to make sure initial values are correct.
     let data = (await getChartData(chartDom)).data;
     assert.deepEqual(data[0].type, 'bar');
     assert.deepEqual(data[0].x, [ 61, 5, 4, 3, 2, 1 ]);