[setup] Refactor popover parent finding logic

- move to separate method - create instance var - specify `initialPopover` and add `transitionType` check, as the popover doesn't re-initialize when moving between panels in the same popover, and we don't want this to re-fire unnecessarily - add E2E tests for popover behavior
elastic · May 5, 2022 · 1e3d257 · 1e3d257
1 parent a5be54f
commit 1e3d257
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 34 deletions.
diff --git a/src/components/context_menu/context_menu_panel.spec.tsx b/src/components/context_menu/context_menu_panel.spec.tsx
@@ -208,7 +208,7 @@ describe('EuiContextMenuPanel', () => {
         });
       });
 
-      it('does not lose focus while using left/right arrow navigation between panels', () => {
+      describe('panels', () => {
         const panels = [
           {
             id: 0,
@@ -245,21 +245,31 @@ describe('EuiContextMenuPanel', () => {
             initialFocusedItemIndex: 0,
           },
         ];
-        cy.mount(<EuiContextMenu panels={panels} initialPanelId={0} />);
-        cy.realPress('{downarrow}');
-        cy.focused().should('have.attr', 'data-test-subj', 'itemA');
-        cy.realPress('{rightarrow}');
-        cy.focused().should('have.attr', 'data-test-subj', 'itemB');
-        cy.realPress('{rightarrow}');
-        cy.focused().should('have.attr', 'data-test-subj', 'itemC');
 
-        // Test extremely rapid left/right arrow usage
-        cy.repeatRealPress('{leftarrow}');
-        cy.focused().should('have.attr', 'data-test-subj', 'itemA');
-        cy.repeatRealPress('{rightarrow}');
-        cy.focused().should('have.attr', 'data-test-subj', 'itemC');
-        cy.repeatRealPress('{leftarrow}');
-        cy.focused().should('have.attr', 'data-test-subj', 'itemA');
+        it('does not lose focus while using left/right arrow navigation between panels', () => {
+          cy.mount(<EuiContextMenu panels={panels} initialPanelId={0} />);
+          cy.realPress('{downarrow}');
+          cy.focused().should('have.attr', 'data-test-subj', 'itemA');
+          cy.realPress('{rightarrow}');
+          cy.focused().should('have.attr', 'data-test-subj', 'itemB');
+          cy.realPress('{rightarrow}');
+          cy.focused().should('have.attr', 'data-test-subj', 'itemC');
+        });
+
+        it('does not lose focus when inside an EuiPopover and during rapid left/right arrow usage', () => {
+          cy.mount(
+            <EuiPopover isOpen={true} button={<button />}>
+              <EuiContextMenu panels={panels} initialPanelId={0} />
+            </EuiPopover>
+          );
+          cy.wait(350); // Wait for EuiContextMenuPanel to reclaim focus from popover
+          cy.realPress('{downarrow}');
+          cy.focused().should('have.attr', 'data-test-subj', 'itemA');
+          cy.repeatRealPress('{rightarrow}');
+          cy.focused().should('have.attr', 'data-test-subj', 'itemC');
+          cy.repeatRealPress('{leftarrow}');
+          cy.focused().should('have.attr', 'data-test-subj', 'itemA');
+        });
       });
     });
 

diff --git a/src/components/context_menu/context_menu_panel.tsx b/src/components/context_menu/context_menu_panel.tsx
@@ -94,6 +94,7 @@ export class EuiContextMenuPanel extends Component<Props, State> {
   private _isMounted = false;
   private backButton?: HTMLElement | null = null;
   private panel?: HTMLElement | null = null;
+  private initialPopoverParent?: HTMLElement | null = null;
 
   constructor(props: Props) {
     super(props);
@@ -269,26 +270,8 @@ export class EuiContextMenuPanel extends Component<Props, State> {
   // 350ms after the popover finishes transitioning in. This workaround
   // reclaims focus from parent EuiPopovers that do not set an `initialFocus`
   reclaimPopoverFocus() {
-    if (!this.panel) return;
-
-    const parent = this.panel.parentNode as HTMLElement;
-    if (!parent) return;
-    const hasEuiContextMenuParent = parent.classList.contains('euiContextMenu');
-
-    // It's possible to use an EuiContextMenuPanel directly in a popover without
-    // an EuiContextMenu, so we need to account for that when searching parent nodes
-    const popoverParent = hasEuiContextMenuParent
-      ? (parent?.parentNode?.parentNode as HTMLElement)
-      : (parent?.parentNode as HTMLElement);
-    if (!popoverParent) return;
-
-    const hasPopoverParent = popoverParent.classList.contains(
-      'euiPopover__panel'
-    );
-    if (!hasPopoverParent) return;
-
     // If the popover panel gains focus, switch it to the context menu panel instead
-    popoverParent.addEventListener('focus', () => {
+    this.initialPopoverParent?.addEventListener('focus', () => {
       this.updateFocus();
     });
   }
@@ -417,10 +400,37 @@ export class EuiContextMenuPanel extends Component<Props, State> {
     }
   }
 
+  getInitialPopoverParent() {
+    // If `transitionType` exists, that means we're navigating between panels
+    // and the initial popover has already loaded, so we shouldn't need this logic
+    if (this.props.transitionType) return;
+
+    if (!this.panel) return;
+
+    const parent = this.panel.parentNode as HTMLElement;
+    if (!parent) return;
+    const hasEuiContextMenuParent = parent.classList.contains('euiContextMenu');
+
+    // It's possible to use an EuiContextMenuPanel directly in a popover without
+    // an EuiContextMenu, so we need to account for that when searching parent nodes
+    const popoverParent = hasEuiContextMenuParent
+      ? (parent?.parentNode?.parentNode as HTMLElement)
+      : (parent?.parentNode as HTMLElement);
+    if (!popoverParent) return;
+
+    const hasPopoverParent = popoverParent.classList.contains(
+      'euiPopover__panel'
+    );
+    if (!hasPopoverParent) return;
+
+    this.initialPopoverParent = popoverParent;
+  }
+
   panelRef = (node: HTMLElement | null) => {
     this.panel = node;
 
     this.updateHeight();
+    this.getInitialPopoverParent();
   };
 
   render() {