Save query form validation on blur (#43726) (#43827)

This PR changes the save query form to only validate its inputs on blur. Previously we validated on every change. This could lead to the errors flashing in the user's face before they're done typing a valid input. For example, we allow spaces in the name field, but not at the beginning or end of the name. So if a user typed this is a long name with spaces they would see the error pop up every time they type a space, only to have it disappear when they type the next letter.

Author: Bargs

src/legacy/core_plugins/data/public/search/search_bar/components/saved_query_management/save_query_form.tsx

@@ -34,7 +34,7 @@ import {
   EuiText,
 } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
-import { sortBy } from 'lodash';
+import { sortBy, isEqual } from 'lodash';
 import { SavedQuery, SavedQueryAttributes } from '../../index';
 import { SavedQueryService } from '../../lib/saved_query_service';
 
@@ -74,6 +74,7 @@ export const SaveQueryForm: FunctionComponent<Props> = ({
   const [shouldIncludeTimefilter, setIncludeTimefilter] = useState(
     savedQuery ? !!savedQuery.timefilter : false
   );
+  const [formErrors, setFormErrors] = useState<string[]>([]);
 
   useEffect(() => {
     const fetchQueries = async () => {
@@ -91,12 +92,6 @@ export const SaveQueryForm: FunctionComponent<Props> = ({
     }
   );
 
-  const hasTitleConflict = !!savedQueries.find(
-    existingSavedQuery => !savedQuery && existingSavedQuery.attributes.title === title
-  );
-
-  const hasWhitespaceError = title.length > title.trim().length;
-
   const titleConflictErrorText = i18n.translate(
     'data.search.searchBar.savedQueryForm.titleConflictText',
     {
@@ -106,18 +101,36 @@ export const SaveQueryForm: FunctionComponent<Props> = ({
   const whitespaceErrorText = i18n.translate(
     'data.search.searchBar.savedQueryForm.whitespaceErrorText',
     {
-      defaultMessage: 'Title cannot contain leading or trailing white space',
+      defaultMessage: 'Title cannot contain leading or trailing whitespace',
     }
   );
-  const hasErrors = hasWhitespaceError || hasTitleConflict;
 
-  const errors = () => {
-    if (hasWhitespaceError) return [whitespaceErrorText];
-    if (hasTitleConflict) return [titleConflictErrorText];
-    return [];
+  const validate = () => {
+    const errors = [];
+    if (title.length > title.trim().length) {
+      errors.push(whitespaceErrorText);
+    }
+    if (
+      !!savedQueries.find(
+        existingSavedQuery => !savedQuery && existingSavedQuery.attributes.title === title
+      )
+    ) {
+      errors.push(titleConflictErrorText);
+    }
+
+    if (!isEqual(errors, formErrors)) {
+      setFormErrors(errors);
+    }
   };
+
+  const hasErrors = formErrors.length > 0;
+
+  if (hasErrors) {
+    validate();
+  }
+
   const saveQueryForm = (
-    <EuiForm isInvalid={hasErrors} error={errors()}>
+    <EuiForm isInvalid={hasErrors} error={formErrors}>
       <EuiFormRow>
         <EuiText color="subdued">{savedQueryDescriptionText}</EuiText>
       </EuiFormRow>
@@ -140,6 +153,7 @@ export const SaveQueryForm: FunctionComponent<Props> = ({
           }}
           data-test-subj="saveQueryFormTitle"
           isInvalid={hasErrors}
+          onBlur={validate}
         />
       </EuiFormRow>
 

test/functional/services/saved_query_management_component.js

@@ -38,9 +38,20 @@ export function SavedQueryManagementComponentProvider({ getService }) {
       if (name) {
         await testSubjects.setValue('saveQueryFormTitle', name);
       }
+
+      // Form input validation only happens onBlur. Clicking the save button should de-focus the
+      // input element and the validation should prevent a save from actually happening if there's
+      // an error.
+      await testSubjects.click('savedQueryFormSaveButton');
+
       const saveQueryFormSaveButtonStatus = await testSubjects.isEnabled('savedQueryFormSaveButton');
-      expect(saveQueryFormSaveButtonStatus).to.not.eql(true);
-      await testSubjects.click('savedQueryFormCancelButton');
+
+      try {
+        expect(saveQueryFormSaveButtonStatus).to.not.eql(true);
+      }
+      finally {
+        await testSubjects.click('savedQueryFormCancelButton');
+      }
     }
 
     async saveCurrentlyLoadedAsNewQuery(name, description, includeFilters, includeTimeFilter) {