Skip to content

changing vis.API.events #25280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ppisljar merged 3 commits into from
Nov 16, 2018
Merged

changing vis.API.events #25280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3cedf9e
moving filters out of Vis
ppisljar Nov 7, 2018
188fdd5
updating unit tests
ppisljar Nov 7, 2018
88e6af8
updating based on review
ppisljar Nov 8, 2018
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
3 changes: 3 additions & 0 deletions src/core_plugins/kbn_vislib_vis_types/public/gauge.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ export default function GaugeVisType(Private, i18n) {
}
},
},
events: {
brush: { disabled: true },
},
editorConfig: {
collections: {
gaugeTypes: ['Arc', 'Circle'],
Expand Down
3 changes: 3 additions & 0 deletions src/core_plugins/kbn_vislib_vis_types/public/goal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ export default function GoalVisType(Private, i18n) {
}
},
},
events: {
brush: { disabled: true },
},
editorConfig: {
collections: {
gaugeTypes: ['Arc', 'Circle'],
Expand Down
3 changes: 3 additions & 0 deletions src/core_plugins/kbn_vislib_vis_types/public/pie.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ export default function HistogramVisType(Private, i18n) {
}
},
},
events: {
brush: { disabled: true },
},
editorConfig: {
collections: {
legendPositions: [{
Expand Down
2 changes: 1 addition & 1 deletion src/core_plugins/metric_vis/public/metric_vis_controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ export class MetricVisComponent extends Component {
return;
}
const table = this.props.visData;
this.props.vis.API.events.addFilter(table, metric.columnIndex, metric.rowIndex);
this.props.vis.API.events.filter({ table, column: metric.columnIndex, row: metric.rowIndex });
};

_renderMetric = (metric, index) => {
Expand Down
2 changes: 1 addition & 1 deletion src/core_plugins/region_map/public/region_map_visualization.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export function RegionMapsVisualizationProvider(Private, config, i18n) {
}

const rowIndex = this._chartData.rows.findIndex(row => row[0] === event);
this._vis.API.events.addFilter(this._chartData, 0, rowIndex, event);
this._vis.API.events.filter({ table: this._chartData, column: 0, row: rowIndex, value: event });
});

this._choroplethLayer.on('styleChanged', (event) => {
Expand Down
6 changes: 3 additions & 3 deletions src/core_plugins/tagcloud/public/tag_cloud_visualization.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ export class TagCloudVisualization {
if (!this._bucketAgg) {
return;
}
this._vis.API.events.addFilter(
event.meta.data, 0, event.meta.rowIndex
);
this._vis.API.events.filter({
table: event.meta.data, column: 0, row: event.meta.rowIndex
});
});
this._renderComplete$ = Rx.fromEvent(this._tagCloud, 'renderComplete');

Expand Down
47 changes: 0 additions & 47 deletions src/ui/public/vis/__tests__/_vis.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

import _ from 'lodash';
import ngMock from 'ng_mock';
import sinon from 'sinon';
import expect from 'expect.js';
import { VisProvider } from '..';
import FixturesStubbedLogstashIndexPatternProvider from 'fixtures/stubbed_logstash_index_pattern';
Expand All @@ -42,15 +41,6 @@ describe('Vis Class', function () {
listeners: { click: _.noop }
};

// Wrap the given vis type definition in a state, that can be passed to vis
const state = (type) => ({
type: {
visConfig: { defaults: {} },
schemas: {},
...type,
}
});

beforeEach(ngMock.module('kibana'));
beforeEach(ngMock.inject(function (Private) {
Vis = Private(VisProvider);
Expand Down Expand Up @@ -116,41 +106,4 @@ describe('Vis Class', function () {
});
});

describe('vis addFilter method', () => {
let aggConfig;
let data;

beforeEach(() => {
aggConfig = {
type: { name: 'terms' },
params: {},
createFilter: sinon.stub()
};

data = {
columns: [{
id: 'col-0',
title: 'test',
aggConfig
}],
rows: [{ 'col-0': 'US' }]
};
});


it('adds a simple filter', () => {
const vis = new Vis(indexPattern, state({ requestHandler: 'none' }));
vis.API.events.addFilter(data, 0, 0);
expect(aggConfig.createFilter.callCount).to.be(1);
expect(aggConfig.createFilter.getCall(0).args[0]).to.be('US');
});

it('adds a filter if value is provided instead of row index', () => {
const vis = new Vis(indexPattern, state({ requestHandler: 'none' }));
vis.API.events.addFilter(data, 0, -1, 'UK');
expect(aggConfig.createFilter.callCount).to.be(1);
expect(aggConfig.createFilter.getCall(0).args[0]).to.be('UK');
});
});

});
9 changes: 8 additions & 1 deletion src/ui/public/vis/__tests__/vis_types/base_vis_type.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,16 @@
*/

import expect from 'expect.js';
import { BaseVisType } from '../../vis_types/base_vis_type';
import ngMock from 'ng_mock';
import { BaseVisTypeProvider } from '../../vis_types/base_vis_type';

describe('Base Vis Type', function () {
let BaseVisType;

beforeEach(ngMock.module('kibana'));
beforeEach(ngMock.inject(function (Private) {
BaseVisType = Private(BaseVisTypeProvider);
}));

describe('initialization', () => {
it('should throw if mandatory properties are missing', () => {
Expand Down
10 changes: 9 additions & 1 deletion src/ui/public/vis/__tests__/vis_types/react_vis_type.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,13 @@
*/

import expect from 'expect.js';
import { ReactVisType } from '../../vis_types/react_vis_type';
import ngMock from 'ng_mock';
import { ReactVisTypeProvider } from '../../vis_types/react_vis_type';

describe('React Vis Type', function () {

let ReactVisType;

const visConfig = {
name: 'test',
title: 'test',
Expand All @@ -31,6 +34,11 @@ describe('React Vis Type', function () {
type: { visConfig: { component: 'test' } }
};

beforeEach(ngMock.module('kibana'));
beforeEach(ngMock.inject(function (Private) {
ReactVisType = Private(ReactVisTypeProvider);
}));

describe('initialization', () => {
it('should throw if component is not set', () => {
expect(() => {
Expand Down
13 changes: 2 additions & 11 deletions src/ui/public/vis/vis.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,17 @@ import _ from 'lodash';
import { VisTypesRegistryProvider } from '../registry/vis_types';
import { AggConfigs } from './agg_configs';
import { PersistedState } from '../persisted_state';
import { onBrushEvent } from '../utils/brush_event';
import { FilterBarQueryFilterProvider } from '../filter_bar/query_filter';
import { updateVisualizationConfig } from './vis_update';
import { SearchSourceProvider } from '../courier/search_source';
import { SavedObjectsClientProvider } from '../saved_objects';
import { timefilter } from 'ui/timefilter';
import { VisFiltersProvider } from './vis_filters';

export function VisProvider(Private, indexPatterns, getAppState) {
const visTypes = Private(VisTypesRegistryProvider);
const queryFilter = Private(FilterBarQueryFilterProvider);
const SearchSource = Private(SearchSourceProvider);
const savedObjectsClient = Private(SavedObjectsClientProvider);
const visFilter = Private(VisFiltersProvider);

class Vis extends EventEmitter {
constructor(indexPattern, visState) {
Expand Down Expand Up @@ -73,14 +70,8 @@ export function VisProvider(Private, indexPatterns, getAppState) {
timeFilter: timefilter,
queryFilter: queryFilter,
events: {
// the filter method will be removed in the near feature
// you should rather use addFilter method below
filter: visFilter.filter,
createFilter: visFilter.createFilter,
addFilter: visFilter.addFilter,
brush: (event) => {
onBrushEvent(event, getAppState());
}
filter: data => this.eventsSubject.next({ name: 'filterBucket', data }),
brush: data => this.eventsSubject.next({ name: 'brush', data }),
},
getAppState,
};
Expand Down
4 changes: 3 additions & 1 deletion src/ui/public/vis/vis_factory.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@
* under the License.
*/

import { BaseVisType, AngularVisTypeProvider, ReactVisType, VislibVisTypeProvider } from './vis_types';
import { BaseVisTypeProvider, AngularVisTypeProvider, ReactVisTypeProvider, VislibVisTypeProvider } from './vis_types';

export const VisFactoryProvider = (Private) => {
const AngularVisType = Private(AngularVisTypeProvider);
const VislibVisType = Private(VislibVisTypeProvider);
const BaseVisType = Private(BaseVisTypeProvider);
const ReactVisType = Private(ReactVisTypeProvider);

return {
createBaseVisualization: (config) => {
Expand Down
49 changes: 27 additions & 22 deletions src/ui/public/vis/vis_filters.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import _ from 'lodash';
import { FilterBarPushFiltersProvider } from '../filter_bar/push_filters';
import { FilterBarQueryFilterProvider } from '../filter_bar/query_filter';
import { onBrushEvent } from '../utils/brush_event';

const getTerms = (table, columnIndex, rowIndex) => {
if (rowIndex === -1) {
Expand All @@ -41,26 +42,26 @@ const getTerms = (table, columnIndex, rowIndex) => {
}))];
};

export function VisFiltersProvider(Private, getAppState) {
const createFilter = (data, columnIndex, rowIndex, cellValue) => {
Copy link
Copy Markdown
Contributor

@markov00 markov00 Nov 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Is there a reason to use arrow function here? can't we use named functions?

Copy link
Copy Markdown
Contributor Author

@ppisljar ppisljar Nov 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason not to use arrow functions ?

Copy link
Copy Markdown
Contributor

@markov00 markov00 Nov 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

touché

const { aggConfig, id: columnId } = data.columns[columnIndex];
let filter = [];
const value = rowIndex > -1 ? data.rows[rowIndex][columnId] : cellValue;
if (value === null || value === undefined) {
return;
}
if (aggConfig.type.name === 'terms' && aggConfig.params.otherBucket) {
const terms = getTerms(data, columnIndex, rowIndex);
filter = aggConfig.createFilter(value, { terms });
} else {
filter = aggConfig.createFilter(value);
}
return filter;
};

const VisFiltersProvider = (Private, getAppState) => {
const filterBarPushFilters = Private(FilterBarPushFiltersProvider);
const queryFilter = Private(FilterBarQueryFilterProvider);

const createFilter = (data, columnIndex, rowIndex, cellValue) => {
const { aggConfig, id: columnId } = data.columns[columnIndex];
let filter = [];
const value = rowIndex > -1 ? data.rows[rowIndex][columnId] : cellValue;
if (value === null || value === undefined) {
return;
}
if (aggConfig.type.name === 'terms' && aggConfig.params.otherBucket) {
const terms = getTerms(data, columnIndex, rowIndex);
filter = aggConfig.createFilter(value, { terms });
} else {
filter = aggConfig.createFilter(value);
}
return filter;
};

const filter = (event, { simulate } = {}) => {
let data = event.datum.aggConfigResult;
const filters = [];
Expand All @@ -87,14 +88,18 @@ export function VisFiltersProvider(Private, getAppState) {
return filters;
};

const addFilter = (data, columnIndex, rowIndex, cellValue) => {
const filter = createFilter(data, columnIndex, rowIndex, cellValue);
const addFilter = (event) => {
const filter = createFilter(event.table, event.column, event.row, event.value);
queryFilter.addFilters(filter);
};

return {
createFilter,
addFilter,
filter
filter,
brush: (event) => {
onBrushEvent(event, getAppState());
},
};
}
};

export { VisFiltersProvider, createFilter };
5 changes: 3 additions & 2 deletions src/ui/public/vis/vis_types/angular_vis_type.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@
* under the License.
*/

import { BaseVisType } from './base_vis_type';
import { BaseVisTypeProvider } from './base_vis_type';
import $ from 'jquery';


export function AngularVisTypeProvider($compile, $rootScope) {
export function AngularVisTypeProvider(Private, $compile, $rootScope) {
const BaseVisType = Private(BaseVisTypeProvider);

class AngularVisController {
constructor(domeElement, vis) {
Expand Down
Loading