[Ingest Pipeline] Processor Editor Item Styling tweak (elastic#70786)

* Small styling tweaks to processor items - Moved the move button to the before the processor name - Cancel button is still after description if there is one - Made inline text description a bit taller and changed border style * Commit code that moves the cancel move button 🤦🏼‍♂️ * Do not completely hide the move button, prevent ui from jumping * Update styling and UX of move button; EuiToggleButton - Bring the styling of the button more in line with this comment elastic#70786 (comment) * use cross icon for cancelling move * replace hard values with EUI values in SCSS * Address rerendering triggered by context - also prevent re-renders basded on contstructing objects on each render * Similarly move use of context to settings form container We are only interested in the es docs path string in the settings form component so no need to render for other updates. Co-authored-by: Elastic Machine <[email protected]>
jloleysens · Jul 7, 2020 · 4f59aa7 · 4f59aa7
1 parent 1f5e67e
commit 4f59aa7
Show file tree

Hide file tree

Showing 13 changed files with 184 additions and 103 deletions.
diff --git a/...ion/components/pipeline_processors_editor/__jest__/pipeline_processors_editor.helpers.tsx b/...ion/components/pipeline_processors_editor/__jest__/pipeline_processors_editor.helpers.tsx
@@ -112,7 +112,7 @@ const createActions = (testBed: TestBed<TestSubject>) => {
 
     moveProcessor(processorSelector: string, dropZoneSelector: string) {
       act(() => {
-        find(`${processorSelector}.moveItemButton`).simulate('click');
+        find(`${processorSelector}.moveItemButton`).simulate('change');
       });
       component.update();
       act(() => {
@@ -144,12 +144,13 @@ const createActions = (testBed: TestBed<TestSubject>) => {
 
     startAndCancelMove(processorSelector: string) {
       act(() => {
-        find(`${processorSelector}.moveItemButton`).simulate('click');
+        find(`${processorSelector}.moveItemButton`).simulate('change');
       });
       component.update();
       act(() => {
-        find(`${processorSelector}.cancelMoveItemButton`).simulate('click');
+        find(`${processorSelector}.cancelMoveItemButton`).simulate('change');
       });
+      component.update();
     },
 
     duplicateProcessor(processorSelector: string) {

diff --git a/...cation/components/pipeline_processors_editor/__jest__/pipeline_processors_editor.test.tsx b/...cation/components/pipeline_processors_editor/__jest__/pipeline_processors_editor.test.tsx
@@ -153,7 +153,7 @@ describe('Pipeline Editor', () => {
       const processorSelector = 'processors>0';
       actions.startAndCancelMove(processorSelector);
       // Assert that we have exited move mode for this processor
-      expect(exists(`moveItemButton-${processorSelector}`));
+      expect(exists(`${processorSelector}.moveItemButton`)).toBe(true);
       const [onUpdateResult] = onUpdate.mock.calls[onUpdate.mock.calls.length - 1];
       const { processors } = onUpdateResult.getData();
       // Assert that nothing has changed

diff --git a/...ipelines/public/application/components/pipeline_processors_editor/components/_shared.scss b/...ipelines/public/application/components/pipeline_processors_editor/components/_shared.scss
@@ -1,2 +1,2 @@
-$dropZoneZIndex: 1;  /* Prevent the next item down from obscuring the button */
-$cancelButtonZIndex: 2;
+$dropZoneZIndex: $euiZLevel1;  /* Prevent the next item down from obscuring the button */
+$cancelButtonZIndex: $euiZLevel2;
diff --git a/...components/pipeline_processors_editor/components/pipeline_processors_editor_item/index.ts b/...components/pipeline_processors_editor/components/pipeline_processors_editor_item/index.ts
@@ -4,4 +4,6 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-export { PipelineProcessorsEditorItem, Handlers } from './pipeline_processors_editor_item';
+export { PipelineProcessorsEditorItem } from './pipeline_processors_editor_item.container';
+
+export { Handlers } from './types';
diff --git a/.../components/pipeline_processors_editor_item/pipeline_processors_editor_item.container.tsx b/.../components/pipeline_processors_editor_item/pipeline_processors_editor_item.container.tsx
@@ -0,0 +1,28 @@
+/*
+ * 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.
+ */
+
+import React, { FunctionComponent } from 'react';
+
+import { usePipelineProcessorsContext } from '../../context';
+
+import {
+  PipelineProcessorsEditorItem as ViewComponent,
+  Props as ViewComponentProps,
+} from './pipeline_processors_editor_item';
+
+type Props = Omit<ViewComponentProps, 'editor' | 'processorsDispatch'>;
+
+export const PipelineProcessorsEditorItem: FunctionComponent<Props> = (props) => {
+  const { state } = usePipelineProcessorsContext();
+
+  return (
+    <ViewComponent
+      {...props}
+      editor={state.editor}
+      processorsDispatch={state.processors.dispatch}
+    />
+  );
+};
diff --git a/...rs_editor/components/pipeline_processors_editor_item/pipeline_processors_editor_item.scss b/...rs_editor/components/pipeline_processors_editor_item/pipeline_processors_editor_item.scss
@@ -1,10 +1,12 @@
 @import '../shared';
 
 .pipelineProcessorsEditor__item {
-  transition: border-color 1s;
+  transition: border-color $euiAnimSpeedExtraSlow $euiAnimSlightResistance;
   min-height: 50px;
+
   &--selected {
-    border: 1px solid $euiColorPrimary;
+    border: $euiBorderThin;
+    border-color: $euiColorPrimary;
   }
 
   &--displayNone {
@@ -25,15 +27,14 @@
   }
 
   &__textContainer {
-    padding: 4px;
-    border-radius: 2px;
-
-    transition: border-color 0.3s;
-    border: 2px solid transparent;
+    cursor: text;
+    border-bottom: 1px dashed transparent;
 
     &--notEditing {
+      border-bottom: $euiBorderEditable;
+      border-width: $euiBorderWidthThin;
       &:hover {
-        border: 2px solid $euiColorLightShade;
+        border-color: $euiColorMediumShade;
       }
     }
   }
@@ -46,12 +47,17 @@
   }
 
   &__textInput {
-    height: 21px;
-    min-width: 150px;
+    height: $euiSizeL;
+    min-width: 200px;
   }
 
-  &__cancelMoveButton {
-    // Ensure that the cancel button is above the drop zones
-    z-index: $cancelButtonZIndex;
+  &__moveButton {
+    &:hover {
+      transform: none !important;
+    }
+    &--cancel {
+      // Ensure that the cancel button is above the drop zones
+      z-index: $cancelButtonZIndex;
+    }
   }
 }
diff --git a/...ors_editor/components/pipeline_processors_editor_item/pipeline_processors_editor_item.tsx b/...ors_editor/components/pipeline_processors_editor_item/pipeline_processors_editor_item.tsx
@@ -8,33 +8,31 @@ import classNames from 'classnames';
 import React, { FunctionComponent, memo } from 'react';
 import {
   EuiButtonIcon,
-  EuiButton,
+  EuiButtonToggle,
   EuiFlexGroup,
   EuiFlexItem,
   EuiPanel,
   EuiText,
   EuiToolTip,
 } from '@elastic/eui';
 
-import { ProcessorInternal, ProcessorSelector } from '../../types';
+import { ProcessorInternal, ProcessorSelector, ContextValueEditor } from '../../types';
 import { selectorToDataTestSubject } from '../../utils';
+import { ProcessorsDispatch } from '../../processors_reducer';
 
-import { usePipelineProcessorsContext } from '../../context';
+import { ProcessorInfo } from '../processors_tree';
 
 import './pipeline_processors_editor_item.scss';
 
 import { InlineTextInput } from './inline_text_input';
 import { ContextMenu } from './context_menu';
 import { i18nTexts } from './i18n_texts';
-import { ProcessorInfo } from '../processors_tree';
-
-export interface Handlers {
-  onMove: () => void;
-  onCancelMove: () => void;
-}
+import { Handlers } from './types';
 
 export interface Props {
   processor: ProcessorInternal;
+  processorsDispatch: ProcessorsDispatch;
+  editor: ContextValueEditor;
   handlers: Handlers;
   selector: ProcessorSelector;
   description?: string;
@@ -43,18 +41,16 @@ export interface Props {
 }
 
 export const PipelineProcessorsEditorItem: FunctionComponent<Props> = memo(
-  ({
+  function PipelineProcessorsEditorItem({
     processor,
     description,
     handlers: { onCancelMove, onMove },
     selector,
     movingProcessor,
     renderOnFailureHandlers,
-  }) => {
-    const {
-      state: { editor, processors },
-    } = usePipelineProcessorsContext();
-
+    editor,
+    processorsDispatch,
+  }) {
     const isDisabled = editor.mode.id !== 'idle';
     const isInMoveMode = Boolean(movingProcessor);
     const isMovingThisProcessor = processor.id === movingProcessor?.id;
@@ -78,9 +74,41 @@ export const PipelineProcessorsEditorItem: FunctionComponent<Props> = memo(
       'pipelineProcessorsEditor__item--displayNone': isInMoveMode && !processor.options.description,
     });
 
-    const cancelMoveButtonClasses = classNames('pipelineProcessorsEditor__item__cancelMoveButton', {
-      'pipelineProcessorsEditor__item--displayNone': !isMovingThisProcessor,
-    });
+    const renderMoveButton = () => {
+      const label = !isMovingThisProcessor
+        ? i18nTexts.moveButtonLabel
+        : i18nTexts.cancelMoveButtonLabel;
+      const dataTestSubj = !isMovingThisProcessor ? 'moveItemButton' : 'cancelMoveItemButton';
+      const moveButtonClasses = classNames('pipelineProcessorsEditor__item__moveButton', {
+        'pipelineProcessorsEditor__item__moveButton--cancel': isMovingThisProcessor,
+      });
+      const icon = isMovingThisProcessor ? 'cross' : 'sortable';
+      const moveButton = (
+        <EuiButtonToggle
+          isEmpty={!isMovingThisProcessor}
+          fill={isMovingThisProcessor}
+          isIconOnly
+          iconType={icon}
+          data-test-subj={dataTestSubj}
+          size="s"
+          disabled={isDisabled && !isMovingThisProcessor}
+          label={label}
+          aria-label={label}
+          onChange={() => (!isMovingThisProcessor ? onMove() : onCancelMove())}
+        />
+      );
+      // Remove the tooltip from the DOM to prevent it from lingering if the mouse leave event
+      // did not fire.
+      return (
+        <div className={moveButtonClasses}>
+          {!isInMoveMode ? (
+            <EuiToolTip content={i18nTexts.moveButtonLabel}>{moveButton}</EuiToolTip>
+          ) : (
+            moveButton
+          )}
+        </div>
+      );
+    };
 
     return (
       <EuiPanel className={panelClasses} paddingSize="s">
@@ -93,6 +121,7 @@ export const PipelineProcessorsEditorItem: FunctionComponent<Props> = memo(
         >
           <EuiFlexItem>
             <EuiFlexGroup gutterSize="m" alignItems="center" responsive={false}>
+              <EuiFlexItem grow={false}>{renderMoveButton()}</EuiFlexItem>
               <EuiFlexItem grow={false}>
                 <EuiText
                   className="pipelineProcessorsEditor__item__processorTypeLabel"
@@ -115,7 +144,7 @@ export const PipelineProcessorsEditorItem: FunctionComponent<Props> = memo(
                         description: nextDescription,
                       };
                     }
-                    processors.dispatch({
+                    processorsDispatch({
                       type: 'updateProcessor',
                       payload: {
                         processor: {
@@ -149,25 +178,6 @@ export const PipelineProcessorsEditorItem: FunctionComponent<Props> = memo(
                   </EuiToolTip>
                 )}
               </EuiFlexItem>
-              <EuiFlexItem className={actionElementClasses} grow={false}>
-                {!isInMoveMode && (
-                  <EuiToolTip content={i18nTexts.moveButtonLabel}>
-                    <EuiButtonIcon
-                      data-test-subj="moveItemButton"
-                      size="s"
-                      disabled={isDisabled}
-                      aria-label={i18nTexts.moveButtonLabel}
-                      onClick={onMove}
-                      iconType="sortable"
-                    />
-                  </EuiToolTip>
-                )}
-              </EuiFlexItem>
-              <EuiFlexItem grow={false} className={cancelMoveButtonClasses}>
-                <EuiButton data-test-subj="cancelMoveItemButton" size="s" onClick={onCancelMove}>
-                  {i18nTexts.cancelMoveButtonLabel}
-                </EuiButton>
-              </EuiFlexItem>
             </EuiFlexGroup>
           </EuiFlexItem>
           <EuiFlexItem grow={false}>
@@ -183,7 +193,7 @@ export const PipelineProcessorsEditorItem: FunctionComponent<Props> = memo(
                 editor.setMode({ id: 'removingProcessor', arg: { selector } });
               }}
               onDuplicate={() => {
-                processors.dispatch({
+                processorsDispatch({
                   type: 'duplicateProcessor',
                   payload: {
                     source: selector,

diff --git a/...components/pipeline_processors_editor/components/pipeline_processors_editor_item/types.ts b/...components/pipeline_processors_editor/components/pipeline_processors_editor_item/types.ts
@@ -0,0 +1,10 @@
+/*
+ * 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 interface Handlers {
+  onMove: () => void;
+  onCancelMove: () => void;
+}
diff --git a/...rocessors_editor/components/processor_settings_form/processor_settings_form.container.tsx b/...rocessors_editor/components/processor_settings_form/processor_settings_form.container.tsx
@@ -10,6 +10,7 @@ import { useForm, OnFormUpdateArg, FormData } from '../../../../../shared_import
 import { ProcessorInternal } from '../../types';
 
 import { ProcessorSettingsForm as ViewComponent } from './processor_settings_form';
+import { usePipelineProcessorsContext } from '../../context';
 
 export type ProcessorSettingsFromOnSubmitArg = Omit<ProcessorInternal, 'id'>;
 
@@ -32,6 +33,10 @@ export const ProcessorSettingsForm: FunctionComponent<Props> = ({
   onSubmit,
   ...rest
 }) => {
+  const {
+    links: { esDocsBasePath },
+  } = usePipelineProcessorsContext();
+
   const handleSubmit = useCallback(
     async (data: FormData, isValid: boolean) => {
       if (isValid) {
@@ -60,5 +65,7 @@ export const ProcessorSettingsForm: FunctionComponent<Props> = ({
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [onFormUpdate]);
 
-  return <ViewComponent processor={processor} form={form} {...rest} />;
+  return (
+    <ViewComponent {...rest} processor={processor} form={form} esDocsBasePath={esDocsBasePath} />
+  );
 };
diff --git a/...pipeline_processors_editor/components/processor_settings_form/processor_settings_form.tsx b/...pipeline_processors_editor/components/processor_settings_form/processor_settings_form.tsx
@@ -21,7 +21,6 @@ import {
 } from '@elastic/eui';
 
 import { Form, FormDataProvider, FormHook } from '../../../../../shared_imports';
-import { usePipelineProcessorsContext } from '../../context';
 import { ProcessorInternal } from '../../types';
 
 import { DocumentationButton } from './documentation_button';
@@ -35,6 +34,7 @@ export interface Props {
   form: FormHook;
   onClose: () => void;
   onOpen: () => void;
+  esDocsBasePath: string;
 }
 
 const updateButtonLabel = i18n.translate(
@@ -52,11 +52,7 @@ const cancelButtonLabel = i18n.translate(
 );
 
 export const ProcessorSettingsForm: FunctionComponent<Props> = memo(
-  ({ processor, form, isOnFailure, onClose, onOpen }) => {
-    const {
-      links: { esDocsBasePath },
-    } = usePipelineProcessorsContext();
-
+  ({ processor, form, isOnFailure, onClose, onOpen, esDocsBasePath }) => {
     const flyoutTitleContent = isOnFailure ? (
       <FormattedMessage
         id="xpack.ingestPipelines.settingsFormOnFailureFlyout.title"

diff --git a/...components/pipeline_processors_editor/components/processors_tree/components/tree_node.tsx b/...components/pipeline_processors_editor/components/processors_tree/components/tree_node.tsx
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import React, { FunctionComponent, useMemo } from 'react';
+import React, { FunctionComponent, useMemo, useCallback } from 'react';
 import { i18n } from '@kbn/i18n';
 import { EuiText } from '@elastic/eui';
 
@@ -46,7 +46,7 @@ export const TreeNode: FunctionComponent<Props> = ({
     };
   }, [onAction, stringSelector, processor]); // eslint-disable-line react-hooks/exhaustive-deps
 
-  const renderOnFailureHandlersTree = () => {
+  const renderOnFailureHandlersTree = useCallback(() => {
     if (!processor.onFailure?.length) {
       return;
     }
@@ -79,7 +79,7 @@ export const TreeNode: FunctionComponent<Props> = ({
         />
       </div>
     );
-  };
+  }, [processor.onFailure, stringSelector, onAction, movingProcessor, level]); // eslint-disable-line react-hooks/exhaustive-deps
 
   return (
     <PipelineProcessorsEditorItem