From b1f8e20d713ffe387d6936fc546f8f305dbb37d5 Mon Sep 17 00:00:00 2001 From: Joe Fleming Date: Mon, 11 Jun 2018 14:32:09 -0700 Subject: [PATCH] Feat: Element done handler (#641) * chore: move done handler into element handlers and remove the contructor from ElementHandlers * chore: stricter, better defined prop types also add some comments explaining where various handler functions come from * feat: listen for element done calls * feat: dispatch on element done handler also log a warning if the done handler wasn't called within the time limit * fix: image, call done handler once * chore: create ElementShareContainer component move all the done handler and event dispatching stuff into it, and wrap the element renderer * fix: mark elements render as incomplete * fix: move data-render-complete to correct element also track completed state in ElementShareContainer to avoid event race conditions in reporting * chore: switch isProduction to isDevelopment check --- .../element_content/element_content.js | 42 ++++++++----- public/components/element_content/index.js | 7 +-- .../element_share_container.js | 59 +++++++++++++++++++ .../element_share_container/index.js | 1 + public/components/element_wrapper/index.js | 4 +- .../element_wrapper/lib/handlers.js | 13 ++++ public/components/render_with_fn/index.js | 3 +- .../components/render_with_fn/lib/handlers.js | 7 +-- .../render_with_fn/render_with_fn.js | 24 ++++++-- public/render_functions/image.js | 2 - 10 files changed, 129 insertions(+), 33 deletions(-) create mode 100644 public/components/element_share_container/element_share_container.js create mode 100644 public/components/element_share_container/index.js diff --git a/public/components/element_content/element_content.js b/public/components/element_content/element_content.js index dbecd847d971c2..788b5b0bb10cb4 100644 --- a/public/components/element_content/element_content.js +++ b/public/components/element_content/element_content.js @@ -1,10 +1,11 @@ import React from 'react'; import PropTypes from 'prop-types'; -import { pure, compose, branch, renderComponent, withProps } from 'recompose'; +import { pure, compose, branch, renderComponent } from 'recompose'; import Style from 'style-it'; +import { getType } from '../../../common/lib/get_type'; import { Loading } from '../loading'; import { RenderWithFn } from '../render_with_fn'; -import { getType } from '../../../common/lib/get_type'; +import { ElementShareContainer } from '../element_share_container'; import { InvalidExpression } from './invalid_expression'; import { InvalidElementType } from './invalid_element_type'; @@ -31,21 +32,20 @@ const branches = [ !renderFunction // We can't find an element in the registry for this ); }, renderComponent(InvalidExpression)), - withProps(({ handlers }) => ({ - handlers: { - done() {}, - ...handlers, - }, - })), ]; -// NOTE: the data-shared-* attributes here are used for reporting export const ElementContent = compose(pure, ...branches)( ({ renderable, renderFunction, size, handlers }) => { + const { getFilter, setFilter, done, onComplete } = handlers; + return Style.it( renderable.css,
-
+ -
+
); } ); ElementContent.propTypes = { - renderable: PropTypes.object, - renderFunction: PropTypes.object, + renderable: PropTypes.shape({ + css: PropTypes.string, + value: PropTypes.object, + }), + renderFunction: PropTypes.shape({ + name: PropTypes.string, + render: PropTypes.func, + reuseDomNode: PropTypes.bool, + }), size: PropTypes.object, - handlers: PropTypes.object, + handlers: PropTypes.shape({ + setFilter: PropTypes.func.isRequired, + getFilter: PropTypes.func.isRequired, + done: PropTypes.func.isRequired, + onComplete: PropTypes.func.isRequired, // local, not passed through + }).isRequired, state: PropTypes.string, }; diff --git a/public/components/element_content/index.js b/public/components/element_content/index.js index 6c2b013aebc3e9..d881dfb8e91e1d 100644 --- a/public/components/element_content/index.js +++ b/public/components/element_content/index.js @@ -11,8 +11,7 @@ export const ElementContent = compose( )(Component); ElementContent.propTypes = { - renderable: PropTypes.object, - size: PropTypes.object, - handlers: PropTypes.object, - state: PropTypes.string, + renderable: PropTypes.shape({ + as: PropTypes.string, + }), }; diff --git a/public/components/element_share_container/element_share_container.js b/public/components/element_share_container/element_share_container.js new file mode 100644 index 00000000000000..0ec83522798443 --- /dev/null +++ b/public/components/element_share_container/element_share_container.js @@ -0,0 +1,59 @@ +import React from 'react'; +import PropTypes from 'prop-types'; + +export class ElementShareContainer extends React.PureComponent { + static propTypes = { + functionName: PropTypes.string.isRequired, + onComplete: PropTypes.func.isRequired, + className: PropTypes.string, + children: PropTypes.node.isRequired, + }; + + state = { + renderComplete: false, + }; + + componentDidMount() { + const { functionName, onComplete } = this.props; + const isDevelopment = process.env.NODE_ENV !== 'production'; + let t; + + // check that the done event is called within a certain time + if (isDevelopment) { + const timeout = 15000; // 15 seconds + t = setTimeout(() => { + // TODO: show this message in a proper notification + console.warn(`done handler never called in render function: ${functionName}`); + }, timeout); + } + + // dispatches a custom DOM event on the container when the element is complete + onComplete(() => { + clearTimeout(t); + const ev = new Event('renderComplete'); + this.sharedItemRef.dispatchEvent(ev); + + // if the element is finished before reporting is listening for then + // renderComplete event, the report never completes. to get around that + // issue, track the completed state locally and set the + // [data-render-complete] value accordingly. + // this is similar to renderComplete directive in Kibana, + // see: src/ui/public/render_complete/directive.js + this.setState({ renderComplete: true }); + }); + } + + render() { + // NOTE: the data-shared-item and data-render-complete attributes are used for reporting + return ( +
(this.sharedItemRef = ref)} + > + {this.props.children} +
+ ); + } +} diff --git a/public/components/element_share_container/index.js b/public/components/element_share_container/index.js new file mode 100644 index 00000000000000..6196708b280c30 --- /dev/null +++ b/public/components/element_share_container/index.js @@ -0,0 +1 @@ +export { ElementShareContainer } from './element_share_container'; diff --git a/public/components/element_wrapper/index.js b/public/components/element_wrapper/index.js index 475c25d7516aeb..84373c6c18b5eb 100644 --- a/public/components/element_wrapper/index.js +++ b/public/components/element_wrapper/index.js @@ -47,5 +47,7 @@ const mergeProps = (stateProps, dispatchProps, { element }) => { export const ElementWrapper = connect(mapStateToProps, mapDispatchToProps, mergeProps)(Component); ElementWrapper.propTypes = { - element: PropTypes.object, + element: PropTypes.shape({ + id: PropTypes.string.isRequired, + }).isRequired, }; diff --git a/public/components/element_wrapper/lib/handlers.js b/public/components/element_wrapper/lib/handlers.js index c386729ddbe0c6..1fa5816adf2a4b 100644 --- a/public/components/element_wrapper/lib/handlers.js +++ b/public/components/element_wrapper/lib/handlers.js @@ -1,6 +1,9 @@ import { setFilter } from '../../../state/actions/elements'; export function createHandlers(element, pageId, dispatch) { + let isComplete = false; + let completeFn = () => {}; + return { setFilter(text) { dispatch(setFilter(text, element.id, pageId, true)); @@ -9,5 +12,15 @@ export function createHandlers(element, pageId, dispatch) { getFilter() { return element.filter; }, + + onComplete(fn) { + completeFn = fn; + }, + + done() { + if (isComplete) return; // don't emit if the element is already done + isComplete = true; + completeFn(); + }, }; } diff --git a/public/components/render_with_fn/index.js b/public/components/render_with_fn/index.js index ce06e6580046d1..92c9d582518385 100644 --- a/public/components/render_with_fn/index.js +++ b/public/components/render_with_fn/index.js @@ -12,11 +12,12 @@ export const RenderWithFn = compose( }) ), withProps(({ handlers, elementHandlers }) => ({ - handlers: Object.assign(elementHandlers, handlers, { done: () => {} }), + handlers: Object.assign(elementHandlers, handlers), onError: message => notify.error(message), })) )(Component); RenderWithFn.propTypes = { handlers: PropTypes.object, + elementHandlers: PropTypes.object, }; diff --git a/public/components/render_with_fn/lib/handlers.js b/public/components/render_with_fn/lib/handlers.js index 259857d9fc29db..40aa92fa862f8f 100644 --- a/public/components/render_with_fn/lib/handlers.js +++ b/public/components/render_with_fn/lib/handlers.js @@ -1,8 +1,7 @@ export class ElementHandlers { - constructor() { - this.resize = () => {}; - this.destroy = () => {}; - } + resize() {} + + destroy() {} onResize(fn) { this.resize = fn; diff --git a/public/components/render_with_fn/render_with_fn.js b/public/components/render_with_fn/render_with_fn.js index 38e92a4e09a2b7..1fa2f067f309f4 100644 --- a/public/components/render_with_fn/render_with_fn.js +++ b/public/components/render_with_fn/render_with_fn.js @@ -6,16 +6,29 @@ import { RenderToDom } from '../render_to_dom'; export class RenderWithFn extends React.Component { static propTypes = { - name: PropTypes.string, + name: PropTypes.string.isRequired, renderFn: PropTypes.func.isRequired, reuseNode: PropTypes.bool, - handlers: PropTypes.object, - destroyFn: PropTypes.func, - config: PropTypes.object, - size: PropTypes.object, + handlers: PropTypes.shape({ + // element handlers, see components/element_wrapper/lib/handlers.js + setFilter: PropTypes.func.isRequired, + getFilter: PropTypes.func.isRequired, + done: PropTypes.func.isRequired, + // render handlers, see lib/handlers.js + resize: PropTypes.func.isRequired, + onResize: PropTypes.func.isRequired, + destroy: PropTypes.func.isRequired, + onDestroy: PropTypes.func.isRequired, + }), + config: PropTypes.object.isRequired, + size: PropTypes.object.isRequired, onError: PropTypes.func.isRequired, }; + static defaultProps = { + reuseNode: false, + }; + static domNode = null; componentDidMount() { @@ -109,7 +122,6 @@ export class RenderWithFn extends React.Component { return (
({ ReactDOM.render(
, domNode, () => handlers.done()); handlers.onDestroy(() => ReactDOM.unmountComponentAtNode(domNode)); - - handlers.done(); }, });