Skip to content

Commit

Permalink
[Expressions] [Lens] Implement a loading state and error state in the…
Browse files Browse the repository at this point in the history
… ExpressionRenderer (#48841)

* Add loading indicator to Lens workspace panel

* [Expressions] [Lens] Handle loading and errors in ExpressionRenderer

* Using loading$ observable and improve tests

* Using CSS and to handle layout of expression renderer

Added TODO for using chart loader when area is completely empty

* Improve error handling and simplify code

* Fix cleanup behavior

* Fix double render and prevent error cases in xy chart

* Fix context for use in dashboards

* Remove className from expression rendere component

* Improve handling of additional interpreter args

* More layout fixes

- Hide chart if Empty not Loading
- Fix relative positioning for progress bar since className is no longer passed (super hacky)
  • Loading branch information
Wylie Conlon authored Oct 25, 2019
1 parent 9bd8f74 commit ed07fb4
Show file tree
Hide file tree
Showing 26 changed files with 832 additions and 399 deletions.
1 change: 1 addition & 0 deletions src/legacy/core_plugins/expressions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export default function DataExpressionsPlugin(kibana: any) {
init: (server: Legacy.Server) => ({}),
uiExports: {
injectDefaultVars: () => ({}),
styleSheetPaths: resolve(__dirname, 'public/index.scss'),
},
};

Expand Down
13 changes: 13 additions & 0 deletions src/legacy/core_plugins/expressions/public/index.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Import the EUI global scope so we can use EUI constants
@import 'src/legacy/ui/public/styles/_styling_constants';

/* Expressions plugin styles */

// Prefix all styles with "exp" to avoid conflicts.
// Examples
// expChart
// expChart__legend
// expChart__legend--small
// expChart__legend-isLoading

@import './np_ready/public/index';
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
.expExpressionRenderer {
position: relative;
display: flex;
align-items: center;
justify-content: center;
width: 100%;
height: 100%;
}

.expExpressionRenderer__expression {
width: 100%;
height: 100%;
}

.expExpressionRenderer-isEmpty,
.expExpressionRenderer-hasError {
.expExpressionRenderer__expression {
display: none;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import './expression_renderer';
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export class ExpressionDataHandler {
this.promise = interpreter.interpretAst(this.ast, params.context || defaultContext, {
getInitialContext,
inspectorAdapters: this.inspectorAdapters,
abortSignal: this.abortController.signal,
});
}

Expand All @@ -69,7 +70,18 @@ export class ExpressionDataHandler {
};

getData = async () => {
return await this.promise;
try {
return await this.promise;
} catch (e) {
return {
type: 'error',
error: {
type: e.type,
message: e.message,
stack: e.stack,
},
};
}
};

getExpression = () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import React from 'react';
import { Subject } from 'rxjs';
import { share } from 'rxjs/operators';
import { ExpressionRendererImplementation } from './expression_renderer';
import { ExpressionLoader } from './loader';
import { mount } from 'enzyme';
import { EuiProgress } from '@elastic/eui';

jest.mock('./loader', () => {
return {
ExpressionLoader: jest.fn().mockImplementation(() => {
return {};
}),
loader: jest.fn(),
};
});

describe('ExpressionRenderer', () => {
it('starts to load, resolves, and goes back to loading', () => {
const dataSubject = new Subject();
const data$ = dataSubject.asObservable().pipe(share());
const renderSubject = new Subject();
const render$ = renderSubject.asObservable().pipe(share());
const loadingSubject = new Subject();
const loading$ = loadingSubject.asObservable().pipe(share());

(ExpressionLoader as jest.Mock).mockImplementation(() => {
return {
render$,
data$,
loading$,
update: jest.fn(),
};
});

const instance = mount(<ExpressionRendererImplementation expression="" />);

loadingSubject.next();
instance.update();

expect(instance.find(EuiProgress)).toHaveLength(1);

renderSubject.next(1);

instance.update();

expect(instance.find(EuiProgress)).toHaveLength(0);

instance.setProps({ expression: 'something new' });
loadingSubject.next();
instance.update();

expect(instance.find(EuiProgress)).toHaveLength(1);

renderSubject.next(1);
instance.update();

expect(instance.find(EuiProgress)).toHaveLength(0);
});

it('should display an error message when the expression fails', () => {
const dataSubject = new Subject();
const data$ = dataSubject.asObservable().pipe(share());
const renderSubject = new Subject();
const render$ = renderSubject.asObservable().pipe(share());
const loadingSubject = new Subject();
const loading$ = loadingSubject.asObservable().pipe(share());

(ExpressionLoader as jest.Mock).mockImplementation(() => {
return {
render$,
data$,
loading$,
update: jest.fn(),
};
});

const instance = mount(<ExpressionRendererImplementation expression="" />);

dataSubject.next('good data');
renderSubject.next({
type: 'error',
error: { message: 'render error' },
});
instance.update();

expect(instance.find(EuiProgress)).toHaveLength(0);
expect(instance.find('[data-test-subj="expression-renderer-error"]')).toHaveLength(1);
});

it('should display a custom error message if the user provides one', () => {
const dataSubject = new Subject();
const data$ = dataSubject.asObservable().pipe(share());
const renderSubject = new Subject();
const render$ = renderSubject.asObservable().pipe(share());
const loadingSubject = new Subject();
const loading$ = loadingSubject.asObservable().pipe(share());

(ExpressionLoader as jest.Mock).mockImplementation(() => {
return {
render$,
data$,
loading$,
update: jest.fn(),
};
});

const renderErrorFn = jest.fn().mockReturnValue(null);

const instance = mount(
<ExpressionRendererImplementation expression="" renderError={renderErrorFn} />
);

renderSubject.next({
type: 'error',
error: { message: 'render error' },
});
instance.update();

expect(renderErrorFn).toHaveBeenCalledWith('render error');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,58 +17,116 @@
* under the License.
*/

import { useRef, useEffect } from 'react';
import { useRef, useEffect, useState } from 'react';
import React from 'react';
import { ExpressionAST, IExpressionLoaderParams, IInterpreterResult } from './types';
import { IExpressionLoader, ExpressionLoader } from './loader';
import classNames from 'classnames';
import { EuiLoadingChart, EuiProgress } from '@elastic/eui';
import { ExpressionAST, IExpressionLoaderParams, IInterpreterErrorResult } from './types';
import { ExpressionLoader } from './loader';

// Accept all options of the runner as props except for the
// dom element which is provided by the component itself
export interface ExpressionRendererProps extends IExpressionLoaderParams {
className: string;
dataAttrs?: string[];
expression: string | ExpressionAST;
/**
* If an element is specified, but the response of the expression run can't be rendered
* because it isn't a valid response or the specified renderer isn't available,
* this callback is called with the given result.
*/
onRenderFailure?: (result: IInterpreterResult) => void;
renderError?: (error?: string | null) => React.ReactElement | React.ReactElement[];
}

interface State {
isEmpty: boolean;
isLoading: boolean;
error: null | Error;
}

export type ExpressionRenderer = React.FC<ExpressionRendererProps>;

export const createRenderer = (loader: IExpressionLoader): ExpressionRenderer => ({
className,
const defaultState: State = {
isEmpty: true,
isLoading: false,
error: null,
};

export const ExpressionRendererImplementation = ({
dataAttrs,
expression,
onRenderFailure,
renderError,
...options
}: ExpressionRendererProps) => {
const mountpoint: React.MutableRefObject<null | HTMLDivElement> = useRef(null);
const handlerRef: React.MutableRefObject<null | ExpressionLoader> = useRef(null);
const [state, setState] = useState<State>({ ...defaultState });

// Re-fetch data automatically when the inputs change
useEffect(() => {
if (mountpoint.current) {
if (!handlerRef.current) {
handlerRef.current = loader(mountpoint.current, expression, options);
} else {
handlerRef.current.update(expression, options);
}
handlerRef.current.data$.toPromise().catch(result => {
if (onRenderFailure) {
onRenderFailure(result);
}
});
if (handlerRef.current) {
handlerRef.current.update(expression, options);
}
}, [
expression,
options.searchContext,
options.context,
options.variables,
options.disableCaching,
mountpoint.current,
]);

return <div {...dataAttrs} className={className} ref={mountpoint} />;
// Initialize the loader only once
useEffect(() => {
if (mountpoint.current && !handlerRef.current) {
handlerRef.current = new ExpressionLoader(mountpoint.current, expression, options);

handlerRef.current.loading$.subscribe(() => {
if (!handlerRef.current) {
return;
}
setState(prevState => ({ ...prevState, isLoading: true }));
});
handlerRef.current.render$.subscribe((item: number | IInterpreterErrorResult) => {
if (!handlerRef.current) {
return;
}
if (typeof item !== 'number') {
setState(() => ({
...defaultState,
isEmpty: false,
error: item.error,
}));
} else {
setState(() => ({
...defaultState,
isEmpty: false,
}));
}
});
}
}, [mountpoint.current]);

useEffect(() => {
// We only want a clean up to run when the entire component is unloaded, not on every render
return function cleanup() {
if (handlerRef.current) {
handlerRef.current.destroy();
handlerRef.current = null;
}
};
}, []);

const classes = classNames('expExpressionRenderer', {
'expExpressionRenderer-isEmpty': state.isEmpty,
'expExpressionRenderer-hasError': !!state.error,
});

return (
<div {...dataAttrs} className={classes}>
{state.isEmpty ? <EuiLoadingChart mono size="l" /> : null}
{state.isLoading ? <EuiProgress size="xs" color="accent" position="absolute" /> : null}
{!state.isLoading && state.error ? (
renderError ? (
renderError(state.error.message)
) : (
<div data-test-subj="expression-renderer-error">{state.error.message}</div>
)
) : null}
<div className="expExpressionRenderer__expression" ref={mountpoint} />
</div>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { ExpressionsPublicPlugin } from './plugin';

export * from './plugin';
export { ExpressionRenderer, ExpressionRendererProps } from './expression_renderer';
export { IInterpreterRenderFunction } from './types';
export { IInterpreterRenderFunction, IInterpreterRenderHandlers } from './types';

export function plugin(initializerContext: PluginInitializerContext) {
return new ExpressionsPublicPlugin(initializerContext);
Expand Down
Loading

0 comments on commit ed07fb4

Please sign in to comment.