Skip to content

Commit

Permalink
Decouple arg types (elastic#357)
Browse files Browse the repository at this point in the history
* Refactored render_element

* Refactored render_element. Need to fix componentDidUpdate

* Refactored render_element. Fixed resetRenderTarget.

* Fixed resetRenderTarget.

* Added comment to render-to-dom

* chore: post about cafe canvas

cross-link to the elastic blog

* Refactored arg_form. Created arg_template_form.

* Refactored datacolumn arg type

* Added workpad selector to arg_form

* Refactored color_picker to no longer access the redux state. Created workpad_color_picker which still uses state
Refactored font arg_type.

* example workpads

* example workpads image

* example workpads + python script zip file

* chore: publish exmaple workpads blog post

* Added isReact prop to arg_type to indicate the arg type is a react component.

* Fixed workpadcolorpicker export and import into  color_picker_mini

* Bug Fix: Changed PropType for value in  NumberArgInput from string to number

* Removed unused proptype in arg_template_form and done function from handlers

* Removed getArgumentProps function and moved logic

* Removed unused prop labels from font extended template

* moved error rendering to arg_template_form. Fixed logic for expandable label

* Removed isReact prop

* Refactored arg_types

* Created lib function to turn a template into a react component

* Moved exp type handlers into common/lib

* Fixed error handling for errors thrown in arg templates

* Added promise to renderError

* Datacolumn returns null when mathValue is invalid

* Removed error handler from expression_form_handlers

* Extracted colors from workpad in arg_types that have ColorPickerMini as a child component
  • Loading branch information
cqliu1 authored and Rashid Khan committed Apr 5, 2018
1 parent e184364 commit 80c2682
Show file tree
Hide file tree
Showing 35 changed files with 354 additions and 138 deletions.
10 changes: 10 additions & 0 deletions common/lib/expression_form_handlers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export class ExpressionFormHandlers {
constructor() {
this.destroy = () => {};
this.done = () => {};
}

onDestroy(fn) {
this.destroy = fn;
}
}
100 changes: 54 additions & 46 deletions public/components/arg_form/arg_form.js
Original file line number Diff line number Diff line change
@@ -1,46 +1,13 @@
import React, { createElement } from 'react';
import React from 'react';
import PropTypes from 'prop-types';
import { ErrorBoundary } from '../enhance/error_boundary';
import { RenderError } from '../../../common/lib/errors';
import { ArgLabel } from './arg_label';
import { ArgSimpleForm } from './arg_simple_form';
import { ArgTemplateForm } from './arg_template_form';
import { SimpleFailure } from './simple_failure';
import { AdvancedFailure } from './advanced_failure';
import './arg_form.less';

const getTemplates = (
err,
{ argTypeInstance, label, setLabel, resetErrorState, templateProps }
) => {
const { template, simpleTemplate } = argTypeInstance.argType;
const argumentProps = {
...templateProps,
defaultValue: argTypeInstance.default,
// Provide templates with a renderError method, and wrap the error in a known error type
// to stop Kibana's window.error from being called
// see window_error_handler.js for details
renderError: msg => {
throw new RenderError(msg || 'Render failed');
},
setLabel,
resetErrorState,
label,
};

// if provided any kind of error, render the fallback failure forms
if (err) {
return {
simpleForm: createElement(SimpleFailure, argumentProps),
advancedForm: createElement(AdvancedFailure, argumentProps),
};
}

return {
simpleForm: simpleTemplate && createElement(simpleTemplate, argumentProps),
advancedForm: template && createElement(template, argumentProps),
};
};

// This is what is being generated by render() from the Arg class. It is called in FunctionForm
export const ArgForm = props => {
const {
Expand All @@ -52,27 +19,53 @@ export const ArgForm = props => {
expand,
setExpand,
onValueRemove,
workpad,
renderError,
setRenderError,
} = props;

return (
<ErrorBoundary>
{({ error, resetErrorState }) => {
const { simpleForm, advancedForm } = getTemplates(error, {
argTypeInstance,
label,
const { template, simpleTemplate } = argTypeInstance.argType;

const hasError = Boolean(error) || renderError;

const argumentProps = {
...templateProps,
defaultValue: argTypeInstance.default,

renderError: () => {
// TODO: don't do this
// It's an ugly hack to avoid React's render cycle and ensure the error happens on the next tick
// This is important; Otherwise we end up updating state in the middle of a render cycle
Promise.resolve().then(() => {
// Provide templates with a renderError method, and wrap the error in a known error type
// to stop Kibana's window.error from being called
// see window_error_handler.js for details,
setRenderError(true);
});
},
error: hasError,
setLabel,
resetErrorState,
templateProps,
});
const expandableLabel = Boolean(simpleForm && advancedForm);
resetErrorState: () => {
resetErrorState();
setRenderError(false);
},
label,
workpad,
};

const expandableLabel = hasError || Boolean(simpleTemplate && template);

const showAdvanced = hasError ? expand : template && (expand || !simpleTemplate);
return (
<div className="canvas__arg">
<div className="canvas__arg--header">
<ArgLabel
label={label}
help={argTypeInstance.help}
expandable={expandableLabel || error}
expandable={expandableLabel}
expanded={expand}
setExpand={setExpand}
/>
Expand All @@ -82,15 +75,27 @@ export const ArgForm = props => {
valueMissing={valueMissing}
onRemove={onValueRemove}
>
{simpleForm}
<ArgTemplateForm
template={simpleTemplate}
errorTemplate={SimpleFailure}
error={hasError}
argumentProps={argumentProps}
/>
</ArgSimpleForm>
</div>

<div
className="canvas__arg--controls"
style={{ display: advancedForm && (expand || !simpleForm) ? 'block' : 'none' }}
style={{
display: showAdvanced ? 'block' : 'none',
}}
>
{advancedForm}
<ArgTemplateForm
template={template}
errorTemplate={AdvancedFailure}
error={hasError}
argumentProps={argumentProps}
/>
</div>
</div>
);
Expand All @@ -100,6 +105,7 @@ export const ArgForm = props => {
};

ArgForm.propTypes = {
workpad: PropTypes.object.isRequired,
argTypeInstance: PropTypes.object,
templateProps: PropTypes.object,
valueMissing: PropTypes.bool,
Expand All @@ -108,4 +114,6 @@ ArgForm.propTypes = {
expand: PropTypes.bool,
setExpand: PropTypes.func,
onValueRemove: PropTypes.func,
renderError: PropTypes.bool.isRequired,
setRenderError: PropTypes.func.isRequired,
};
80 changes: 80 additions & 0 deletions public/components/arg_form/arg_template_form.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import React from 'react';
import PropTypes from 'prop-types';
import { compose, withPropsOnChange, withProps } from 'recompose';
import { RenderToDom } from '../render_to_dom';
import { ExpressionFormHandlers } from '../../../common/lib/expression_form_handlers';
import './arg_form.less';

class ArgTemplateFormComponent extends React.Component {
static propTypes = {
template: PropTypes.func,
argumentProps: PropTypes.shape({
valueMissing: PropTypes.bool,
label: PropTypes.string,
setLabel: PropTypes.func.isRequired,
expand: PropTypes.bool,
setExpand: PropTypes.func,
onValueRemove: PropTypes.func,
resetErrorState: PropTypes.func.isRequired,
}),
handlers: PropTypes.object.isRequired,
error: PropTypes.oneOfType([PropTypes.object, PropTypes.bool]).isRequired,
errorTemplate: PropTypes.oneOfType([PropTypes.element, PropTypes.func]).isRequired,
};

static domNode = null;

componentWillUpdate(prevProps) {
//see if error state changed
if (this.props.error !== prevProps.error) this.props.handlers.destroy();
}
componentDidUpdate() {
if (this.props.error) return this.renderErrorTemplate();
this.renderTemplate(this.domNode);
}

componentWillUnmount() {
this.props.handlers.destroy();
}

renderTemplate = domNode => {
const { template, argumentProps, handlers } = this.props;
if (template) {
return template(domNode, argumentProps, handlers);
}
};

renderErrorTemplate = () => {
const { errorTemplate, argumentProps } = this.props;
return React.createElement(errorTemplate, argumentProps);
};

render() {
const { template, error } = this.props;

if (error) return this.renderErrorTemplate();

if (!template) return null;

return (
<RenderToDom
render={domNode => {
this.domNode = domNode;
this.renderTemplate(domNode);
}}
/>
);
}
}

export const ArgTemplateForm = compose(
withPropsOnChange(
() => false,
() => ({
expressionFormHandlers: new ExpressionFormHandlers(),
})
),
withProps(({ handlers, expressionFormHandlers }) => ({
handlers: Object.assign(expressionFormHandlers, handlers),
}))
)(ArgTemplateFormComponent);
14 changes: 12 additions & 2 deletions public/components/arg_form/index.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,22 @@
import PropTypes from 'prop-types';
import { compose, withState } from 'recompose';
import { connect } from 'react-redux';
import { compose, withState, lifecycle } from 'recompose';
import { getWorkpadInfo } from '../../state/selectors/workpad';
import { ArgForm as Component } from './arg_form';

export const ArgForm = compose(
withState('label', 'setLabel', ({ label, argTypeInstance }) => {
return label || argTypeInstance.displayName || argTypeInstance.name;
}),
withState('expand', 'setExpand', ({ argTypeInstance }) => argTypeInstance.expanded)
withState('expand', 'setExpand', ({ argTypeInstance }) => argTypeInstance.expanded),
withState('renderError', 'setRenderError', false),
lifecycle({
componentWillUpdate(prevProps) {
if (prevProps.templateProps.argValue !== this.props.templateProps.argValue)
this.props.setRenderError(false);
},
}),
connect(state => ({ workpad: getWorkpadInfo(state) }))
)(Component);

ArgForm.propTypes = {
Expand Down
18 changes: 11 additions & 7 deletions public/components/color_manager/color_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,21 @@ export const ColorManager = ({ value, addColor, removeColor, onChange }) => (
onChange={e => onChange(e.target.value)}
/>
</div>
<i onClick={() => addColor(value)} className="canvas__color-manager--add fa fa-plus-circle" />
<i
onClick={() => removeColor(value)}
className="canvas__color-manager--remove fa fa-times-circle"
/>
{addColor && (
<i onClick={() => addColor(value)} className="canvas__color-manager--add fa fa-plus-circle" />
)}
{removeColor && (
<i
onClick={() => removeColor(value)}
className="canvas__color-manager--remove fa fa-times-circle"
/>
)}
</div>
);

ColorManager.propTypes = {
value: PropTypes.string,
addColor: PropTypes.func.isRequired,
removeColor: PropTypes.func.isRequired,
addColor: PropTypes.func,
removeColor: PropTypes.func,
onChange: PropTypes.func.isRequired,
};
15 changes: 2 additions & 13 deletions public/components/color_picker/index.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,5 @@
import { connect } from 'react-redux';
import { getWorkpadColors } from '../../state/selectors/workpad';
import { addColor, removeColor } from '../../state/actions/workpad';
import { pure } from 'recompose';

import { ColorPicker as Component } from './color_picker';

const mapStateToProps = state => ({
colors: getWorkpadColors(state),
});

const mapDispatchToProps = {
addColor,
removeColor,
};

export const ColorPicker = connect(mapStateToProps, mapDispatchToProps)(Component);
export const ColorPicker = pure(Component);
10 changes: 8 additions & 2 deletions public/components/color_picker_mini/color_picker_mini.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,16 @@ import { Popover, OverlayTrigger } from 'react-bootstrap';
import { ColorPicker } from '../color_picker';
import { ColorDot } from '../color_dot';
import './color_picker_mini.less';
import { WorkpadColorPicker } from '../workpad_color_picker/';

export const ColorPickerMini = ({ onChange, value, placement }) => {
export const ColorPickerMini = ({ onChange, value, placement, colors }) => {
const picker = (
<Popover id="popover-trigger-click" style={{ width: 207 }}>
<ColorPicker onChange={onChange} value={value} />
{colors ? (
<ColorPicker onChange={onChange} value={value} colors={colors} />
) : (
<WorkpadColorPicker onChange={onChange} value={value} />
)}
</Popover>
);

Expand All @@ -24,6 +29,7 @@ export const ColorPickerMini = ({ onChange, value, placement }) => {
};

ColorPickerMini.propTypes = {
colors: PropTypes.array,
value: PropTypes.string,
onChange: PropTypes.func,
placement: PropTypes.string,
Expand Down
7 changes: 4 additions & 3 deletions public/components/render_to_dom/render_to_dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@ export class RenderToDom extends React.Component {
}

render() {
const { domNode, setDomNode, style } = this.props;
const linkRef = refNode => {
if (!this.props.domNode && refNode) {
if (!domNode && refNode) {
// Initialize the domNode property. This should only happen once, even if config changes.
this.props.setDomNode(refNode);
setDomNode(refNode);
}
};

return <div className="render_to_dom" ref={linkRef} style={this.props.style} />;
return <div className="render_to_dom" ref={linkRef} style={style} />;
}
}
8 changes: 7 additions & 1 deletion public/components/text_style_picker/text_style_picker.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export const TextStylePicker = ({
underline,
italic,
onChange,
colors,
}) => {
function doChange(propName, value) {
onChange({
Expand Down Expand Up @@ -48,7 +49,11 @@ export const TextStylePicker = ({
<FontPicker value={family} onSelect={value => doChange('family', value)} />
</FormGroup>
<FormGroup className="canvas__text-style-picker--bottom">
<ColorPickerMini value={color} onChange={value => doChange('color', value)} />
<ColorPickerMini
value={color}
onChange={value => doChange('color', value)}
colors={colors}
/>
<ButtonGroup bsSize="small">
<Button
active={weight === 'bold'}
Expand Down Expand Up @@ -89,4 +94,5 @@ TextStylePicker.propTypes = {
underline: PropTypes.bool,
italic: PropTypes.bool,
onChange: PropTypes.func.isRequired,
colors: PropTypes.array,
};
Loading

0 comments on commit 80c2682

Please sign in to comment.