Skip to content

Commit

Permalink
Fix 0 values in percentage mode (#15765)
Browse files Browse the repository at this point in the history
* percentage mode should not change the value of injected zeros

* ensure that zero values do not mutate after the call to the expand method

* pass fixed expand function to stack.offset
  • Loading branch information
scampi authored and timroes committed Apr 12, 2018
1 parent d8ddd27 commit d6d4ce8
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 2 deletions.
49 changes: 49 additions & 0 deletions src/ui/public/vislib/__tests__/visualizations/column_chart.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import histogramRows from 'fixtures/vislib/mock_data/histogram/_rows';
import stackedSeries from 'fixtures/vislib/mock_data/date_histogram/_stacked_series';
import { seriesMonthlyInterval } from 'fixtures/vislib/mock_data/date_histogram/_series_monthly_interval';
import { rowsSeriesWithHoles } from 'fixtures/vislib/mock_data/date_histogram/_rows_series_with_holes';
import rowsWithZeros from 'fixtures/vislib/mock_data/date_histogram/_rows';
import $ from 'jquery';
import FixturesVislibVisFixtureProvider from 'fixtures/vislib/_vis_fixture';
import 'ui/persisted_state';
Expand Down Expand Up @@ -191,6 +192,54 @@ dataTypesArray.forEach(function (dataType) {
});
});

describe('stackData method - data set with zeros in percentage mode', function () {
let vis;
let persistedState;
const visLibParams = {
type: 'histogram',
addLegend: true,
addTooltip: true,
mode: 'percentage',
zeroFill: true
};

beforeEach(ngMock.module('kibana'));
beforeEach(ngMock.inject(function (Private, $injector) {
vis = Private(FixturesVislibVisFixtureProvider)(visLibParams);
persistedState = new ($injector.get('PersistedState'))();
vis.on('brush', _.noop);
}));

afterEach(function () {
vis.destroy();
});

it('should not mutate the injected zeros', function () {
vis.render(seriesMonthlyInterval, persistedState);

expect(vis.handler.charts).to.have.length(1);
const chart = vis.handler.charts[0];
expect(chart.chartData.series).to.have.length(1);
const series = chart.chartData.series[0].values;
// with the interval set in seriesMonthlyInterval data, the point at x=1454309600000 does not exist
const point = _.find(series, 'x', 1454309600000);
expect(point).to.not.be(undefined);
expect(point.y).to.be(0);
});

it('should not mutate zeros that exist in the data', function () {
vis.render(rowsWithZeros, persistedState);

expect(vis.handler.charts).to.have.length(2);
const chart = vis.handler.charts[0];
expect(chart.chartData.series).to.have.length(5);
const series = chart.chartData.series[0].values;
const point = _.find(series, 'x', 1415826240000);
expect(point).to.not.be(undefined);
expect(point.y).to.be(0);
});
});

describe('datumWidth - split chart data set with holes', function () {
let vis;
let persistedState;
Expand Down
2 changes: 1 addition & 1 deletion src/ui/public/vislib/lib/axis/axis.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export function VislibLibAxisProvider(Private) {
return d.x;
})
.y(d => {
if (this.axisConfig.get('scale.offset') === 'expand') {
if (typeof this.axisConfig.get('scale.offset') === 'function' && this.axisConfig.get('scale.offset').name === 'expand') {
return Math.abs(d.y);
}
return d.y;
Expand Down
24 changes: 23 additions & 1 deletion src/ui/public/vislib/lib/axis/axis_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,29 @@ export function VislibLibAxisConfigProvider() {
stacked = false;
break;
case SCALE_MODES.PERCENTAGE:
offset = 'expand';
offset = function expand(data) {
// taken from https://github.com/d3/d3/blob/v3.5.6/src/layout/stack.js#L193
// fixed to support zeros
const n = data.length;
const m = data[0].length;
const y0 = [];

for (let j = 0; j < m; ++j) {
let o = 0;
for (let i = 0; i < n; i++) {
o += data[i][j][1];
}
if (o) {
for (let i = 0; i < n; i++) {
data[i][j][1] /= o;
}
}
}
for (let j = 0; j < m; ++j) {
y0[j] = 0;
}
return y0;
};
break;
default:
offset = this.get('scale.mode');
Expand Down

0 comments on commit d6d4ce8

Please sign in to comment.