From fcc6e0e28d6c46fad8c9139362c6c8dcdc42eaa0 Mon Sep 17 00:00:00 2001
From: Jean-Louis Leysens <jloleysens@gmail.com>
Date: Mon, 5 Oct 2020 18:47:18 +0200
Subject: [PATCH] [Ingest Pipelines] Processors editor a11y focus states
 (#79122) (#79481)

* Fix showing of accessibility border

- fix use of flex items (removed unnecessary use thereof)
- also fixed overflow when tabbing through drop zones (compressed)

* refactor isLast to compressed

* optimize keyboard focus states in move mode

* fix jest test

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
---
 .../inline_text_input.tsx                     |  18 ++-
 .../pipeline_processors_editor_item.tsx       |   1 +
 .../components/drop_zone_button.tsx           |  25 ++-
 .../components/private_tree.tsx               | 151 +++++++++---------
 .../processors_tree/processors_tree.scss      |   9 +-
 .../processors_tree/processors_tree.tsx       |  17 +-
 6 files changed, 117 insertions(+), 104 deletions(-)

diff --git a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/pipeline_processors_editor_item/inline_text_input.tsx b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/pipeline_processors_editor_item/inline_text_input.tsx
index e91974adca20a..833888e9062df 100644
--- a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/pipeline_processors_editor_item/inline_text_input.tsx
+++ b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/pipeline_processors_editor_item/inline_text_input.tsx
@@ -11,15 +11,23 @@ export interface Props {
   placeholder: string;
   ariaLabel: string;
   onChange: (value: string) => void;
-  disabled: boolean;
+  /**
+   * Whether the containing element of the text input can be focused.
+   *
+   * If it cannot be focused, this component cannot switch to showing
+   * the text input field.
+   *
+   * Defaults to false.
+   */
+  disabled?: boolean;
   text?: string;
 }
 
 export const InlineTextInput: FunctionComponent<Props> = ({
-  disabled,
   placeholder,
   text,
   ariaLabel,
+  disabled = false,
   onChange,
 }) => {
   const [isShowingTextInput, setIsShowingTextInput] = useState<boolean>(false);
@@ -71,7 +79,11 @@ export const InlineTextInput: FunctionComponent<Props> = ({
       />
     </div>
   ) : (
-    <div className={containerClasses} tabIndex={0} onFocus={() => setIsShowingTextInput(true)}>
+    <div
+      className={containerClasses}
+      tabIndex={disabled ? -1 : 0}
+      onFocus={() => setIsShowingTextInput(true)}
+    >
       <EuiText size="s" color="subdued">
         <div className="pipelineProcessorsEditor__item__description">
           {text || <em>{placeholder}</em>}
diff --git a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/pipeline_processors_editor_item/pipeline_processors_editor_item.tsx b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/pipeline_processors_editor_item/pipeline_processors_editor_item.tsx
index bf69f817183ab..aa76f67413306 100644
--- a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/pipeline_processors_editor_item/pipeline_processors_editor_item.tsx
+++ b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/pipeline_processors_editor_item/pipeline_processors_editor_item.tsx
@@ -159,6 +159,7 @@ export const PipelineProcessorsEditorItem: FunctionComponent<Props> = memo(
                   color={isDimmed ? 'subdued' : undefined}
                 >
                   <EuiLink
+                    tabIndex={isEditorNotInIdleMode ? -1 : 0}
                     disabled={isEditorNotInIdleMode}
                     onClick={() => {
                       editor.setMode({
diff --git a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processors_tree/components/drop_zone_button.tsx b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processors_tree/components/drop_zone_button.tsx
index 57ecb6f7f1187..cd32e2ec54726 100644
--- a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processors_tree/components/drop_zone_button.tsx
+++ b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processors_tree/components/drop_zone_button.tsx
@@ -7,11 +7,15 @@
 import { i18n } from '@kbn/i18n';
 import React, { FunctionComponent } from 'react';
 import classNames from 'classnames';
-import { EuiButtonIcon, EuiToolTip } from '@elastic/eui';
+import { EuiButtonIcon } from '@elastic/eui';
 
 export interface Props {
   isVisible: boolean;
   isDisabled: boolean;
+  /**
+   * Useful for buttons at the very top or bottom of lists to avoid any overflow.
+   */
+  compressed?: boolean;
   onClick: (event: React.MouseEvent<HTMLButtonElement, MouseEvent>) => void;
   'data-test-subj'?: string;
 }
@@ -29,7 +33,7 @@ const cannotMoveHereLabel = i18n.translate(
 );
 
 export const DropZoneButton: FunctionComponent<Props> = (props) => {
-  const { onClick, isDisabled, isVisible } = props;
+  const { onClick, isDisabled, isVisible, compressed } = props;
   const isUnavailable = isVisible && isDisabled;
   const containerClasses = classNames({
     // eslint-disable-next-line @typescript-eslint/naming-convention
@@ -40,14 +44,16 @@ export const DropZoneButton: FunctionComponent<Props> = (props) => {
   const buttonClasses = classNames({
     // eslint-disable-next-line @typescript-eslint/naming-convention
     'pipelineProcessorsEditor__tree__dropZoneButton--visible': isVisible,
+    // eslint-disable-next-line @typescript-eslint/naming-convention
+    'pipelineProcessorsEditor__tree__dropZoneButton--compressed': compressed,
   });
 
-  const content = (
+  return (
     <div className={`pipelineProcessorsEditor__tree__dropZoneContainer ${containerClasses}`}>
       <EuiButtonIcon
         data-test-subj={props['data-test-subj']}
         className={`pipelineProcessorsEditor__tree__dropZoneButton ${buttonClasses}`}
-        aria-label={moveHereLabel}
+        aria-label={isUnavailable ? cannotMoveHereLabel : moveHereLabel}
         // We artificially disable the button so that hover and pointer events are
         // still enabled
         onClick={isDisabled ? () => {} : onClick}
@@ -55,15 +61,4 @@ export const DropZoneButton: FunctionComponent<Props> = (props) => {
       />
     </div>
   );
-
-  return isUnavailable ? (
-    <EuiToolTip
-      className="pipelineProcessorsEditor__tree__dropZoneContainer__toolTip"
-      content={cannotMoveHereLabel}
-    >
-      {content}
-    </EuiToolTip>
-  ) : (
-    content
-  );
 };
diff --git a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processors_tree/components/private_tree.tsx b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processors_tree/components/private_tree.tsx
index 89407fd4366d8..27e612f196667 100644
--- a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processors_tree/components/private_tree.tsx
+++ b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processors_tree/components/private_tree.tsx
@@ -5,7 +5,7 @@
  */
 
 import React, { FunctionComponent, MutableRefObject, useEffect } from 'react';
-import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
+import { EuiFlexGroup } from '@elastic/eui';
 import { AutoSizer, List, WindowScroller } from 'react-virtualized';
 
 import { DropSpecialLocations } from '../../../constants';
@@ -78,50 +78,45 @@ export const PrivateTree: FunctionComponent<PrivateProps> = ({
     return (
       <>
         {idx === 0 ? (
-          <EuiFlexItem>
-            <DropZoneButton
-              data-test-subj={`dropButtonAbove-${stringifiedSelector}`}
-              onClick={(event) => {
-                event.preventDefault();
-                onAction({
-                  type: 'move',
-                  payload: {
-                    destination: selector.concat(DropSpecialLocations.top),
-                    source: movingProcessor!.selector,
-                  },
-                });
-              }}
-              isVisible={Boolean(movingProcessor)}
-              isDisabled={!movingProcessor || isDropZoneAboveDisabled(info, movingProcessor)}
-            />
-          </EuiFlexItem>
-        ) : undefined}
-        <EuiFlexItem>
-          <TreeNode
-            level={level}
-            processor={processor}
-            processorInfo={info}
-            onAction={onAction}
-            movingProcessor={movingProcessor}
-          />
-        </EuiFlexItem>
-        <EuiFlexItem>
           <DropZoneButton
-            data-test-subj={`dropButtonBelow-${stringifiedSelector}`}
-            isVisible={Boolean(movingProcessor)}
-            isDisabled={!movingProcessor || isDropZoneBelowDisabled(info, movingProcessor)}
+            data-test-subj={`dropButtonAbove-${stringifiedSelector}`}
             onClick={(event) => {
               event.preventDefault();
               onAction({
                 type: 'move',
                 payload: {
-                  destination: selector.concat(String(idx + 1)),
+                  destination: selector.concat(DropSpecialLocations.top),
                   source: movingProcessor!.selector,
                 },
               });
             }}
+            isVisible={Boolean(movingProcessor)}
+            isDisabled={!movingProcessor || isDropZoneAboveDisabled(info, movingProcessor)}
           />
-        </EuiFlexItem>
+        ) : undefined}
+        <TreeNode
+          level={level}
+          processor={processor}
+          processorInfo={info}
+          onAction={onAction}
+          movingProcessor={movingProcessor}
+        />
+        <DropZoneButton
+          compressed={level === 1 && idx + 1 === processors.length}
+          data-test-subj={`dropButtonBelow-${stringifiedSelector}`}
+          isVisible={Boolean(movingProcessor)}
+          isDisabled={!movingProcessor || isDropZoneBelowDisabled(info, movingProcessor)}
+          onClick={(event) => {
+            event.preventDefault();
+            onAction({
+              type: 'move',
+              payload: {
+                destination: selector.concat(String(idx + 1)),
+                source: movingProcessor!.selector,
+              },
+            });
+          }}
+        />
       </>
     );
   };
@@ -141,52 +136,50 @@ export const PrivateTree: FunctionComponent<PrivateProps> = ({
       <WindowScroller ref={windowScrollerRef} scrollElement={window}>
         {({ height, registerChild, isScrolling, onChildScroll, scrollTop }: any) => {
           return (
-            <EuiFlexGroup direction="column" responsive={false} gutterSize="none">
-              <AutoSizer disableHeight>
-                {({ width }) => {
-                  return (
-                    <div ref={registerChild}>
-                      <List
-                        ref={listRef}
-                        autoHeight
-                        height={height}
-                        width={width}
-                        overScanRowCount={5}
-                        isScrolling={isScrolling}
-                        onChildScroll={onChildScroll}
-                        scrollTop={scrollTop}
-                        rowCount={processors.length}
-                        rowHeight={({ index }) => {
-                          const processor = processors[index];
-                          return calculateItemHeight({
-                            processor,
-                            isFirstInArray: index === 0,
-                          });
-                        }}
-                        rowRenderer={({ index: idx, style }) => {
-                          const processor = processors[idx];
-                          const above = processors[idx - 1];
-                          const below = processors[idx + 1];
-                          const info: ProcessorInfo = {
-                            id: processor.id,
-                            selector: selector.concat(String(idx)),
-                            aboveId: above?.id,
-                            belowId: below?.id,
-                          };
+            <AutoSizer disableHeight>
+              {({ width }) => {
+                return (
+                  <div style={{ width: '100%' }} ref={registerChild}>
+                    <List
+                      ref={listRef}
+                      autoHeight
+                      height={height}
+                      width={width}
+                      overScanRowCount={5}
+                      isScrolling={isScrolling}
+                      onChildScroll={onChildScroll}
+                      scrollTop={scrollTop}
+                      rowCount={processors.length}
+                      rowHeight={({ index }) => {
+                        const processor = processors[index];
+                        return calculateItemHeight({
+                          processor,
+                          isFirstInArray: index === 0,
+                        });
+                      }}
+                      rowRenderer={({ index: idx, style }) => {
+                        const processor = processors[idx];
+                        const above = processors[idx - 1];
+                        const below = processors[idx + 1];
+                        const info: ProcessorInfo = {
+                          id: processor.id,
+                          selector: selector.concat(String(idx)),
+                          aboveId: above?.id,
+                          belowId: below?.id,
+                        };
 
-                          return (
-                            <div style={style} key={processor.id}>
-                              {renderRow({ processor, info, idx })}
-                            </div>
-                          );
-                        }}
-                        processors={processors}
-                      />
-                    </div>
-                  );
-                }}
-              </AutoSizer>
-            </EuiFlexGroup>
+                        return (
+                          <div style={style} key={processor.id}>
+                            {renderRow({ processor, info, idx })}
+                          </div>
+                        );
+                      }}
+                      processors={processors}
+                    />
+                  </div>
+                );
+              }}
+            </AutoSizer>
           );
         }}
       </WindowScroller>
diff --git a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processors_tree/processors_tree.scss b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processors_tree/processors_tree.scss
index 25e4eb7320bf4..f1e399428cdf2 100644
--- a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processors_tree/processors_tree.scss
+++ b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processors_tree/processors_tree.scss
@@ -31,15 +31,14 @@
     }
   }
   $dropZoneButtonHeight: 60px;
-  $dropZoneButtonOffsetY: $dropZoneButtonHeight * -0.5;
+  $dropZoneButtonOffsetY: $dropZoneButtonHeight * 0.5;
 
   &__dropZoneButton {
     position: absolute;
     padding: 0;
     height: $dropZoneButtonHeight;
-    margin-top: $dropZoneButtonOffsetY;
+    margin-top: -$dropZoneButtonOffsetY;
     width: 100%;
-    opacity: 0;
     text-decoration: none !important;
     z-index: $dropZoneZIndex;
 
@@ -49,6 +48,10 @@
         transform: none !important;
       }
     }
+
+    &--compressed {
+      height: $dropZoneButtonOffsetY;
+    }
   }
 
   &__addProcessorButton {
diff --git a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processors_tree/processors_tree.tsx b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processors_tree/processors_tree.tsx
index ffc0a1459b791..46d237e1467e7 100644
--- a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processors_tree/processors_tree.tsx
+++ b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processors_tree/processors_tree.tsx
@@ -98,9 +98,16 @@ export const ProcessorsTree: FunctionComponent<Props> = memo((props) => {
         />
       </EuiFlexItem>
       <EuiFlexItem grow={false}>
-        <EuiFlexGroup responsive={false} alignItems="flexStart" gutterSize="none">
-          <EuiFlexItem data-test-subj={selectorToDataTestSubject(baseSelector)} grow={false}>
-            {!processors.length && (
+        <EuiFlexGroup
+          data-test-subj={selectorToDataTestSubject(baseSelector)}
+          responsive={false}
+          alignItems="flexStart"
+          gutterSize="none"
+          direction="column"
+        >
+          {!processors.length && (
+            // We want to make this dropzone the max length of its container
+            <EuiFlexItem style={{ width: '100%' }}>
               <DropZoneButton
                 data-test-subj="dropButtonEmptyTree"
                 isVisible={Boolean(movingProcessor)}
@@ -116,7 +123,9 @@ export const ProcessorsTree: FunctionComponent<Props> = memo((props) => {
                   });
                 }}
               />
-            )}
+            </EuiFlexItem>
+          )}
+          <EuiFlexItem grow={false}>
             <AddProcessorButton
               onClick={() => {
                 onAction({ type: 'addProcessor', payload: { target: baseSelector } });