diff --git a/client/src/campaigns/triggers/CUD.js b/client/src/campaigns/triggers/CUD.js index 2db20013..aef1321e 100644 --- a/client/src/campaigns/triggers/CUD.js +++ b/client/src/campaigns/triggers/CUD.js @@ -98,7 +98,7 @@ export default class CUD extends Component { componentDidMount() { if (this.props.entity) { - this.getFormValuesFromEntity(this.props.entity, ::this.getFormValuesMutator); + this.getFormValuesFromEntity(this.props.entity); } else { this.populateFormValues({ diff --git a/client/src/lib/form.js b/client/src/lib/form.js index b7b79d73..5c5a69d5 100644 --- a/client/src/lib/form.js +++ b/client/src/lib/form.js @@ -983,6 +983,19 @@ const withForm = createComponentMixin([], [], (TargetClass, InnerClass) => { isServerValidationRunning: false }); + const getSaveData = (self, formStateData) => { + let data = formStateData.map(attr => attr.get('value')).toJS(); + + if (self.submitFormValuesMutator) { + const newData = self.submitFormValuesMutator(data); + if (newData !== undefined) { + data = newData; + } + } + + return data; + }; + // formValidateResolve is called by "validateForm" once client receives validation response from server that does not // trigger another server validation let formValidateResolve = null; @@ -1100,7 +1113,7 @@ const withForm = createComponentMixin([], [], (TargetClass, InnerClass) => { delete data.hash; if (this.getFormValuesMutator) { - this.getFormValuesMutator(data); + this.getFormValuesMutator(data, this.getFormValues()); } this.populateFormValues(data); @@ -1126,7 +1139,7 @@ const withForm = createComponentMixin([], [], (TargetClass, InnerClass) => { delete data.hash; if (this.getFormValuesMutator) { - const newData = this.getFormValuesMutator(data); + const newData = this.getFormValuesMutator(data, this.getFormValues()); if (newData !== undefined) { data = newData; @@ -1167,7 +1180,7 @@ const withForm = createComponentMixin([], [], (TargetClass, InnerClass) => { if (settings.leaveConfirmation) { await new Promise((resolve, reject) => { this.setState(previousState => ({ - formState: previousState.formState.set('savedData', previousState.formState.get('data')) + formState: previousState.formState.set('savedData', getSaveData(this, previousState.formState.get('data'))) }), resolve); }); } @@ -1197,7 +1210,7 @@ const withForm = createComponentMixin([], [], (TargetClass, InnerClass) => { })); if (settings.leaveConfirmation) { - mutState.set('savedData', mutState.get('data')); + mutState.set('savedData', getSaveData(this, mutState.get('data'))); } validateFormState(this, mutState); @@ -1304,6 +1317,7 @@ const withForm = createComponentMixin([], [], (TargetClass, InnerClass) => { }; proto.getFormValues = function(name) { + if (!this.state || !this.state.formState) return undefined; return this.state.formState.get('data').map(attr => attr.get('value')).toJS(); }; @@ -1324,21 +1338,8 @@ const withForm = createComponentMixin([], [], (TargetClass, InnerClass) => { }; const _isFormChanged = self => { - const settings = self.state.formSettings; - - const mutateData = data => { - if (self.submitFormValuesMutator) { - const newData = self.submitFormValuesMutator(data); - if (newData !== undefined) { - data = newData; - } - } - - return data; - }; - - const currentData = mutateData(self.state.formState.get('data').map(attr => attr.get('value')).toJS()); - const savedData = mutateData(self.state.formState.get('savedData').map(attr => attr.get('value')).toJS()); + const currentData = getSaveData(self, self.state.formState.get('data')); + const savedData = self.state.formState.get('savedData'); return !deepEqual(currentData, savedData); }; diff --git a/client/src/lists/forms/CUD.js b/client/src/lists/forms/CUD.js index a10ae8d7..120bcf0b 100644 --- a/client/src/lists/forms/CUD.js +++ b/client/src/lists/forms/CUD.js @@ -81,10 +81,8 @@ export default class CUD extends Component { changed: this.serverValidatedFields }, onChange: { - previewList: () => { - this.setState({ - previewContents: null - }); + previewList: (newState, key, oldValue, newValue) => { + newState.formState.setIn(['data', 'previewContents', 'value'], null); } } }); @@ -293,9 +291,9 @@ export default class CUD extends Component { } } - getFormValuesMutator(data) { + getFormValuesMutator(data, originalData) { this.supplyDefaults(data); - data.selectedTemplate = data.selectedTemplate || 'layout'; + data.selectedTemplate = (originalData && originalData.selectedTemplate) || 'layout'; } submitFormValuesMutator(data) { diff --git a/client/src/lists/segments/CUD.js b/client/src/lists/segments/CUD.js index ec625623..8a04cc47 100644 --- a/client/src/lists/segments/CUD.js +++ b/client/src/lists/segments/CUD.js @@ -27,6 +27,7 @@ import {ActionLink, Button, Icon} from "../../lib/bootstrap-components"; import {getRuleHelpers} from "./helpers"; import RuleSettingsPane from "./RuleSettingsPane"; import {withComponentMixins} from "../../lib/decorator-helpers"; +import clone from "clone"; // https://stackoverflow.com/a/4819886/1601953 const isTouchDevice = !!('ontouchstart' in window || navigator.maxTouchPoints); @@ -41,7 +42,7 @@ const isTouchDevice = !!('ontouchstart' in window || navigator.maxTouchPoints); ]) export default class CUD extends Component { // The code below keeps the segment settings in form value. However, it uses it as a mutable datastructure. - // After initilization, segment settings is never set using setState. This is OK we update the state.rulesTree + // After initilization, segment settings is never set using setState. This is OK since we update the state.rulesTree // from the segment settings on relevant events (changes in the tree and closing the rule settings pane). constructor(props) { @@ -104,9 +105,9 @@ export default class CUD extends Component { return tree; } - getFormValuesMutator(data) { + getFormValuesMutator(data, originalData) { data.rootRuleType = data.settings.rootRule.type; - data.selectedRule = null; // Validation errors of the selected rule are attached to this which makes sure we don't submit the segment if the opened rule has errors + data.selectedRule = (originalData && originalData.selectedRule) || null; // Validation errors of the selected rule are attached to this which makes sure we don't submit the segment if the opened rule has errors this.setState({ rulesTree: this.getTreeFromRules(data.settings.rootRule.rules) @@ -115,6 +116,10 @@ export default class CUD extends Component { submitFormValuesMutator(data) { data.settings.rootRule.type = data.rootRuleType; + + // We have to clone the data here otherwise the form change detection doesn't work. This is because we use the state as a mutable structure. + data = clone(data); + return filterData(data, ['name', 'settings']); } diff --git a/client/src/lists/segments/RuleSettingsPane.js b/client/src/lists/segments/RuleSettingsPane.js index d8b4728b..a1b2b754 100644 --- a/client/src/lists/segments/RuleSettingsPane.js +++ b/client/src/lists/segments/RuleSettingsPane.js @@ -31,8 +31,7 @@ export default class RuleSettingsPane extends PureComponent { this.initForm({ leaveConfirmation: false, - onChangeBeforeValidation: ::this.populateRuleDefaults, - onChange: ::this.onFormChange + onChangeBeforeValidation: ::this.populateRuleDefaults }); } @@ -45,7 +44,8 @@ export default class RuleSettingsPane extends PureComponent { forceShowValidation: PropTypes.bool.isRequired } - updateStateFromProps(props, populateForm) { + updateStateFromProps(populateForm) { + const props = this.props; if (populateForm) { const rule = props.rule; const ruleHelpers = this.ruleHelpers; @@ -74,15 +74,32 @@ export default class RuleSettingsPane extends PureComponent { if (props.forceShowValidation) { this.showFormValidation(); } - } componentDidMount() { - this.updateStateFromProps(this.props, true); + this.updateStateFromProps(true); } - componentWillReceiveProps(nextProps) { - this.updateStateFromProps(nextProps, this.props.rule !== nextProps.rule); + componentDidUpdate(prevProps) { + this.updateStateFromProps(this.props.rule !== prevProps.rule); + + if (this.isFormWithoutErrors()) { + const rule = this.props.rule; + const ruleHelpers = this.ruleHelpers; + + rule.type = this.getFormValue('type'); + + if (!ruleHelpers.isCompositeRuleType(rule.type)) { + rule.column = this.getFormValue('column'); + + const settings = this.ruleHelpers.getRuleTypeSettings(rule); + settings.assignRuleSettings(rule, key => this.getFormValue(key)); + } + + this.props.onChange(false); + } else { + this.props.onChange(true); + } } localValidateFormValues(state) { @@ -95,16 +112,17 @@ export default class RuleSettingsPane extends PureComponent { const ruleType = state.getIn(['type', 'value']); if (!ruleHelpers.isCompositeRuleType(ruleType)) { - const column = state.getIn(['column', 'value']); + if (!ruleType) { + state.setIn(['type', 'error'], t('typeMustBeSelected')); + } + const column = state.getIn(['column', 'value']); if (column) { const colType = ruleHelpers.getColumnType(column); if (ruleType) { const settings = ruleHelpers.primitiveRuleTypes[colType][ruleType]; settings.validate(state); - } else { - state.setIn(['type', 'error'], t('typeMustBeSelected')); } } else { state.setIn(['column', 'error'], t('fieldMustBeSelected')); @@ -133,28 +151,6 @@ export default class RuleSettingsPane extends PureComponent { } } - onFormChange(newState) { - const noErrors = !newState.formState.get('data').find(attr => attr.get('error')); - - if (noErrors) { - const rule = this.props.rule; - const ruleHelpers = this.ruleHelpers; - - rule.type = newState.formState.getIn(['data','type','value']); - - if (!ruleHelpers.isCompositeRuleType(rule.type)) { - rule.column = newState.formState.getIn(['data','column','value']); - - const settings = this.ruleHelpers.getRuleTypeSettings(rule); - settings.assignRuleSettings(rule, key => newState.formState.getIn(['data', key, 'value'])); - } - - this.props.onChange(false); - } else { - this.props.onChange(true); - } - } - async closeForm() { if (this.isFormWithoutErrors()) { this.props.onClose(); diff --git a/client/src/lists/subscriptions/List.js b/client/src/lists/subscriptions/List.js index dad4c2c1..c69f6490 100644 --- a/client/src/lists/subscriptions/List.js +++ b/client/src/lists/subscriptions/List.js @@ -67,8 +67,8 @@ export default class List extends Component { this.updateSegmentSelection(this.props); } - componentWillReceiveProps(nextProps) { - this.updateSegmentSelection(nextProps); + componentDidUpdate() { + this.updateSegmentSelection(this.props); } render() {