You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by el...@apache.org on 2022/09/20 20:50:17 UTC
[superset] 07/29: fix(plugin-chart-echarts): invalid total label location for negative values in stacked bar chart (#21032)
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch 2.0-test
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 2db82d578c10abc14fde3936102049b3919f5994
Author: JUST.in DO IT <ju...@airbnb.com>
AuthorDate: Thu Aug 11 11:28:18 2022 -0700
fix(plugin-chart-echarts): invalid total label location for negative values in stacked bar chart (#21032)
(cherry picked from commit a8ba544e609ad3af449239c1fb956bb18c7066c4)
---
.../plugin-chart-echarts/Timeseries/Stories.tsx | 35 +++++-
.../Timeseries/negativeNumData.ts | 111 +++++++++++++++++++
.../src/Timeseries/transformProps.ts | 2 +
.../src/Timeseries/transformers.ts | 5 +-
.../plugin-chart-echarts/src/utils/series.ts | 15 ++-
.../plugin-chart-echarts/test/utils/series.test.ts | 119 +++++++++++++++++++++
6 files changed, 284 insertions(+), 3 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-demo/storybook/stories/plugins/plugin-chart-echarts/Timeseries/Stories.tsx b/superset-frontend/packages/superset-ui-demo/storybook/stories/plugins/plugin-chart-echarts/Timeseries/Stories.tsx
index 3f219ed6e6..b44ba252ea 100644
--- a/superset-frontend/packages/superset-ui-demo/storybook/stories/plugins/plugin-chart-echarts/Timeseries/Stories.tsx
+++ b/superset-frontend/packages/superset-ui-demo/storybook/stories/plugins/plugin-chart-echarts/Timeseries/Stories.tsx
@@ -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()
@@ -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,
@@ -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',
+ ),
+ }}
+ />
+);
diff --git a/superset-frontend/packages/superset-ui-demo/storybook/stories/plugins/plugin-chart-echarts/Timeseries/negativeNumData.ts b/superset-frontend/packages/superset-ui-demo/storybook/stories/plugins/plugin-chart-echarts/Timeseries/negativeNumData.ts
new file mode 100644
index 0000000000..8dc0f7e9b9
--- /dev/null
+++ b/superset-frontend/packages/superset-ui-demo/storybook/stories/plugins/plugin-chart-echarts/Timeseries/negativeNumData.ts
@@ -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,
+ },
+];
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
index ca0e079609..d96fe1b9b0 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -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),
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts
index 93565de46c..420662647d 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts
@@ -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 '';
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
index 23710cd6d1..8d93ce3f34 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
@@ -77,6 +77,8 @@ export function extractShowValueIndexes(
series: SeriesOption[],
opts: {
stack: StackType;
+ onlyTotal?: boolean;
+ isHorizontal?: boolean;
},
): number[] {
const showValueIndexes: number[] = [];
@@ -84,9 +86,20 @@ export function extractShowValueIndexes(
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;
+ }
+ }
});
});
}
diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
index ae3871c821..3f20e0d5f8 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
@@ -25,6 +25,7 @@ import {
getChartPadding,
getLegendProps,
sanitizeHtml,
+ extractShowValueIndexes,
} from '../../src/utils/series';
import { LegendOrientation, LegendType } from '../../src/types';
import { defaultLegendPadding } from '../../src/defaults';
@@ -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();