Skip to content

Commit

Permalink
Feat: Element done handler (elastic#641)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
w33ble authored Jun 11, 2018
1 parent cf5ca79 commit b1f8e20
Show file tree
Hide file tree
Showing 10 changed files with 129 additions and 33 deletions.
42 changes: 27 additions & 15 deletions public/components/element_content/element_content.js
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -31,40 +32,51 @@ 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,
<div style={{ ...renderable.containerStyle, ...size }}>
<div className="canvas__element--content" data-shared-item>
<ElementShareContainer
className="canvas__element--content"
onComplete={onComplete}
functionName={renderFunction.name}
>
<RenderWithFn
name={renderFunction.name}
renderFn={renderFunction.render}
reuseNode={renderFunction.reuseDomNode}
config={renderable.value}
css={renderable.css} // This is an actual CSS stylesheet string, it will be scoped by RenderElement
size={size} // Size is only passed for the purpose of triggering the resize event, it isn't really used otherwise
handlers={handlers}
handlers={{ getFilter, setFilter, done }}
/>
</div>
</ElementShareContainer>
</div>
);
}
);

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,
};
7 changes: 3 additions & 4 deletions public/components/element_content/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}),
};
Original file line number Diff line number Diff line change
@@ -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 (
<div
data-shared-item
data-render-complete={this.state.renderComplete}
className={this.props.className}
ref={ref => (this.sharedItemRef = ref)}
>
{this.props.children}
</div>
);
}
}
1 change: 1 addition & 0 deletions public/components/element_share_container/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { ElementShareContainer } from './element_share_container';
4 changes: 3 additions & 1 deletion public/components/element_wrapper/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
13 changes: 13 additions & 0 deletions public/components/element_wrapper/lib/handlers.js
Original file line number Diff line number Diff line change
@@ -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));
Expand All @@ -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();
},
};
}
3 changes: 2 additions & 1 deletion public/components/render_with_fn/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
7 changes: 3 additions & 4 deletions public/components/render_with_fn/lib/handlers.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
export class ElementHandlers {
constructor() {
this.resize = () => {};
this.destroy = () => {};
}
resize() {}

destroy() {}

onResize(fn) {
this.resize = fn;
Expand Down
24 changes: 18 additions & 6 deletions public/components/render_with_fn/render_with_fn.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -109,7 +122,6 @@ export class RenderWithFn extends React.Component {
return (
<div
className="canvas__workpad--element_render canvas__element"
data-render-complete="true"
style={{ height: '100%', width: '100%' }}
>
<RenderToDom
Expand Down
2 changes: 0 additions & 2 deletions public/render_functions/image.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,5 @@ export const image = () => ({
ReactDOM.render(<div style={style} />, domNode, () => handlers.done());

handlers.onDestroy(() => ReactDOM.unmountComponentAtNode(domNode));

handlers.done();
},
});

0 comments on commit b1f8e20

Please sign in to comment.