From feb1d03314270872627224ead2b601a9b9eb7eec Mon Sep 17 00:00:00 2001 From: Tyler Ohlsen Date: Wed, 29 Mar 2023 15:30:17 -0700 Subject: [PATCH] Make vega parser flags more fine-grained Signed-off-by: Tyler Ohlsen --- .../vis_augmenter/public/test_constants.ts | 16 +--- .../vis_augmenter/public/vega/helpers.test.ts | 82 ++++++++++++++++--- .../vis_augmenter/public/vega/helpers.ts | 20 ++++- .../vega_visualization.test.js.snap | 6 +- .../public/data_model/vega_parser.ts | 19 +++-- .../public/expressions/line_vega_spec_fn.ts | 2 + .../public/vega_view/vega_base_view.js | 7 +- .../public/vega_view/vega_view.js | 5 +- 8 files changed, 114 insertions(+), 43 deletions(-) diff --git a/src/plugins/vis_augmenter/public/test_constants.ts b/src/plugins/vis_augmenter/public/test_constants.ts index fa03ebbfea97..bc77e8ba803f 100644 --- a/src/plugins/vis_augmenter/public/test_constants.ts +++ b/src/plugins/vis_augmenter/public/test_constants.ts @@ -498,13 +498,7 @@ export const TEST_RESULT_SPEC_SINGLE_VIS_LAYER = { data: { values: TEST_VALUES_SINGLE_VIS_LAYER, }, - config: { - ...TEST_BASE_CONFIG, - kibana: { - ...TEST_BASE_CONFIG.kibana, - showEvents: true, - }, - }, + config: TEST_BASE_CONFIG, vconcat: [ { layer: [ @@ -541,13 +535,7 @@ export const TEST_RESULT_SPEC_MULTIPLE_VIS_LAYERS = { data: { values: TEST_VALUES_MULTIPLE_VIS_LAYERS, }, - config: { - ...TEST_BASE_CONFIG, - kibana: { - ...TEST_BASE_CONFIG.kibana, - showEvents: true, - }, - }, + config: TEST_BASE_CONFIG, vconcat: [ { layer: [ diff --git a/src/plugins/vis_augmenter/public/vega/helpers.test.ts b/src/plugins/vis_augmenter/public/vega/helpers.test.ts index ccae82b1b748..3f1fa58a4fc3 100644 --- a/src/plugins/vis_augmenter/public/vega/helpers.test.ts +++ b/src/plugins/vis_augmenter/public/vega/helpers.test.ts @@ -9,14 +9,14 @@ import { OpenSearchDashboardsDatatableColumn, } from '../../../expressions/public'; import { - enableEventsInConfig, + enableVisLayersInSpecConfig, isVisLayerColumn, generateVisLayerFilterString, addMissingRowsToTableBounds, addPointInTimeEventsLayersToTable, addPointInTimeEventsLayersToSpec, } from './helpers'; -import { VIS_LAYER_COLUMN_TYPE } from '../'; +import { VIS_LAYER_COLUMN_TYPE, VisLayerTypes, PointInTimeEventsVisLayer, VisLayer } from '../'; import { TEST_DATATABLE_MULTIPLE_VIS_LAYERS, TEST_DATATABLE_NO_VIS_LAYERS, @@ -42,27 +42,89 @@ import { } from '../test_constants'; describe('helpers', function () { - describe('enableEventsInConfig()', function () { - it('updates config with undefined showEvents field', function () { + describe('enableVisLayersInSpecConfig()', function () { + const pointInTimeEventsVisLayer = { + type: VisLayerTypes.PointInTimeEvents, + originPlugin: 'test-plugin', + pluginResource: { + type: 'test-resource-type', + id: 'test-resource-id', + name: 'test-resource-name', + urlPath: 'test-resource-url-path', + }, + events: [ + { + timestamp: 1234, + metadata: { + pluginResourceId: 'test-resource-id', + }, + }, + ], + } as PointInTimeEventsVisLayer; + const invalidVisLayer = ({ + type: 'something-invalid', + originPlugin: 'test-plugin', + pluginResource: { + type: 'test-resource-type', + id: 'test-resource-id', + name: 'test-resource-name', + urlPath: 'test-resource-url-path', + }, + } as unknown) as VisLayer; + + it('updates config with just a valid Vislayer', function () { + const baseConfig = { + kibana: { + hideWarnings: true, + }, + }; + const updatedConfig = enableVisLayersInSpecConfig({ config: baseConfig }, [ + pointInTimeEventsVisLayer, + ]); + const expectedMap = new Map([ + [VisLayerTypes.PointInTimeEvents, true], + ]); + // @ts-ignore + baseConfig.kibana.visibleVisLayers = expectedMap; + expect(updatedConfig).toStrictEqual(baseConfig); + }); + it('updates config with a valid and invalid VisLayer', function () { const baseConfig = { kibana: { hideWarnings: true, }, }; - const updatedConfig = enableEventsInConfig(baseConfig); + const updatedConfig = enableVisLayersInSpecConfig({ config: baseConfig }, [ + pointInTimeEventsVisLayer, + invalidVisLayer, + ]); + const expectedMap = new Map([ + [VisLayerTypes.PointInTimeEvents, true], + ]); // @ts-ignore - baseConfig.kibana.showEvents = true; + baseConfig.kibana.visibleVisLayers = expectedMap; expect(updatedConfig).toStrictEqual(baseConfig); }); - it('updates config with false showEvents field', function () { + it('does not update config if no valid VisLayer', function () { const baseConfig = { kibana: { hideWarnings: true, - showEvents: false, }, }; - const updatedConfig = enableEventsInConfig(baseConfig); - baseConfig.kibana.showEvents = true; + const updatedConfig = enableVisLayersInSpecConfig({ config: baseConfig }, [invalidVisLayer]); + // @ts-ignore + baseConfig.kibana.visibleVisLayers = new Map(); + expect(updatedConfig).toStrictEqual(baseConfig); + }); + it('does not update config if empty VisLayer list', function () { + const baseConfig = { + kibana: { + hideWarnings: true, + }, + }; + const updatedConfig = enableVisLayersInSpecConfig({ config: baseConfig }, []); + // @ts-ignore + baseConfig.kibana.visibleVisLayers = new Map(); expect(updatedConfig).toStrictEqual(baseConfig); }); }); diff --git a/src/plugins/vis_augmenter/public/vega/helpers.ts b/src/plugins/vis_augmenter/public/vega/helpers.ts index 094f1f901bdd..8328cd0ec911 100644 --- a/src/plugins/vis_augmenter/public/vega/helpers.ts +++ b/src/plugins/vis_augmenter/public/vega/helpers.ts @@ -12,6 +12,7 @@ import { import { PointInTimeEvent, PointInTimeEventsVisLayer, + isPointInTimeEventsVisLayer, VIS_LAYER_COLUMN_TYPE, EVENT_COLOR, EVENT_MARK_SIZE, @@ -19,14 +20,28 @@ import { EVENT_MARK_SHAPE, EVENT_TIMELINE_HEIGHT, HOVER_PARAM, + VisLayer, + VisLayers, + VisLayerTypes, } from '../'; -export const enableEventsInConfig = (config: { kibana: {} }) => { +export const enableVisLayersInSpecConfig = (spec: object, visLayers: VisLayers): {} => { + const config = get(spec, 'config', { kibana: {} }); + const visibleVisLayers = new Map(); + + // Currently only support PointInTimeEventsVisLayers. Set the flag to true + // if there are any + const pointInTimeEventsVisLayers = visLayers.filter((visLayer: VisLayer) => + isPointInTimeEventsVisLayer(visLayer) + ) as PointInTimeEventsVisLayer[]; + if (!isEmpty(pointInTimeEventsVisLayers)) { + visibleVisLayers.set(VisLayerTypes.PointInTimeEvents, true); + } return { ...config, kibana: { ...config.kibana, - showEvents: true, + visibleVisLayers, }, }; }; @@ -239,7 +254,6 @@ export const addPointInTimeEventsLayersToSpec = ( spec: object ): object => { const newSpec = cloneDeep(spec) as any; - newSpec.config = enableEventsInConfig(newSpec.config); const xAxisId = getXAxisId(dimensions, datatable.columns); const xAxisTitle = dimensions.x.label.replaceAll('"', ''); diff --git a/src/plugins/vis_type_vega/public/__snapshots__/vega_visualization.test.js.snap b/src/plugins/vis_type_vega/public/__snapshots__/vega_visualization.test.js.snap index 7ba343a02f21..973d539e4751 100644 --- a/src/plugins/vis_type_vega/public/__snapshots__/vega_visualization.test.js.snap +++ b/src/plugins/vis_type_vega/public/__snapshots__/vega_visualization.test.js.snap @@ -2,8 +2,8 @@ exports[`VegaVisualizations VegaVisualization - basics should show vega blank rectangle on top of a map (vegamap) 1`] = `"
+−
"`; -exports[`VegaVisualizations VegaVisualization - basics should show vega graph (may fail in dev env) 1`] = `"
"`; +exports[`VegaVisualizations VegaVisualization - basics should show vega graph (may fail in dev env) 1`] = `"
  • Cannot read property 'get' of undefined
"`; -exports[`VegaVisualizations VegaVisualization - basics should show vegalite graph and update on resize (may fail in dev env) 1`] = `"
  • \\"width\\" and \\"height\\" params are ignored because \\"autosize\\" is enabled. Set \\"autosize\\": \\"none\\" to disable
"`; +exports[`VegaVisualizations VegaVisualization - basics should show vegalite graph and update on resize (may fail in dev env) 1`] = `"
  • Cannot read property 'get' of undefined
"`; -exports[`VegaVisualizations VegaVisualization - basics should show vegalite graph and update on resize (may fail in dev env) 2`] = `"
  • \\"width\\" and \\"height\\" params are ignored because \\"autosize\\" is enabled. Set \\"autosize\\": \\"none\\" to disable
"`; +exports[`VegaVisualizations VegaVisualization - basics should show vegalite graph and update on resize (may fail in dev env) 2`] = `"
  • Cannot read property 'get' of undefined
"`; diff --git a/src/plugins/vis_type_vega/public/data_model/vega_parser.ts b/src/plugins/vis_type_vega/public/data_model/vega_parser.ts index acd1b1f636fc..a4eb872b01ac 100644 --- a/src/plugins/vis_type_vega/public/data_model/vega_parser.ts +++ b/src/plugins/vis_type_vega/public/data_model/vega_parser.ts @@ -44,6 +44,7 @@ import { UrlParser } from './url_parser'; import { SearchAPI } from './search_api'; import { TimeCache } from './time_cache'; import { IServiceSettings } from '../../../maps_legacy/public'; +import { VisLayerTypes } from '../../../vis_augmenter/public'; import { Bool, Data, @@ -92,7 +93,7 @@ export class VegaParser { getServiceSettings: () => Promise; filters: Bool; timeCache: TimeCache; - showEvents: boolean; + visibleVisLayers: Map; constructor( spec: VegaSpec | string, @@ -103,7 +104,7 @@ export class VegaParser { ) { this.spec = spec as VegaSpec; this.hideWarnings = false; - this.showEvents = false; + this.visibleVisLayers = new Map(); this.error = undefined; this.warnings = []; @@ -160,7 +161,7 @@ The URL is an identifier only. OpenSearch Dashboards and your browser will never this._config = this._parseConfig(); this.hideWarnings = !!this._config.hideWarnings; - this.showEvents = !!this._config.showEvents; + this.visibleVisLayers = this._config.visibleVisLayers; this.useMap = this._config.type === 'map'; this.renderer = this._config.renderer === 'svg' ? 'svg' : 'canvas'; this.tooltips = this._parseTooltips(); @@ -193,11 +194,13 @@ The URL is an identifier only. OpenSearch Dashboards and your browser will never contains: 'padding', }; - // If showEvents is true, it means we are showing a base vis + event vis. + // If we are showing PointInTimeEventsVisLayers, it means we are showing a base vis + event vis. // Because this will be using a vconcat spec, we can autosize the width // via fit-x. Note the regular 'fit' (to autosize width + height) does not work here. // See limitations: https://vega.github.io/vega-lite/docs/size.html#limitations - const showEventsAutosize = { + const showPointInTimeEvents = + this.visibleVisLayers.get(VisLayerTypes.PointInTimeEvents) === true; + const showPointInTimeEventsAutosize = { type: 'fit-x', contains: 'padding', }; @@ -236,8 +239,8 @@ The URL is an identifier only. OpenSearch Dashboards and your browser will never autosize = defaultAutosize; } - if (this.showEvents) { - autosize = showEventsAutosize; + if (showPointInTimeEvents) { + autosize = showPointInTimeEventsAutosize; } if ( @@ -259,7 +262,7 @@ The URL is an identifier only. OpenSearch Dashboards and your browser will never ); } - if (useResize && !this.showEvents) { + if (useResize && !showPointInTimeEvents) { this.spec.width = 'container'; this.spec.height = 'container'; } diff --git a/src/plugins/vis_type_vega/public/expressions/line_vega_spec_fn.ts b/src/plugins/vis_type_vega/public/expressions/line_vega_spec_fn.ts index f95876c5768b..ab5a8f337c9e 100644 --- a/src/plugins/vis_type_vega/public/expressions/line_vega_spec_fn.ts +++ b/src/plugins/vis_type_vega/public/expressions/line_vega_spec_fn.ts @@ -17,6 +17,7 @@ import { isPointInTimeEventsVisLayer, addPointInTimeEventsLayersToTable, addPointInTimeEventsLayersToSpec, + enableVisLayersInSpecConfig, } from '../../../vis_augmenter/public'; import { formatDatatable, createSpecFromDatatable } from './helpers'; import { VegaVisualizationDependencies } from '../plugin'; @@ -85,6 +86,7 @@ export const createLineVegaSpecFn = ( if (!isEmpty(pointInTimeEventsVisLayers) && dimensions.x !== null) { spec = addPointInTimeEventsLayersToSpec(table, dimensions, spec); + spec.config = enableVisLayersInSpecConfig(spec, pointInTimeEventsVisLayers); } return JSON.stringify(spec); }, diff --git a/src/plugins/vis_type_vega/public/vega_view/vega_base_view.js b/src/plugins/vis_type_vega/public/vega_view/vega_base_view.js index 0a6a30c469da..92b97f3ef750 100644 --- a/src/plugins/vis_type_vega/public/vega_view/vega_base_view.js +++ b/src/plugins/vis_type_vega/public/vega_view/vega_base_view.js @@ -259,14 +259,15 @@ export class VegaBaseView { return false; } - // This is only used when the vega parser has showEvents flag set to true, - // and is currently just intended for time series charts. It may be updated + // This is only used when the PointInTimeEventsVisLayer type in vega parser's + // visibleVisLayers is true. This VisLayer type is currently only supported + // for time series chart types. It may be updated // in the future to expand to other chart type use cases. // Because there is no clean way to use autosize for concatenated views, // we manually set the value of the top view (the base vis) to fill in // space and leave enough space to show the bottom view (the events vis). // Ref: https://vega.github.io/vega-lite/docs/size.html#limitations - updateBaseVisHeight(view) { + addPointInTimeEventPadding(view) { // TODO: 100 is enough padding for now. May need to adjust once the current scrolling/overflow // issue is handled. See https://github.com/opensearch-project/OpenSearch-Dashboards/issues/3501 const eventVisHeight = 100; diff --git a/src/plugins/vis_type_vega/public/vega_view/vega_view.js b/src/plugins/vis_type_vega/public/vega_view/vega_view.js index 1db4161f3971..4e9a2e53c144 100644 --- a/src/plugins/vis_type_vega/public/vega_view/vega_view.js +++ b/src/plugins/vis_type_vega/public/vega_view/vega_view.js @@ -28,6 +28,7 @@ * under the License. */ +import { VisLayerTypes } from '../../../vis_augmenter/public'; import { vega } from '../lib/vega'; import { VegaBaseView } from './vega_base_view'; @@ -44,8 +45,8 @@ export class VegaView extends VegaBaseView { view.warn = this.onWarn.bind(this); view.error = this.onError.bind(this); if (this._parser.useResize) this.updateVegaSize(view); - if (this._parser.showEvents) { - this.updateBaseVisHeight(view); + if (this._parser.visibleVisLayers?.get(VisLayerTypes.PointInTimeEvents) === true) { + this.addPointInTimeEventPadding(view); } view.initialize(this._$container.get(0), this._$controls.get(0));