Skip to content
This repository has been archived by the owner on Jun 25, 2020. It is now read-only.

fix: single y axis bounds #148

Merged
merged 1 commit into from
Jul 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 49 additions & 11 deletions packages/superset-ui-legacy-preset-chart-nvd3/src/NVD3Vis.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ import isTruthy from './utils/isTruthy';
import {
cleanColorInput,
computeBarChartWidth,
computeYDomain,
computeStackedYDomain,
drawBarValues,
generateBubbleTooltipContent,
generateMultiLineTooltipContent,
Expand Down Expand Up @@ -440,7 +442,7 @@ function nvd3Vis(element, props) {

if (showBarValue) {
drawBarValues(svg, data, isBarStacked, yAxisFormat);
chart.dispatch.on('stateChange', () => {
chart.dispatch.on('stateChange.drawBarValues', () => {
drawBarValues(svg, data, isBarStacked, yAxisFormat);
});
}
Expand Down Expand Up @@ -607,16 +609,52 @@ function nvd3Vis(element, props) {
xTicks.selectAll('text').attr('dx', -6.5);
}

// Apply y-axis bounds
if (chart.yDomain && Array.isArray(yAxisBounds) && yAxisBounds.length === 2) {
const [min, max] = yAxisBounds;
const hasCustomMin = isDefined(min) && !Number.isNaN(min);
const hasCustomMax = isDefined(max) && !Number.isNaN(max);
if (hasCustomMin && hasCustomMax) {
chart.yDomain([min, max]);
chart.clipEdge(true);
const applyYAxisBounds = () => {
if (chart.yDomain && Array.isArray(yAxisBounds) && yAxisBounds.length === 2) {
const [customMin, customMax] = yAxisBounds;
const hasCustomMin = isDefined(customMin) && !Number.isNaN(customMin);
const hasCustomMax = isDefined(customMax) && !Number.isNaN(customMax);

if ((hasCustomMin || hasCustomMax) && vizType === 'area' && chart.style() === 'expand') {
// Because there are custom bounds, we need to override them back to 0%-100% since this
// is an expanded area chart
chart.yDomain([0, 1]);
} else if (
(hasCustomMin || hasCustomMax) &&
vizType === 'area' &&
chart.style() === 'stream'
) {
// Because there are custom bounds, we need to override them back to the domain of the
// data since this is a stream area chart
chart.yDomain(computeStackedYDomain(data));
} else if (hasCustomMin && hasCustomMax) {
// Override the y domain if there's both a custom min and max
chart.yDomain([customMin, customMax]);
chart.clipEdge(true);
} else if (hasCustomMin || hasCustomMax) {
// Only one of the bounds has been set, so we need to manually calculate the other one
let [trueMin, trueMax] = [0, 1];

// These viz types can be stacked
// They correspond to the nvd3 stackedAreaChart and multiBarChart
if (vizType === 'area' || (isVizTypes(['bar', 'dist_bar']) && chart.stacked())) {
// This is a stacked area chart or a stacked bar chart
[trueMin, trueMax] = computeStackedYDomain(data);
} else {
[trueMin, trueMax] = computeYDomain(data);
}

const min = hasCustomMin ? customMin : trueMin;
const max = hasCustomMax ? customMax : trueMax;
chart.yDomain([min, max]);
chart.clipEdge(true);
}
}
}
};
applyYAxisBounds();

// Also reapply on each state change to account for enabled/disabled series
chart.dispatch.on('stateChange.applyYAxisBounds', applyYAxisBounds);

// align yAxis1 and yAxis2 ticks
if (isVizTypes(['dual_line', 'line_multi'])) {
Expand Down Expand Up @@ -659,7 +697,7 @@ function nvd3Vis(element, props) {

// redo on legend toggle; nvd3 calls the callback *before* the line is
// drawn, so we need to add a small delay here
chart.dispatch.on('stateChange', () => {
chart.dispatch.on('stateChange.showMarkers', () => {
setTimeout(() => {
svg
.selectAll('.nv-point')
Expand Down
13 changes: 12 additions & 1 deletion packages/superset-ui-legacy-preset-chart-nvd3/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ export function setAxisShowMaxMin(axis, showminmax) {

export function computeYDomain(data) {
if (Array.isArray(data) && data.length > 0 && Array.isArray(data[0].values)) {
const extents = data.map(row => d3.extent(row.values, v => v.y));
const extents = data.filter(d => !d.disabled).map(row => d3.extent(row.values, v => v.y));
const minOfMin = d3.min(extents, ([min]) => min);
const maxOfMax = d3.max(extents, ([, max]) => max);

Expand All @@ -371,3 +371,14 @@ export function computeYDomain(data) {

return [0, 1];
}

export function computeStackedYDomain(data) {
if (Array.isArray(data) && data.length > 0 && Array.isArray(data[0].values)) {
const series = data.filter(d => !d.disabled).map(d => d.values.map(v => v.y));
const stackedValues = series[0].map((_, i) => series.reduce((acc, cur) => acc + cur[i], 0));

return [Math.min(0, ...stackedValues), Math.max(0, ...stackedValues)];
}

return [0, 1];
}
122 changes: 121 additions & 1 deletion packages/superset-ui-legacy-preset-chart-nvd3/test/utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,99 @@
* specific language governing permissions and limitations
* under the License.
*/
import { getTimeOrNumberFormatter, formatLabel, tryNumify } from '../src/utils';
import {
computeStackedYDomain,
computeYDomain,
getTimeOrNumberFormatter,
formatLabel,
tryNumify,
} from '../src/utils';

const DATA = [
{
key: ['East Asia & Pacific'],
values: [
{
x: -315619200000.0,
y: 1031863394.0,
},
{
x: -283996800000.0,
y: 1034767718.0,
},
],
},
{
key: ['South Asia'],
values: [
{
x: -315619200000.0,
y: 572036107.0,
},
{
x: -283996800000.0,
y: 584143236.0,
},
],
},
{
key: ['Europe & Central Asia'],
values: [
{
x: -315619200000.0,
y: 660881033.0,
},
{
x: -283996800000.0,
y: 668526708.0,
},
],
},
];

const DATA_WITH_DISABLED_SERIES = [
{
disabled: true,
key: ['East Asia & Pacific'],
values: [
{
x: -315619200000.0,
y: 1031863394.0,
},
{
x: -283996800000.0,
y: 1034767718.0,
},
],
},
{
disabled: true,
key: ['South Asia'],
values: [
{
x: -315619200000.0,
y: 572036107.0,
},
{
x: -283996800000.0,
y: 584143236.0,
},
],
},
{
key: ['Europe & Central Asia'],
values: [
{
x: -315619200000.0,
y: 660881033.0,
},
{
x: -283996800000.0,
y: 668526708.0,
},
],
},
];

describe('nvd3/utils', () => {
describe('getTimeOrNumberFormatter(format)', () => {
Expand Down Expand Up @@ -66,4 +158,32 @@ describe('nvd3/utils', () => {
expect(tryNumify('a string')).toBe('a string');
});
});

describe('computeYDomain()', () => {
it('works with invalid data', () => {
expect(computeYDomain('foo')).toEqual([0, 1]);
});

it('works with all series enabled', () => {
expect(computeYDomain(DATA)).toEqual([572036107.0, 1034767718.0]);
});

it('works with some series disabled', () => {
expect(computeYDomain(DATA_WITH_DISABLED_SERIES)).toEqual([660881033.0, 668526708.0]);
});
});

describe('computeStackedYDomain()', () => {
it('works with invalid data', () => {
expect(computeStackedYDomain('foo')).toEqual([0, 1]);
});

it('works with all series enabled', () => {
expect(computeStackedYDomain(DATA)).toEqual([0, 2287437662.0]);
});

it('works with some series disabled', () => {
expect(computeStackedYDomain(DATA_WITH_DISABLED_SERIES)).toEqual([0, 668526708.0]);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export default [
metrics: ['sum__SP_POP_TOTL'],
richTooltip: true,
showBrush: 'auto',
showControls: false,
showControls: true,
showLegend: true,
stackedStyle: 'stack',
vizType: 'area',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,22 @@ export default [
width: 400,
}}
/>
<pre>yAxisBounds=[40000, null] with Legend</pre>
<SuperChart
chartType="line"
chartProps={{
datasource: { verboseMap: {} },
formData: {
richTooltip: true,
showLegend: true,
vizType: 'line',
yAxisBounds: [40000, null],
},
height: 200,
payload: { data },
width: 400,
}}
/>
</div>
),
storyName: 'yAxisBounds',
Expand Down