diff --git a/packages/eui/changelogs/upcoming/8325.md b/packages/eui/changelogs/upcoming/8325.md new file mode 100644 index 000000000000..4073f1f33704 --- /dev/null +++ b/packages/eui/changelogs/upcoming/8325.md @@ -0,0 +1,3 @@ +**Bug fixes** + +- Fixed a bug in `EuiHeader` where the navigation of `EuiCollapsibleNavBeta` would render below the `EuiFlyout`'s overlay diff --git a/packages/eui/src/components/collapsible_nav_beta/collapsible_nav_beta.stories.tsx b/packages/eui/src/components/collapsible_nav_beta/collapsible_nav_beta.stories.tsx index 0f6c27ac4da2..490ac67fe30f 100644 --- a/packages/eui/src/components/collapsible_nav_beta/collapsible_nav_beta.stories.tsx +++ b/packages/eui/src/components/collapsible_nav_beta/collapsible_nav_beta.stories.tsx @@ -278,7 +278,8 @@ const MockConsumerFlyout: FunctionComponent = () => { setFlyoutOpen(false)}> This flyout's mask should overlay / sit on top of the collapsible - nav, on both desktop and mobile + nav only on mobile. On desktop, the collapsible nav should always be + visible and reachable. )} @@ -289,18 +290,38 @@ const MockConsumerFlyout: FunctionComponent = () => { export const FlyoutOverlay: Story = { render: (_) => { return ( - - - - Click the "Toggle flyout" button in the top right hand corner - - - - - - - - + <> + + + + + + + + + + + + + + + + + Click the "Toggle flyout" button in the top right hand corner, and + tab through the page + + + ); }, parameters: hideAllStorybookControls, diff --git a/packages/eui/src/components/flyout/flyout.spec.tsx b/packages/eui/src/components/flyout/flyout.spec.tsx index c3b2b8419650..7a67e773160f 100644 --- a/packages/eui/src/components/flyout/flyout.spec.tsx +++ b/packages/eui/src/components/flyout/flyout.spec.tsx @@ -13,8 +13,15 @@ import React, { useState } from 'react'; import { EuiGlobalToastList } from '../toast'; -import { EuiHeader } from '../header'; +import { + EuiHeader, + EuiHeaderSection, + EuiHeaderSectionItemButton, +} from '../header'; +import { EuiCollapsibleNavBeta } from '../collapsible_nav_beta'; import { EuiFlyout } from './flyout'; +import { EuiCollapsibleNav, EuiCollapsibleNavGroup } from '../collapsible_nav'; +import { EuiIcon } from '../icon'; const childrenDefault = ( <> @@ -171,18 +178,72 @@ describe('EuiFlyout', () => { }); describe('EuiHeader shards', () => { - const FlyoutWithHeader = ({ children = childrenDefault, ...rest }) => { + const FlyoutWithHeader = ({ + children = childrenDefault, + collapsibleNavVariant = undefined, + ...rest + }: { + children?: React.ReactNode; + collapsibleNavVariant?: 'beta' | 'default'; + }) => { const [isOpen, setIsOpen] = useState(false); + const [navIsOpen, setNavIsOpen] = useState(false); + + // We need to toggle it in order to properly test + // the expected focus behavior + const collapsibleNav = ( + setNavIsOpen(!navIsOpen)} + > + + ); + // No need to toggle… + const collapsibleNavBeta = ( + + + + + + ); return ( <> - + {collapsibleNavVariant && ( + + {collapsibleNavVariant === 'beta' + ? collapsibleNavBeta + : collapsibleNav} + + )} + + + {isOpen ? ( { ); }); + it('includes EuiCollapsibleNavBeta items in tab rotation, inside EuiHeaders shards', () => { + cy.viewport(800, 600); + cy.mount( + + ); + cy.get('[data-test-subj="toggleFlyoutFromHeader"]').click(); + cy.repeatRealPress('Tab', 4); + cy.focused().should('have.text', 'Item B'); + cy.repeatRealPress('Tab', 2); + cy.focused().should( + 'have.attr', + 'data-test-subj', + 'toggleFlyoutFromHeader' + ); + }); + + /** + * @todo this fails with React 18, but passes with previous versions + * Focus behaviour is different in React 18, depending on whether 1 or more flyouts are open + * + * @see https://github.com/elastic/eui/pull/8325#discussion_r1973266414 + * @see https://github.com/elastic/eui/issues/8376 + */ + it.skip('includes EuiCollapsibleNav items in tab rotation, inside EuiHeaders shards', () => { + cy.viewport(800, 600); + cy.mount( + + ); + // Open the collapsible nav + // should be accessible by tabbing + cy.get('[data-test-subj="toggleNavButton"]').click(); + cy.repeatRealPress('Tab', 2); + cy.focused().should('have.text', 'Link A'); + cy.repeatRealPress('Tab', 1); + cy.focused().should('have.attr', 'data-test-subj', 'toggleNavButton'); + // Open the flyout, without closing the collapsible nav; + // nav should not be accessible by tabbing + // though it's visible, behind the overlay + cy.get('[data-test-subj="toggleFlyoutFromHeader"]').click(); + cy.realPress('Tab'); + cy.focused().should('not.have.text', 'Link A'); + cy.realPress('Tab'); + cy.focused().should('not.have.text', 'Link A'); + cy.realPress('Tab'); + cy.focused().should( + 'have.attr', + 'data-test-subj', + 'toggleFlyoutFromHeader' + ); + }); + it('does not shard fixed headers if `includeFixedHeadersInFocusTrap` is set to false', () => { cy.mount(); cy.get('[data-test-subj="toggleFlyoutFromHeader"]').click(); diff --git a/packages/eui/src/components/header/header.spec.tsx b/packages/eui/src/components/header/header.spec.tsx new file mode 100644 index 000000000000..3a52af0d6883 --- /dev/null +++ b/packages/eui/src/components/header/header.spec.tsx @@ -0,0 +1,39 @@ +/* + * 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 from 'react'; + +import { EuiHeader } from './header'; +import { EuiFlyout } from '../flyout'; + +describe('EuiHeader', () => { + describe('fixed position', () => { + it('is above EuiFlyout', () => { + cy.mount( + <> + + + + ); + + cy.get('[data-test-subj="flyout"]').then(($flyout) => { + const styles = window.getComputedStyle($flyout[0]); + const flyoutZIndex = parseInt(styles.getPropertyValue('z-index'), 10); + + cy.get('[data-test-subj="header"]') + .should('have.css', 'z-index') + .then((value) => parseInt(value, 10)) + .and('be.above', flyoutZIndex); + }); + }); + }); +}); diff --git a/packages/eui/src/components/header/header.styles.ts b/packages/eui/src/components/header/header.styles.ts index e1e7fa9de368..f15b5ae45e1e 100644 --- a/packages/eui/src/components/header/header.styles.ts +++ b/packages/eui/src/components/header/header.styles.ts @@ -8,7 +8,7 @@ import { css } from '@emotion/react'; -import { euiShadowSmall } from '../../themes/amsterdam/global_styling/mixins'; +import { euiShadowXSmall } from '../../themes/amsterdam/global_styling/mixins'; import { logicalCSS } from '../../global_styling'; import { UseEuiTheme, @@ -38,7 +38,7 @@ export const euiHeaderStyles = (euiThemeContext: UseEuiTheme) => { ${logicalCSS('height', height)} ${logicalCSS('padding-horizontal', padding)} ${logicalCSS('border-bottom', euiTheme.border.thin)} - ${euiShadowSmall(euiThemeContext)} + ${euiShadowXSmall(euiThemeContext)} `, // Position static: css` @@ -47,7 +47,8 @@ export const euiHeaderStyles = (euiThemeContext: UseEuiTheme) => { position: relative; `, fixed: css` - z-index: ${euiTheme.levels.header}; + /* Ensure it's above EuiFlyout */ + z-index: ${Number(euiTheme.levels.header!) + 1}; position: fixed; ${logicalCSS('top', 0)} ${logicalCSS('horizontal', 0)}