From 3f6c0d688cf38d5b20db788b7a51231ebd95e7c6 Mon Sep 17 00:00:00 2001 From: Daniil Suleiman <31325372+sulemanof@users.noreply.github.com> Date: Mon, 28 Sep 2020 12:43:34 +0300 Subject: [PATCH] Lazy load metric & mardown visualizations (#78391) * Lazy load metrics vis * Use common chart spinner * Simplify markdown renderer * Update tests * Update types for metric vis * Fix tests * Fix merge conflict Co-authored-by: Elastic Machine --- src/plugins/vis_type_markdown/kibana.json | 2 +- .../__snapshots__/markdown_fn.test.ts.snap | 2 +- .../vis_type_markdown/public/index.scss | 8 -- .../vis_type_markdown/public/markdown_fn.ts | 8 +- .../public/markdown_renderer.tsx | 50 ++++------ .../{_markdown_vis.scss => markdown_vis.scss} | 7 ++ .../public/markdown_vis_controller.test.tsx | 46 ++-------- .../public/markdown_vis_controller.tsx | 92 +++++-------------- .../vis_type_markdown/public/plugin.ts | 8 +- .../metric_vis.scss} | 7 ++ .../components/metric_vis_component.test.tsx | 2 +- .../components/metric_vis_component.tsx | 10 +- src/plugins/vis_type_metric/public/index.scss | 8 -- src/plugins/vis_type_metric/public/index.ts | 1 - .../vis_type_metric/public/metric_vis_fn.ts | 4 +- .../public/metric_vis_renderer.tsx | 34 +++---- src/plugins/vis_type_metric/public/plugin.ts | 3 +- .../vis_type_metric/public/services.ts | 3 - .../public/components/_visualization.scss | 7 ++ .../components/visualization_container.tsx | 29 +++++- 20 files changed, 132 insertions(+), 199 deletions(-) delete mode 100644 src/plugins/vis_type_markdown/public/index.scss rename src/plugins/vis_type_markdown/public/{_markdown_vis.scss => markdown_vis.scss} (55%) rename src/plugins/vis_type_metric/public/{_metric_vis.scss => components/metric_vis.scss} (78%) delete mode 100644 src/plugins/vis_type_metric/public/index.scss diff --git a/src/plugins/vis_type_markdown/kibana.json b/src/plugins/vis_type_markdown/kibana.json index 5723fdefe1e4c..c0afcb0e99d13 100644 --- a/src/plugins/vis_type_markdown/kibana.json +++ b/src/plugins/vis_type_markdown/kibana.json @@ -4,5 +4,5 @@ "ui": true, "server": true, "requiredPlugins": ["expressions", "visualizations"], - "requiredBundles": ["kibanaUtils", "kibanaReact", "charts", "visualizations", "expressions", "visDefaultEditor"] + "requiredBundles": ["kibanaReact", "charts", "visualizations", "expressions", "visDefaultEditor"] } diff --git a/src/plugins/vis_type_markdown/public/__snapshots__/markdown_fn.test.ts.snap b/src/plugins/vis_type_markdown/public/__snapshots__/markdown_fn.test.ts.snap index 473e2cba742b7..9983f67d4be4d 100644 --- a/src/plugins/vis_type_markdown/public/__snapshots__/markdown_fn.test.ts.snap +++ b/src/plugins/vis_type_markdown/public/__snapshots__/markdown_fn.test.ts.snap @@ -5,7 +5,7 @@ Object { "as": "markdown_vis", "type": "render", "value": Object { - "visConfig": Object { + "visParams": Object { "fontSize": 12, "markdown": "## hello _markdown_", "openLinksInNewTab": true, diff --git a/src/plugins/vis_type_markdown/public/index.scss b/src/plugins/vis_type_markdown/public/index.scss deleted file mode 100644 index ddb7fe3a6b0d9..0000000000000 --- a/src/plugins/vis_type_markdown/public/index.scss +++ /dev/null @@ -1,8 +0,0 @@ -// Prefix all styles with "mkd" to avoid conflicts. -// Examples -// mkdChart -// mkdChart__legend -// mkdChart__legend--small -// mkdChart__legend-isLoading - -@import './markdown_vis'; diff --git a/src/plugins/vis_type_markdown/public/markdown_fn.ts b/src/plugins/vis_type_markdown/public/markdown_fn.ts index 4b3c9989431f9..eaa2c840f8046 100644 --- a/src/plugins/vis_type_markdown/public/markdown_fn.ts +++ b/src/plugins/vis_type_markdown/public/markdown_fn.ts @@ -21,16 +21,16 @@ import { i18n } from '@kbn/i18n'; import { ExpressionFunctionDefinition, Render } from '../../expressions/public'; import { Arguments, MarkdownVisParams } from './types'; -interface RenderValue { +export interface MarkdownVisRenderValue { visType: 'markdown'; - visConfig: MarkdownVisParams; + visParams: MarkdownVisParams; } export type MarkdownVisExpressionFunctionDefinition = ExpressionFunctionDefinition< 'markdownVis', unknown, Arguments, - Render + Render >; export const createMarkdownVisFn = (): MarkdownVisExpressionFunctionDefinition => ({ @@ -70,7 +70,7 @@ export const createMarkdownVisFn = (): MarkdownVisExpressionFunctionDefinition = as: 'markdown_vis', value: { visType: 'markdown', - visConfig: { + visParams: { markdown: args.markdown, openLinksInNewTab: args.openLinksInNewTab, fontSize: parseInt(args.font.spec.fontSize || '12', 10), diff --git a/src/plugins/vis_type_markdown/public/markdown_renderer.tsx b/src/plugins/vis_type_markdown/public/markdown_renderer.tsx index 5950a762635b2..8071196c6a213 100644 --- a/src/plugins/vis_type_markdown/public/markdown_renderer.tsx +++ b/src/plugins/vis_type_markdown/public/markdown_renderer.tsx @@ -17,41 +17,29 @@ * under the License. */ -import React from 'react'; +import React, { lazy } from 'react'; import { render, unmountComponentAtNode } from 'react-dom'; import { VisualizationContainer } from '../../visualizations/public'; import { ExpressionRenderDefinition } from '../../expressions/common/expression_renderers'; -import { MarkdownVisWrapper } from './markdown_vis_controller'; -import { StartServicesGetter } from '../../kibana_utils/public'; +import { MarkdownVisRenderValue } from './markdown_fn'; -export const getMarkdownRenderer = (start: StartServicesGetter) => { - const markdownVisRenderer: () => ExpressionRenderDefinition = () => ({ - name: 'markdown_vis', - displayName: 'markdown visualization', - reuseDomNode: true, - render: async (domNode: HTMLElement, config: any, handlers: any) => { - const { visConfig } = config; +// @ts-ignore +const MarkdownVisComponent = lazy(() => import('./markdown_vis_controller')); - const I18nContext = await start().core.i18n.Context; +export const markdownVisRenderer: ExpressionRenderDefinition = { + name: 'markdown_vis', + displayName: 'markdown visualization', + reuseDomNode: true, + render: async (domNode, { visParams }, handlers) => { + handlers.onDestroy(() => { + unmountComponentAtNode(domNode); + }); - handlers.onDestroy(() => { - unmountComponentAtNode(domNode); - }); - - render( - - - - - , - domNode - ); - }, - }); - - return markdownVisRenderer; + render( + + + , + domNode + ); + }, }; diff --git a/src/plugins/vis_type_markdown/public/_markdown_vis.scss b/src/plugins/vis_type_markdown/public/markdown_vis.scss similarity index 55% rename from src/plugins/vis_type_markdown/public/_markdown_vis.scss rename to src/plugins/vis_type_markdown/public/markdown_vis.scss index fb0a3d05e5e85..2356562a86ed0 100644 --- a/src/plugins/vis_type_markdown/public/_markdown_vis.scss +++ b/src/plugins/vis_type_markdown/public/markdown_vis.scss @@ -1,3 +1,10 @@ +// Prefix all styles with "mkd" to avoid conflicts. +// Examples +// mkdChart +// mkdChart__legend +// mkdChart__legend--small +// mkdChart__legend-isLoading + .mkdVis { padding: $euiSizeS; width: 100%; diff --git a/src/plugins/vis_type_markdown/public/markdown_vis_controller.test.tsx b/src/plugins/vis_type_markdown/public/markdown_vis_controller.test.tsx index 6df205b21d910..36850fc820ded 100644 --- a/src/plugins/vis_type_markdown/public/markdown_vis_controller.test.tsx +++ b/src/plugins/vis_type_markdown/public/markdown_vis_controller.test.tsx @@ -20,7 +20,7 @@ import React from 'react'; import { wait } from '@testing-library/dom'; import { render, cleanup } from '@testing-library/react/pure'; -import { MarkdownVisWrapper } from './markdown_vis_controller'; +import MarkdownVisComponent from './markdown_vis_controller'; afterEach(cleanup); @@ -36,7 +36,7 @@ describe('markdown vis controller', () => { }; const { getByTestId, getByText } = render( - + ); await wait(() => getByTestId('markdownBody')); @@ -60,7 +60,7 @@ describe('markdown vis controller', () => { }; const { getByTestId, getByText } = render( - + ); await wait(() => getByTestId('markdownBody')); @@ -82,7 +82,7 @@ describe('markdown vis controller', () => { }; const { getByTestId, getByText, rerender } = render( - + ); await wait(() => getByTestId('markdownBody')); @@ -90,9 +90,7 @@ describe('markdown vis controller', () => { expect(getByText(/initial/i)).toBeInTheDocument(); vis.params.markdown = 'Updated'; - rerender( - - ); + rerender(); expect(getByText(/Updated/i)).toBeInTheDocument(); }); @@ -114,11 +112,7 @@ describe('markdown vis controller', () => { it('should be called on initial rendering', async () => { const { getByTestId } = render( - + ); await wait(() => getByTestId('markdownBody')); @@ -128,11 +122,7 @@ describe('markdown vis controller', () => { it('should be called on successive render when params change', async () => { const { getByTestId, rerender } = render( - + ); await wait(() => getByTestId('markdownBody')); @@ -142,24 +132,14 @@ describe('markdown vis controller', () => { renderComplete.mockClear(); vis.params.markdown = 'changed'; - rerender( - - ); + rerender(); expect(renderComplete).toHaveBeenCalledTimes(1); }); it('should be called on successive render even without data change', async () => { const { getByTestId, rerender } = render( - + ); await wait(() => getByTestId('markdownBody')); @@ -168,13 +148,7 @@ describe('markdown vis controller', () => { renderComplete.mockClear(); - rerender( - - ); + rerender(); expect(renderComplete).toHaveBeenCalledTimes(1); }); diff --git a/src/plugins/vis_type_markdown/public/markdown_vis_controller.tsx b/src/plugins/vis_type_markdown/public/markdown_vis_controller.tsx index e1155ca42df72..a2387b96eab6d 100644 --- a/src/plugins/vis_type_markdown/public/markdown_vis_controller.tsx +++ b/src/plugins/vis_type_markdown/public/markdown_vis_controller.tsx @@ -17,83 +17,35 @@ * under the License. */ -import React from 'react'; +import React, { useEffect } from 'react'; import { Markdown } from '../../kibana_react/public'; import { MarkdownVisParams } from './types'; +import './markdown_vis.scss'; + interface MarkdownVisComponentProps extends MarkdownVisParams { renderComplete: () => void; } -/** - * The MarkdownVisComponent renders markdown to HTML and presents it. - */ -class MarkdownVisComponent extends React.Component { - /** - * Will be called after the first render when the component is present in the DOM. - * - * We call renderComplete here, to signal, that we are done with rendering. - */ - componentDidMount() { - this.props.renderComplete(); - } - - /** - * Will be called after the component has been updated and the changes has been - * flushed into the DOM. - * - * We will use this to signal that we are done rendering by calling the - * renderComplete property. - */ - componentDidUpdate() { - this.props.renderComplete(); - } +const MarkdownVisComponent = ({ + fontSize, + markdown, + openLinksInNewTab, + renderComplete, +}: MarkdownVisComponentProps) => { + useEffect(renderComplete); // renderComplete will be called after each render to signal, that we are done with rendering. - /** - * Render the actual HTML. - * Note: if only fontSize parameter has changed, this method will be called - * and return the appropriate JSX, but React will detect, that only the - * style argument has been updated, and thus only set this attribute to the DOM. - */ - render() { - return ( -
- -
- ); - } -} - -/** - * This is a wrapper component, that is actually used as the visualization. - * The sole purpose of this component is to extract all required parameters from - * the properties and pass them down as separate properties to the actual component. - * That way the actual (MarkdownVisComponent) will properly trigger it's prop update - * callback (componentWillReceiveProps) if one of these params change. It wouldn't - * trigger otherwise (e.g. it doesn't for this wrapper), since it only triggers - * if the reference to the prop changes (in this case the reference to vis). - * - * The way React works, this wrapper nearly brings no overhead, but allows us - * to use proper lifecycle methods in the actual component. - */ - -export interface MarkdownVisWrapperProps { - visParams: MarkdownVisParams; - fireEvent: (event: any) => void; - renderComplete: () => void; -} - -export function MarkdownVisWrapper(props: MarkdownVisWrapperProps) { return ( - +
+ +
); -} +}; + +// default export required for React.Lazy +// eslint-disable-next-line import/no-default-export +export { MarkdownVisComponent as default }; diff --git a/src/plugins/vis_type_markdown/public/plugin.ts b/src/plugins/vis_type_markdown/public/plugin.ts index c117df7e0fa33..790b19876d366 100644 --- a/src/plugins/vis_type_markdown/public/plugin.ts +++ b/src/plugins/vis_type_markdown/public/plugin.ts @@ -24,10 +24,7 @@ import { VisualizationsSetup } from '../../visualizations/public'; import { markdownVisDefinition } from './markdown_vis'; import { createMarkdownVisFn } from './markdown_fn'; import { ConfigSchema } from '../config'; - -import './index.scss'; -import { getMarkdownRenderer } from './markdown_renderer'; -import { createStartServicesGetter } from '../../kibana_utils/public'; +import { markdownVisRenderer } from './markdown_renderer'; /** @internal */ export interface MarkdownPluginSetupDependencies { @@ -44,9 +41,8 @@ export class MarkdownPlugin implements Plugin { } public setup(core: CoreSetup, { expressions, visualizations }: MarkdownPluginSetupDependencies) { - const start = createStartServicesGetter(core.getStartServices); visualizations.createBaseVisualization(markdownVisDefinition); - expressions.registerRenderer(getMarkdownRenderer(start)); + expressions.registerRenderer(markdownVisRenderer); expressions.registerFunction(createMarkdownVisFn); } diff --git a/src/plugins/vis_type_metric/public/_metric_vis.scss b/src/plugins/vis_type_metric/public/components/metric_vis.scss similarity index 78% rename from src/plugins/vis_type_metric/public/_metric_vis.scss rename to src/plugins/vis_type_metric/public/components/metric_vis.scss index b1f04cc93c4b7..5665ba8e8d099 100644 --- a/src/plugins/vis_type_metric/public/_metric_vis.scss +++ b/src/plugins/vis_type_metric/public/components/metric_vis.scss @@ -1,3 +1,10 @@ +// Prefix all styles with "mtr" to avoid conflicts. +// Examples +// mtrChart +// mtrChart__legend +// mtrChart__legend--small +// mtrChart__legend-isLoading + .mtrVis { width: 100%; display: flex; diff --git a/src/plugins/vis_type_metric/public/components/metric_vis_component.test.tsx b/src/plugins/vis_type_metric/public/components/metric_vis_component.test.tsx index b56d4e4f62e41..7f82c6adb5694 100644 --- a/src/plugins/vis_type_metric/public/components/metric_vis_component.test.tsx +++ b/src/plugins/vis_type_metric/public/components/metric_vis_component.test.tsx @@ -20,7 +20,7 @@ import React from 'react'; import { shallow } from 'enzyme'; -import { MetricVisComponent, MetricVisComponentProps } from './metric_vis_component'; +import MetricVisComponent, { MetricVisComponentProps } from './metric_vis_component'; jest.mock('../services', () => ({ getFormatService: () => ({ diff --git a/src/plugins/vis_type_metric/public/components/metric_vis_component.tsx b/src/plugins/vis_type_metric/public/components/metric_vis_component.tsx index 9ce3820ee4e23..e5c7db65c09a8 100644 --- a/src/plugins/vis_type_metric/public/components/metric_vis_component.tsx +++ b/src/plugins/vis_type_metric/public/components/metric_vis_component.tsx @@ -30,14 +30,16 @@ import { getFormatService } from '../services'; import { SchemaConfig } from '../../../visualizations/public'; import { Range } from '../../../expressions/public'; +import './metric_vis.scss'; + export interface MetricVisComponentProps { - visParams: VisParams; + visParams: Pick; visData: Input; fireEvent: (event: any) => void; renderComplete: () => void; } -export class MetricVisComponent extends Component { +class MetricVisComponent extends Component { private getLabels() { const config = this.props.visParams.metric; const isPercentageMode = config.percentageMode; @@ -209,3 +211,7 @@ export class MetricVisComponent extends Component { return metricsHtml; } } + +// default export required for React.Lazy +// eslint-disable-next-line import/no-default-export +export { MetricVisComponent as default }; diff --git a/src/plugins/vis_type_metric/public/index.scss b/src/plugins/vis_type_metric/public/index.scss deleted file mode 100644 index 638f9ac1ef93a..0000000000000 --- a/src/plugins/vis_type_metric/public/index.scss +++ /dev/null @@ -1,8 +0,0 @@ -// Prefix all styles with "mtr" to avoid conflicts. -// Examples -// mtrChart -// mtrChart__legend -// mtrChart__legend--small -// mtrChart__legend-isLoading - -@import 'metric_vis'; diff --git a/src/plugins/vis_type_metric/public/index.ts b/src/plugins/vis_type_metric/public/index.ts index 3d3e1879a51d9..ac541a9577cfc 100644 --- a/src/plugins/vis_type_metric/public/index.ts +++ b/src/plugins/vis_type_metric/public/index.ts @@ -16,7 +16,6 @@ * specific language governing permissions and limitations * under the License. */ -import './index.scss'; import { PluginInitializerContext } from 'kibana/public'; import { MetricVisPlugin as Plugin } from './plugin'; diff --git a/src/plugins/vis_type_metric/public/metric_vis_fn.ts b/src/plugins/vis_type_metric/public/metric_vis_fn.ts index b58be63581724..97b1e6822333e 100644 --- a/src/plugins/vis_type_metric/public/metric_vis_fn.ts +++ b/src/plugins/vis_type_metric/public/metric_vis_fn.ts @@ -46,7 +46,7 @@ interface Arguments { bucket: any; // these aren't typed yet } -interface RenderValue { +export interface MetricVisRenderValue { visType: typeof visType; visData: Input; visConfig: Pick; @@ -57,7 +57,7 @@ export type MetricVisExpressionFunctionDefinition = ExpressionFunctionDefinition 'metricVis', Input, Arguments, - Render + Render >; export const createMetricVisFn = (): MetricVisExpressionFunctionDefinition => ({ diff --git a/src/plugins/vis_type_metric/public/metric_vis_renderer.tsx b/src/plugins/vis_type_metric/public/metric_vis_renderer.tsx index 2bae668b080ea..bf0d6da9fba05 100644 --- a/src/plugins/vis_type_metric/public/metric_vis_renderer.tsx +++ b/src/plugins/vis_type_metric/public/metric_vis_renderer.tsx @@ -17,37 +17,33 @@ * under the License. */ -import React from 'react'; +import React, { lazy } from 'react'; import { render, unmountComponentAtNode } from 'react-dom'; -import { MetricVisComponent } from './components/metric_vis_component'; -import { getI18n } from './services'; + import { VisualizationContainer } from '../../visualizations/public'; import { ExpressionRenderDefinition } from '../../expressions/common/expression_renderers'; +import { MetricVisRenderValue } from './metric_vis_fn'; +// @ts-ignore +const MetricVisComponent = lazy(() => import('./components/metric_vis_component')); -export const metricVisRenderer: () => ExpressionRenderDefinition = () => ({ +export const metricVisRenderer: () => ExpressionRenderDefinition = () => ({ name: 'metric_vis', displayName: 'metric visualization', reuseDomNode: true, - render: async (domNode: HTMLElement, config: any, handlers: any) => { - const { visData, visConfig } = config; - - const I18nContext = getI18n().Context; - + render: async (domNode, { visData, visConfig }, handlers) => { handlers.onDestroy(() => { unmountComponentAtNode(domNode); }); render( - - - - - , + + + , domNode ); }, diff --git a/src/plugins/vis_type_metric/public/plugin.ts b/src/plugins/vis_type_metric/public/plugin.ts index b9e094aa76889..c653d1bdaf965 100644 --- a/src/plugins/vis_type_metric/public/plugin.ts +++ b/src/plugins/vis_type_metric/public/plugin.ts @@ -25,7 +25,7 @@ import { createMetricVisFn } from './metric_vis_fn'; import { createMetricVisTypeDefinition } from './metric_vis_type'; import { ChartsPluginSetup } from '../../charts/public'; import { DataPublicPluginStart } from '../../data/public'; -import { setFormatService, setI18n } from './services'; +import { setFormatService } from './services'; import { ConfigSchema } from '../config'; import { metricVisRenderer } from './metric_vis_renderer'; @@ -59,7 +59,6 @@ export class MetricVisPlugin implements Plugin { } public start(core: CoreStart, { data }: MetricVisPluginStartDependencies) { - setI18n(core.i18n); setFormatService(data.fieldFormats); } } diff --git a/src/plugins/vis_type_metric/public/services.ts b/src/plugins/vis_type_metric/public/services.ts index 0e19cfdce228d..681afbaf0b268 100644 --- a/src/plugins/vis_type_metric/public/services.ts +++ b/src/plugins/vis_type_metric/public/services.ts @@ -17,12 +17,9 @@ * under the License. */ -import { I18nStart } from 'kibana/public'; import { createGetterSetter } from '../../kibana_utils/common'; import { DataPublicPluginStart } from '../../data/public'; export const [getFormatService, setFormatService] = createGetterSetter< DataPublicPluginStart['fieldFormats'] >('metric data.fieldFormats'); - -export const [getI18n, setI18n] = createGetterSetter('I18n'); diff --git a/src/plugins/visualizations/public/components/_visualization.scss b/src/plugins/visualizations/public/components/_visualization.scss index 785968d2883e7..f5e2d4fcf2862 100644 --- a/src/plugins/visualizations/public/components/_visualization.scss +++ b/src/plugins/visualizations/public/components/_visualization.scss @@ -70,3 +70,10 @@ flex-direction: column; } +.visChart__spinner { + display: flex; + flex: 1 1 auto; + justify-content: center; + align-items: center; +} + diff --git a/src/plugins/visualizations/public/components/visualization_container.tsx b/src/plugins/visualizations/public/components/visualization_container.tsx index d6f87d4cea123..007a9e6e9dde4 100644 --- a/src/plugins/visualizations/public/components/visualization_container.tsx +++ b/src/plugins/visualizations/public/components/visualization_container.tsx @@ -17,14 +17,35 @@ * under the License. */ -import React, { ReactNode } from 'react'; +import React, { ReactNode, Suspense } from 'react'; +import { EuiLoadingChart } from '@elastic/eui'; +import classNames from 'classnames'; +import { VisualizationNoResults } from './visualization_noresults'; interface VisualizationContainerProps { className?: string; children: ReactNode; + showNoResult?: boolean; } -export const VisualizationContainer = (props: VisualizationContainerProps) => { - const classes = `visualization ${props.className}`; - return
{props.children}
; +export const VisualizationContainer = ({ + className, + children, + showNoResult = false, +}: VisualizationContainerProps) => { + const classes = classNames('visualization', className); + + const fallBack = ( +
+ +
+ ); + + return ( +
+ + {showNoResult ? : children} + +
+ ); };