mirror of
https://github.com/elastic/kibana.git
synced 2025-04-23 17:28:26 -04:00
Backport PR #8538
--------- **Commit 1:** [agg_response/point_series] always assign series labels to points Since forever, the vislib has been compensating for a behavior/bug in the agg_response module where "series" labels were not assigned to points if there wasn't a "series" aggregation or multiple y-axis. To compensate for this, the vislib was assigning the yAxisLabel to the series, which had the same effect, but now that the legend is rendered in it's own tick, followed by the visualization, and that behavior was being executed during the vislib render, the legend has been flashing an empty row for a while (this was worsened by the fact that vislib rendering did not trigger a digest cycle). These changes remove the workaround code from the vislib, and update the agg_response to always send the y-axis label as the series label when there is no series aggregation. Behavior with multiple-y-axis and multiple series aggs should be preserved. * Original sha:7460c0ac2d* Authored by spalger <email@spalger.com> on 2016-10-04T18:40:41Z **Commit 2:** [agg_response/point_series] update tests to reflect changes in 19773a6 * Original sha:f681d6cf86* Authored by spalger <email@spalger.com> on 2016-10-04T19:22:55Z **Commit 3:** [agg_response/point_series] apply PR feedback * Original sha:a119577af8* Authored by spalger <email@spalger.com> on 2016-10-04T20:41:29Z **Commit 4:** [agg_response/point_series] use no-digest promises in tests * Original sha:8d370826c4* Authored by spalger <email@spalger.com> on 2016-10-04T20:45:02Z
This commit is contained in:
Elastic Jasper
spalger
parent
aad5e9a5fe
commit
6a05615b8c
13 changed files with 40 additions and 124 deletions
|
|
@ -18,13 +18,15 @@ describe('getPoint', function () {
|
|||
describe('Without series aspect', function () {
|
||||
let seriesAspect;
|
||||
let xAspect;
|
||||
let yCol;
|
||||
let yAspect;
|
||||
let yScale;
|
||||
|
||||
beforeEach(function () {
|
||||
seriesAspect = null;
|
||||
xAspect = { i: 0 };
|
||||
yAspect = { i: 1 };
|
||||
yCol = { title: 'Y', aggConfig: {} };
|
||||
yAspect = { i: 1, col: yCol };
|
||||
yScale = 5;
|
||||
});
|
||||
|
||||
|
|
@ -37,8 +39,8 @@ describe('getPoint', function () {
|
|||
.to.have.property('x', 1)
|
||||
.and.have.property('y', 10)
|
||||
.and.have.property('z', 3)
|
||||
.and.have.property('aggConfigResult', row[1])
|
||||
.and.not.have.property('series');
|
||||
.and.have.property('series', yCol.title)
|
||||
.and.have.property('aggConfigResult', row[1]);
|
||||
});
|
||||
|
||||
it('ignores points with a y value of NaN', function () {
|
||||
|
|
|
|||
|
|
@ -27,10 +27,11 @@ describe('getSeries', function () {
|
|||
[1, 2, 3]
|
||||
].map(wrapRows);
|
||||
|
||||
const yCol = { aggConfig: {}, title: 'y' };
|
||||
let chart = {
|
||||
aspects: {
|
||||
x: { i: 0 },
|
||||
y: { i: 1 },
|
||||
y: { i: 1, col: yCol },
|
||||
z: { i: 2 }
|
||||
}
|
||||
};
|
||||
|
|
@ -44,7 +45,7 @@ describe('getSeries', function () {
|
|||
let siri = series[0];
|
||||
expect(siri)
|
||||
.to.be.an('object')
|
||||
.and.have.property('label', '')
|
||||
.and.have.property('label', yCol.title)
|
||||
.and.have.property('values');
|
||||
|
||||
expect(siri.values)
|
||||
|
|
|
|||
|
|
@ -57,7 +57,7 @@ describe('pointSeriesChartDataFromTable', function () {
|
|||
|
||||
let y = {
|
||||
agg: vis.aggs[0],
|
||||
col: { aggConfig: vis.aggs[0] },
|
||||
col: { aggConfig: vis.aggs[0], title: vis.aggs[0].makeLabel() },
|
||||
at: function (i) { return 100 * i; }
|
||||
};
|
||||
|
||||
|
|
@ -81,6 +81,7 @@ describe('pointSeriesChartDataFromTable', function () {
|
|||
expect(chartData.series).to.be.an('array');
|
||||
expect(chartData.series).to.have.length(1);
|
||||
let series = chartData.series[0];
|
||||
expect(series).to.have.property('label', y.col.title);
|
||||
expect(series.values).to.have.length(rowCount);
|
||||
series.values.forEach(function (point, i) {
|
||||
expect(point)
|
||||
|
|
@ -152,8 +153,7 @@ describe('pointSeriesChartDataFromTable', function () {
|
|||
|
||||
expect(point).to.have.property('y');
|
||||
expect(point.y).to.be.a('number');
|
||||
|
||||
expect(point).to.not.have.property('series');
|
||||
expect(point).to.have.property('series', siri.label);
|
||||
|
||||
expect(point).to.have.property('aggConfigResult');
|
||||
expect(point.aggConfigResult)
|
||||
|
|
|
|||
|
|
@ -27,6 +27,11 @@ export default function PointSeriesGetPoint() {
|
|||
if (series) {
|
||||
point.aggConfig = series.agg;
|
||||
point.series = series.agg.fieldFormatter()(unwrap(row[series.i]));
|
||||
} else if (y) {
|
||||
// If the data is not split up with a series aspect, then
|
||||
// each point's "series" becomes the y-agg that produced it
|
||||
point.aggConfig = y.col.aggConfig;
|
||||
point.series = y.col.title;
|
||||
}
|
||||
|
||||
if (yScale) {
|
||||
|
|
|
|||
|
|
@ -23,9 +23,17 @@ export default function PointSeriesGetSeries(Private) {
|
|||
let point = partGetPoint(row, y, aspects.z);
|
||||
if (!point) return;
|
||||
|
||||
let prefix = point.series ? point.series + ': ' : '';
|
||||
let seriesId = prefix + y.agg.id;
|
||||
let seriesLabel = prefix + y.col.title;
|
||||
// use the point's y-axis as it's series by default,
|
||||
// but augment that with series aspect if it's actually
|
||||
// available
|
||||
let seriesId = y.agg.id;
|
||||
let seriesLabel = y.col.title;
|
||||
|
||||
if (aspects.series) {
|
||||
const prefix = point.series ? point.series + ': ' : '';
|
||||
seriesId = prefix + seriesId;
|
||||
seriesLabel = prefix + seriesLabel;
|
||||
}
|
||||
|
||||
addToSiri(series, point, seriesId, seriesLabel);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -19,7 +19,6 @@ describe('Vislib AxisTitle Class Test Suite', function () {
|
|||
let yTitle;
|
||||
const data = {
|
||||
hits: 621,
|
||||
label: '',
|
||||
ordered: {
|
||||
date: true,
|
||||
interval: 30000,
|
||||
|
|
@ -28,6 +27,7 @@ describe('Vislib AxisTitle Class Test Suite', function () {
|
|||
},
|
||||
series: [
|
||||
{
|
||||
label: 'Count',
|
||||
values: [
|
||||
{
|
||||
x: 1408734060000,
|
||||
|
|
|
|||
|
|
@ -17,7 +17,6 @@ describe('Vislib ChartTitle Class Test Suite', function () {
|
|||
let dataObj;
|
||||
const data = {
|
||||
hits: 621,
|
||||
label: '',
|
||||
ordered: {
|
||||
date: true,
|
||||
interval: 30000,
|
||||
|
|
@ -26,6 +25,7 @@ describe('Vislib ChartTitle Class Test Suite', function () {
|
|||
},
|
||||
series: [
|
||||
{
|
||||
label: 'Count',
|
||||
values: [
|
||||
{
|
||||
x: 1408734060000,
|
||||
|
|
|
|||
|
|
@ -120,84 +120,6 @@ describe('Vislib Data Class Test Suite', function () {
|
|||
const rowIn = new Data(rowsData, {}, persistedState);
|
||||
expect(_.isObject(rowIn)).to.be(true);
|
||||
});
|
||||
|
||||
it('should update label in series data', function () {
|
||||
const seriesDataWithoutLabelInSeries = {
|
||||
'label': '',
|
||||
'series': [
|
||||
{
|
||||
'label': '',
|
||||
'values': [{x: 0, y: 1}, {x: 1, y: 2}, {x: 2, y: 3}]
|
||||
}
|
||||
],
|
||||
'yAxisLabel': 'customLabel'
|
||||
};
|
||||
const modifiedData = new Data(seriesDataWithoutLabelInSeries, {}, persistedState);
|
||||
expect(modifiedData.data.series[0].label).to.be('customLabel');
|
||||
});
|
||||
|
||||
it('should update label in row data', function () {
|
||||
const seriesDataWithoutLabelInRow = {
|
||||
'rows': [
|
||||
{
|
||||
'label': '',
|
||||
'series': [
|
||||
{
|
||||
'label': '',
|
||||
'values': [{x: 0, y: 1}, {x: 1, y: 2}, {x: 2, y: 3}]
|
||||
}
|
||||
],
|
||||
'yAxisLabel': 'customLabel'
|
||||
},
|
||||
{
|
||||
'label': '',
|
||||
'series': [
|
||||
{
|
||||
'label': '',
|
||||
'values': [{x: 0, y: 1}, {x: 1, y: 2}, {x: 2, y: 3}]
|
||||
}
|
||||
],
|
||||
'yAxisLabel': 'customLabel'
|
||||
}
|
||||
],
|
||||
};
|
||||
|
||||
const modifiedData = new Data(seriesDataWithoutLabelInRow, {}, persistedState);
|
||||
expect(modifiedData.data.rows[0].series[0].label).to.be('customLabel');
|
||||
expect(modifiedData.data.rows[1].series[0].label).to.be('customLabel');
|
||||
});
|
||||
|
||||
it('should update label in column data', function () {
|
||||
const seriesDataWithoutLabelInRow = {
|
||||
'columns': [
|
||||
{
|
||||
'label': '',
|
||||
'series': [
|
||||
{
|
||||
'label': '',
|
||||
'values': [{x: 0, y: 1}, {x: 1, y: 2}, {x: 2, y: 3}]
|
||||
}
|
||||
],
|
||||
'yAxisLabel': 'customLabel'
|
||||
},
|
||||
{
|
||||
'label': '',
|
||||
'series': [
|
||||
{
|
||||
'label': '',
|
||||
'values': [{x: 0, y: 1}, {x: 1, y: 2}, {x: 2, y: 3}]
|
||||
}
|
||||
],
|
||||
'yAxisLabel': 'customLabel'
|
||||
}
|
||||
],
|
||||
'yAxisLabel': 'customLabel'
|
||||
};
|
||||
|
||||
const modifiedData = new Data(seriesDataWithoutLabelInRow, {}, persistedState);
|
||||
expect(modifiedData.data.columns[0].series[0].label).to.be('customLabel');
|
||||
expect(modifiedData.data.columns[1].series[0].label).to.be('customLabel');
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -11,7 +11,6 @@ describe('Vislib Column Layout Test Suite', function () {
|
|||
let el;
|
||||
const data = {
|
||||
hits: 621,
|
||||
label: '',
|
||||
ordered: {
|
||||
date: true,
|
||||
interval: 30000,
|
||||
|
|
@ -20,6 +19,7 @@ describe('Vislib Column Layout Test Suite', function () {
|
|||
},
|
||||
series: [
|
||||
{
|
||||
label: 'Count',
|
||||
values: [
|
||||
{
|
||||
x: 1408734060000,
|
||||
|
|
|
|||
|
|
@ -18,7 +18,6 @@ describe('Vislib xAxis Class Test Suite', function () {
|
|||
let dataObj;
|
||||
const data = {
|
||||
hits: 621,
|
||||
label: '',
|
||||
ordered: {
|
||||
date: true,
|
||||
interval: 30000,
|
||||
|
|
@ -27,6 +26,7 @@ describe('Vislib xAxis Class Test Suite', function () {
|
|||
},
|
||||
series: [
|
||||
{
|
||||
label: 'Count',
|
||||
values: [
|
||||
{
|
||||
x: 1408734060000,
|
||||
|
|
|
|||
|
|
@ -65,32 +65,8 @@ export default function DataFactory(Private) {
|
|||
}
|
||||
}
|
||||
|
||||
_updateData() {
|
||||
if (this.data.rows) {
|
||||
_.map(this.data.rows, this._updateDataSeriesLabel, this);
|
||||
} else if (this.data.columns) {
|
||||
_.map(this.data.columns, this._updateDataSeriesLabel, this);
|
||||
} else {
|
||||
this._updateDataSeriesLabel(this.data);
|
||||
}
|
||||
};
|
||||
|
||||
_updateDataSeriesLabel(eachData) {
|
||||
if (eachData.series) {
|
||||
eachData.series[0].label = this.get('yAxisLabel');
|
||||
}
|
||||
};
|
||||
|
||||
_getLabels(data) {
|
||||
if (this.type === 'series') {
|
||||
const noLabel = getLabels(data).length === 1 && getLabels(data)[0] === '';
|
||||
if (noLabel) {
|
||||
this._updateData();
|
||||
return [(this.get('yAxisLabel'))];
|
||||
}
|
||||
return getLabels(data);
|
||||
}
|
||||
return this.pieNames();
|
||||
return this.type === 'series' ? getLabels(data) : this.pieNames();
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -9,6 +9,8 @@ import VisRenderbotProvider from 'ui/vis/renderbot';
|
|||
import VislibVisTypeVislibRenderbotProvider from 'ui/vislib_vis_type/vislib_renderbot';
|
||||
import PersistedStatePersistedStateProvider from 'ui/persisted_state/persisted_state';
|
||||
import AggResponseIndexProvider from 'ui/agg_response/index';
|
||||
import noDigestPromises from 'test_utils/no_digest_promises';
|
||||
|
||||
describe('renderbot', function exportWrapper() {
|
||||
let vislib;
|
||||
let Vis;
|
||||
|
|
@ -119,6 +121,8 @@ describe('renderbot', function exportWrapper() {
|
|||
});
|
||||
|
||||
describe('render', function () {
|
||||
noDigestPromises.activateForSuite();
|
||||
|
||||
let vis = { type: mockVisType, isHierarchical: _.constant(false) };
|
||||
let $el = $('<div>testing</div>');
|
||||
let stubs = {};
|
||||
|
|
|
|||
|
|
@ -2,7 +2,8 @@ import _ from 'lodash';
|
|||
import VislibProvider from 'ui/vislib';
|
||||
import VisRenderbotProvider from 'ui/vis/renderbot';
|
||||
import VislibVisTypeBuildChartDataProvider from 'ui/vislib_vis_type/build_chart_data';
|
||||
module.exports = function VislibRenderbotFactory(Private) {
|
||||
module.exports = function VislibRenderbotFactory(Private, $injector) {
|
||||
const AngularPromise = $injector.get('Promise');
|
||||
let vislib = Private(VislibProvider);
|
||||
let Renderbot = Private(VisRenderbotProvider);
|
||||
let buildChartData = Private(VislibVisTypeBuildChartDataProvider);
|
||||
|
|
@ -46,11 +47,8 @@ module.exports = function VislibRenderbotFactory(Private) {
|
|||
VislibRenderbot.prototype.buildChartData = buildChartData;
|
||||
VislibRenderbot.prototype.render = function (esResponse) {
|
||||
this.chartData = this.buildChartData(esResponse);
|
||||
// to allow legend to render first (wait for angular digest cycle to complete)
|
||||
return new Promise((resolve, reject) => {
|
||||
setTimeout(() => {
|
||||
resolve(this.vislibVis.render(this.chartData, this.uiState));
|
||||
});
|
||||
return AngularPromise.delay(1).then(() => {
|
||||
this.vislibVis.render(this.chartData, this.uiState);
|
||||
});
|
||||
};
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue