Address some early feedback and use FormDataProvider

FormDataProvider gives a more declarative way to listen to form
state updates.

Also refactored the state reader mechanism.

x-pack/plugins/ingest_pipelines/public/application/components/pipeline_form/pipeline_form.tsx

@@ -3,7 +3,7 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-import React, { useState } from 'react';
+import React, { useRef, useState } from 'react';
 import { FormattedMessage } from '@kbn/i18n/react';
 import { i18n } from '@kbn/i18n';
 import {
@@ -25,12 +25,13 @@ import {
   FormConfig,
   JsonEditorField,
   useKibana,
+  FormDataProvider,
 } from '../../../shared_imports';
 import { Pipeline } from '../../../../common/types';
 
 import { SectionError, PipelineRequestFlyout } from '../';
 import { pipelineFormSchema } from './schema';
-import { PipelineProcessorsEditor } from '../../pipeline_processors_editor';
+import { PipelineProcessorsEditor, StateReader } from '../../pipeline_processors_editor';
 
 interface Props {
   onSave: (pipeline: Pipeline) => void;
@@ -60,7 +61,7 @@ export const PipelineForm: React.FunctionComponent<Props> = ({
 }) => {
   const { services } = useKibana();
 
-  const [processorStateReaderRef, setProcessorStateReaderRef] = useState<any>();
+  const processorStateReaderRef = useRef<StateReader>(undefined as any);
   const [showJsonProcessorsEditor, setShowJsonProcessorsEditor] = useState(false);
   const [isVersionVisible, setIsVersionVisible] = useState<boolean>(Boolean(defaultValue.version));
   const [isOnFailureEditorVisible, setIsOnFailureEditorVisible] = useState<boolean>(
@@ -71,7 +72,7 @@ export const PipelineForm: React.FunctionComponent<Props> = ({
   const handleSave: FormConfig['onSubmit'] = (formData, isValid) => {
     if (isValid) {
       if (!showJsonProcessorsEditor) {
-        const { processors } = processorStateReaderRef.current();
+        const { processors } = processorStateReaderRef.current.read();
         onSave({ ...formData, processors } as Pipeline);
       } else {
         onSave(formData as Pipeline);
@@ -131,21 +132,25 @@ export const PipelineForm: React.FunctionComponent<Props> = ({
       );
     }
 
-    const formProcessorValue = form.getFields().processors?.value ?? defaultValue?.processors;
-
-    const processors =
-      typeof formProcessorValue === 'string' && formProcessorValue
-        ? JSON.parse(formProcessorValue)
-        : defaultValue?.processors ?? [];
-
     return (
-      <>
-        <PipelineProcessorsEditor
-          stateReaderRef={ref => setProcessorStateReaderRef(ref)}
-          processors={processors}
-        />
-        <EuiSpacer size="l" />
-      </>
+      <FormDataProvider pathsToWatch="processors">
+        {({ processors }) => {
+          const processorProp =
+            typeof processors === 'string' && processors
+              ? JSON.parse(processors)
+              : defaultValue?.processors ?? [];
+
+          return (
+            <>
+              <PipelineProcessorsEditor
+                stateReaderRef={reader => (processorStateReaderRef.current = reader)}
+                processors={processorProp}
+              />
+              <EuiSpacer size="l" />
+            </>
+          );
+        }}
+      </FormDataProvider>
     );
   };
 

x-pack/plugins/ingest_pipelines/public/application/pipeline_processors_editor/components/index.ts

@@ -4,5 +4,5 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-export { FormFlyout } from './form_flyout';
-export { ProcessorSettingsForm } from './processor_settings_form';
+export { SettingsFormFlyout } from './settings_form_flyout';
+export { ProcessorSettingsForm, ProcessorSettingsFromOnSubmitArg } from './processor_settings_form';

x-pack/plugins/ingest_pipelines/public/application/pipeline_processors_editor/components/processor_settings_form/map_processor_type_to_form.tsx

@@ -18,6 +18,6 @@ export const types = Object.keys(mapProcessorTypeToForm);
 
 export type ProcessorType = keyof typeof mapProcessorTypeToForm;
 
-export const getProcessorFormDescriptor = (type: string): FunctionComponent => {
+export const getProcessorForm = (type: string): FunctionComponent => {
   return mapProcessorTypeToForm[type as ProcessorType];
 };

x-pack/plugins/ingest_pipelines/public/application/pipeline_processors_editor/components/processor_settings_form/processor_settings_form.tsx

@@ -3,14 +3,15 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-
-import React, { FunctionComponent, useEffect, useState } from 'react';
+import { i18n } from '@kbn/i18n';
+import React, { FunctionComponent } from 'react';
 import { EuiButton } from '@elastic/eui';
-import { Form, useForm } from '../../../../shared_imports';
+
+import { Form, useForm, FormDataProvider } from '../../../../shared_imports';
 
 import { PipelineEditorProcessor } from '../../types';
 
-import { getProcessorFormDescriptor } from './map_processor_type_to_form';
+import { getProcessorForm } from './map_processor_type_to_form';
 import { CommonProcessorFields, ProcessorTypeField } from './processors/common_fields';
 
 export type ProcessorSettingsFromOnSubmitArg = Omit<PipelineEditorProcessor, 'id'>;
@@ -31,43 +32,43 @@ export const ProcessorSettingsForm: FunctionComponent<Props> = ({ processor, onS
     }
   };
 
-  const [type, setType] = useState<string | undefined>(processor?.type);
-
   const { form } = useForm({
     defaultValue: processor ? { ...processor.options } : undefined,
     onSubmit: handleSubmit,
   });
 
-  useEffect(() => {
-    const subscription = form.subscribe(({ data }) => {
-      if (data.raw.type !== type) {
-        setType(data.raw.type);
-      }
-    });
-    return subscription.unsubscribe;
-  });
-
-  let FormFields: FunctionComponent | undefined;
-
-  if (type) {
-    FormFields = getProcessorFormDescriptor(type as any);
-
-    // TODO: Handle this error in a different way
-    if (!FormFields) {
-      throw new Error(`Could not find form for type ${type}`);
-    }
-  }
-
   return (
     <Form form={form}>
       <ProcessorTypeField initialType={processor?.type} />
-      {FormFields && (
-        <>
-          <FormFields />
-          <CommonProcessorFields />
-          <EuiButton onClick={form.submit}>Submit</EuiButton>
-        </>
-      )}
+      <FormDataProvider pathsToWatch="type">
+        {({ type }) => {
+          let FormFields: FunctionComponent | null = null;
+
+          if (type) {
+            FormFields = getProcessorForm(type as any);
+
+            // TODO: Handle this error in a different way
+            if (!FormFields) {
+              throw new Error(`Could not find form for type ${type}`);
+            }
+          }
+
+          return (
+            FormFields && (
+              <>
+                <FormFields />
+                <CommonProcessorFields />
+                <EuiButton onClick={form.submit}>
+                  {i18n.translate(
+                    'xpack.ingestPipelines.pipelineEditor.settingsForm.submitButtonLabel',
+                    { defaultMessage: 'Submit' }
+                  )}
+                </EuiButton>
+              </>
+            )
+          );
+        }}
+      </FormDataProvider>
     </Form>
   );
 };

x-pack/plugins/ingest_pipelines/public/application/pipeline_processors_editor/components/form_flyout.tsx → x-pack/plugins/ingest_pipelines/public/application/pipeline_processors_editor/components/settings_form_flyout.tsx

@@ -10,17 +10,17 @@ import React, { FunctionComponent } from 'react';
 
 import { PipelineEditorProcessor } from '../types';
 
-import { ProcessorSettingsForm, ProcessorSettingsFromOnSubmitArg } from './processor_settings_form';
+import { ProcessorSettingsForm, ProcessorSettingsFromOnSubmitArg } from '.';
 
 export interface Props {
   processor: PipelineEditorProcessor | undefined;
   onSubmit: (processor: ProcessorSettingsFromOnSubmitArg) => void;
   onClose: () => void;
 }
 
-export const FormFlyout: FunctionComponent<Props> = ({ onClose, processor, onSubmit }) => {
+export const SettingsFormFlyout: FunctionComponent<Props> = ({ onClose, processor, onSubmit }) => {
   return (
-    <EuiFlyout onClose={onClose} aria-labelledby="flyoutComplicatedTitle">
+    <EuiFlyout onClose={onClose}>
       <EuiFlyoutBody>
         <ProcessorSettingsForm processor={processor as any} onSubmit={onSubmit} />
       </EuiFlyoutBody>

x-pack/plugins/ingest_pipelines/public/application/pipeline_processors_editor/index.ts

@@ -4,4 +4,4 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-export { PipelineProcessorsEditor } from './pipeline_processors_editor';
+export { PipelineProcessorsEditor, StateReader } from './pipeline_processors_editor';

x-pack/plugins/ingest_pipelines/public/application/pipeline_processors_editor/pipeline_processors_editor.tsx

@@ -5,14 +5,7 @@
  */
 
 import { i18n } from '@kbn/i18n';
-import React, {
-  FunctionComponent,
-  useState,
-  useMemo,
-  useRef,
-  useEffect,
-  MutableRefObject,
-} from 'react';
+import React, { FunctionComponent, useState, useMemo, useRef, useEffect } from 'react';
 import {
   EuiPanel,
   EuiFlexGroup,
@@ -21,30 +14,44 @@ import {
   EuiDroppable,
   EuiDraggable,
   EuiIcon,
-  EuiTitle,
-  EuiSpacer,
   EuiButton,
   EuiButtonEmpty,
 } from '@elastic/eui';
 
 import { Processor } from '../../../common/types';
 
-import { FormFlyout } from './components';
+import { SettingsFormFlyout } from './components';
 import { prepareDataIn } from './data_in';
 import { prepareDataOut, DataOutResult } from './data_out';
 import { useEditorState } from './reducer';
 import { PipelineEditorProcessor } from './types';
 
 export interface Props {
   processors: Processor[];
-  stateReaderRef: (state: MutableRefObject<() => DataOutResult>) => void;
+  /**
+   * This follows the pattern of other components that wish to expose
+   * a state without emitting state updates to parents, e.g.:
+   *
+   * <input ref={myInputRef => ... } />
+   *
+   * In this case we share a well defined API that contains the latest
+   * state reader. Callers can create a ref of their own to point to
+   * this reader, e.g., useRef<{@link StateReader}>() so they can
+   * imperatively choose when to read state.
+   */
+  stateReaderRef: (stateReader: { read: () => DataOutResult }) => void;
   onFailure?: Processor[];
 }
 
+export interface StateReader {
+  read: () => DataOutResult;
+}
+
 export const PipelineProcessorsEditor: FunctionComponent<Props> = ({
   processors: originalProcessors,
   stateReaderRef,
 }) => {
+  const internalStateReaderRef = useRef<StateReader>({ read: undefined as any });
   const dataInResult = useMemo(() => prepareDataIn({ processors: originalProcessors }), [
     originalProcessors,
   ]);
@@ -54,14 +61,14 @@ export const PipelineProcessorsEditor: FunctionComponent<Props> = ({
     undefined
   );
   const [isAddingNewProcessor, setIsAddingNewProcessor] = useState<boolean>(false);
-  const stateGetterRef = useRef(() => prepareDataOut(state));
 
   useEffect(() => {
-    stateGetterRef.current = () => prepareDataOut(state);
+    internalStateReaderRef.current.read = () => prepareDataOut(state);
   }, [state]);
 
   useEffect(() => {
-    stateReaderRef(stateGetterRef);
+    // Share our state reader instance with callers, do this once only
+    stateReaderRef(internalStateReaderRef.current);
   });
 
   const dismissFlyout = () => {
@@ -71,10 +78,6 @@ export const PipelineProcessorsEditor: FunctionComponent<Props> = ({
 
   return (
     <>
-      <EuiTitle>
-        <h1>Pipeline Editor</h1>
-      </EuiTitle>
-      <EuiSpacer size="m" />
       <EuiPanel>
         <EuiDragDropContext
           onDragEnd={({ source, destination }) => {
@@ -137,7 +140,7 @@ export const PipelineProcessorsEditor: FunctionComponent<Props> = ({
         <EuiButton onClick={() => setIsAddingNewProcessor(true)}>Add a processor</EuiButton>
       </EuiPanel>
       {selectedProcessor || isAddingNewProcessor ? (
-        <FormFlyout
+        <SettingsFormFlyout
           processor={selectedProcessor}
           onClose={() => {
             dismissFlyout();

x-pack/plugins/ingest_pipelines/public/application/pipeline_processors_editor/reducer.ts

@@ -34,11 +34,11 @@ const reducer: Reducer<State, Action> = (state, action) => {
 
   if (action.type === 'removeProcessor') {
     const idx = findProcessorIdx(state.processors, action.payload.processor);
-    const copyProcessors = state.processors.slice();
-    copyProcessors.splice(idx, 1);
+    const processorsCopy = state.processors.slice();
+    processorsCopy.splice(idx, 1);
     return {
       ...state,
-      processors: copyProcessors,
+      processors: processorsCopy,
     };
   }
 
@@ -55,11 +55,11 @@ const reducer: Reducer<State, Action> = (state, action) => {
   if (action.type === 'updateProcessor') {
     const { processor } = action.payload;
     const idx = findProcessorIdx(state.processors, processor);
-    const copyProcessors = state.processors.slice();
-    copyProcessors.splice(idx, 1, processor);
+    const processorsCopy = state.processors.slice();
+    processorsCopy.splice(idx, 1, processor);
     return {
       ...state,
-      processors: copyProcessors,
+      processors: processorsCopy,
     };
   }
 

x-pack/plugins/ingest_pipelines/public/shared_imports.ts

@@ -28,6 +28,7 @@ export {
   UseField,
   FormHook,
   useFormContext,
+  FormDataProvider,
 } from '../../../../src/plugins/es_ui_shared/static/forms/hook_form_lib';
 
 export {