From a5be54ff54af3c1465ac3cef81e5315175d9ee90 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Thu, 5 May 2022 10:15:42 -0700 Subject: [PATCH 1/9] [misc] move componentDidUpdate fn - move it closer to updateFocus / componentDidMount for easier context between the other two methods --- src/components/context_menu/context_menu_panel.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/context_menu/context_menu_panel.tsx b/src/components/context_menu/context_menu_panel.tsx index 8840760618f..6d1c7d47eaf 100644 --- a/src/components/context_menu/context_menu_panel.tsx +++ b/src/components/context_menu/context_menu_panel.tsx @@ -299,6 +299,10 @@ export class EuiContextMenuPanel extends Component { } }; + componentDidUpdate() { + this.updateFocus(); + } + componentDidMount() { this.updateFocus(); this.reclaimPopoverFocus(); @@ -413,10 +417,6 @@ export class EuiContextMenuPanel extends Component { } } - componentDidUpdate() { - this.updateFocus(); - } - panelRef = (node: HTMLElement | null) => { this.panel = node; From 1e3d2570540ef53fa4d22f062f131da806320671 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Thu, 5 May 2022 14:02:51 -0700 Subject: [PATCH 2/9] [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 --- .../context_menu/context_menu_panel.spec.tsx | 40 ++++++++++------ .../context_menu/context_menu_panel.tsx | 48 +++++++++++-------- 2 files changed, 54 insertions(+), 34 deletions(-) diff --git a/src/components/context_menu/context_menu_panel.spec.tsx b/src/components/context_menu/context_menu_panel.spec.tsx index 97448b5c60c..745fd1eb11b 100644 --- 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(); - 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(); + 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( + }> + + + ); + 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 index 6d1c7d47eaf..4fe3f45d170 100644 --- 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 { 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 { // 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 { } } + 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() { From f6c99b8f2938aa85f67a2db74903a597eb12902f Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Thu, 5 May 2022 14:12:37 -0700 Subject: [PATCH 3/9] [fix] Add a wait condition/state for popover focus - this makes it so EuiContextMenu doesn't call `.focus()` too early and hijack the `returnFocus` element that our focus trap dependency sets via `document.activeElement` - see https://github.com/elastic/eui/pull/5760#discussion_r844388025 + Add Cypress E2E tests for popover close focus return (w/ bonus `initialFocus` regression test) --- .../context_menu/context_menu_panel.spec.tsx | 66 +++++++++++++++++-- .../context_menu/context_menu_panel.tsx | 49 ++++++++++---- 2 files changed, 96 insertions(+), 19 deletions(-) diff --git a/src/components/context_menu/context_menu_panel.spec.tsx b/src/components/context_menu/context_menu_panel.spec.tsx index 745fd1eb11b..148a0d07f22 100644 --- a/src/components/context_menu/context_menu_panel.spec.tsx +++ b/src/components/context_menu/context_menu_panel.spec.tsx @@ -8,7 +8,7 @@ /// -import React from 'react'; +import React, { useState } from 'react'; import { EuiPopover } from '../popover'; import { EuiContextMenu } from './context_menu'; @@ -123,17 +123,69 @@ describe('EuiContextMenuPanel', () => { }); }); - describe('when inside an EuiPopover', () => { - it('reclaims focus from the parent popover panel', () => { - cy.mount( - }> - + describe('within an EuiPopover', () => { + const ContextMenuInPopover: React.FC = ({ children, ...rest }) => { + const [isOpen, setIsOpen] = useState(false); + const closePopover = () => setIsOpen(false); + const openPopover = () => setIsOpen(true); + return ( + + Toggle popover + + } + {...rest} + > + + Closes popover from context menu + + ), + ]} + /> ); - cy.wait(400); // EuiPopover's updateFocus() takes ~350ms to run + }; + + const mountAndOpenPopover = (component = ) => { + cy.realMount(component); + cy.get('[data-test-subj="popoverToggle"]').click(); + cy.wait(350); // EuiPopover's updateFocus() takes ~350ms to run + }; + + it('reclaims focus from the parent popover panel', () => { + mountAndOpenPopover(); cy.focused().should('not.have.attr', 'class', 'euiPopover__panel'); cy.focused().should('have.attr', 'class', 'euiContextMenuPanel'); }); + + it('does not hijack focus from the EuiPopover if `initialFocus` is set', () => { + mountAndOpenPopover( + + + + ); + cy.focused().should('not.have.attr', 'class', 'euiContextMenuPanel'); + cy.focused().should('have.attr', 'id', 'testInitialFocus'); + }); + + it('restores focus to the toggling button on popover close', () => { + mountAndOpenPopover(); + cy.realPress('Tab'); + cy.realPress('Enter'); + cy.focused().should('have.attr', 'data-test-subj', 'popoverToggle'); + }); + + it('restores focus to the toggling button on popover escape key', () => { + mountAndOpenPopover(); + cy.realPress('{esc}'); + cy.focused().should('have.attr', 'data-test-subj', 'popoverToggle'); + }); }); }); diff --git a/src/components/context_menu/context_menu_panel.tsx b/src/components/context_menu/context_menu_panel.tsx index 4fe3f45d170..0990792e25c 100644 --- a/src/components/context_menu/context_menu_panel.tsx +++ b/src/components/context_menu/context_menu_panel.tsx @@ -84,6 +84,7 @@ interface State { focusedItemIndex?: number; currentHeight?: number; height?: number; + waitingForInitialPopover: boolean; } export class EuiContextMenuPanel extends Component { @@ -109,6 +110,7 @@ export class EuiContextMenuPanel extends Component { ? props.initialFocusedItemIndex + 1 // Account for panel title back button : props.initialFocusedItemIndex, currentHeight: undefined, + waitingForInitialPopover: false, }; } @@ -226,6 +228,12 @@ export class EuiContextMenuPanel extends Component { return; } + // Don't take focus yet if EuiContextMenu is in a popover + // and the popover is initially opening/transitioning in + if (this.initialPopoverParent && this.state.waitingForInitialPopover) { + return; + } + // Setting focus while transitioning causes the animation to glitch, so we have to wait // until it's finished before we focus anything. if (this.props.transitionType) { @@ -265,16 +273,10 @@ 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 the popover panel gains focus, switch it to the context menu panel instead - this.initialPopoverParent?.addEventListener('focus', () => { - this.updateFocus(); - }); - } + reclaimPopoverFocus = () => { + this.setState({ waitingForInitialPopover: false }); + this.updateFocus(); + }; onTransitionComplete = () => { if (this.props.onTransitionComplete) { @@ -287,12 +289,28 @@ export class EuiContextMenuPanel extends Component { } componentDidMount() { - this.updateFocus(); - this.reclaimPopoverFocus(); + // If EuiContextMenu is used within an EuiPopover, we need to wait for EuiPopover to: + // 1. Correctly set its `returnFocus` to the toggling button, + // so focus is correctly restored to the popover toggle on close + // 2. Finish its own `updateFocus()` call 350ms after transitioning in, + // so the panel can handle its own focus without focus fighting + if (this.initialPopoverParent) { + this.initialPopoverParent.addEventListener( + 'focus', + this.reclaimPopoverFocus, + { once: true } + ); + } else { + this.updateFocus(); + } this._isMounted = true; } componentWillUnmount() { + this.initialPopoverParent?.removeEventListener( + 'focus', + this.reclaimPopoverFocus + ); this._isMounted = false; } @@ -365,6 +383,12 @@ export class EuiContextMenuPanel extends Component { return true; } + if ( + nextState.waitingForInitialPopover !== this.state.waitingForInitialPopover + ) { + return true; + } + // ** // this component should have either items or children, // if there are items we can determine via `watchedItemProps` if we should update @@ -424,6 +448,7 @@ export class EuiContextMenuPanel extends Component { if (!hasPopoverParent) return; this.initialPopoverParent = popoverParent; + this.setState({ waitingForInitialPopover: true }); } panelRef = (node: HTMLElement | null) => { From 6ef934299cf0cf1334709e67abf55311ea1eb247 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Thu, 5 May 2022 15:30:30 -0700 Subject: [PATCH 4/9] [!!!] EuiContextMenuPanels with `children` are still broken and do not correctly return focus :( - this is because of `shouldComponentUpdate` - the `items` API updates focus less than `children`, so `children` is still updating/hijacking focus after the popover focus trap returns focus to the button --- .../context_menu/context_menu_panel.spec.tsx | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/components/context_menu/context_menu_panel.spec.tsx b/src/components/context_menu/context_menu_panel.spec.tsx index 148a0d07f22..1758b91118d 100644 --- a/src/components/context_menu/context_menu_panel.spec.tsx +++ b/src/components/context_menu/context_menu_panel.spec.tsx @@ -139,15 +139,12 @@ describe('EuiContextMenuPanel', () => { } {...rest} > - - Closes popover from context menu - - ), - ]} - /> + + {children} + + ); }; From 1fb9b246b785b511ff884da6d1785db98c46e1e9 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Thu, 5 May 2022 15:48:46 -0700 Subject: [PATCH 5/9] [!!!] Remove `shouldComponentUpdate` logic & `watchedItemProps` - replace `updateFocus` with `takeInitialFocus`, and do not continue to update/hijack focus once initial focus has been set - this removes the need to restrict how often `EuiContextMenuPanel` updates (which also requires a bunch of tedious `items` diffing that we will no longer need) --- .../context_menu_panel.test.tsx.snap | 140 ------------------ .../context_menu/context_menu_panel.test.tsx | 92 ------------ .../context_menu/context_menu_panel.tsx | 102 ++----------- .../table/mobile/table_sort_mobile.tsx | 1 - 4 files changed, 16 insertions(+), 319 deletions(-) diff --git a/src/components/context_menu/__snapshots__/context_menu_panel.test.tsx.snap b/src/components/context_menu/__snapshots__/context_menu_panel.test.tsx.snap index 2feb29e822a..916b2547649 100644 --- a/src/components/context_menu/__snapshots__/context_menu_panel.test.tsx.snap +++ b/src/components/context_menu/__snapshots__/context_menu_panel.test.tsx.snap @@ -130,143 +130,3 @@ exports[`EuiContextMenuPanel props transitionDirection previous with transitionT
`; - -exports[`EuiContextMenuPanel updating items and content updates to items should not re-render if any items's watchedItemProps did not change 1`] = ` -" -
- -
- - - - - - -
-
-
-
" -`; - -exports[`EuiContextMenuPanel updating items and content updates to items should not re-render if any items's watchedItemProps did not change 2`] = ` -" -
- -
- - - - - - -
-
-
-
" -`; - -exports[`EuiContextMenuPanel updating items and content updates to items should re-render at all times when children exists 1`] = ` -" -
- -
- Hello World -
-
-
-
" -`; - -exports[`EuiContextMenuPanel updating items and content updates to items should re-render at all times when children exists 2`] = ` -" -
- -
- More Salutations -
-
-
-
" -`; - -exports[`EuiContextMenuPanel updating items and content updates to items should re-render if any items's watchedItemProps did change 1`] = ` -" -
- -
- - - - - - -
-
-
-
" -`; - -exports[`EuiContextMenuPanel updating items and content updates to items should re-render if any items's watchedItemProps did change 2`] = ` -" -
- -
- - - - - - -
-
-
-
" -`; diff --git a/src/components/context_menu/context_menu_panel.test.tsx b/src/components/context_menu/context_menu_panel.test.tsx index 724afe9298e..2b005d8fa7b 100644 --- a/src/components/context_menu/context_menu_panel.test.tsx +++ b/src/components/context_menu/context_menu_panel.test.tsx @@ -255,96 +255,4 @@ describe('EuiContextMenuPanel', () => { }); // @see Cypress context_menu_panel.spec.tsx for focus & keyboard nav testing - - describe('updating items and content', () => { - describe('updates to items', () => { - it("should not re-render if any items's watchedItemProps did not change", () => { - expect.assertions(2); // make sure the assertion in the `setProps` callback is executed - - // by not passing `watchedItemProps` no changes to items should cause a re-render - const component = mount( - - Option A - , - - Option B - , - ]} - /> - ); - - expect(component.debug()).toMatchSnapshot(); - - component.setProps( - { - items: [ - - Option A - , - - Option B - , - ], - }, - () => { - expect(component.debug()).toMatchSnapshot(); - } - ); - }); - - it("should re-render if any items's watchedItemProps did change", () => { - expect.assertions(2); // make sure the assertion in the `setProps` callback is executed - - // by referencing the `data-counter` property in `watchedItemProps` - // changes to the items should be picked up and re-rendered - const component = mount( - - Option A - , - - Option B - , - ]} - /> - ); - - expect(component.debug()).toMatchSnapshot(); - - component.setProps( - { - items: [ - - Option A - , - - Option B - , - ], - }, - () => { - expect(component.debug()).toMatchSnapshot(); - } - ); - }); - - it('should re-render at all times when children exists', () => { - expect.assertions(2); // make sure the assertion in the `setProps` callback is executed - - const component = mount( - Hello World - ); - - expect(component.debug()).toMatchSnapshot(); - - component.setProps({ children: 'More Salutations' }, () => { - expect(component.debug()).toMatchSnapshot(); - }); - }); - }); - }); }); diff --git a/src/components/context_menu/context_menu_panel.tsx b/src/components/context_menu/context_menu_panel.tsx index 0990792e25c..2bcaedd3653 100644 --- a/src/components/context_menu/context_menu_panel.tsx +++ b/src/components/context_menu/context_menu_panel.tsx @@ -51,7 +51,6 @@ export interface EuiContextMenuPanelProps { title?: ReactNode; transitionDirection?: EuiContextMenuPanelTransitionDirection; transitionType?: EuiContextMenuPanelTransitionType; - watchedItemProps?: string[]; /** * Alters the size of the items and the title */ @@ -85,6 +84,7 @@ interface State { currentHeight?: number; height?: number; waitingForInitialPopover: boolean; + tookInitialFocus: boolean; } export class EuiContextMenuPanel extends Component { @@ -111,6 +111,7 @@ export class EuiContextMenuPanel extends Component { : props.initialFocusedItemIndex, currentHeight: undefined, waitingForInitialPopover: false, + tookInitialFocus: false, }; } @@ -221,7 +222,7 @@ export class EuiContextMenuPanel extends Component { } }; - updateFocus() { + takeInitialFocus() { // Give positioning time to render before focus is applied. Otherwise page jumps. requestAnimationFrame(() => { if (!this._isMounted) { @@ -244,6 +245,11 @@ export class EuiContextMenuPanel extends Component { return; } + // Initial focus has already been handled, no need to continue and potentially hijack/focus fight + if (this.state.tookInitialFocus) { + return; + } + // If menuItems has been cleared, iterate through and set menuItems from tabbableItems if (!this.state.menuItems.length && this.panel) { const tabbableItems = tabbable(this.panel); @@ -256,26 +262,27 @@ export class EuiContextMenuPanel extends Component { // If an item is focused, focus it if (this.state.focusedItemIndex != null) { this.state.menuItems[this.state.focusedItemIndex].focus(); - return; + return this.setState({ tookInitialFocus: true }); } // Otherwise, if the back button panel title is present, focus it if (this.props.onClose) { this.setState({ focusedItemIndex: 0 }); this.state.menuItems[0].focus(); - return; + return this.setState({ tookInitialFocus: true }); } } // Focus on the panel as a last resort. if (this.panel && !this.panel.contains(document.activeElement)) { this.panel.focus(); + this.setState({ tookInitialFocus: true }); } }); } reclaimPopoverFocus = () => { this.setState({ waitingForInitialPopover: false }); - this.updateFocus(); + this.takeInitialFocus(); }; onTransitionComplete = () => { @@ -285,7 +292,9 @@ export class EuiContextMenuPanel extends Component { }; componentDidUpdate() { - this.updateFocus(); + // Focus isn't always ready to be taken on mount, so we need to call it + // on update as well just in case + this.takeInitialFocus(); } componentDidMount() { @@ -301,7 +310,7 @@ export class EuiContextMenuPanel extends Component { { once: true } ); } else { - this.updateFocus(); + this.takeInitialFocus(); } this._isMounted = true; } @@ -334,84 +343,6 @@ export class EuiContextMenuPanel extends Component { return null; } - getWatchedPropsForItems(items: ReactElement[]) { - // This lets us compare prevProps and nextProps among items so we can re-render if our items - // have changed. - const { watchedItemProps } = this.props; - - // Create fingerprint of all item's watched properties - if (items.length && watchedItemProps && watchedItemProps.length) { - return JSON.stringify( - items.map((item) => { - // Create object of item properties and values - const props: any = { - key: item.key, - }; - watchedItemProps.forEach((prop: string) => { - props[prop] = item.props[prop]; - }); - return props; - }) - ); - } - - return null; - } - - didItemsChange(prevItems: ReactElement[], nextItems: ReactElement[]) { - // If the count of items has changed then update - if (prevItems.length !== nextItems.length) { - return true; - } - - // Check if any watched item properties changed by quick string comparison - if ( - this.getWatchedPropsForItems(nextItems) !== - this.getWatchedPropsForItems(prevItems) - ) { - return true; - } - } - - shouldComponentUpdate(nextProps: Props, nextState: State) { - // Prevent calling `this.updateFocus()` below if we don't have to. - if (nextProps.transitionType !== this.props.transitionType) { - return true; - } - - if (nextState.focusedItemIndex !== this.state.focusedItemIndex) { - return true; - } - - if ( - nextState.waitingForInitialPopover !== this.state.waitingForInitialPopover - ) { - return true; - } - - // ** - // this component should have either items or children, - // if there are items we can determine via `watchedItemProps` if we should update - // if there are children we can't know if they have changed so return true - // ** - - if ( - (this.props.items && this.props.items.length > 0) || - (nextProps.items && nextProps.items.length > 0) - ) { - if (this.didItemsChange(this.props.items!, nextProps.items!)) { - return true; - } - } - - // it's not possible (in any good way) to know if `children` has changed, assume they might have - if (this.props.children != null) { - return true; - } - - return false; - } - updateHeight() { const currentHeight = this.panel ? this.panel.clientHeight : 0; @@ -470,7 +401,6 @@ export class EuiContextMenuPanel extends Component { onTransitionComplete, onUseKeyboardToNavigate, items, - watchedItemProps, initialFocusedItemIndex, showNextPanel, showPreviousPanel, diff --git a/src/components/table/mobile/table_sort_mobile.tsx b/src/components/table/mobile/table_sort_mobile.tsx index 8d52e177455..39fb5eb0f50 100644 --- a/src/components/table/mobile/table_sort_mobile.tsx +++ b/src/components/table/mobile/table_sort_mobile.tsx @@ -97,7 +97,6 @@ export class EuiTableSortMobile extends Component< }) : undefined } - watchedItemProps={['isSorted', 'isSortAscending']} /> ); From 40dc5c4df8cd6db93e35869ec6a8ded87256cd96 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Thu, 5 May 2022 16:01:51 -0700 Subject: [PATCH 6/9] [!!!] Move `tabbable` iteration out of focus logic and into its own method - it shouldn't be tied to the focus call anymore since the focus call no longer occurs after update, and makes more sense as a separate call + updates to logic: - do not run `tabbable` on `children` API since it won't even use the navigation - return early - use `this.backButton` instead for back button focusing, since `children` will no longer have `menuItems` - Check for a valid focusedItem - it's possible for consumers to either pass in bad indices or for `items` to update and the index to no longer exist - Add E2E tests confirming changes & new logic work as expected --- .../context_menu/context_menu_panel.spec.tsx | 53 +++++++++++++++---- .../context_menu/context_menu_panel.tsx | 47 ++++++++++------ 2 files changed, 75 insertions(+), 25 deletions(-) diff --git a/src/components/context_menu/context_menu_panel.spec.tsx b/src/components/context_menu/context_menu_panel.spec.tsx index 1758b91118d..1434c86d8fd 100644 --- a/src/components/context_menu/context_menu_panel.spec.tsx +++ b/src/components/context_menu/context_menu_panel.spec.tsx @@ -42,15 +42,6 @@ describe('EuiContextMenuPanel', () => { cy.focused().should('have.attr', 'class', 'euiContextMenuPanel'); }); - it('sets initial focus from `initialFocusedItemIndex`', () => { - cy.mount( - - {children} - - ); - cy.focused().should('have.attr', 'data-test-subj', 'itemC'); - }); - describe('with `children`', () => { it('ignores arrow key navigation, which only toggles for `items`', () => { cy.mount({children}); @@ -60,6 +51,20 @@ describe('EuiContextMenuPanel', () => { }); describe('with `items`', () => { + it('sets initial focus from `initialFocusedItemIndex`', () => { + cy.mount( + + ); + cy.focused().should('have.attr', 'data-test-subj', 'itemC'); + }); + + it('falls back to the panel if given an invalid `focusedItemIndex`', () => { + cy.mount( + + ); + cy.focused().should('have.attr', 'class', 'euiContextMenuPanel'); + }); + it('focuses and registers any tabbable child as navigable menu items', () => { cy.mount( { cy.realPress('{downarrow}'); cy.focused().should('have.attr', 'data-test-subj', 'itemA'); }); + + it('correctly re-finds navigable menu items if `items` changes', () => { + const DynanicItemsTest = () => { + const [dynamicItems, setDynamicItems] = useState([ + items[0], + items[1], + ]); + const appendItems = () => setDynamicItems(items); + return ( + <> + + + + ); + }; + cy.mount(); + cy.realPress('{downarrow}'); + cy.focused().should('have.attr', 'data-test-subj', 'itemA'); + cy.realPress('{downarrow}'); + cy.focused().should('have.attr', 'data-test-subj', 'itemB'); + cy.realPress('{downarrow}'); + cy.focused().should('have.attr', 'data-test-subj', 'itemA'); + + cy.get('[data-test-subj="appendItems"]').click(); + cy.get('[data-test-subj="itemA"]').click(); + cy.realPress('{uparrow}'); + cy.focused().should('have.attr', 'data-test-subj', 'itemC'); + }); }); describe('with `panels`', () => { diff --git a/src/components/context_menu/context_menu_panel.tsx b/src/components/context_menu/context_menu_panel.tsx index 2bcaedd3653..7fdfbdc5067 100644 --- a/src/components/context_menu/context_menu_panel.tsx +++ b/src/components/context_menu/context_menu_panel.tsx @@ -115,6 +115,19 @@ export class EuiContextMenuPanel extends Component { }; } + // Find all tabbable menu items on both panel init and + // whenever `menuItems` resets when `props.items` changes + findMenuItems = () => { + if (!this.panel) return; + if (!this.props.items?.length) return; // We only need menu items/arrow key navigation for the `items` API + if (this.state.menuItems.length) return; // If we already have menu items, no need to continue + + const tabbableItems = tabbable(this.panel); + if (tabbableItems.length) { + this.setState({ menuItems: tabbableItems }); + } + }; + incrementFocusedItemIndex = (amount: number) => { let nextFocusedItemIndex; @@ -250,26 +263,24 @@ export class EuiContextMenuPanel extends Component { return; } - // If menuItems has been cleared, iterate through and set menuItems from tabbableItems - if (!this.state.menuItems.length && this.panel) { - const tabbableItems = tabbable(this.panel); - if (tabbableItems.length) { - this.setState({ menuItems: tabbableItems }); + // If an item should be focused, focus it (if it exists) + if (this.state.focusedItemIndex != null && this.state.menuItems.length) { + const focusedItem = this.state.menuItems[this.state.focusedItemIndex]; + if (focusedItem) { + focusedItem.focus(); + return this.setState({ tookInitialFocus: true }); } } - if (this.state.menuItems.length) { - // If an item is focused, focus it - if (this.state.focusedItemIndex != null) { - this.state.menuItems[this.state.focusedItemIndex].focus(); - return this.setState({ tookInitialFocus: true }); - } - // Otherwise, if the back button panel title is present, focus it - if (this.props.onClose) { + // Otherwise, if the back button panel title is present, focus it + if (this.backButton) { + // Focus the back button for both `items` and `children` APIs + this.backButton.focus(); + // If `items`, ensure our focused item index is correct + if (this.state.menuItems.length) { this.setState({ focusedItemIndex: 0 }); - this.state.menuItems[0].focus(); - return this.setState({ tookInitialFocus: true }); } + return this.setState({ tookInitialFocus: true }); } // Focus on the panel as a last resort. @@ -291,7 +302,10 @@ export class EuiContextMenuPanel extends Component { } }; - componentDidUpdate() { + componentDidUpdate(_: Props, prevState: State) { + if (prevState.menuItems !== this.state.menuItems) { + this.findMenuItems(); + } // Focus isn't always ready to be taken on mount, so we need to call it // on update as well just in case this.takeInitialFocus(); @@ -387,6 +401,7 @@ export class EuiContextMenuPanel extends Component { this.updateHeight(); this.getInitialPopoverParent(); + this.findMenuItems(); }; render() { From 4b397b02eab0057fbdc6e18b0160ce721e61949d Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Thu, 5 May 2022 16:06:27 -0700 Subject: [PATCH 7/9] [!!!] Restore up/down key navigation behavior - rename `incrementFocusedItemIndex` to `focusMenuItem` and change args to be a bit more human readable - instead of having the previous `updateFocus` handle up/down nav, we can simply call `.focus()` from within this method, and arrow navigation works as before - note `?.focus();` - this is important to keep as users can start mashing up/down before `tabbable` is done running and there are any menu items to focus - no specific E2E tests for this, tests should simply not be failing --- .../context_menu/context_menu_panel.tsx | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/components/context_menu/context_menu_panel.tsx b/src/components/context_menu/context_menu_panel.tsx index 7fdfbdc5067..d2fa06749f8 100644 --- a/src/components/context_menu/context_menu_panel.tsx +++ b/src/components/context_menu/context_menu_panel.tsx @@ -128,15 +128,17 @@ export class EuiContextMenuPanel extends Component { } }; - incrementFocusedItemIndex = (amount: number) => { + focusMenuItem = (direction: 'up' | 'down') => { + const indexOffset = direction === 'up' ? -1 : 1; let nextFocusedItemIndex; if (this.state.focusedItemIndex === undefined) { // If this is the beginning of the user's keyboard navigation of the menu, then we'll focus // either the first or last item. - nextFocusedItemIndex = amount < 0 ? this.state.menuItems.length - 1 : 0; + nextFocusedItemIndex = + direction === 'up' ? this.state.menuItems.length - 1 : 0; } else { - nextFocusedItemIndex = this.state.focusedItemIndex + amount; + nextFocusedItemIndex = this.state.focusedItemIndex + indexOffset; if (nextFocusedItemIndex < 0) { nextFocusedItemIndex = this.state.menuItems.length - 1; @@ -145,9 +147,8 @@ export class EuiContextMenuPanel extends Component { } } - this.setState({ - focusedItemIndex: nextFocusedItemIndex, - }); + this.setState({ focusedItemIndex: nextFocusedItemIndex }); + this.state.menuItems[nextFocusedItemIndex]?.focus(); }; onKeyDown = (event: React.KeyboardEvent) => { @@ -198,7 +199,7 @@ export class EuiContextMenuPanel extends Component { case cascadingMenuKeys.ARROW_UP: event.preventDefault(); - this.incrementFocusedItemIndex(-1); + this.focusMenuItem('up'); if (this.props.onUseKeyboardToNavigate) { this.props.onUseKeyboardToNavigate(); @@ -207,7 +208,7 @@ export class EuiContextMenuPanel extends Component { case cascadingMenuKeys.ARROW_DOWN: event.preventDefault(); - this.incrementFocusedItemIndex(1); + this.focusMenuItem('down'); if (this.props.onUseKeyboardToNavigate) { this.props.onUseKeyboardToNavigate(); From 1721abfb409667b9d92974c78732327c91861b6a Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Fri, 6 May 2022 09:07:48 -0700 Subject: [PATCH 8/9] changelog --- upcoming_changelogs/5880.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 upcoming_changelogs/5880.md diff --git a/upcoming_changelogs/5880.md b/upcoming_changelogs/5880.md new file mode 100644 index 00000000000..98a0087364d --- /dev/null +++ b/upcoming_changelogs/5880.md @@ -0,0 +1,7 @@ +**Bug fixes** + +- Fixed `EuiContextMenuPanel` (when used within an `EuiPopover`) to correctly return focus to its popover toggle in all scenarios, not just keyboard Escape press + +**Breaking changes** + +- Removed `watchedItemProps` from `EuiContextMenuPanel`, which now updates like a standard component and no longer needs this logic From f8e7a5cf16debc2ebd3c253a9eee44c650020354 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Mon, 9 May 2022 14:53:49 -0700 Subject: [PATCH 9/9] [PR feedback] Prefer not to hook into className for popover panel --- src/components/context_menu/context_menu_panel.tsx | 4 +--- .../__snapshots__/column_selector.test.tsx.snap | 3 +++ .../__snapshots__/column_sorting.test.tsx.snap | 3 +++ .../__snapshots__/super_select.test.tsx.snap | 4 ++++ .../popover/__snapshots__/popover.test.tsx.snap | 12 ++++++++++++ src/components/popover/popover.tsx | 1 + .../tour/__snapshots__/tour_step.test.tsx.snap | 5 +++++ 7 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/components/context_menu/context_menu_panel.tsx b/src/components/context_menu/context_menu_panel.tsx index d2fa06749f8..8691238fb4d 100644 --- a/src/components/context_menu/context_menu_panel.tsx +++ b/src/components/context_menu/context_menu_panel.tsx @@ -388,9 +388,7 @@ export class EuiContextMenuPanel extends Component { : (parent?.parentNode as HTMLElement); if (!popoverParent) return; - const hasPopoverParent = popoverParent.classList.contains( - 'euiPopover__panel' - ); + const hasPopoverParent = !!popoverParent.dataset.popoverPanel; if (!hasPopoverParent) return; this.initialPopoverParent = popoverParent; diff --git a/src/components/datagrid/controls/__snapshots__/column_selector.test.tsx.snap b/src/components/datagrid/controls/__snapshots__/column_selector.test.tsx.snap index beb97537b57..7bee1dad864 100644 --- a/src/components/datagrid/controls/__snapshots__/column_selector.test.tsx.snap +++ b/src/components/datagrid/controls/__snapshots__/column_selector.test.tsx.snap @@ -117,6 +117,7 @@ exports[`useDataGridColumnSelector columnSelector renders a toolbar button/popov aria-modal="true" class="euiPanel euiPanel--paddingSmall euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--bottom euiDataGrid__controlPopoverWithDragDrop" data-autofocus="true" + data-popover-panel="true" role="dialog" style="top: 16px; left: -22px; will-change: transform, opacity; z-index: 2000;" tabindex="0" @@ -473,6 +474,7 @@ exports[`useDataGridColumnSelector columnSelector renders a toolbar button/popov aria-live="off" aria-modal="true" className="euiPopover__panel euiPopover__panel--bottom euiDataGrid__controlPopoverWithDragDrop" + data-popover-panel={true} hasShadow={false} paddingSize="s" panelRef={[Function]} @@ -492,6 +494,7 @@ exports[`useDataGridColumnSelector columnSelector renders a toolbar button/popov aria-live="off" aria-modal="true" className="euiPanel euiPanel--paddingSmall euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--bottom euiDataGrid__controlPopoverWithDragDrop" + data-popover-panel={true} role="dialog" style={ Object { diff --git a/src/components/datagrid/controls/__snapshots__/column_sorting.test.tsx.snap b/src/components/datagrid/controls/__snapshots__/column_sorting.test.tsx.snap index 703b12fd7c0..b5cf61f5c8f 100644 --- a/src/components/datagrid/controls/__snapshots__/column_sorting.test.tsx.snap +++ b/src/components/datagrid/controls/__snapshots__/column_sorting.test.tsx.snap @@ -107,6 +107,7 @@ exports[`useDataGridColumnSorting columnSorting renders a toolbar button/popover aria-modal="true" class="euiPanel euiPanel--paddingSmall euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--bottom euiDataGrid__controlPopoverWithDragDrop" data-autofocus="true" + data-popover-panel="true" role="dialog" style="top: 16px; left: -22px; will-change: transform, opacity; z-index: 2000;" tabindex="0" @@ -484,6 +485,7 @@ exports[`useDataGridColumnSorting columnSorting renders a toolbar button/popover aria-live="off" aria-modal="true" className="euiPopover__panel euiPopover__panel--bottom euiDataGrid__controlPopoverWithDragDrop" + data-popover-panel={true} hasShadow={false} paddingSize="s" panelRef={[Function]} @@ -503,6 +505,7 @@ exports[`useDataGridColumnSorting columnSorting renders a toolbar button/popover aria-live="off" aria-modal="true" className="euiPanel euiPanel--paddingSmall euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--bottom euiDataGrid__controlPopoverWithDragDrop" + data-popover-panel={true} role="dialog" style={ Object { diff --git a/src/components/form/super_select/__snapshots__/super_select.test.tsx.snap b/src/components/form/super_select/__snapshots__/super_select.test.tsx.snap index f48fb0c5ef3..82484dca42a 100644 --- a/src/components/form/super_select/__snapshots__/super_select.test.tsx.snap +++ b/src/components/form/super_select/__snapshots__/super_select.test.tsx.snap @@ -166,6 +166,7 @@ exports[`EuiSuperSelect props custom display is propagated to dropdown 1`] = ` aria-live="assertive" aria-modal="true" class="euiPanel euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--bottom euiPopover__panel-noArrow euiPopover__panel-isAttached" + data-popover-panel="true" role="dialog" style="top: 8px; left: 0px; will-change: transform, opacity; z-index: 2000;" > @@ -441,6 +442,7 @@ exports[`EuiSuperSelect props more props are propogated to each option 1`] = ` aria-live="assertive" aria-modal="true" class="euiPanel euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--bottom euiPopover__panel-noArrow euiPopover__panel-isAttached" + data-popover-panel="true" role="dialog" style="top: 8px; left: 0px; will-change: transform, opacity; z-index: 2000;" > @@ -605,6 +607,7 @@ exports[`EuiSuperSelect props options are rendered when select is open 1`] = ` aria-live="assertive" aria-modal="true" class="euiPanel euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--bottom euiPopover__panel-noArrow euiPopover__panel-isAttached" + data-popover-panel="true" role="dialog" style="top: 8px; left: 0px; will-change: transform, opacity; z-index: 2000;" > @@ -768,6 +771,7 @@ exports[`EuiSuperSelect props renders popoverProps on the underlying EuiPopover aria-live="assertive" aria-modal="true" class="euiPanel euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--bottom euiPopover__panel-noArrow euiPopover__panel-isAttached goes-on-popover-panel" + data-popover-panel="true" role="dialog" style="top: 8px; left: 0px; will-change: transform, opacity; z-index: 2000;" > diff --git a/src/components/popover/__snapshots__/popover.test.tsx.snap b/src/components/popover/__snapshots__/popover.test.tsx.snap index 81a1cee5ae4..2561fc0df45 100644 --- a/src/components/popover/__snapshots__/popover.test.tsx.snap +++ b/src/components/popover/__snapshots__/popover.test.tsx.snap @@ -109,6 +109,7 @@ exports[`EuiPopover props arrowChildren is rendered 1`] = ` aria-modal="true" class="euiPanel euiPanel--paddingMedium euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--bottom euiPopover__panel-isOpen" data-autofocus="true" + data-popover-panel="true" role="dialog" style="top: 16px; left: -22px; will-change: transform, opacity; z-index: 2000;" tabindex="0" @@ -167,6 +168,7 @@ exports[`EuiPopover props buffer 1`] = ` aria-modal="true" class="euiPanel euiPanel--paddingMedium euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--bottom euiPopover__panel-isOpen" data-autofocus="true" + data-popover-panel="true" role="dialog" style="top: 16px; left: -22px; will-change: transform, opacity; z-index: 2000;" tabindex="0" @@ -223,6 +225,7 @@ exports[`EuiPopover props buffer for all sides 1`] = ` aria-modal="true" class="euiPanel euiPanel--paddingMedium euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--bottom euiPopover__panel-isOpen" data-autofocus="true" + data-popover-panel="true" role="dialog" style="top: 16px; left: -22px; will-change: transform, opacity; z-index: 2000;" tabindex="0" @@ -292,6 +295,7 @@ exports[`EuiPopover props focusTrapProps is rendered 1`] = ` aria-modal="true" class="euiPanel euiPanel--paddingMedium euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--bottom euiPopover__panel-isOpen" data-autofocus="true" + data-popover-panel="true" role="dialog" style="top: 16px; left: -22px; will-change: transform, opacity; z-index: 2000;" tabindex="0" @@ -361,6 +365,7 @@ exports[`EuiPopover props isOpen renders true 1`] = ` aria-modal="true" class="euiPanel euiPanel--paddingMedium euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--bottom euiPopover__panel-isOpen" data-autofocus="true" + data-popover-panel="true" role="dialog" style="top: 16px; left: -22px; will-change: transform, opacity; z-index: 2000;" tabindex="0" @@ -418,6 +423,7 @@ exports[`EuiPopover props offset with arrow 1`] = ` aria-modal="true" class="euiPanel euiPanel--paddingMedium euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--bottom euiPopover__panel-isOpen" data-autofocus="true" + data-popover-panel="true" role="dialog" style="top: 26px; left: -22px; will-change: transform, opacity; z-index: 2000;" tabindex="0" @@ -475,6 +481,7 @@ exports[`EuiPopover props offset without arrow 1`] = ` aria-modal="true" class="euiPanel euiPanel--paddingMedium euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--bottom euiPopover__panel-isOpen euiPopover__panel-noArrow" data-autofocus="true" + data-popover-panel="true" role="dialog" style="top: 18px; left: -22px; will-change: transform, opacity; z-index: 2000;" tabindex="0" @@ -530,6 +537,7 @@ exports[`EuiPopover props ownFocus defaults to true 1`] = ` aria-modal="true" class="euiPanel euiPanel--paddingMedium euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--bottom euiPopover__panel-isOpen" data-autofocus="true" + data-popover-panel="true" role="dialog" style="top: 16px; left: -22px; will-change: transform, opacity; z-index: 2000;" tabindex="0" @@ -584,6 +592,7 @@ exports[`EuiPopover props ownFocus renders false 1`] = ` aria-live="assertive" aria-modal="true" class="euiPanel euiPanel--paddingMedium euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--bottom euiPopover__panel-isOpen" + data-popover-panel="true" role="dialog" style="top: 16px; left: -22px; will-change: transform, opacity; z-index: 2000;" > @@ -633,6 +642,7 @@ exports[`EuiPopover props panelClassName is rendered 1`] = ` aria-modal="true" class="euiPanel euiPanel--paddingMedium euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--bottom euiPopover__panel-isOpen test" data-autofocus="true" + data-popover-panel="true" role="dialog" style="top: 16px; left: -22px; will-change: transform, opacity; z-index: 2000;" tabindex="0" @@ -689,6 +699,7 @@ exports[`EuiPopover props panelPaddingSize is rendered 1`] = ` aria-modal="true" class="euiPanel euiPanel--paddingSmall euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--bottom euiPopover__panel-isOpen" data-autofocus="true" + data-popover-panel="true" role="dialog" style="top: 16px; left: -22px; will-change: transform, opacity; z-index: 2000;" tabindex="0" @@ -745,6 +756,7 @@ exports[`EuiPopover props panelProps is rendered 1`] = ` aria-modal="true" class="euiPanel euiPanel--paddingMedium euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--bottom euiPopover__panel-isOpen testClass1 testClass2" data-autofocus="true" + data-popover-panel="true" data-test-subj="test subject string" role="dialog" style="top: 16px; left: -22px; will-change: transform, opacity; z-index: 2000;" diff --git a/src/components/popover/popover.tsx b/src/components/popover/popover.tsx index 36a80306406..850e2902c64 100644 --- a/src/components/popover/popover.tsx +++ b/src/components/popover/popover.tsx @@ -801,6 +801,7 @@ export class EuiPopover extends Component { > @@ -145,6 +146,7 @@ exports[`EuiTourStep can have subtitle 1`] = ` aria-live="assertive" aria-modal="true" class="euiPanel euiPanel--paddingMedium euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--left euiPopover__panel-isOpen euiTour testClass1 testClass2" + data-popover-panel="true" role="dialog" style="top: -22px; left: -26px; will-change: transform, opacity; max-width: 600px; min-width: 300px; z-index: 2000;" > @@ -247,6 +249,7 @@ exports[`EuiTourStep can override the footer action 1`] = ` aria-live="assertive" aria-modal="true" class="euiPanel euiPanel--paddingMedium euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--left euiPopover__panel-isOpen euiTour testClass1 testClass2" + data-popover-panel="true" role="dialog" style="top: -22px; left: -26px; will-change: transform, opacity; max-width: 600px; min-width: 300px; z-index: 2000;" > @@ -333,6 +336,7 @@ exports[`EuiTourStep can turn off the beacon 1`] = ` aria-live="assertive" aria-modal="true" class="euiPanel euiPanel--paddingMedium euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--left euiPopover__panel-isOpen euiTour testClass1 testClass2" + data-popover-panel="true" role="dialog" style="top: -22px; left: -16px; will-change: transform, opacity; max-width: 600px; min-width: 300px; z-index: 2000;" > @@ -425,6 +429,7 @@ exports[`EuiTourStep is rendered 1`] = ` aria-live="assertive" aria-modal="true" class="euiPanel euiPanel--paddingMedium euiPanel--borderRadiusMedium euiPanel--plain euiPanel--noShadow euiPopover__panel euiPopover__panel--left euiPopover__panel-isOpen euiTour testClass1 testClass2" + data-popover-panel="true" role="dialog" style="top: -22px; left: -26px; will-change: transform, opacity; max-width: 600px; min-width: 300px; z-index: 2000;" >