Skip to content

Commit

Permalink
Fix EuiPopover continuing to hijack EuiContextMenu focus
Browse files Browse the repository at this point in the history
- because EuiPopover's `updateFocus` is called after the transition ends (350ms currently), it supercedes EuiContextMenu's `updateFocus`

- I'm not a super huge fan of this DOM-heavy workaround but I can't think of any other options - very open to more ideas
  • Loading branch information
cee-chen committed Apr 13, 2022
1 parent a605c21 commit 4958d95
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 0 deletions.
14 changes: 14 additions & 0 deletions src/components/context_menu/context_menu_panel.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import React from 'react';

import { EuiPopover } from '../popover';
import { EuiContextMenuItem } from './context_menu_item';
import { EuiContextMenuPanel } from './context_menu_panel';

Expand Down Expand Up @@ -44,6 +45,19 @@ describe('EuiContextMenuPanel', () => {
);
cy.focused().should('not.exist');
});

describe('when inside an EuiPopover', () => {
it('reclaims focus from the parent popover panel', () => {
cy.mount(
<EuiPopover isOpen={true} button={<button />}>
<EuiContextMenuPanel items={items} />
</EuiPopover>
);
cy.wait(400); // EuiPopover's updateFocus() takes ~350ms to run
cy.focused().should('not.have.attr', 'class', 'euiPopover__panel');
cy.focused().should('have.attr', 'class', 'euiContextMenuPanel');
});
});
});

describe('Keyboard navigation of items', () => {
Expand Down
30 changes: 30 additions & 0 deletions src/components/context_menu/context_menu_panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,35 @@ export class EuiContextMenuPanel extends Component<Props, State> {
});
}

// If EuiContextMenu is used within an EuiPopover, EuiPopover's own
// `updateFocus()` method hijacks EuiContextMenuPanel's `updateFocus()`
// 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.updateFocus();
});
}

onTransitionComplete = () => {
if (this.props.onTransitionComplete) {
this.props.onTransitionComplete();
Expand All @@ -280,6 +309,7 @@ export class EuiContextMenuPanel extends Component<Props, State> {

componentDidMount() {
this.updateFocus();
this.reclaimPopoverFocus();
this._isMounted = true;
}

Expand Down

0 comments on commit 4958d95

Please sign in to comment.