address review feedback

elastic · Sep 15, 2020 · 1aa0ef4 · 1aa0ef4
1 parent 53f278b
commit 1aa0ef4
Show file tree

Hide file tree

Showing 10 changed files with 71 additions and 59 deletions.
diff --git a/...on/components/pipeline_processors_editor/components/processor_form/add_processor_form.tsx b/...on/components/pipeline_processors_editor/components/processor_form/add_processor_form.tsx
@@ -33,7 +33,6 @@ export interface Props {
   form: FormHook<Fields>;
   onOpen: () => void;
   esDocsBasePath: string;
-  getDefaultProcessorOptions: () => Fields;
   closeFlyout: () => void;
   handleSubmit: (shouldCloseFlyout?: boolean) => Promise<void>;
 }
@@ -67,7 +66,6 @@ export const AddProcessorForm: FunctionComponent<Props> = ({
   onOpen,
   form,
   esDocsBasePath,
-  getDefaultProcessorOptions,
   closeFlyout,
   handleSubmit,
 }) => {
@@ -110,7 +108,7 @@ export const AddProcessorForm: FunctionComponent<Props> = ({
           </EuiFlexGroup>
         </EuiFlyoutHeader>
         <EuiFlyoutBody>
-          <ProcessorSettingsFields getDefaultProcessorOptions={getDefaultProcessorOptions} />
+          <ProcessorSettingsFields />
         </EuiFlyoutBody>
         <EuiFlyoutFooter>
           <EuiFlexGroup justifyContent="flexEnd">

diff --git a/...n/components/pipeline_processors_editor/components/processor_form/edit_processor_form.tsx b/...n/components/pipeline_processors_editor/components/processor_form/edit_processor_form.tsx
@@ -34,14 +34,13 @@ import { Fields } from './processor_form.container';
 
 export interface Props {
   isOnFailure: boolean;
-  processor: ProcessorInternal;
   form: FormHook<Fields>;
   onOpen: () => void;
   esDocsBasePath: string;
-  getDefaultProcessorOptions: () => Fields;
   closeFlyout: () => void;
   resetProcessors: () => void;
   handleSubmit: (shouldCloseFlyout?: boolean) => Promise<void>;
+  getProcessor: () => ProcessorInternal;
 }
 
 const updateButtonLabel = i18n.translate(
@@ -94,12 +93,11 @@ const getFlyoutTitle = (isOnFailure: boolean) => {
 };
 
 export const EditProcessorForm: FunctionComponent<Props> = ({
-  processor,
+  getProcessor,
   form,
   isOnFailure,
   onOpen,
   esDocsBasePath,
-  getDefaultProcessorOptions,
   closeFlyout,
   handleSubmit,
   resetProcessors,
@@ -111,6 +109,8 @@ export const EditProcessorForm: FunctionComponent<Props> = ({
     isExecutingPipeline,
   } = testPipelineData;
 
+  const processor = getProcessor();
+
   const processorOutput =
     processor &&
     testOutputPerProcessor &&
@@ -149,12 +149,7 @@ export const EditProcessorForm: FunctionComponent<Props> = ({
       />
     );
   } else {
-    flyoutContent = (
-      <ProcessorSettingsFields
-        processor={processor}
-        getDefaultProcessorOptions={getDefaultProcessorOptions}
-      />
-    );
+    flyoutContent = <ProcessorSettingsFields processor={processor} />;
   }
 
   return (
@@ -203,7 +198,7 @@ export const EditProcessorForm: FunctionComponent<Props> = ({
                   if (tab.id === 'output') {
                     await handleSubmit(false);
                   } else {
-                    form.reset({ defaultValue: getDefaultProcessorOptions() });
+                    form.reset({ defaultValue: { fields: processor.options } });
                   }
                   setActiveTab(tab.id);
                 }}

diff --git a/...ponents/pipeline_processors_editor/components/processor_form/processor_form.container.tsx b/...ponents/pipeline_processors_editor/components/processor_form/processor_form.container.tsx
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import React, { FunctionComponent, useCallback, useEffect } from 'react';
+import React, { FunctionComponent, useCallback, useEffect, useRef } from 'react';
 
 import { useForm, OnFormUpdateArg, FormData, useKibana } from '../../../../../shared_imports';
 import { ProcessorInternal } from '../../types';
@@ -29,34 +29,36 @@ interface Props {
   onOpen: () => void;
   onClose: () => void;
   processor?: ProcessorInternal;
-  unsavedFormData?: Omit<ProcessorInternal, 'id'>;
 }
 
 export const ProcessorFormContainer: FunctionComponent<Props> = ({
   processor,
   onFormUpdate,
   onSubmit,
-  unsavedFormData,
   onClose,
   ...rest
 }) => {
   const { services } = useKibana();
 
-  const getDefaultProcessorOptions = (): Fields => {
-    let defaultFields;
+  // We need to keep track of the processor form state if the user
+  // has made config changes, navigated between tabs (Configuration vs. Output)
+  // and has not yet submitted the form
+  const unsavedFormState = useRef<ProcessorInternal['options'] | undefined>();
 
-    if (unsavedFormData) {
-      const { options } = unsavedFormData;
-      defaultFields = { fields: options };
+  const getProcessor = useCallback((): ProcessorInternal => {
+    let options;
+
+    if (unsavedFormState?.current) {
+      options = unsavedFormState.current;
     } else {
-      defaultFields = { fields: processor?.options ?? {} };
+      options = processor?.options ?? {};
     }
 
-    return defaultFields;
-  };
+    return { ...processor, options } as ProcessorInternal;
+  }, [processor, unsavedFormState]);
 
   const { form } = useForm({
-    defaultValue: getDefaultProcessorOptions(),
+    defaultValue: { fields: getProcessor().options },
   });
 
   const handleSubmit = useCallback(
@@ -65,9 +67,13 @@ export const ProcessorFormContainer: FunctionComponent<Props> = ({
 
       if (isValid) {
         const { type, customOptions, fields } = data as FormData;
+        const options = customOptions ? customOptions : fields;
+
+        unsavedFormState.current = options;
+
         onSubmit({
           type,
-          options: customOptions ? customOptions : fields,
+          options,
         });
 
         if (shouldCloseFlyout) {
@@ -78,12 +84,12 @@ export const ProcessorFormContainer: FunctionComponent<Props> = ({
     [form, onClose, onSubmit]
   );
 
-  const resetProcessors = () => {
+  const resetProcessors = useCallback(() => {
     onSubmit({
       type: processor!.type,
       options: processor?.options || {},
     });
-  };
+  }, [onSubmit, processor]);
 
   useEffect(() => {
     const subscription = form.subscribe(onFormUpdate);
@@ -99,9 +105,8 @@ export const ProcessorFormContainer: FunctionComponent<Props> = ({
     return (
       <EditProcessorForm
         {...rest}
-        processor={processor!}
         form={form}
-        getDefaultProcessorOptions={getDefaultProcessorOptions}
+        getProcessor={getProcessor}
         esDocsBasePath={services.documentation.getEsDocsBasePath()}
         closeFlyout={onClose}
         resetProcessors={resetProcessors}
@@ -113,7 +118,6 @@ export const ProcessorFormContainer: FunctionComponent<Props> = ({
       <AddProcessorForm
         {...rest}
         form={form}
-        getDefaultProcessorOptions={getDefaultProcessorOptions}
         esDocsBasePath={services.documentation.getEsDocsBasePath()}
         closeFlyout={onClose}
         handleSubmit={handleSubmit}

diff --git a/...peline_processors_editor/components/processor_form/processor_output/processor_output.scss b/...peline_processors_editor/components/processor_form/processor_output/processor_output.scss
@@ -1,5 +1,12 @@
 .processorOutput {
-  &__callOutCodeBlock > pre {
-    background: transparent;
+  &__callOut {
+    &--customIcon {
+      .euiCallOutHeader {
+        align-items: center;
+      }
+    }
+    &__codeBlock > pre {
+      background: transparent;
+    }
   }
 }
diff --git a/...ipeline_processors_editor/components/processor_form/processor_output/processor_output.tsx b/...ipeline_processors_editor/components/processor_form/processor_output/processor_output.tsx
@@ -112,7 +112,13 @@ export const ProcessorOutput: FunctionComponent<Props> = ({
   const getOutputContent = () => {
     switch (status) {
       case 'skipped':
-        return <EuiCallOut title={i18nTexts.skippedCalloutTitle} iconType={SkippedIcon} />;
+        return (
+          <EuiCallOut
+            title={i18nTexts.skippedCalloutTitle}
+            iconType={SkippedIcon}
+            className="processorOutput__callOut processorOutput__callOut--customIcon"
+          />
+        );
       case 'dropped':
         return <EuiCallOut title={i18nTexts.droppedCalloutTitle} iconType="indexClose" />;
       case 'success':
@@ -127,11 +133,16 @@ export const ProcessorOutput: FunctionComponent<Props> = ({
         return <NoOutputCallOut />;
       case 'error':
         return (
-          <EuiCallOut iconType={ErrorIcon} title={i18nTexts.processorErrorTitle} color="danger">
+          <EuiCallOut
+            iconType={ErrorIcon}
+            title={i18nTexts.processorErrorTitle}
+            color="danger"
+            className="processorOutput__callOut processorOutput__callOut--customIcon"
+          >
             <EuiCodeBlock
               language="json"
               paddingSize="none"
-              className="processorOutput__callOutCodeBlock"
+              className="processorOutput__callOut__codeBlock"
               transparentBackground
             >
               {JSON.stringify(error, null, 2)}
@@ -144,9 +155,10 @@ export const ProcessorOutput: FunctionComponent<Props> = ({
             iconType={ErrorIgnoredIcon}
             title={i18nTexts.processorIgnoredErrorTitle}
             color="warning"
+            className="processorOutput__callOut processorOutput__callOut--customIcon"
           >
             <EuiCodeBlock
-              className="processorOutput__callOutCodeBlock"
+              className="processorOutput__callOut__codeBlock"
               language="json"
               paddingSize="none"
               transparentBackground

diff --git a/...onents/pipeline_processors_editor/components/processor_form/processor_settings_fields.tsx b/...onents/pipeline_processors_editor/components/processor_form/processor_settings_fields.tsx
@@ -13,17 +13,12 @@ import { ProcessorInternal } from '../../types';
 import { getProcessorDescriptor } from '../shared';
 import { CommonProcessorFields, ProcessorTypeField } from './processors/common_fields';
 import { Custom } from './processors/custom';
-import { Fields } from './processor_form.container';
 
 export interface Props {
   processor?: ProcessorInternal;
-  getDefaultProcessorOptions: () => Fields;
 }
 
-export const ProcessorSettingsFields: FunctionComponent<Props> = ({
-  processor,
-  getDefaultProcessorOptions,
-}) => {
+export const ProcessorSettingsFields: FunctionComponent<Props> = ({ processor }) => {
   return (
     <>
       <ProcessorTypeField initialType={processor?.type} />
@@ -33,7 +28,6 @@ export const ProcessorSettingsFields: FunctionComponent<Props> = ({
       <FormDataProvider pathsToWatch="type">
         {(arg: any) => {
           const { type } = arg;
-          const { fields: defaultOptions } = getDefaultProcessorOptions();
 
           if (type?.length) {
             const formDescriptor = getProcessorDescriptor(type as any);
@@ -59,7 +53,7 @@ export const ProcessorSettingsFields: FunctionComponent<Props> = ({
                 </>
               );
             }
-            return <Custom defaultOptions={defaultOptions} />;
+            return <Custom defaultOptions={processor?.options} />;
           }
 
           // If the user has not yet defined a type, we do not show any settings fields

diff --git a/...ine_processors_editor/components/test_pipeline/documents_dropdown/documents_dropdown.scss b/...ine_processors_editor/components/test_pipeline/documents_dropdown/documents_dropdown.scss
@@ -0,0 +1,3 @@
+.documentsDropdownPanel {
+  min-width: 200px;
+}
diff --git a/...ents/test_pipeline/documents_dropdown.tsx → ...documents_dropdown/documents_dropdown.tsx b/...ents/test_pipeline/documents_dropdown.tsx → ...documents_dropdown/documents_dropdown.tsx
@@ -17,9 +17,11 @@ import {
   EuiSpacer,
 } from '@elastic/eui';
 
-import { Document } from '../../types';
+import { Document } from '../../../types';
 
-import { TestPipelineFlyoutTab } from '../test_pipeline/test_pipeline_flyout_tabs';
+import { TestPipelineFlyoutTab } from '../test_pipeline_flyout_tabs';
+
+import './documents_dropdown.scss';
 
 const i18nTexts = {
   dropdownLabel: i18n.translate(
@@ -82,6 +84,7 @@ export const DocumentsDropdown: FunctionComponent<Props> = ({
       withTitle
       repositionOnScroll
       data-test-subj="documentsDropdown"
+      panelClassName="documentsDropdownPanel"
     >
       <EuiSelectable
         singleSelection

diff --git a/...omponents/pipeline_processors_editor/components/test_pipeline/documents_dropdown/index.ts b/...omponents/pipeline_processors_editor/components/test_pipeline/documents_dropdown/index.ts
@@ -0,0 +1,7 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+export { DocumentsDropdown } from './documents_dropdown';
diff --git a/...s/public/application/components/pipeline_processors_editor/context/processors_context.tsx b/...s/public/application/components/pipeline_processors_editor/context/processors_context.tsx
@@ -106,13 +106,6 @@ export const PipelineProcessorsContextProvider: FunctionComponent<Props> = ({
     validate: () => Promise.resolve(true),
   });
 
-  // We need to keep track of the processor form state if the user
-  // has made config changes, navigated between tabs (Configuration vs. Output)
-  // and has not yet submitted the form
-  const [unsavedProcessorFormData, setUnsavedProcessorFormData] = useState<
-    Omit<ProcessorInternal, 'id'> | undefined
-  >(undefined);
-
   const onFormUpdate = useCallback<(arg: OnFormUpdateArg<any>) => void>(
     ({ isValid, validate }) => {
       setFormState({
@@ -156,8 +149,6 @@ export const PipelineProcessorsContextProvider: FunctionComponent<Props> = ({
           });
           break;
         case 'managingProcessor':
-          setUnsavedProcessorFormData(processorTypeAndOptions);
-
           processorsDispatch({
             type: 'updateProcessor',
             payload: {
@@ -179,7 +170,6 @@ export const PipelineProcessorsContextProvider: FunctionComponent<Props> = ({
   const onCloseSettingsForm = useCallback(() => {
     setMode({ id: 'idle' });
     setFormState({ validate: () => Promise.resolve(true) });
-    setUnsavedProcessorFormData(undefined);
   }, [setFormState, setMode]);
 
   const onTreeAction = useCallback<OnActionHandler>(
@@ -250,7 +240,6 @@ export const PipelineProcessorsContextProvider: FunctionComponent<Props> = ({
           onFormUpdate={onFormUpdate}
           onSubmit={onSubmit}
           onClose={onCloseSettingsForm}
-          unsavedFormData={unsavedProcessorFormData}
         />
       ) : undefined}
       {mode.id === 'removingProcessor' && (