diff --git a/src-docs/src/views/popover/initial_focus.js b/src-docs/src/views/popover/initial_focus.js new file mode 100644 index 00000000000..7ccac2c5365 --- /dev/null +++ b/src-docs/src/views/popover/initial_focus.js @@ -0,0 +1,40 @@ +import React, { useState } from 'react'; + +import { + EuiButton, + EuiFormRow, + EuiPopover, + EuiSpacer, + EuiFieldText, +} from '../../../../src/components'; + +export default () => { + const [isPopoverOpen, setIsPopoverOpen] = useState(false); + + const onButtonClick = () => + setIsPopoverOpen((isPopoverOpen) => !isPopoverOpen); + const closePopover = () => setIsPopoverOpen(false); + + const button = ( + + Show popover + + ); + + return ( + + + + + + + + Submit + + ); +}; diff --git a/src-docs/src/views/popover/popover_example.js b/src-docs/src/views/popover/popover_example.js index d8b6ed3e533..f86611fb483 100644 --- a/src-docs/src/views/popover/popover_example.js +++ b/src-docs/src/views/popover/popover_example.js @@ -15,6 +15,9 @@ import { import Popover from './popover'; const popoverSource = require('!!raw-loader!./popover'); +import InitialFocus from './initial_focus'; +const initialFocusSource = require('!!raw-loader!./initial_focus'); + import TrapFocus from './trap_focus'; const trapFocusSource = require('!!raw-loader!./trap_focus'); @@ -60,6 +63,14 @@ const trapFocusSnippet = ` `; +const initialFocusSnippet = ` + +`; + const popoverAnchorSnippet = `, }, + { + title: 'Setting an initial focus', + source: [ + { + type: GuideSectionTypes.JS, + code: initialFocusSource, + }, + ], + text: ( + <> +

+ If you want a specific child element of the popover to immediately + gain focus when the popover is open, use the{' '} + initialFocus prop to pass either a + selector or DOM node. +

+ + It can be jarring for keyboard and screen reader users to + immediately land on an element with no other context. To + alleviate this, ensure that your initial focus target makes + sense alone or is the primary goal of the popover. + + } + /> + + ), + props: { EuiPopover }, + snippet: initialFocusSnippet, + demo: , + }, { title: 'Removing the focus trap', source: [ diff --git a/src-docs/src/views/popover/trap_focus.js b/src-docs/src/views/popover/trap_focus.js index ed6da6d9a92..4404c49058b 100644 --- a/src-docs/src/views/popover/trap_focus.js +++ b/src-docs/src/views/popover/trap_focus.js @@ -1,4 +1,4 @@ -import React, { useState } from 'react'; +import React, { useState, useEffect } from 'react'; import { EuiButton, @@ -13,15 +13,6 @@ import { useGeneratedHtmlId } from '../../../../src/services'; export default () => { const [isPopoverOpen, setIsPopoverOpen] = useState(false); - const trapFocusFormRowId__1 = useGeneratedHtmlId({ - prefix: 'trapFocusFormRow', - suffix: 'first', - }); - const trapFocusFormRowId__2 = useGeneratedHtmlId({ - prefix: 'trapFocusFormRow', - suffix: 'second', - }); - const onButtonClick = () => setIsPopoverOpen((isPopoverOpen) => !isPopoverOpen); const closePopover = () => setIsPopoverOpen(false); @@ -32,17 +23,24 @@ export default () => { ); + // Since `hasFocus={false}` disables popover auto focus, we need to manually set it ourselves + const focusId = useGeneratedHtmlId(); + useEffect(() => { + if (isPopoverOpen) { + document.getElementById(focusId).focus({ preventScroll: true }); + } + }, [isPopoverOpen, focusId]); + return ( { /> - + { cy.focused().should('have.attr', 'data-test-subj', 'panelA'); }); }); + + describe('when inside an EuiPopover', () => { + it('reclaims focus from the parent popover panel', () => { + cy.mount( + }> + + + ); + 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', () => { diff --git a/src/components/context_menu/context_menu_panel.tsx b/src/components/context_menu/context_menu_panel.tsx index 613622703e4..8840760618f 100644 --- a/src/components/context_menu/context_menu_panel.tsx +++ b/src/components/context_menu/context_menu_panel.tsx @@ -264,6 +264,35 @@ export class EuiContextMenuPanel extends Component { }); } + // 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(); @@ -272,6 +301,7 @@ export class EuiContextMenuPanel extends Component { componentDidMount() { this.updateFocus(); + this.reclaimPopoverFocus(); this._isMounted = true; } diff --git a/src/components/datagrid/data_grid.spec.tsx b/src/components/datagrid/data_grid.spec.tsx index 7163ca3cb59..cc9b0ec3115 100644 --- a/src/components/datagrid/data_grid.spec.tsx +++ b/src/components/datagrid/data_grid.spec.tsx @@ -366,12 +366,17 @@ describe('EuiDataGrid', () => { cy.focused().type('{enter}'); cy.focused().should('have.attr', 'data-test-subj', 'focusOnMe'); - // fourth cell is non-expandable with multiple interactives, click should focus on the cell + // fourth cell is expandable & interactive, click should focus on the popover cy.get( '[data-gridcell-column-index="3"][data-gridcell-row-index="0"]' ).click(); cy.focused().type('{enter}'); - cy.focused().should('have.attr', 'data-test-subj', 'focusOnMe'); // focus trap focuses the link + // focus trap focuses the popover + cy.focused().should( + 'have.attr', + 'data-test-subj', + 'euiDataGridExpansionPopover' + ); cy.focused().type('{esc}'); cy.focused() .should('have.attr', 'data-gridcell-column-index', '3') @@ -404,10 +409,14 @@ describe('EuiDataGrid', () => { .should('have.attr', 'data-gridcell-column-index', '5') .should('have.attr', 'data-gridcell-row-index', '0'); cy.focused().type('{enter}'); // trigger expansion popover - cy.focused().should('have.attr', 'data-test-subj', 'btn-yes'); // focus trap should move focus to the first button - cy.focused().parentsUntil( - '[data-test-subj="euiDataGridExpansionPopover"]' - ); // ensure focus is in the popover + // focus trap focuses the popover + cy.focused().should( + 'have.attr', + 'data-test-subj', + 'euiDataGridExpansionPopover' + ); + cy.realPress('Tab'); + cy.focused().should('have.attr', 'data-test-subj', 'btn-yes'); cy.realPress('Tab'); cy.focused().should('have.attr', 'data-test-subj', 'btn-no'); cy.realPress('Tab'); diff --git a/src/components/popover/popover.spec.tsx b/src/components/popover/popover.spec.tsx new file mode 100644 index 00000000000..73f30c2140d --- /dev/null +++ b/src/components/popover/popover.spec.tsx @@ -0,0 +1,87 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +/// + +import React, { useState } from 'react'; + +import { EuiButton } from '../button'; +import { EuiPopover } from './popover'; + +const PopoverComponent = ({ children, ...rest }) => { + const [isPopoverOpen, setIsPopoverOpen] = useState(false); + const closePopover = () => setIsPopoverOpen(false); + const togglePopover = () => + setIsPopoverOpen((isPopoverOpen) => !isPopoverOpen); + + const button = ( + + Show popover + + ); + + return ( + + {children} + + ); +}; + +describe('EuiPopover', () => { + describe('focus behavior', () => { + it('focuses the panel wrapper by default', () => { + cy.mount(Test); + cy.get('[data-test-subj="togglePopover"]').click(); + cy.focused().should('have.attr', 'data-test-subj', 'popoverPanel'); + }); + + it('does not focus anything if `ownFocus` is false', () => { + cy.mount(Test); + cy.get('[data-test-subj="togglePopover"]').click(); + cy.focused().should('have.attr', 'data-test-subj', 'togglePopover'); + }); + + describe('initialFocus', () => { + it('does not focus anything if `initialFocus` is false', () => { + cy.mount( + Test + ); + cy.get('[data-test-subj="togglePopover"]').click(); + cy.focused().should('have.attr', 'data-test-subj', 'togglePopover'); + }); + + it('focuses selector strings', () => { + cy.mount( + + + + ); + cy.get('[data-test-subj="togglePopover"]').click(); + cy.focused().should('have.attr', 'id', 'test'); + }); + + it('focuses functions returning DOM Nodes', () => { + cy.mount( + document.getElementById('test')} + > + + + ); + cy.get('[data-test-subj="togglePopover"]').click(); + cy.focused().should('have.attr', 'id', 'test'); + }); + }); + }); +}); diff --git a/src/components/popover/popover.tsx b/src/components/popover/popover.tsx index 6d915a85c49..36a80306406 100644 --- a/src/components/popover/popover.tsx +++ b/src/components/popover/popover.tsx @@ -16,7 +16,7 @@ import React, { RefCallback, } from 'react'; import classNames from 'classnames'; -import { tabbable, focusable } from 'tabbable'; +import { focusable } from 'tabbable'; import { CommonProps, NoArgCallback } from '../common'; import { FocusTarget, EuiFocusTrap, EuiFocusTrapProps } from '../focus_trap'; @@ -438,16 +438,11 @@ export class EuiPopover extends Component { return; } - // Otherwise let's focus the first tabbable item and expedite input from the user. + // Otherwise focus either `initialFocus` or the panel let focusTarget; if (this.props.initialFocus != null) { focusTarget = getElementFromInitialFocus(this.props.initialFocus); - } else { - const tabbableItems = tabbable(this.panel); - if (tabbableItems.length) { - focusTarget = tabbableItems[0]; - } } // there's a race condition between the popover content becoming visible and this function call @@ -456,7 +451,7 @@ export class EuiPopover extends Component { if (focusTarget == null) { // there isn't a focus target, one of two reasons: // #1 is the whole panel hidden? If so, schedule another check - // #2 panel is visible but no tabbables exist, move focus to the panel + // #2 panel is visible and no `initialFocus` was set, move focus to the panel const panelVisibility = window.getComputedStyle(this.panel).opacity; if (panelVisibility === '0') { // #1 diff --git a/upcoming_changelogs/5784.md b/upcoming_changelogs/5784.md new file mode 100644 index 00000000000..90d941180a7 --- /dev/null +++ b/upcoming_changelogs/5784.md @@ -0,0 +1,3 @@ +**Breaking changes** + +- `EuiPopover`s will no longer focus the first tabbable child by default - instead, the popover panel will be focused. This change should be a better experience for both keyboard and screen reader users. Consumers who want to set an initial focus on specific popover element should use the `initialFocus` prop.