[Ingest Pipelines Editor] First round of UX improvements (#69381) (#7…

…0076) * First round of UX tweaks - Fixed potential text overflow issue on descriptions - Removed border around text input when editing description * Updated the on-failure pipeline description copy * Properly encode URI component pipeline names * use xjson editor in flyout * also hide the test flyout if we are editing a component * add much stronger dimming effect when in edit mode * also added dimming effect to moving state * remove box shadow if dimmed * add tooltips to dropzones * fix CITs after master merge * fix nested rendering of processors tree * only show the tooltip when the dropzone is unavaiable and visible * keep white background on dim * hide controls when moving * fix on blur bug * Rename variables and prefix booleans with "is" * Remove box shadow on all nested tree items * use classNames as it is intended to be used * Refactor SCSS values to variables * Added cancel move button - also hide the description in move mode when it is empty - update and refactor some shared sass variables - some number of sass changes to make labels play nice in move mode - changed the logic to not render the buttons when in move mode instead of display: none on them. The issue is with the tooltip not hiding when when we change to move mode and the mouse event "leave" does get through the tooltip element causing tooltips to hang even though the mouse has left them. * Fixes for monaco XJSON grammar parser and update form copy - Monaco XJSON worker was not handling trailing whitespace - Update copy in the processor configuration form Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
elastic · Jun 26, 2020 · e9e72a8 · e9e72a8
1 parent 0a901a8
commit e9e72a8
Show file tree

Hide file tree

Showing 25 changed files with 511 additions and 316 deletions.
diff --git a/packages/kbn-monaco/src/xjson/grammar.ts b/packages/kbn-monaco/src/xjson/grammar.ts
@@ -200,12 +200,13 @@ export const createParser = () => {
 
       try {
         value();
+        white();
       } catch (e) {
         errored = true;
         annos.push({ type: AnnoTypes.error, at: e.at - 1, text: e.message });
       }
       if (!errored && ch) {
-        error('Syntax error');
+        annos.push({ type: AnnoTypes.error, at: at, text: 'Syntax Error' });
       }
       return { annotations: annos };
     }

diff --git a/packages/kbn-monaco/src/xjson/language.ts b/packages/kbn-monaco/src/xjson/language.ts
@@ -52,7 +52,10 @@ export const registerGrammarChecker = (editor: monaco.editor.IEditor) => {
 
   const updateAnnos = async () => {
     const { annotations } = await wps.getAnnos();
-    const model = editor.getModel() as monaco.editor.ITextModel;
+    const model = editor.getModel() as monaco.editor.ITextModel | null;
+    if (!model) {
+      return;
+    }
     monaco.editor.setModelMarkers(
       model,
       OWNER,

diff --git a/...ck/plugins/ingest_pipelines/public/application/components/pipeline_form/pipeline_form.tsx b/...ck/plugins/ingest_pipelines/public/application/components/pipeline_form/pipeline_form.tsx
@@ -81,6 +81,7 @@ export const PipelineForm: React.FunctionComponent<PipelineFormProps> = ({
   });
 
   const onEditorFlyoutOpen = useCallback(() => {
+    setIsTestingPipeline(false);
     setIsRequestVisible(false);
   }, [setIsRequestVisible]);
 

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
@@ -24,8 +24,15 @@ jest.mock('@elastic/eui', () => {
         }}
       />
     ),
-    // Mocking EuiCodeEditor, which uses React Ace under the hood
-    EuiCodeEditor: (props: any) => (
+  };
+});
+
+jest.mock('../../../../../../../../src/plugins/kibana_react/public', () => {
+  const original = jest.requireActual('../../../../../../../../src/plugins/kibana_react/public');
+  return {
+    ...original,
+    // Mocking CodeEditor, which uses React Monaco under the hood
+    CodeEditor: (props: any) => (
       <input
         data-test-subj={props['data-test-subj'] || 'mockCodeEditor'}
         data-currentvalue={props.value}
@@ -95,8 +102,9 @@ const createActions = (testBed: TestBed<TestSubject>) => {
       act(() => {
         find(`${processorSelector}.moveItemButton`).simulate('click');
       });
+      component.update();
       act(() => {
-        find(dropZoneSelector).last().simulate('click');
+        find(dropZoneSelector).simulate('click');
       });
       component.update();
     },
@@ -122,13 +130,6 @@ const createActions = (testBed: TestBed<TestSubject>) => {
       });
     },
 
-    duplicateProcessor(processorSelector: string) {
-      find(`${processorSelector}.moreMenu.button`).simulate('click');
-      act(() => {
-        find(`${processorSelector}.moreMenu.duplicateButton`).simulate('click');
-      });
-    },
-
     startAndCancelMove(processorSelector: string) {
       act(() => {
         find(`${processorSelector}.moveItemButton`).simulate('click');
@@ -139,6 +140,13 @@ const createActions = (testBed: TestBed<TestSubject>) => {
       });
     },
 
+    duplicateProcessor(processorSelector: string) {
+      find(`${processorSelector}.moreMenu.button`).simulate('click');
+      act(() => {
+        find(`${processorSelector}.moreMenu.duplicateButton`).simulate('click');
+      });
+    },
+
     toggleOnFailure() {
       find('pipelineEditorOnFailureToggle').simulate('click');
     },

diff --git a/...ipelines/public/application/components/pipeline_processors_editor/components/_shared.scss b/...ipelines/public/application/components/pipeline_processors_editor/components/_shared.scss
@@ -0,0 +1,2 @@
+$dropZoneZIndex: 1;  /* Prevent the next item down from obscuring the button */
+$cancelButtonZIndex: 2;
diff --git a/...lication/components/pipeline_processors_editor/components/on_failure_processors_title.tsx b/...lication/components/pipeline_processors_editor/components/on_failure_processors_title.tsx
@@ -31,7 +31,7 @@ export const OnFailureProcessorsTitle: FunctionComponent = () => {
         <EuiText size="s" color="subdued">
           <FormattedMessage
             id="xpack.ingestPipelines.pipelineEditor.onFailureTreeDescription"
-            defaultMessage="The processors used to pre-process documents before indexing. {learnMoreLink}"
+            defaultMessage="The processors used to handle exceptions in this pipeline. {learnMoreLink}"
             values={{
               learnMoreLink: (
                 <EuiLink

diff --git a/...ts/pipeline_processors_editor/components/pipeline_processors_editor_item/context_menu.tsx b/...ts/pipeline_processors_editor/components/pipeline_processors_editor_item/context_menu.tsx
@@ -4,6 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
+import classNames from 'classnames';
 import React, { FunctionComponent, useState } from 'react';
 
 import { EuiContextMenuItem, EuiContextMenuPanel, EuiPopover, EuiButtonIcon } from '@elastic/eui';
@@ -12,6 +13,7 @@ import { editorItemMessages } from './messages';
 
 interface Props {
   disabled: boolean;
+  hidden: boolean;
   showAddOnFailure: boolean;
   onDuplicate: () => void;
   onDelete: () => void;
@@ -20,9 +22,13 @@ interface Props {
 }
 
 export const ContextMenu: FunctionComponent<Props> = (props) => {
-  const { showAddOnFailure, onDuplicate, onAddOnFailure, onDelete, disabled } = props;
+  const { showAddOnFailure, onDuplicate, onAddOnFailure, onDelete, disabled, hidden } = props;
   const [isOpen, setIsOpen] = useState<boolean>(false);
 
+  const containerClasses = classNames({
+    'pipelineProcessorsEditor__item--displayNone': hidden,
+  });
+
   const contextMenuItems = [
     <EuiContextMenuItem
       data-test-subj="duplicateButton"
@@ -63,23 +69,25 @@ export const ContextMenu: FunctionComponent<Props> = (props) => {
   ].filter(Boolean) as JSX.Element[];
 
   return (
-    <EuiPopover
-      data-test-subj={props['data-test-subj']}
-      anchorPosition="leftCenter"
-      panelPaddingSize="none"
-      isOpen={isOpen}
-      closePopover={() => setIsOpen(false)}
-      button={
-        <EuiButtonIcon
-          data-test-subj="button"
-          disabled={disabled}
-          onClick={() => setIsOpen((v) => !v)}
-          iconType="boxesHorizontal"
-          aria-label={editorItemMessages.moreButtonAriaLabel}
-        />
-      }
-    >
-      <EuiContextMenuPanel items={contextMenuItems} />
-    </EuiPopover>
+    <div className={containerClasses}>
+      <EuiPopover
+        data-test-subj={props['data-test-subj']}
+        anchorPosition="leftCenter"
+        panelPaddingSize="none"
+        isOpen={isOpen}
+        closePopover={() => setIsOpen(false)}
+        button={
+          <EuiButtonIcon
+            data-test-subj="button"
+            disabled={disabled}
+            onClick={() => setIsOpen((v) => !v)}
+            iconType="boxesHorizontal"
+            aria-label={editorItemMessages.moreButtonAriaLabel}
+          />
+        }
+      >
+        <EuiContextMenuPanel items={contextMenuItems} />
+      </EuiPopover>
+    </div>
   );
 };
diff --git a/...peline_processors_editor/components/pipeline_processors_editor_item/inline_text_input.tsx b/...peline_processors_editor/components/pipeline_processors_editor_item/inline_text_input.tsx
@@ -3,18 +3,20 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-
+import classNames from 'classnames';
 import React, { FunctionComponent, useState, useEffect, useCallback } from 'react';
 import { EuiFieldText, EuiText, keyCodes } from '@elastic/eui';
 
 export interface Props {
   placeholder: string;
   ariaLabel: string;
   onChange: (value: string) => void;
+  disabled: boolean;
   text?: string;
 }
 
 export const InlineTextInput: FunctionComponent<Props> = ({
+  disabled,
   placeholder,
   text,
   ariaLabel,
@@ -23,26 +25,17 @@ export const InlineTextInput: FunctionComponent<Props> = ({
   const [isShowingTextInput, setIsShowingTextInput] = useState<boolean>(false);
   const [textValue, setTextValue] = useState<string>(text ?? '');
 
-  const content = isShowingTextInput ? (
-    <EuiFieldText
-      controlOnly
-      fullWidth
-      compressed
-      value={textValue}
-      aria-label={ariaLabel}
-      className="pipelineProcessorsEditor__item__textInput"
-      inputRef={(el) => el?.focus()}
-      onChange={(event) => setTextValue(event.target.value)}
-    />
-  ) : (
-    <EuiText size="s" color="subdued">
-      {text || <em>{placeholder}</em>}
-    </EuiText>
-  );
+  const containerClasses = classNames('pipelineProcessorsEditor__item__textContainer', {
+    'pipelineProcessorsEditor__item__textContainer--notEditing': !isShowingTextInput && !disabled,
+  });
 
   const submitChange = useCallback(() => {
-    setIsShowingTextInput(false);
-    onChange(textValue);
+    // Give any on blur handlers the chance to complete if the user is
+    // tabbing over this component.
+    setTimeout(() => {
+      setIsShowingTextInput(false);
+      onChange(textValue);
+    });
   }, [setIsShowingTextInput, onChange, textValue]);
 
   useEffect(() => {
@@ -62,14 +55,27 @@ export const InlineTextInput: FunctionComponent<Props> = ({
     };
   }, [isShowingTextInput, submitChange, setIsShowingTextInput]);
 
-  return (
-    <div
-      className="pipelineProcessorsEditor__item__textContainer"
-      tabIndex={0}
-      onFocus={() => setIsShowingTextInput(true)}
-      onBlur={submitChange}
-    >
-      {content}
+  return isShowingTextInput && !disabled ? (
+    <div className={`pipelineProcessorsEditor__item__textContainer ${containerClasses}`}>
+      <EuiFieldText
+        controlOnly
+        onBlur={submitChange}
+        fullWidth
+        compressed
+        value={textValue}
+        aria-label={ariaLabel}
+        className="pipelineProcessorsEditor__item__textInput"
+        inputRef={(el) => el?.focus()}
+        onChange={(event) => setTextValue(event.target.value)}
+      />
+    </div>
+  ) : (
+    <div className={containerClasses} tabIndex={0} onFocus={() => setIsShowingTextInput(true)}>
+      <EuiText size="s" color="subdued">
+        <div className="pipelineProcessorsEditor__item__description">
+          {text || <em>{placeholder}</em>}
+        </div>
+      </EuiText>
     </div>
   );
 };
diff --git a/...ponents/pipeline_processors_editor/components/pipeline_processors_editor_item/messages.ts b/...ponents/pipeline_processors_editor/components/pipeline_processors_editor_item/messages.ts
@@ -10,12 +10,9 @@ export const editorItemMessages = {
   moveButtonLabel: i18n.translate('xpack.ingestPipelines.pipelineEditor.item.moveButtonLabel', {
     defaultMessage: 'Move this processor',
   }),
-  editorButtonLabel: i18n.translate(
-    'xpack.ingestPipelines.pipelineEditor.item.editButtonAriaLabel',
-    {
-      defaultMessage: 'Edit this processor',
-    }
-  ),
+  editButtonLabel: i18n.translate('xpack.ingestPipelines.pipelineEditor.item.editButtonAriaLabel', {
+    defaultMessage: 'Edit this processor',
+  }),
   duplicateButtonLabel: i18n.translate(
     'xpack.ingestPipelines.pipelineEditor.item.moreMenu.duplicateButtonLabel',
     {
@@ -31,7 +28,7 @@ export const editorItemMessages = {
   cancelMoveButtonLabel: i18n.translate(
     'xpack.ingestPipelines.pipelineEditor.item.cancelMoveButtonAriaLabel',
     {
-      defaultMessage: 'Cancel moving this processor',
+      defaultMessage: 'Cancel move',
     }
   ),
   deleteButtonLabel: i18n.translate(

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,17 +1,57 @@
+@import '../shared';
+
 .pipelineProcessorsEditor__item {
+  transition: border-color 1s;
+  min-height: 50px;
+  &--selected {
+    border: 1px solid $euiColorPrimary;
+  }
+
+  &--displayNone {
+    display: none;
+  }
+
+  &--dimmed {
+    box-shadow: none;
+  }
+
+  // Remove the box-shadow on all nested items
+  .pipelineProcessorsEditor__item {
+    box-shadow: none !important;
+  }
+
+  &__processorTypeLabel {
+    line-height: $euiButtonHeightSmall;
+  }
+
   &__textContainer {
     padding: 4px;
     border-radius: 2px;
 
-    transition: border-color .3s;
-    border: 2px solid #FFF;
+    transition: border-color 0.3s;
+    border: 2px solid transparent;
 
-    &:hover {
-      border: 2px solid $euiColorLightShade;
+    &--notEditing {
+      &:hover {
+        border: 2px solid $euiColorLightShade;
+      }
     }
   }
+
+  &__description {
+    overflow-x: hidden;
+    white-space: nowrap;
+    text-overflow: ellipsis;
+    max-width: 600px;
+  }
+
   &__textInput {
     height: 21px;
-    min-width: 100px;
+    min-width: 150px;
+  }
+
+  &__cancelMoveButton {
+    // Ensure that the cancel button is above the drop zones
+    z-index: $cancelButtonZIndex;
   }
 }
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		$dropZoneZIndex: 1; /* Prevent the next item down from obscuring the button */
		$cancelButtonZIndex: 2;