From 8d180d19b927f0d585893c4875ca498e1fd1affd Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Wed, 3 Apr 2019 11:35:53 -0700 Subject: [PATCH 1/3] Clarify UX intentions with comments and use null to represent 'no error'. --- .../components/auto_follow_pattern_form.js | 24 +++++-------------- .../auto_follow_pattern_validators.js | 12 ++++++++-- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/x-pack/plugins/cross_cluster_replication/public/app/components/auto_follow_pattern_form.js b/x-pack/plugins/cross_cluster_replication/public/app/components/auto_follow_pattern_form.js index 71444cd46abdc..1c20929a10308 100644 --- a/x-pack/plugins/cross_cluster_replication/public/app/components/auto_follow_pattern_form.js +++ b/x-pack/plugins/cross_cluster_replication/public/app/components/auto_follow_pattern_form.js @@ -112,7 +112,7 @@ export class AutoFollowPatternForm extends PureComponent { onCreateLeaderIndexPattern = (indexPattern) => { const error = validateLeaderIndexPattern(indexPattern); - if (error.message) { + if (error) { const errors = { leaderIndexPatterns: { @@ -169,16 +169,9 @@ export class AutoFollowPatternForm extends PureComponent { this.setState(({ fieldsErrors }) => updateFormErrors(errors, fieldsErrors)); } else { this.setState(({ fieldsErrors, autoFollowPattern: { leaderIndexPatterns } }) => { - let errors; - if (!leaderIndexPatterns.length) { - // If we don't have yet any pattern in our state, - // we validate the *current value* of the auto-follow pattern form input - errors = validateAutoFollowPattern({ leaderIndexPatterns: [leaderIndexPattern] }); - } else { - // If we do have some auto-follow pattern in our state, - // we validate the *value in the state* - errors = validateAutoFollowPattern({ leaderIndexPatterns }); - } + // If the user has fixed invalid input, then we need to update the validation state to clear + // the outdated errors. + const errors = validateAutoFollowPattern({ leaderIndexPatterns }); return updateFormErrors(errors, fieldsErrors); }); } @@ -195,12 +188,7 @@ export class AutoFollowPatternForm extends PureComponent { }; isFormValid() { - return Object.values(this.state.fieldsErrors).every(error => { - if (error !== null && typeof error === 'object') { - return error.message === null; - } - return error === undefined || error === null; - }); + return Object.values(this.state.fieldsErrors).every(error => error === undefined || error === null); } sendForm = () => { @@ -383,7 +371,7 @@ export class AutoFollowPatternForm extends PureComponent { * Leader index pattern(s) */ const renderLeaderIndexPatterns = () => { - const hasError = !!fieldsErrors.leaderIndexPatterns.message; + const hasError = !!(fieldsErrors.leaderIndexPatterns && fieldsErrors.leaderIndexPatterns.message); const isInvalid = hasError && (fieldsErrors.leaderIndexPatterns.alwaysVisible || areErrorsVisible); const formattedLeaderIndexPatterns = leaderIndexPatterns.map(pattern => ({ label: pattern })); diff --git a/x-pack/plugins/cross_cluster_replication/public/app/services/auto_follow_pattern_validators.js b/x-pack/plugins/cross_cluster_replication/public/app/services/auto_follow_pattern_validators.js index 0d3b8ff711bda..6411b34b73feb 100644 --- a/x-pack/plugins/cross_cluster_replication/public/app/services/auto_follow_pattern_validators.js +++ b/x-pack/plugins/cross_cluster_replication/public/app/services/auto_follow_pattern_validators.js @@ -88,7 +88,7 @@ export const validateLeaderIndexPattern = (indexPattern) => { }; } - return { message: null }; + return null; }; export const validatePrefix = (prefix) => { @@ -184,7 +184,15 @@ export const validateAutoFollowPattern = (autoFollowPattern = {}) => { break; case 'leaderIndexPatterns': - error = validateLeaderIndexPattern(fieldValue[0]); + // We only need to check if a value has been provided, because validation for this field + // has already been executed as the user has entered input into it. + if (!fieldValue.length) { + error = { + message: i18n.translate('xpack.crossClusterReplication.autoFollowPattern.leaderIndexPatternValidation.isEmpty', { + defaultMessage: 'At least one leader index pattern is required.', + }) + }; + } break; case 'followIndexPatternPrefix': From 255e7291f3d9bdc97fd9d333ac61f244df310fcc Mon Sep 17 00:00:00 2001 From: sebelga Date: Thu, 4 Apr 2019 10:50:54 +0200 Subject: [PATCH 2/3] Small refactor --- .../components/auto_follow_pattern_form.js | 9 ++++-- .../auto_follow_pattern_validators.js | 28 +++++++++++++------ 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/cross_cluster_replication/public/app/components/auto_follow_pattern_form.js b/x-pack/plugins/cross_cluster_replication/public/app/components/auto_follow_pattern_form.js index 1c20929a10308..b3adfce3ded2a 100644 --- a/x-pack/plugins/cross_cluster_replication/public/app/components/auto_follow_pattern_form.js +++ b/x-pack/plugins/cross_cluster_replication/public/app/components/auto_follow_pattern_form.js @@ -169,9 +169,12 @@ export class AutoFollowPatternForm extends PureComponent { this.setState(({ fieldsErrors }) => updateFormErrors(errors, fieldsErrors)); } else { this.setState(({ fieldsErrors, autoFollowPattern: { leaderIndexPatterns } }) => { - // If the user has fixed invalid input, then we need to update the validation state to clear - // the outdated errors. - const errors = validateAutoFollowPattern({ leaderIndexPatterns }); + // If have at least 1 auto-follow pattern in our state, we validate its value + // otherwise, we validate the *current value* of the form input + const errors = Boolean(leaderIndexPatterns.length) + ? validateAutoFollowPattern({ leaderIndexPatterns }) + : validateAutoFollowPattern({ leaderIndexPatterns: [leaderIndexPattern] }); + return updateFormErrors(errors, fieldsErrors); }); } diff --git a/x-pack/plugins/cross_cluster_replication/public/app/services/auto_follow_pattern_validators.js b/x-pack/plugins/cross_cluster_replication/public/app/services/auto_follow_pattern_validators.js index 6411b34b73feb..720221e47926e 100644 --- a/x-pack/plugins/cross_cluster_replication/public/app/services/auto_follow_pattern_validators.js +++ b/x-pack/plugins/cross_cluster_replication/public/app/services/auto_follow_pattern_validators.js @@ -91,6 +91,24 @@ export const validateLeaderIndexPattern = (indexPattern) => { return null; }; +export const validateLeaderIndexPatterns = (indexPatterns) => { + if (!indexPatterns.length) { + return { + message: i18n.translate('xpack.crossClusterReplication.autoFollowPattern.leaderIndexPatternValidation.isEmpty', { + defaultMessage: 'At least one leader index pattern is required.', + }) + }; + } + + let error = null; + indexPatterns.forEach((indexPattern) => { + if (!error) { + error = validateLeaderIndexPattern(indexPattern); + } + }); + return error; +}; + export const validatePrefix = (prefix) => { // If it's empty, it is valid if (!prefix || !prefix.trim()) { @@ -184,15 +202,7 @@ export const validateAutoFollowPattern = (autoFollowPattern = {}) => { break; case 'leaderIndexPatterns': - // We only need to check if a value has been provided, because validation for this field - // has already been executed as the user has entered input into it. - if (!fieldValue.length) { - error = { - message: i18n.translate('xpack.crossClusterReplication.autoFollowPattern.leaderIndexPatternValidation.isEmpty', { - defaultMessage: 'At least one leader index pattern is required.', - }) - }; - } + error = validateLeaderIndexPatterns(fieldValue); break; case 'followIndexPatternPrefix': From d7453da9682bfa69348f65e5c36d0466d29d014a Mon Sep 17 00:00:00 2001 From: sebelga Date: Sat, 6 Apr 2019 06:26:19 +0200 Subject: [PATCH 3/3] Add CR changes --- .../public/app/components/auto_follow_pattern_form.js | 4 ++-- .../app/services/auto_follow_pattern_validators.js | 10 +++------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/cross_cluster_replication/public/app/components/auto_follow_pattern_form.js b/x-pack/plugins/cross_cluster_replication/public/app/components/auto_follow_pattern_form.js index b3adfce3ded2a..9f0423c366bbe 100644 --- a/x-pack/plugins/cross_cluster_replication/public/app/components/auto_follow_pattern_form.js +++ b/x-pack/plugins/cross_cluster_replication/public/app/components/auto_follow_pattern_form.js @@ -169,10 +169,10 @@ export class AutoFollowPatternForm extends PureComponent { this.setState(({ fieldsErrors }) => updateFormErrors(errors, fieldsErrors)); } else { this.setState(({ fieldsErrors, autoFollowPattern: { leaderIndexPatterns } }) => { - // If have at least 1 auto-follow pattern in our state, we validate its value - // otherwise, we validate the *current value* of the form input const errors = Boolean(leaderIndexPatterns.length) + // Validate existing patterns, so we can surface an error if this required input is missing. ? validateAutoFollowPattern({ leaderIndexPatterns }) + // Validate the input as the user types so they have immediate feedback about errors. : validateAutoFollowPattern({ leaderIndexPatterns: [leaderIndexPattern] }); return updateFormErrors(errors, fieldsErrors); diff --git a/x-pack/plugins/cross_cluster_replication/public/app/services/auto_follow_pattern_validators.js b/x-pack/plugins/cross_cluster_replication/public/app/services/auto_follow_pattern_validators.js index 720221e47926e..87bbceedb201b 100644 --- a/x-pack/plugins/cross_cluster_replication/public/app/services/auto_follow_pattern_validators.js +++ b/x-pack/plugins/cross_cluster_replication/public/app/services/auto_follow_pattern_validators.js @@ -92,6 +92,8 @@ export const validateLeaderIndexPattern = (indexPattern) => { }; export const validateLeaderIndexPatterns = (indexPatterns) => { + // We only need to check if a value has been provided, because validation for this field + // has already been executed as the user has entered input into it. if (!indexPatterns.length) { return { message: i18n.translate('xpack.crossClusterReplication.autoFollowPattern.leaderIndexPatternValidation.isEmpty', { @@ -100,13 +102,7 @@ export const validateLeaderIndexPatterns = (indexPatterns) => { }; } - let error = null; - indexPatterns.forEach((indexPattern) => { - if (!error) { - error = validateLeaderIndexPattern(indexPattern); - } - }); - return error; + return null; }; export const validatePrefix = (prefix) => {