Skip to content
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.

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

[Visualizations] Remove jQuery from owned plugins + remove flot-charts from shared bundle #181543

Merged
merged 15 commits into from
Apr 29, 2024
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
1 change: 0 additions & 1 deletion packages/kbn-ui-shared-deps-src/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ webpack_cli(
"//packages/kbn-babel-register",
"//packages/kbn-babel-preset",
# packages included in the shared deps src bundle
"//packages/kbn-flot-charts",
"//packages/kbn-ui-theme",
"//packages/kbn-i18n",
"//packages/kbn-i18n-react",
Expand Down
2 changes: 0 additions & 2 deletions packages/kbn-ui-shared-deps-src/src/entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ require('./polyfills');

export const Jquery = require('jquery');
window.$ = window.jQuery = Jquery;
// mutates window.jQuery and window.$
require('@kbn/flot-charts');

// stateful deps
export const KbnUiTheme = require('@kbn/ui-theme');
Expand Down
7 changes: 5 additions & 2 deletions src/plugins/vis_types/vega/public/data_model/url_parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
* Side Public License, v 1.
*/

import $ from 'jquery';
import { i18n } from '@kbn/i18n';
import { UrlObject } from './types';

Expand Down Expand Up @@ -51,7 +50,11 @@ export class UrlParser {
})
);
} else {
url += (url.includes('?') ? '&' : '?') + $.param(query);
const urlParams = new URLSearchParams();
for (const [name, value] of Object.entries(query)) {
urlParams.append(name, String(value));
}
url += (url.includes('?') ? '&' : '?') + urlParams.toString();
}

obj.url = url;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ export class VegaBaseView {
destroy(): Promise<void>;
resize(dimensions?: { height: number; width: number }): Promise<void>;

_$container: any;
_$controls: any;
_container: HTMLDivElement;
_controls: HTMLDivElement;
_parser: any;
_vegaViewConfig: any;
_serviceSettings: VegaViewParams['serviceSettings'];
Expand Down
81 changes: 49 additions & 32 deletions src/plugins/vis_types/vega/public/vega_view/vega_base_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
* Side Public License, v 1.
*/

import $ from 'jquery';
import moment from 'moment';
import dateMath from '@kbn/datemath';
import { scheme, loader, logger, Warn, version as vegaVersion, expressionFunction } from 'vega';
Expand Down Expand Up @@ -77,15 +76,15 @@ const getExternalUrlServiceError = (uri) =>

export class VegaBaseView {
constructor(opts) {
this._$parentEl = $(opts.parentEl);
this._parentEl = opts.parentEl;
this._parser = opts.vegaParser;
this._serviceSettings = opts.serviceSettings;
this._filterManager = opts.filterManager;
this._fireEvent = opts.fireEvent;
this._timefilter = opts.timefilter;
this._view = null;
this._vegaViewConfig = null;
this._$messages = null;
this._messages = null;
this._destroyHandlers = [];
this._initialized = false;
this._externalUrl = opts.externalUrl;
Expand All @@ -100,11 +99,13 @@ export class VegaBaseView {

try {
if (this._parser.useResize) {
this._$parentEl.addClass('vgaVis--autoresize');
this._parentEl.classList.add('vgaVis--autoresize');
} else {
this._$parentEl.removeClass('vgaVis--autoresize');
this._parentEl.classList.remove('vgaVis--autoresize');
}
this._$parentEl.empty().addClass(`vgaVis`).css('flex-direction', this._parser.containerDir);
this._parentEl.replaceChildren();
this._parentEl.classList.add('vgaVis');
this._parentEl.style.flexDirection = this._parser.containerDir;

// bypass the onWarn warning checks - in some cases warnings may still need to be shown despite being disabled
for (const warn of this._parser.warnings) {
Expand All @@ -116,23 +117,29 @@ export class VegaBaseView {
return;
}

this._$container = $('<div class="vgaVis__view">').appendTo(this._$parentEl);
this._$controls = $(
`<div class="vgaVis__controls vgaVis__controls--${this._parser.controlsDir}">`
).appendTo(this._$parentEl);
this._container = document.createElement('div');
this._container.classList.add('vgaVis__view');
this._parentEl.append(this._container);

this._controls = document.createElement('div');
this._controls.classList.add(
`vgaVis__controls`,
`vgaVis__controls--${this._parser.controlsDir}`
);
this._parentEl.append(this._controls);

this._addDestroyHandler(() => {
if (this._$container) {
this._$container.remove();
this._$container = null;
if (this._container) {
this._container.remove();
this._container = null;
}
if (this._$controls) {
this._$controls.remove();
this._$controls = null;
if (this._controls) {
this._controls.remove();
this._controls = null;
}
if (this._$messages) {
this._$messages.remove();
this._$messages = null;
if (this._messages) {
this._messages.remove();
this._messages = null;
}
if (this._view) {
const state = this._view.getState();
Expand Down Expand Up @@ -253,18 +260,24 @@ export class VegaBaseView {
}

_addMessage(type, text) {
if (!this._$messages) {
this._$messages = $(`<ul class="vgaVis__messages">`).appendTo(this._$parentEl);
if (!this._messages) {
this._messages = document.createElement('ul');
this._messages.classList.add('vgaVis__messages');
this._parentEl.append(this._messages);
}
const isMessageAlreadyDisplayed = this._$messages
.find(`pre.vgaVis__messageCode`)
.filter((index, element) => element.textContent === text).length;
const isMessageAlreadyDisplayed = [
...this._messages.querySelectorAll(`:scope pre.vgaVis__messageCode`),
].filter((index, element) => element.textContent === text).length;
if (!isMessageAlreadyDisplayed) {
this._$messages.append(
$(`<li class="vgaVis__message vgaVis__message--${type}">`).append(
$(`<pre class="vgaVis__messageCode">`).text(text)
)
);
const messageCodeEl = document.createElement('pre');
messageCodeEl.classList.add('vgaVis__messageCode');
messageCodeEl.textContent = text;

const messageItemEl = document.createElement('li');
messageItemEl.classList.add(`vgaVis__message`, `vgaVis__message--${type}`);
messageItemEl.append(messageCodeEl);

this._messages.append(messageItemEl);
}
}

Expand All @@ -279,8 +292,12 @@ export class VegaBaseView {
}

updateVegaSize(view, dimensions) {
const width = Math.floor(Math.max(0, dimensions?.width ?? this._$container.width()));
const height = Math.floor(Math.max(0, dimensions?.height ?? this._$container.height()));
const width = Math.floor(
Math.max(0, dimensions?.width ?? this._container.getBoundingClientRect().width)
);
const height = Math.floor(
Math.max(0, dimensions?.height ?? this._container.getBoundingClientRect().height)
);

if (view.width() !== width || view.height() !== height) {
view.width(width).height(height);
Expand All @@ -305,7 +322,7 @@ export class VegaBaseView {
if (this._parser.tooltips) {
// position and padding can be specified with
// {config:{kibana:{tooltips: {position: 'top', padding: 15 } }}}
const tthandler = new TooltipHandler(this._$container[0], view, this._parser.tooltips);
const tthandler = new TooltipHandler(this._container, view, this._parser.tooltips);

// Vega bug workaround - need to destroy tooltip by hand
this._addDestroyHandler(() => tthandler.hideTooltip());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ describe('vega_map_view/view', () => {
layers: [],
},
customAttribution: 'tilemap-attribution',
container: vegaMapView._$container.get(0),
container: vegaMapView._container,
minZoom: 0,
maxZoom: 20,
zoom: 3,
Expand All @@ -200,7 +200,7 @@ describe('vega_map_view/view', () => {
layers: [],
},
customAttribution: ['<a rel="noreferrer noopener" href="tms_attributions"></a>'],
container: vegaMapView._$container.get(0),
container: vegaMapView._container,
minZoom: 0,
maxZoom: 20,
zoom: 3,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,14 +136,14 @@ export class VegaMapView extends VegaBaseView {
}

// In some cases, Vega may be initialized twice, e.g. after awaiting...
if (!this._$container) return;
if (!this._container) return;

// For the correct geration of the PDF/PNG report, we must wait until the map is fully rendered.
return new Promise((resolve) => {
const mapBoxInstance = new maplibregl.Map({
style,
customAttribution,
container: this._$container.get(0),
container: this._container,
...this.getMapParams({ ...zoomSettings }),
});

Expand Down Expand Up @@ -206,7 +206,7 @@ export class VegaMapView extends VegaBaseView {
map: mapBoxInstance,
context: {
vegaView,
vegaControls: this._$controls?.get(0),
vegaControls: this._controls,
updateVegaView,
},
});
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/vis_types/vega/public/vega_view/vega_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ import { VegaBaseView } from './vega_base_view';
export class VegaView extends VegaBaseView {
async _initViewCustomizations() {
// In some cases, Vega may be initialized twice... TBD
if (!this._$container) return;
if (!this._container) return;

const view = new View(parse(this._parser.spec, undefined, { ast: true }), this._vegaViewConfig);

if (this._parser.useResize) this.updateVegaSize(view);
view.initialize(this._$container.get(0), this._$controls.get(0));
view.initialize(this._container, this._controls);
// resize again to take controls into account
if (this._parser.useResize) this.updateVegaSize(view);

Expand Down
13 changes: 5 additions & 8 deletions src/plugins/vis_types/vega/public/vega_visualization.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@

import 'jest-canvas-mock';

import $ from 'jquery';

import { createVegaVisualization } from './vega_visualization';

import vegaliteGraph from './test_utils/vegalite_graph.json';
Expand All @@ -32,9 +30,8 @@ describe('VegaVisualizations', () => {
let VegaVisualization;
let vegaVisualizationDependencies;

let mockWidth;
let mockGetBoundingClientRect;
let mockedWidthValue;
let mockHeight;
let mockedHeightValue;

const coreStart = coreMock.createStart();
Expand All @@ -46,8 +43,9 @@ describe('VegaVisualizations', () => {
mockedHeightValue = height;
domNode = document.createElement('div');

mockWidth = jest.spyOn($.prototype, 'width').mockImplementation(() => mockedWidthValue);
mockHeight = jest.spyOn($.prototype, 'height').mockImplementation(() => mockedHeightValue);
mockGetBoundingClientRect = jest
.spyOn(Element.prototype, 'getBoundingClientRect')
.mockImplementation(() => ({ width: mockedWidthValue, height: mockedHeightValue }));
};

const mockGetServiceSettings = () => {
Expand Down Expand Up @@ -78,8 +76,7 @@ describe('VegaVisualizations', () => {
});

afterEach(() => {
mockWidth.mockRestore();
mockHeight.mockRestore();
mockGetBoundingClientRect.mockRestore();
});

test('should show vegalite graph and update on resize (may fail in dev env)', async () => {
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/canvas/public/application.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { i18n } from '@kbn/i18n';
import { Provider } from 'react-redux';
import { BehaviorSubject } from 'rxjs';

import '@kbn/flot-charts';
import { includes, remove } from 'lodash';

import { AppMountParameters, CoreStart, CoreSetup, AppUpdater } from '@kbn/core/public';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
* 2.0.
*/

import $ from 'jquery';

// Kibana wrapper
import d3 from 'd3';
import { getIcon } from '../../helpers/style_choices';
Expand All @@ -15,31 +13,25 @@ import { getIcon } from '../../helpers/style_choices';
// for use outside of Kibana server with direct access to elasticsearch
let graphExplorer = function (indexName, typeName, request, responseHandler) {
const dataForServer = JSON.stringify(request);
$.ajax({
type: 'POST',
url: 'http://localhost:9200/' + indexName + '/_graph/explore',
fetch(`http://localhost:9200/${indexName}/_graph/explore`, {
method: 'POST',
dataType: 'json',
contentType: 'application/json;charset=utf-8',
async: true,
data: dataForServer,
success: function (data) {
responseHandler(data);
},
});
})
.then((response) => response.json())
.then(responseHandler);
};
let searcher = function (indexName, request, responseHandler) {
const dataForServer = JSON.stringify(request);
$.ajax({
type: 'POST',
url: 'http://localhost:9200/' + indexName + '/_search?rest_total_hits_as_int=true',
fetch(`http://localhost:9200/${indexName}/_search?rest_total_hits_as_int=true`, {
method: 'POST',
dataType: 'json',
contentType: 'application/json;charset=utf-8', //Not sure why this was necessary - worked without elsewhere
async: true,
contentType: 'application/json;charset=utf-8',
data: dataForServer,
success: function (data) {
responseHandler(data);
},
});
})
.then((response) => response.json())
.then(responseHandler);
};

// ====== Undo operations =============
Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/monitoring/public/components/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@
* 2.0.
*/

/** Import this dependency here to load it async rather than on startup */
import '@kbn/flot-charts';
/*
* This file should only export page-level components for view controllers to
* mount React to the DOM
*/

export { NoData } from './no_data';
export { License } from './license';
export { PageLoading } from './page_loading';
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/monitoring/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
"@kbn/rule-data-utils",
"@kbn/react-kibana-mount",
"@kbn/react-kibana-context-render",
"@kbn/flot-charts",
],
"exclude": [
"target/**/*",
Expand Down