Merge pull request #9 from cjcenizal/ccr_forms/ux

Clarify UX intentions with comments and use null to represent 'no error'.
cjcenizal · Apr 6, 2019 · 3e8e381 · 3e8e381
2 parents 7b7adb7 + d7453da
commit 3e8e381
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 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
@@ -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,12 @@ 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 });
-        }
+        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);
       });
     }
@@ -195,12 +191,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 +374,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/...k/plugins/cross_cluster_replication/public/app/services/auto_follow_pattern_validators.js b/...k/plugins/cross_cluster_replication/public/app/services/auto_follow_pattern_validators.js
@@ -88,7 +88,21 @@ export const validateLeaderIndexPattern = (indexPattern) => {
     };
   }
 
-  return { message: null };
+  return null;
+};
+
+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', {
+        defaultMessage: 'At least one leader index pattern is required.',
+      })
+    };
+  }
+
+  return null;
 };
 
 export const validatePrefix = (prefix) => {
@@ -184,7 +198,7 @@ export const validateAutoFollowPattern = (autoFollowPattern = {}) => {
         break;
 
       case 'leaderIndexPatterns':
-        error = validateLeaderIndexPattern(fieldValue[0]);
+        error = validateLeaderIndexPatterns(fieldValue);
         break;
 
       case 'followIndexPatternPrefix':