From e99cc3ae089c3f1e571a843fddeb90d3398d6773 Mon Sep 17 00:00:00 2001 From: Cyprien P Date: Mon, 20 Dec 2021 12:21:27 +0100 Subject: [PATCH] (core) Fix pie chart sectors's ordering issue. Summary: By default, plotly's pie chart sort lables by values. This is iconsistent with how bar charts work and especially annoying in case of a linked chart because values can change when user navigate the linked table which causes colors (and display order) of each label to change. Making it hard to keep track values. [[ https://grist.quip.com/wb4gAgrQM2aP/Chart-Improvements-November-2021#temp:C:TZE88067825d66c415da9e839488 | Link to video with more details about the issue ]] Test Plan: Adds a new test case. Reviewers: georgegevoian Reviewed By: georgegevoian Differential Revision: https://phab.getgrist.com/D3193 --- app/client/components/ChartView.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/app/client/components/ChartView.ts b/app/client/components/ChartView.ts index c40ae0d0..fb8413ed 100644 --- a/app/client/components/ChartView.ts +++ b/app/client/components/ChartView.ts @@ -147,6 +147,9 @@ export class ChartView extends Disposable { private _formatterComp: ko.Computed; + // peek section's sort spec + private get _sortSpec() { return this.viewSection.activeSortSpec.peek(); } + public create(gristDoc: GristDoc, viewSectionModel: ViewSectionRec) { BaseView.call(this as any, gristDoc, viewSectionModel); @@ -237,9 +240,14 @@ export class ChartView extends Disposable { const options: ChartOptions = this._options.peek() || {}; let plotData: PlotData = {data: []}; - const sortSpec = this.viewSection.activeSortSpec.peek(); - if (isPieLike(this._chartType.peek()) && sortSpec?.length) { + if (isPieLike(this._chartType.peek())) { + + // Plotly's pie charts have a sort option that is enabled by default. Let's turn it off. dataOptions.sort = false; + + // This line is for labels to stay in order when value changes, which can happen when using + // charts with linked list. + sortByXValues(series); } if (this._chartType.peek() === 'donut') { @@ -262,7 +270,7 @@ export class ChartView extends Disposable { // following the sorting order. This line fixes that issue by consolidating all series to // have at least on entry of each x values. if (this._chartType.peek() === 'bar') { - if (sortSpec?.length) { consolidateValues(gSeries, xvalues); } + if (this._sortSpec?.length) { consolidateValues(gSeries, xvalues); } } const part = chartFunc(gSeries, options, dataOptions);