From 823e4ccdd6fcfee5d0df0d919d87af3100876549 Mon Sep 17 00:00:00 2001 From: Gabriel Dutra Date: Tue, 15 Jan 2019 10:23:33 -0200 Subject: [PATCH] Migrate DynamicForm to React (#3209) * create DynamicForm React component * Render fields based on target in DynamicForm * Add missing title property to fields * Fix style properties in DynamicForm * Render File fields in DynamicForm * Use React for middle component instead of Angular * Functional save button * Update label style * Render functional actions * Handle file inputs * Update render methods to fix code climate issues * Fix ant input number showing duplicate arrows * Update DynamicForm style to be vertical * Separate imports from antd in DynamicForm * Add Feedback Icons to DynamicForm * Change Action props on DynamicForm - use type and pullRight instead of class prop - update data sources and destinations pages accordingly * Remove setDefaults method from DynamicForm fields * Update antd version * Remove unnecessary class selectors * Remove another unnecessary class selector --- client/app/assets/less/ant.less | 11 + client/app/components/dynamic-form.html | 41 --- client/app/components/dynamic-form.js | 124 --------- .../components/dynamic-form/DynamicForm.jsx | 238 ++++++++++++++++++ .../dynamic-form/dynamicFormHelper.js | 92 +++++++ client/app/components/proptypes.js | 21 ++ client/app/pages/data-sources/show.js | 4 +- client/app/pages/destinations/show.html | 3 +- client/app/pages/destinations/show.js | 10 +- client/app/services/toastr.js | 10 + .../data-source/create_data_source_spec.js | 2 +- 11 files changed, 383 insertions(+), 173 deletions(-) delete mode 100644 client/app/components/dynamic-form.html delete mode 100644 client/app/components/dynamic-form.js create mode 100644 client/app/components/dynamic-form/DynamicForm.jsx create mode 100644 client/app/components/dynamic-form/dynamicFormHelper.js create mode 100644 client/app/services/toastr.js diff --git a/client/app/assets/less/ant.less b/client/app/assets/less/ant.less index 50710b8e5f..9607696513 100644 --- a/client/app/assets/less/ant.less +++ b/client/app/assets/less/ant.less @@ -1,10 +1,14 @@ @import '~antd/lib/style/core/iconfont.less'; @import '~antd/lib/style/core/motion.less'; @import '~antd/lib/input/style/index.less'; +@import '~antd/lib/input-number/style/index.less'; @import '~antd/lib/date-picker/style/index.less'; @import '~antd/lib/modal/style/index.less'; @import '~antd/lib/tooltip/style/index.less'; @import '~antd/lib/select/style/index.less'; +@import '~antd/lib/checkbox/style/index.less'; +@import '~antd/lib/upload/style/index.less'; +@import '~antd/lib/form/style/index.less'; @import '~antd/lib/button/style/index.less'; @import '~antd/lib/radio/style/index.less'; @import '~antd/lib/time-picker/style/index.less'; @@ -46,3 +50,10 @@ .ant-dropdown-in-bootstrap-modal { z-index: 1050; } + +// Fix ant input number showing duplicate arrows +.ant-input-number-input::-webkit-outer-spin-button, +.ant-input-number-input::-webkit-inner-spin-button { + -webkit-appearance: none; + margin: 0; +} diff --git a/client/app/components/dynamic-form.html b/client/app/components/dynamic-form.html deleted file mode 100644 index 50037be5c0..0000000000 --- a/client/app/components/dynamic-form.html +++ /dev/null @@ -1,41 +0,0 @@ -
-
- - - -
-
-
- - - - - - - - -
- - - - - - - - - -
diff --git a/client/app/components/dynamic-form.js b/client/app/components/dynamic-form.js deleted file mode 100644 index 5c3e337ad8..0000000000 --- a/client/app/components/dynamic-form.js +++ /dev/null @@ -1,124 +0,0 @@ -import { isUndefined, each, includes } from 'lodash'; -import template from './dynamic-form.html'; - -function orderedInputs(properties, order) { - const inputs = new Array(order.length); - Object.keys(properties).forEach((key) => { - const position = order.indexOf(key); - const input = { name: key, property: properties[key] }; - if (position > -1) { - inputs[position] = input; - } else { - inputs.push(input); - } - }); - return inputs; -} - -function normalizeSchema(configurationSchema) { - each(configurationSchema.properties, (prop, name) => { - if (name === 'password' || name === 'passwd') { - prop.type = 'password'; - } - - if (name.endsWith('File')) { - prop.type = 'file'; - } - - if (prop.type === 'boolean') { - prop.type = 'checkbox'; - } - - prop.required = includes(configurationSchema.required, name); - }); - - configurationSchema.order = configurationSchema.order || []; -} - -function setDefaults(configurationSchema, options) { - if (Object.keys(options).length === 0) { - const properties = configurationSchema.properties; - Object.keys(properties).forEach((property) => { - if (!isUndefined(properties[property].default)) { - options[property] = properties[property].default; - } - }); - } -} - -function DynamicForm($http, toastr) { - return { - restrict: 'E', - replace: 'true', - transclude: true, - template, - scope: { - target: '=', - type: '=', - actions: '=', - }, - link($scope) { - const configurationSchema = $scope.type.configuration_schema; - normalizeSchema(configurationSchema); - $scope.fields = orderedInputs(configurationSchema.properties, configurationSchema.order); - setDefaults(configurationSchema, $scope.target.options); - - $scope.inProgressActions = {}; - if ($scope.actions) { - $scope.actions.forEach((action) => { - const originalCallback = action.callback; - const name = action.name; - action.callback = () => { - action.name = ` ${name}`; - - $scope.inProgressActions[action.name] = true; - function release() { - $scope.inProgressActions[action.name] = false; - action.name = name; - } - - originalCallback(release); - }; - }); - } - - $scope.files = {}; - - $scope.$watchCollection('files', () => { - each($scope.files, (v, k) => { - // THis is needed because angular-base64-upload sets the value to null at initialization, - // causing the field to be marked as dirty even if it wasn't changed. - if (!v && $scope.target.options[k]) { - $scope.dynamicForm.$setPristine(); - } - if (v) { - $scope.target.options[k] = v.base64; - } - }); - }); - - $scope.saveChanges = () => { - $scope.target.$save( - () => { - toastr.success('Saved.'); - $scope.dynamicForm.$setPristine(); - }, - (error) => { - if (error.status === 400 && 'message' in error.data) { - toastr.error(error.data.message); - } else { - toastr.error('Failed saving.'); - } - }, - ); - }; - }, - }; -} - -export default function init(ngModule) { - ngModule.directive('dynamicForm', DynamicForm); -} - -init.init = true; - diff --git a/client/app/components/dynamic-form/DynamicForm.jsx b/client/app/components/dynamic-form/DynamicForm.jsx new file mode 100644 index 0000000000..a35ede9107 --- /dev/null +++ b/client/app/components/dynamic-form/DynamicForm.jsx @@ -0,0 +1,238 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import Form from 'antd/lib/form'; +import Input from 'antd/lib/input'; +import InputNumber from 'antd/lib/input-number'; +import Checkbox from 'antd/lib/checkbox'; +import Button from 'antd/lib/button'; +import Upload from 'antd/lib/upload'; +import Icon from 'antd/lib/icon'; +import { react2angular } from 'react2angular'; +import { toastr } from '@/services/toastr'; +import { Field, Action, AntdForm } from '../proptypes'; +import helper from './dynamicFormHelper'; + +export class DynamicForm extends React.Component { + static propTypes = { + fields: PropTypes.arrayOf(Field), + actions: PropTypes.arrayOf(Action), + feedbackIcons: PropTypes.bool, + onSubmit: PropTypes.func, + form: AntdForm.isRequired, + }; + + static defaultProps = { + fields: [], + actions: [], + feedbackIcons: false, + onSubmit: () => {}, + }; + + constructor(props) { + super(props); + + this.state = { + isSubmitting: false, + inProgressActions: [], + }; + + this.actionCallbacks = this.props.actions.reduce((acc, cur) => ({ + ...acc, + [cur.name]: cur.callback, + }), null); + + props.actions.forEach((action) => { + this.state.inProgressActions[action.name] = false; + }); + } + + setActionInProgress = (actionName, inProgress) => { + this.setState({ + inProgressActions: { + ...this.state.inProgressActions, + [actionName]: inProgress, + }, + }); + } + + handleSubmit = (e) => { + this.setState({ isSubmitting: true }); + e.preventDefault(); + this.props.form.validateFieldsAndScroll((err, values) => { + if (!err) { + this.props.onSubmit( + values, + (msg) => { + const { setFieldsValue, getFieldsValue } = this.props.form; + this.setState({ isSubmitting: false }); + setFieldsValue(getFieldsValue()); // reset form touched state + toastr.success(msg); + }, + (msg) => { + this.setState({ isSubmitting: false }); + toastr.error(msg); + }, + ); + } else this.setState({ isSubmitting: false }); + }); + } + + handleAction = (e) => { + const actionName = e.target.dataset.action; + + this.setActionInProgress(actionName, true); + this.actionCallbacks[actionName](() => { + this.setActionInProgress(actionName, false); + }); + } + + base64File = (fieldName, e) => { + if (e && e.fileList[0]) { + helper.getBase64(e.file).then((value) => { + this.props.form.setFieldsValue({ [fieldName]: value }); + }); + } + } + + renderUpload(field, props) { + const { getFieldDecorator, getFieldValue } = this.props.form; + const { name, initialValue, required } = field; + const fieldLabel = field.title || helper.toHuman(name); + + const fileOptions = { + rules: [{ required, message: `${fieldLabel} is required.` }], + initialValue, + getValueFromEvent: this.base64File.bind(this, name), + }; + + const disabled = getFieldValue(name) !== undefined && getFieldValue(name) !== initialValue; + + const upload = ( + false}> + + + ); + + return getFieldDecorator(name, fileOptions)(upload); + } + + renderField(field, props) { + const { getFieldDecorator } = this.props.form; + const { name, type, initialValue } = field; + const fieldLabel = field.title || helper.toHuman(name); + + const options = { + rules: [{ required: field.required, message: `${fieldLabel} is required.` }], + valuePropName: type === 'checkbox' ? 'checked' : 'value', + initialValue, + }; + + if (type === 'checkbox') { + return getFieldDecorator(name, options)({fieldLabel}); + } else if (type === 'file') { + return this.renderUpload(field, props); + } else if (type === 'number') { + return getFieldDecorator(name, options)(); + } + return getFieldDecorator(name, options)(); + } + + renderFields() { + return this.props.fields.map((field) => { + const [firstItem] = this.props.fields; + const FormItem = Form.Item; + const { name, title, type } = field; + const fieldLabel = title || helper.toHuman(name); + + const formItemProps = { + key: name, + className: 'm-b-10', + hasFeedback: type !== 'checkbox' && type !== 'file' && this.props.feedbackIcons, + label: type === 'checkbox' ? '' : fieldLabel, + }; + + const fieldProps = { + autoFocus: (firstItem === field), + className: 'w-100', + name, + type, + placeholder: field.placeholder, + 'data-test': fieldLabel, + }; + + return ({this.renderField(field, fieldProps)}); + }); + } + + renderActions() { + return this.props.actions.map((action) => { + const inProgress = this.state.inProgressActions[action.name]; + const { isFieldsTouched } = this.props.form; + + const actionProps = { + key: action.name, + htmlType: 'button', + className: action.pullRight ? 'pull-right m-t-10' : 'm-t-10', + type: action.type, + disabled: inProgress || (isFieldsTouched() && action.disableWhenDirty), + loading: inProgress, + onClick: this.handleAction, + }; + + return (); + }); + } + + render() { + const submitProps = { + type: 'primary', + htmlType: 'submit', + className: 'w-100', + disabled: this.state.isSubmitting, + loading: this.state.isSubmitting, + }; + + return ( +
+ {this.renderFields()} + + {this.renderActions()} +
+ ); + } +} + +export default function init(ngModule) { + ngModule.component('dynamicForm', react2angular((props) => { + const UpdatedDynamicForm = Form.create()(DynamicForm); + const fields = helper.getFields(props.type.configuration_schema, props.target); + + const onSubmit = (values, onSuccess, onError) => { + helper.updateTargetWithValues(props.target, values); + props.target.$save( + () => { + onSuccess('Saved.'); + }, + (error) => { + if (error.status === 400 && 'message' in error.data) { + onError(error.data.message); + } else { + onError('Failed saving.'); + } + }, + ); + }; + + const updatedProps = { + fields, + actions: props.target.id ? props.actions : [], + feedbackIcons: true, + onSubmit, + }; + return (); + }, ['target', 'type', 'actions'])); +} + +init.init = true; diff --git a/client/app/components/dynamic-form/dynamicFormHelper.js b/client/app/components/dynamic-form/dynamicFormHelper.js new file mode 100644 index 0000000000..1a5606ab9d --- /dev/null +++ b/client/app/components/dynamic-form/dynamicFormHelper.js @@ -0,0 +1,92 @@ +import { each, includes } from 'lodash'; + +function orderedInputs(properties, order, targetOptions) { + const inputs = new Array(order.length); + Object.keys(properties).forEach((key) => { + const position = order.indexOf(key); + const input = { + name: key, + title: properties[key].title, + type: properties[key].type, + placeholder: properties[key].default && properties[key].default.toString(), + required: properties[key].required, + initialValue: targetOptions[key], + }; + + if (position > -1) { + inputs[position] = input; + } else { + inputs.push(input); + } + }); + return inputs; +} + +function normalizeSchema(configurationSchema) { + each(configurationSchema.properties, (prop, name) => { + if (name === 'password' || name === 'passwd') { + prop.type = 'password'; + } + + if (name.endsWith('File')) { + prop.type = 'file'; + } + + if (prop.type === 'boolean') { + prop.type = 'checkbox'; + } + + if (prop.type === 'string') { + prop.type = 'text'; + } + + prop.required = includes(configurationSchema.required, name); + }); + + configurationSchema.order = configurationSchema.order || []; +} + +function getFields(configurationSchema, target) { + normalizeSchema(configurationSchema); + const inputs = [ + { + name: 'name', + title: 'Name', + type: 'text', + required: true, + initialValue: target.name, + }, + ...orderedInputs(configurationSchema.properties, configurationSchema.order, target.options), + ]; + + return inputs; +} + +function updateTargetWithValues(target, values) { + target.name = values.name; + Object.keys(values).forEach((key) => { + if (key !== 'name') { + target.options[key] = values[key]; + } + }); +} + +function toHuman(text) { + return text.replace(/_/g, ' ').replace(/(?:^|\s)\S/g, a => a.toUpperCase()); +} + +function getBase64(file) { + return new Promise((resolve, reject) => { + const reader = new FileReader(); + reader.readAsDataURL(file); + reader.onload = () => resolve(reader.result.substr(reader.result.indexOf(',') + 1)); + reader.onerror = error => reject(error); + }); +} + +export default { + getFields, + updateTargetWithValues, + toHuman, + getBase64, +}; diff --git a/client/app/components/proptypes.js b/client/app/components/proptypes.js index a6db8a8c18..ff18c69de8 100644 --- a/client/app/components/proptypes.js +++ b/client/app/components/proptypes.js @@ -14,3 +14,24 @@ export const Table = PropTypes.shape({ }); export const Schema = PropTypes.arrayOf(Table); + +export const Field = PropTypes.shape({ + name: PropTypes.string.isRequired, + title: PropTypes.string, + type: PropTypes.oneOf(['text', 'email', 'password', 'number', 'checkbox', 'file']).isRequired, + initialValue: PropTypes.oneOfType([PropTypes.string, PropTypes.number, PropTypes.bool]), + required: PropTypes.bool, + placeholder: PropTypes.string, +}); + +export const Action = PropTypes.shape({ + name: PropTypes.string.isRequired, + callback: PropTypes.func.isRequired, + type: PropTypes.string, + pullRight: PropTypes.bool, + disabledWhenDirty: PropTypes.bool, +}); + +export const AntdForm = PropTypes.shape({ + validateFieldsAndScroll: PropTypes.func, +}); diff --git a/client/app/pages/data-sources/show.js b/client/app/pages/data-sources/show.js index 9d15f094d9..b6446659ed 100644 --- a/client/app/pages/data-sources/show.js +++ b/client/app/pages/data-sources/show.js @@ -81,9 +81,9 @@ function DataSourceCtrl( } $scope.actions = [ - { name: 'Delete', class: 'btn-danger', callback: deleteDataSource }, + { name: 'Delete', type: 'danger', callback: deleteDataSource }, { - name: 'Test Connection', class: 'btn-default pull-right', callback: testConnection, disableWhenDirty: true, + name: 'Test Connection', pullRight: true, callback: testConnection, disableWhenDirty: true, }, ]; } diff --git a/client/app/pages/destinations/show.html b/client/app/pages/destinations/show.html index d96b273ed6..51eaddbdfe 100644 --- a/client/app/pages/destinations/show.html +++ b/client/app/pages/destinations/show.html @@ -21,8 +21,7 @@

{{type.name}}

- - +
diff --git a/client/app/pages/destinations/show.js b/client/app/pages/destinations/show.js index 024ac5718e..f3f3976224 100644 --- a/client/app/pages/destinations/show.js +++ b/client/app/pages/destinations/show.js @@ -28,7 +28,7 @@ function DestinationCtrl( $scope.destination = new Destination({ options: {} }); }; - $scope.delete = () => { + function deleteDestination(callback) { const doDelete = () => { $scope.destination.$delete(() => { toastrSuccessAndPath('Destination', 'destinations', toastr, $location); @@ -40,8 +40,12 @@ function DestinationCtrl( const title = 'Delete Destination'; const message = `Are you sure you want to delete the "${$scope.destination.name}" destination?`; - AlertDialog.open(title, message, deleteConfirm).then(doDelete); - }; + AlertDialog.open(title, message, deleteConfirm).then(doDelete, callback); + } + + $scope.actions = [ + { name: 'Delete', type: 'danger', callback: deleteDestination }, + ]; } export default function init(ngModule) { diff --git a/client/app/services/toastr.js b/client/app/services/toastr.js new file mode 100644 index 0000000000..c46f3728e5 --- /dev/null +++ b/client/app/services/toastr.js @@ -0,0 +1,10 @@ +// eslint-disable-next-line import/no-mutable-exports +export let toastr = null; + +export default function init(ngModule) { + ngModule.run(($injector) => { + toastr = $injector.get('toastr'); + }); +} + +init.init = true; diff --git a/cypress/integration/data-source/create_data_source_spec.js b/cypress/integration/data-source/create_data_source_spec.js index 773b000ab4..52adef85fc 100644 --- a/cypress/integration/data-source/create_data_source_spec.js +++ b/cypress/integration/data-source/create_data_source_spec.js @@ -7,7 +7,7 @@ describe('Create Data Source', () => { it('creates a new PostgreSQL data source', () => { cy.getByTestId('DatabaseSource').contains('PostgreSQL').click(); - cy.getByTestId('TargetName').type('Redash'); + cy.getByTestId('Name').type('Redash'); cy.getByTestId('Host').type('{selectall}postgres'); cy.getByTestId('User').type('postgres'); cy.getByTestId('Password').type('postgres');