Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/eui/changelogs/upcoming/8325.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
**Bug fixes**

- Fixed a bug in `EuiHeader` where the navigation of `EuiCollapsibleNavBeta` would render below the `EuiFlyout`'s overlay
Comment thread
mgadewoll marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,8 @@ const MockConsumerFlyout: FunctionComponent = () => {
<EuiFlyout onClose={() => setFlyoutOpen(false)}>
<EuiCollapsibleNavBeta.Body>
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.
</EuiCollapsibleNavBeta.Body>
</EuiFlyout>
)}
Expand All @@ -289,18 +290,38 @@ const MockConsumerFlyout: FunctionComponent = () => {
export const FlyoutOverlay: Story = {
render: (_) => {
return (
<EuiHeader position="fixed">
<EuiHeaderSection>
<EuiCollapsibleNavBeta>
Click the "Toggle flyout" button in the top right hand corner
</EuiCollapsibleNavBeta>
</EuiHeaderSection>
<EuiHeaderSection>
<EuiHeaderSectionItem>
<MockConsumerFlyout />
</EuiHeaderSectionItem>
</EuiHeaderSection>
</EuiHeader>
<>
<EuiHeader position="fixed">
<EuiHeaderSection>
<EuiCollapsibleNavBeta>
<EuiCollapsibleNavBeta.Body>
<EuiCollapsibleNavBeta.Item
title="Curabitur ornare"
icon="keyboard"
isCollapsible={false}
items={[
{ title: 'Quisque', href: '#' },
{ title: 'Suspendisse euismod', href: '#' },
{ title: 'Aenean nec', href: '#' },
{ title: 'Proin porta', href: '#' },
]}
/>
</EuiCollapsibleNavBeta.Body>
</EuiCollapsibleNavBeta>
</EuiHeaderSection>
<EuiHeaderSection>
<EuiHeaderSectionItem>
<MockConsumerFlyout />
</EuiHeaderSectionItem>
</EuiHeaderSection>
</EuiHeader>
<EuiPageTemplate>
<EuiPageTemplate.Section>
Click the "Toggle flyout" button in the top right hand corner, and
tab through the page
</EuiPageTemplate.Section>
</EuiPageTemplate>
</>
);
},
parameters: hideAllStorybookControls,
Expand Down
128 changes: 120 additions & 8 deletions packages/eui/src/components/flyout/flyout.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = (
<>
Expand Down Expand Up @@ -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 = (
<EuiCollapsibleNav
isOpen={navIsOpen}
button={
<EuiHeaderSectionItemButton
data-test-subj="toggleNavButton"
onClick={() => setNavIsOpen(!navIsOpen)}
>
<EuiIcon type={'menu'} size="m" aria-hidden="true" />
</EuiHeaderSectionItemButton>
}
onClose={() => setNavIsOpen(false)}
>
<EuiCollapsibleNavGroup>
<a href="#">Link A</a>
</EuiCollapsibleNavGroup>
</EuiCollapsibleNav>
);
// No need to toggle…
const collapsibleNavBeta = (
<EuiCollapsibleNavBeta>
<EuiCollapsibleNavBeta.Body>
<EuiCollapsibleNavBeta.Item
title="Items"
isCollapsible={false}
items={[
{ title: 'Item A', href: '#' },
{ title: 'Item B', href: '#' },
{ title: 'Item C', href: '#' },
]}
/>
</EuiCollapsibleNavBeta.Body>
</EuiCollapsibleNavBeta>
);

return (
<>
<EuiHeader position="fixed">
<button
data-test-subj="toggleFlyoutFromHeader"
onClick={() => setIsOpen(!isOpen)}
>
Toggle flyout
</button>
{collapsibleNavVariant && (
<EuiHeaderSection>
{collapsibleNavVariant === 'beta'
? collapsibleNavBeta
: collapsibleNav}
</EuiHeaderSection>
)}
<EuiHeaderSection>
<button
data-test-subj="toggleFlyoutFromHeader"
onClick={() => setIsOpen(!isOpen)}
>
Toggle flyout
</button>
</EuiHeaderSection>
</EuiHeader>
{isOpen ? (
<EuiFlyout
Expand Down Expand Up @@ -214,6 +275,57 @@ describe('EuiFlyout', () => {
);
});

it('includes EuiCollapsibleNavBeta items in tab rotation, inside EuiHeaders shards', () => {
cy.viewport(800, 600);
cy.mount(
<FlyoutWithHeader collapsibleNavVariant="beta"> </FlyoutWithHeader>
);
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(
<FlyoutWithHeader collapsibleNavVariant="default"> </FlyoutWithHeader>
);
// 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(<FlyoutWithHeader includeFixedHeadersInFocusTrap={false} />);
cy.get('[data-test-subj="toggleFlyoutFromHeader"]').click();
Expand Down
39 changes: 39 additions & 0 deletions packages/eui/src/components/header/header.spec.tsx
Original file line number Diff line number Diff line change
@@ -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.
*/

/// <reference types="cypress" />
/// <reference types="cypress-real-events" />
/// <reference types="../../../cypress/support" />

import React from 'react';

import { EuiHeader } from './header';
import { EuiFlyout } from '../flyout';

describe('EuiHeader', () => {
describe('fixed position', () => {
it('is above EuiFlyout', () => {
Comment thread
mgadewoll marked this conversation as resolved.
cy.mount(
<>
<EuiHeader data-test-subj="header" position="fixed" />
<EuiFlyout data-test-subj="flyout" />
</>
);

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);
});
});
});
});
7 changes: 4 additions & 3 deletions packages/eui/src/components/header/header.styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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`
Expand All @@ -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)}
Expand Down