(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
This commit is contained in:
Cyprien P 2021-12-20 12:21:27 +01:00
parent e0fb281eba
commit e99cc3ae08

View File

@ -147,6 +147,9 @@ export class ChartView extends Disposable {
private _formatterComp: ko.Computed<BaseFormatter|undefined>; private _formatterComp: ko.Computed<BaseFormatter|undefined>;
// peek section's sort spec
private get _sortSpec() { return this.viewSection.activeSortSpec.peek(); }
public create(gristDoc: GristDoc, viewSectionModel: ViewSectionRec) { public create(gristDoc: GristDoc, viewSectionModel: ViewSectionRec) {
BaseView.call(this as any, gristDoc, viewSectionModel); BaseView.call(this as any, gristDoc, viewSectionModel);
@ -237,9 +240,14 @@ export class ChartView extends Disposable {
const options: ChartOptions = this._options.peek() || {}; const options: ChartOptions = this._options.peek() || {};
let plotData: PlotData = {data: []}; let plotData: PlotData = {data: []};
const sortSpec = this.viewSection.activeSortSpec.peek(); if (isPieLike(this._chartType.peek())) {
if (isPieLike(this._chartType.peek()) && sortSpec?.length) {
// Plotly's pie charts have a sort option that is enabled by default. Let's turn it off.
dataOptions.sort = false; 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') { 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 // following the sorting order. This line fixes that issue by consolidating all series to
// have at least on entry of each x values. // have at least on entry of each x values.
if (this._chartType.peek() === 'bar') { if (this._chartType.peek() === 'bar') {
if (sortSpec?.length) { consolidateValues(gSeries, xvalues); } if (this._sortSpec?.length) { consolidateValues(gSeries, xvalues); }
} }
const part = chartFunc(gSeries, options, dataOptions); const part = chartFunc(gSeries, options, dataOptions);