From f607547a02d9e1470900d87ca1149c95c28b66cb Mon Sep 17 00:00:00 2001 From: Christopher Joe Date: Tue, 12 Sep 2017 12:49:59 +1200 Subject: [PATCH] Enhancement rename action props with `on` prefix, and action methods with `handle` prefix --- .eslintrc.js | 6 +- .../src/components/Breadcrumb/Breadcrumb.js | 14 ++- .../Breadcrumb/tests/breadcrumb-test.js | 2 +- .../components/CheckboxField/CheckboxField.js | 16 ++-- .../CheckboxSetField/CheckboxSetField.js | 48 +++++----- .../src/components/FieldHolder/FieldHolder.js | 62 ++++++------- client/src/components/Form/Form.js | 1 + .../src/components/FormAction/FormAction.js | 51 +++++------ client/src/components/FormAlert/FormAlert.js | 22 ++--- .../src/components/FormBuilder/FormBuilder.js | 22 +++-- .../FormBuilder/tests/FormBuilder-test.js | 2 +- .../FormBuilderModal/FormBuilderModal.js | 36 +++++--- .../tests/FormBuilderModal-test.js | 2 +- client/src/components/GridField/GridField.js | 72 +++++++-------- .../components/GridField/GridFieldAction.js | 11 +-- .../src/components/GridField/GridFieldCell.js | 18 ++-- .../src/components/ListGroup/ListGroupItem.js | 10 ++- .../MobileMenuToggle/MobileMenuToggle.js | 11 ++- .../components/OptionsetField/OptionField.js | 44 ++++----- .../OptionsetField/OptionsetField.js | 37 ++++---- client/src/components/Toolbar/Toolbar.js | 13 +-- .../tests/TreeDropdownFieldMenu-test.js | 8 +- .../FormBuilderLoader/FormBuilderLoader.js | 6 +- .../InsertLinkModal/InsertLinkModal.js | 5 +- client/src/legacy/AddToCampaignForm.js | 4 +- client/src/lib/formatWrittenNumber.js | 2 +- package.json | 9 +- yarn.lock | 89 ++++--------------- 28 files changed, 295 insertions(+), 328 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index f57d0279c..63009638d 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -45,6 +45,9 @@ module.exports = { "vars": "local" } ], + "react/no-danger": [ + "error" + ], "react/forbid-prop-types": [ "off" ], @@ -60,9 +63,6 @@ module.exports = { "no-useless-escape": [ "off" ], - "react/no-danger": [ - "error" - ], }), "settings": { "import/extensions": [ diff --git a/client/src/components/Breadcrumb/Breadcrumb.js b/client/src/components/Breadcrumb/Breadcrumb.js index 0432c8657..93c3aa093 100644 --- a/client/src/components/Breadcrumb/Breadcrumb.js +++ b/client/src/components/Breadcrumb/Breadcrumb.js @@ -1,4 +1,4 @@ -import React, { Component } from 'react'; +import React, { Component, PropTypes } from 'react'; import { connect } from 'react-redux'; class Breadcrumb extends Component { @@ -44,7 +44,7 @@ class Breadcrumb extends Component { className={iconClassNames.join(' ')} role="button" tabIndex={0} - onClick={crumb.icon.action} + onClick={crumb.icon.onClick} /> )} @@ -67,7 +67,15 @@ class Breadcrumb extends Component { } Breadcrumb.propTypes = { - crumbs: React.PropTypes.array, + crumbs: PropTypes.arrayOf(PropTypes.shape({ + onClick: PropTypes.func, + text: PropTypes.string, + icon: PropTypes.shape({ + className: PropTypes.string, + onClick: PropTypes.func, + action: (props) => { if (props.action) { throw new Error('action: no longer used'); } }, + }) + })), }; function mapStateToProps(state) { diff --git a/client/src/components/Breadcrumb/tests/breadcrumb-test.js b/client/src/components/Breadcrumb/tests/breadcrumb-test.js index 7fa790661..2acbd6273 100644 --- a/client/src/components/Breadcrumb/tests/breadcrumb-test.js +++ b/client/src/components/Breadcrumb/tests/breadcrumb-test.js @@ -25,7 +25,7 @@ describe('BreadcrumbsComponent', () => { href: 'href3', icon: { className: 'breadcrumb3icon', - action: jest.genMockFunction(), + onClick: jest.fn(), }, }, ]; diff --git a/client/src/components/CheckboxField/CheckboxField.js b/client/src/components/CheckboxField/CheckboxField.js index 315765d8a..ba8353e2c 100644 --- a/client/src/components/CheckboxField/CheckboxField.js +++ b/client/src/components/CheckboxField/CheckboxField.js @@ -1,15 +1,13 @@ -import React, { Component } from 'react'; +import React from 'react'; import OptionField from '../OptionsetField/OptionField'; import fieldHolder from 'components/FieldHolder/FieldHolder'; -class CheckboxField extends Component { - render() { - // Build standard checkbox with fieldholder - const FieldHolder = fieldHolder(OptionField); +const CheckboxField = () => { + // Build standard checkbox with fieldholder + const FieldHolder = fieldHolder(OptionField); - // set to not show field holder labels, as checkbox already generates a label - return ; - } -} + // set to not show field holder labels, as checkbox already generates a label + return ; +}; export default CheckboxField; diff --git a/client/src/components/CheckboxSetField/CheckboxSetField.js b/client/src/components/CheckboxSetField/CheckboxSetField.js index 6e5400c25..c4b02de9a 100644 --- a/client/src/components/CheckboxSetField/CheckboxSetField.js +++ b/client/src/components/CheckboxSetField/CheckboxSetField.js @@ -44,30 +44,6 @@ class CheckboxSetField extends Component { return []; } - /** - * Handler for sorting what the value of the field will be, this flows on from the - * OptionField (single checkbox) event handler and adding or removing the corresponding value the - * single checkbox represented to suit the multiple checkbox group. - * - * @param {Event} event - * @param {object} field - */ - handleChange(event, field) { - if (typeof this.props.onChange === 'function') { - const oldValue = this.getValues(); - const newValue = this.props.source - .filter((item, index) => { - if (this.getItemKey(item, index) === field.id) { - return field.value === 1; - } - return oldValue.indexOf(`${item.value}`) > -1; - }) - .map((item) => `${item.value}`); - - this.props.onChange(newValue); - } - } - /** * Fetches properties for an item * @@ -93,6 +69,30 @@ class CheckboxSetField extends Component { }; } + /** + * Handler for sorting what the value of the field will be, this flows on from the + * OptionField (single checkbox) event handler and adding or removing the corresponding value the + * single checkbox represented to suit the multiple checkbox group. + * + * @param {Event} event + * @param {object} field + */ + handleChange(event, field) { + if (typeof this.props.onChange === 'function') { + const oldValue = this.getValues(); + const newValue = this.props.source + .filter((item, index) => { + if (this.getItemKey(item, index) === field.id) { + return field.value === 1; + } + return oldValue.indexOf(`${item.value}`) > -1; + }) + .map((item) => `${item.value}`); + + this.props.onChange(newValue); + } + } + render() { if (!this.props.source) { return null; diff --git a/client/src/components/FieldHolder/FieldHolder.js b/client/src/components/FieldHolder/FieldHolder.js index a172199a4..3d2402374 100644 --- a/client/src/components/FieldHolder/FieldHolder.js +++ b/client/src/components/FieldHolder/FieldHolder.js @@ -27,10 +27,36 @@ function fieldHolder(Field) { return message; } + /** + * Generates the properties for the field holder + * + * @returns {object} + */ + getHolderProps() { + // The extraClass property is defined on both the holder and element + // for legacy reasons (same behaviour as PHP rendering) + const classNames = [ + 'field', + this.props.extraClass, + ]; + if (this.props.readOnly) { + classNames.push('readonly'); + } + + return { + bsClass: this.props.bsClass, + bsSize: this.props.bsSize, + validationState: this.props.validationState, + className: classNames.join(' '), + controlId: this.props.id, + id: this.props.holderId, + }; + } + /** * Build description * - * @returns {Component} + * @returns {React} */ renderDescription() { if (this.props.description === null) { @@ -47,7 +73,7 @@ function fieldHolder(Field) { /** * Build a FormAlert * - * @returns {Component} + * @returns {React} */ renderMessage() { const message = this.getMessage(); @@ -66,7 +92,7 @@ function fieldHolder(Field) { /** * Build title label * - * @returns {Component} + * @returns {React} */ renderLeftTitle() { const labelText = this.props.leftTitle !== null @@ -87,7 +113,7 @@ function fieldHolder(Field) { /** * Build title label * - * @returns {Component} + * @returns {React} */ renderRightTitle() { if (!this.props.rightTitle || this.props.hideLabels) { @@ -101,32 +127,6 @@ function fieldHolder(Field) { ); } - /** - * Generates the properties for the field holder - * - * @returns {object} - */ - getHolderProps() { - // The extraClass property is defined on both the holder and element - // for legacy reasons (same behaviour as PHP rendering) - const classNames = [ - 'field', - this.props.extraClass, - ]; - if (this.props.readOnly) { - classNames.push('readonly'); - } - - return { - bsClass: this.props.bsClass, - bsSize: this.props.bsSize, - validationState: this.props.validationState, - className: classNames.join(' '), - controlId: this.props.id, - id: this.props.holderId, - }; - } - renderField() { const hasMessage = Boolean(this.getMessage()); const props = { @@ -137,7 +137,7 @@ function fieldHolder(Field) { ), }; - const field = ; + const field = ; const prefix = this.props.data.prefix; const suffix = this.props.data.suffix; if (!prefix && !suffix) { diff --git a/client/src/components/Form/Form.js b/client/src/components/Form/Form.js index d3f1340bc..7fa05cf0a 100644 --- a/client/src/components/Form/Form.js +++ b/client/src/components/Form/Form.js @@ -92,6 +92,7 @@ Form.propTypes = { method: PropTypes.string.isRequired, }), fields: PropTypes.array.isRequired, + // props is named `handleSubmit` as it is recieved from redux-form handleSubmit: PropTypes.func, mapActionsToComponents: PropTypes.func.isRequired, mapFieldsToComponents: PropTypes.func.isRequired, diff --git a/client/src/components/FormAction/FormAction.js b/client/src/components/FormAction/FormAction.js index 37a36bfe8..510ccacf6 100644 --- a/client/src/components/FormAction/FormAction.js +++ b/client/src/components/FormAction/FormAction.js @@ -9,17 +9,6 @@ class FormAction extends Component { this.handleClick = this.handleClick.bind(this); } - render() { - const title = this.props.title; - - return ( - - ); - } - /** * Get props for the button * @@ -71,17 +60,6 @@ class FormAction extends Component { return classnames(buttonClasses); } - /** - * @return {boolean} - */ - isPrimary() { - const extraClasses = this.props.extraClass.split(' '); - return ( - this.props.name === 'action_save' || - extraClasses.find(className => className === 'ss-ui-action-constructive') - ); - } - /** * Gets the bootstrap classname for this action * @@ -140,6 +118,17 @@ class FormAction extends Component { return null; } + /** + * @return {boolean} + */ + isPrimary() { + const extraClasses = this.props.extraClass.split(' '); + return ( + this.props.name === 'action_save' || + extraClasses.find(className => className === 'ss-ui-action-constructive') + ); + } + /** * Event handler triggered when a user clicks the button. * @@ -147,16 +136,28 @@ class FormAction extends Component { * @return undefined */ handleClick(event) { - if (typeof this.props.handleClick === 'function') { - this.props.handleClick(event, this.props.name || this.props.id); + if (typeof this.props.onClick === 'function') { + this.props.onClick(event, this.props.name || this.props.id); } } + + render() { + const title = this.props.title; + + return ( + + ); + } } FormAction.propTypes = { id: React.PropTypes.string, name: React.PropTypes.string, - handleClick: React.PropTypes.func, + onClick: React.PropTypes.func, + handleClick: () => { throw new Error('no longer used'); }, title: React.PropTypes.string, type: React.PropTypes.string, loading: React.PropTypes.bool, diff --git a/client/src/components/FormAlert/FormAlert.js b/client/src/components/FormAlert/FormAlert.js index c6850bdea..afcffb48a 100644 --- a/client/src/components/FormAlert/FormAlert.js +++ b/client/src/components/FormAlert/FormAlert.js @@ -17,17 +17,6 @@ class FormAlert extends Component { }; } - /** - * Handler for when the message box is dismissed and hidden - */ - handleDismiss() { - if (typeof this.props.onDismiss === 'function') { - this.props.onDismiss(); - } else { - this.setState({ visible: false }); - } - } - /** * Determines the style for the alert box to be shown based on messageType or other property * by default use "danger". @@ -71,6 +60,17 @@ class FormAlert extends Component { }; } + /** + * Handler for when the message box is dismissed and hidden + */ + handleDismiss() { + if (typeof this.props.onDismiss === 'function') { + this.props.onDismiss(); + } else { + this.setState({ visible: false }); + } + } + render() { // @todo default this.props.visible as null if ((typeof this.props.visible !== 'boolean' && this.state.visible) || this.props.visible) { diff --git a/client/src/components/FormBuilder/FormBuilder.js b/client/src/components/FormBuilder/FormBuilder.js index 951449a7f..bade3e177 100644 --- a/client/src/components/FormBuilder/FormBuilder.js +++ b/client/src/components/FormBuilder/FormBuilder.js @@ -212,8 +212,8 @@ class FormBuilder extends Component { */ handleAction(event) { // Custom handlers - if (typeof this.props.handleAction === 'function') { - this.props.handleAction(event, this.props.values); + if (typeof this.props.onAction === 'function') { + this.props.onAction(event, this.props.values); } // Allow custom handlers to cancel event @@ -257,8 +257,8 @@ class FormBuilder extends Component { throw reason; }); - if (typeof this.props.handleSubmit === 'function') { - return this.props.handleSubmit(dataWithAction, action, submitFn); + if (typeof this.props.onSubmit === 'function') { + return this.props.onSubmit(dataWithAction, action, submitFn); } return submitFn(); @@ -277,7 +277,7 @@ class FormBuilder extends Component { if (action.children) { props.children = this.mapActionsToComponents(action.children); } else { - props.handleClick = this.handleAction; + props.onClick = this.handleAction; // Reset through componentWillReceiveProps() if (this.props.submitting && this.state.submittingAction === action.name) { @@ -396,8 +396,10 @@ const schemaPropType = PropTypes.shape({ const basePropTypes = { createFn: PropTypes.func, - handleSubmit: PropTypes.func, - handleAction: PropTypes.func, + onSubmit: PropTypes.func, + handleSubmit: (props) => { if (props.handleSubmit) { throw new Error('handleSubmit: no longer used'); } }, + onAction: PropTypes.func, + handleAction: (props) => { if (props.handleAction) { throw new Error('handleAction: no longer used'); } }, asyncValidate: PropTypes.func, onSubmitFail: PropTypes.func, onSubmitSuccess: PropTypes.func, @@ -437,5 +439,9 @@ FormBuilder.defaultProps = { autoFocus: false, }; -export { basePropTypes, schemaPropType }; +export { + FormBuilder as Component, + basePropTypes, + schemaPropType +}; export default withInjector(FormBuilder); diff --git a/client/src/components/FormBuilder/tests/FormBuilder-test.js b/client/src/components/FormBuilder/tests/FormBuilder-test.js index 6c6184572..953656e38 100644 --- a/client/src/components/FormBuilder/tests/FormBuilder-test.js +++ b/client/src/components/FormBuilder/tests/FormBuilder-test.js @@ -2,7 +2,7 @@ import React from 'react'; import ReactTestUtils from 'react-addons-test-utils'; -import FormBuilder from '../FormBuilder'; +import { Component as FormBuilder } from '../FormBuilder'; import schemaFieldValues, { findField, schemaMerge } from 'lib/schemaFieldValues'; describe('FormBuilder', () => { diff --git a/client/src/components/FormBuilderModal/FormBuilderModal.js b/client/src/components/FormBuilderModal/FormBuilderModal.js index bb1451a85..610e1bc63 100644 --- a/client/src/components/FormBuilderModal/FormBuilderModal.js +++ b/client/src/components/FormBuilderModal/FormBuilderModal.js @@ -28,8 +28,8 @@ class FormBuilderModal extends Component { return ( @@ -84,13 +84,26 @@ class FormBuilderModal extends Component { }); } + handleLoadingError(schema) { + if (this.props.showErrorMessage) { + const error = schema.errors && schema.errors[0]; + this.setState({ + response: error.value, + error: true, + }); + } + if (typeof this.props.onLoadingError === 'function') { + this.props.onLoadingError(schema); + } + } + /** * Call the callback for hiding this Modal */ handleHide() { this.clearResponse(); - if (typeof this.props.handleHide === 'function') { - this.props.handleHide(); + if (typeof this.props.onHide === 'function') { + this.props.onHide(); } } @@ -105,8 +118,8 @@ class FormBuilderModal extends Component { handleSubmit(data, action, submitFn) { this.clearResponse(); let promise = null; - if (typeof this.props.handleSubmit === 'function') { - promise = this.props.handleSubmit(data, action, submitFn); + if (typeof this.props.onSubmit === 'function') { + promise = this.props.onSubmit(data, action, submitFn); } else { promise = submitFn(); } @@ -145,7 +158,7 @@ class FormBuilderModal extends Component { ); } - if (typeof this.props.handleHide === 'function') { + if (typeof this.props.onHide === 'function') { return (