Skip to content

Commit

Permalink
fix(plugin-chart-echarts): invalid total label location for negative …
Browse files Browse the repository at this point in the history
…values in stacked bar chart (apache#21032)

(cherry picked from commit a8ba544)
  • Loading branch information
justinpark authored and Fahrenheit35 committed Nov 11, 2022
1 parent 23bc02f commit e8e06b2
Show file tree
Hide file tree
Showing 6 changed files with 284 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
TimeseriesTransformProps,
} from '@superset-ui/plugin-chart-echarts';
import data from './data';
import negativeNumData from './negativeNumData';
import { withResizableChartDemo } from '../../../../shared/components/ResizableChartDemo';

new EchartsTimeseriesChartPlugin()
Expand Down Expand Up @@ -61,7 +62,9 @@ export const Timeseries = ({ width, height }) => {
chartType="echarts-timeseries"
width={width}
height={height}
queriesData={[{ data: queryData }]}
queriesData={[
{ data: queryData, colnames: ['__timestamp'], coltypes: [2] },
]}
formData={{
contributionMode: undefined,
forecastEnabled,
Expand All @@ -87,3 +90,33 @@ export const Timeseries = ({ width, height }) => {
/>
);
};

export const WithNegativeNumbers = ({ width, height }) => (
<SuperChart
chartType="echarts-timeseries"
width={width}
height={height}
queriesData={[
{ data: negativeNumData, colnames: ['__timestamp'], coltypes: [2] },
]}
formData={{
contributionMode: undefined,
colorScheme: 'supersetColors',
seriesType: select(
'Line type',
['line', 'scatter', 'smooth', 'bar', 'start', 'middle', 'end'],
'line',
),
yAxisFormat: '$,.2f',
stack: boolean('Stack', true),
showValue: true,
showLegend: true,
onlyTotal: boolean('Only Total', true),
orientation: select(
'Orientation',
['vertical', 'horizontal'],
'vertical',
),
}}
/>
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

export default [
{
__timestamp: 1619827200000,
Boston: -0.88,
NewYork: null,
Washington: -0.3,
JerseyCity: -3.05,
Denver: -8.25,
SF: -0.13,
},
{
__timestamp: 1622505600000,
Boston: -0.81,
NewYork: null,
Washington: -0.29,
JerseyCity: -3.54,
Denver: -13.4,
SF: -0.12,
},
{
__timestamp: 1625097600000,
Boston: 0.91,
NewYork: null,
Washington: 0.25,
JerseyCity: 7.17,
Denver: 7.69,
SF: 0.05,
},
{
__timestamp: 1627776000000,
Boston: -1.05,
NewYork: -1.04,
Washington: -0.19,
JerseyCity: -8.99,
Denver: -7.99,
SF: -0.01,
},
{
__timestamp: 1630454400000,
Boston: -0.92,
NewYork: -1.09,
Washington: -0.17,
JerseyCity: -8.75,
Denver: -7.55,
SF: -0.01,
},
{
__timestamp: 1633046400000,
Boston: 0.79,
NewYork: -0.85,
Washington: 0.13,
JerseyCity: 12.59,
Denver: 3.34,
SF: -0.05,
},
{
__timestamp: 1635724800000,
Boston: 0.72,
NewYork: 0.54,
Washington: 0.15,
JerseyCity: 11.03,
Denver: 7.24,
SF: -0.14,
},
{
__timestamp: 1638316800000,
Boston: 0.61,
NewYork: 0.73,
Washington: 0.15,
JerseyCity: 13.45,
Denver: 5.98,
SF: -0.22,
},
{
__timestamp: 1640995200000,
Boston: 0.51,
NewYork: 1.8,
Washington: 0.15,
JerseyCity: 12.96,
Denver: 3.22,
SF: -0.02,
},
{
__timestamp: 1643673600000,
Boston: -0.47,
NewYork: null,
Washington: -0.18,
JerseyCity: -14.27,
Denver: -6.24,
SF: -0.04,
},
];
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ export default function transformProps(
});
const showValueIndexes = extractShowValueIndexes(rawSeries, {
stack,
onlyTotal,
isHorizontal,
});
const seriesContexts = extractForecastSeriesContexts(
Object.values(rawSeries).map(series => series.name as string),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,10 @@ export function transformSeries(
if (!formatter) return numericValue;
if (!stack || isSelectedLegend) return formatter(numericValue);
if (!onlyTotal) {
if (numericValue >= thresholdValues[dataIndex]) {
if (
numericValue >=
(thresholdValues[dataIndex] || Number.MIN_SAFE_INTEGER)
) {
return formatter(numericValue);
}
return '';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,16 +77,29 @@ export function extractShowValueIndexes(
series: SeriesOption[],
opts: {
stack: StackType;
onlyTotal?: boolean;
isHorizontal?: boolean;
},
): number[] {
const showValueIndexes: number[] = [];
if (opts.stack) {
series.forEach((entry, seriesIndex) => {
const { data = [] } = entry;
(data as [any, number][]).forEach((datum, dataIndex) => {
if (datum[1] !== null) {
if (!opts.onlyTotal && datum[opts.isHorizontal ? 0 : 1] !== null) {
showValueIndexes[dataIndex] = seriesIndex;
}
if (opts.onlyTotal) {
if (datum[opts.isHorizontal ? 0 : 1] > 0) {
showValueIndexes[dataIndex] = seriesIndex;
}
if (
!showValueIndexes[dataIndex] &&
datum[opts.isHorizontal ? 0 : 1] !== null
) {
showValueIndexes[dataIndex] = seriesIndex;
}
}
});
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
getChartPadding,
getLegendProps,
sanitizeHtml,
extractShowValueIndexes,
} from '../../src/utils/series';
import { LegendOrientation, LegendType } from '../../src/types';
import { defaultLegendPadding } from '../../src/defaults';
Expand Down Expand Up @@ -206,6 +207,124 @@ describe('extractGroupbyLabel', () => {
});
});

describe('extractShowValueIndexes', () => {
it('should return the latest index for stack', () => {
expect(
extractShowValueIndexes(
[
{
id: 'abc',
name: 'abc',
data: [
['2000-01-01', null],
['2000-02-01', 0],
['2000-03-01', 1],
['2000-04-01', 0],
['2000-05-01', null],
['2000-06-01', 0],
['2000-07-01', 2],
['2000-08-01', 3],
['2000-09-01', null],
['2000-10-01', null],
],
},
{
id: 'def',
name: 'def',
data: [
['2000-01-01', null],
['2000-02-01', 0],
['2000-03-01', null],
['2000-04-01', 0],
['2000-05-01', null],
['2000-06-01', 0],
['2000-07-01', 2],
['2000-08-01', 3],
['2000-09-01', null],
['2000-10-01', 0],
],
},
{
id: 'def',
name: 'def',
data: [
['2000-01-01', null],
['2000-02-01', null],
['2000-03-01', null],
['2000-04-01', null],
['2000-05-01', null],
['2000-06-01', 3],
['2000-07-01', null],
['2000-08-01', null],
['2000-09-01', null],
['2000-10-01', null],
],
},
],
{ stack: true, onlyTotal: false, isHorizontal: false },
),
).toEqual([undefined, 1, 0, 1, undefined, 2, 1, 1, undefined, 1]);
});

it('should handle the negative numbers for total only', () => {
expect(
extractShowValueIndexes(
[
{
id: 'abc',
name: 'abc',
data: [
['2000-01-01', null],
['2000-02-01', 0],
['2000-03-01', -1],
['2000-04-01', 0],
['2000-05-01', null],
['2000-06-01', 0],
['2000-07-01', -2],
['2000-08-01', -3],
['2000-09-01', null],
['2000-10-01', null],
],
},
{
id: 'def',
name: 'def',
data: [
['2000-01-01', null],
['2000-02-01', 0],
['2000-03-01', null],
['2000-04-01', 0],
['2000-05-01', null],
['2000-06-01', 0],
['2000-07-01', 2],
['2000-08-01', -3],
['2000-09-01', null],
['2000-10-01', 0],
],
},
{
id: 'def',
name: 'def',
data: [
['2000-01-01', null],
['2000-02-01', 0],
['2000-03-01', null],
['2000-04-01', 1],
['2000-05-01', null],
['2000-06-01', 0],
['2000-07-01', -2],
['2000-08-01', 3],
['2000-09-01', null],
['2000-10-01', 0],
],
},
],
{ stack: true, onlyTotal: true, isHorizontal: false },
),
).toEqual([undefined, 1, 0, 2, undefined, 1, 1, 2, undefined, 1]);
});
});

describe('formatSeriesName', () => {
const numberFormatter = getNumberFormatter();
const timeFormatter = getTimeFormatter();
Expand Down

0 comments on commit e8e06b2

Please sign in to comment.