Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EuiFlyout] Enable shards for scoping element clicks; Optionally delay close event #5860

Merged
merged 21 commits into from
May 3, 2022
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
83 changes: 83 additions & 0 deletions src/components/collapsible_nav/collapsible_nav.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
* 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/support"/>

import React, { useState } from 'react';

import { EuiCollapsibleNav } from './collapsible_nav';
import { EuiHeader, EuiHeaderSectionItemButton } from '../header';
import { EuiIcon } from '../icon';

const Nav = () => {
const [isOpen, setIsOpen] = useState(false);

return (
<EuiHeader
style={{ zIndex: 1001 }}
position="fixed"
sections={[
{
items: [
<EuiCollapsibleNav
style={{ top: 48 }}
id="navSpec"
isOpen={isOpen}
button={
<EuiHeaderSectionItemButton
data-test-subj="navSpecButton"
aria-label="Toggle main navigation"
onClick={() => setIsOpen(!isOpen)}
>
<EuiIcon type={'menu'} size="m" aria-hidden="true" />
</EuiHeaderSectionItemButton>
}
onClose={() => setIsOpen(false)}
>
<button data-test-subj="itemA">Item A</button>
<button data-test-subj="itemB">Item B</button>
<button data-test-subj="itemC">Item C</button>
<input data-test-subj="itemD" />
</EuiCollapsibleNav>,
],
},
]}
/>
);
};

describe('EuiCollapsibleNav', () => {
describe('Elastic pattern', () => {
describe('Toggle button behavior', () => {
it('opens and closes nav when the main button is clicked', () => {
cy.mount(<Nav />);
cy.wait(400); // Wait for the button to be clickable
cy.get('[data-test-subj="navSpecButton"]').realClick();
expect(cy.get('#navSpec').should('exist'));
cy.get('[data-test-subj="navSpecButton"]').realClick();
expect(cy.get('#navSpec').should('not.exist'));
});

it('closes the nav when the overlay mask is clicked', () => {
cy.mount(<Nav />);
cy.wait(400);
thompsongl marked this conversation as resolved.
Show resolved Hide resolved
cy.get('[data-test-subj="navSpecButton"]').realClick();
cy.get('.euiOverlayMask').realClick();
expect(cy.get('#navSpec').should('not.exist'));
});

it('closes the nav when the close button is clicked', () => {
cy.mount(<Nav />);
cy.wait(400);
cy.get('[data-test-subj="navSpecButton"]').realClick();
cy.get('[data-test-subj="euiFlyoutCloseButton"]').realClick();
expect(cy.get('#navSpec').should('not.exist'));
});
});
});
});
11 changes: 11 additions & 0 deletions src/components/collapsible_nav/collapsible_nav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@ import React, {
ReactElement,
ReactNode,
useEffect,
useRef,
useState,
} from 'react';
import classNames from 'classnames';
import {
useGeneratedHtmlId,
isWithinMinBreakpoint,
throttle,
useCombinedRefs,
} from '../../services';
import { EuiFlyout, EuiFlyoutProps } from '../flyout';

Expand Down Expand Up @@ -71,12 +73,19 @@ export const EuiCollapsibleNav: FunctionComponent<EuiCollapsibleNavProps> = ({
outsideClickCloses = true,
closeButtonPosition = 'outside',
paddingSize = 'none',
focusTrapProps: _focusTrapProps = {},
...rest
}) => {
const flyoutID = useGeneratedHtmlId({
conditionalId: id,
suffix: 'euiCollapsibleNav',
});
const buttonRef = useRef();
const combinedButtonRef = useCombinedRefs([button?.props.ref, buttonRef]);
const focusTrapProps: EuiFlyoutProps['focusTrapProps'] = {
..._focusTrapProps,
shards: [buttonRef, ...(_focusTrapProps.shards || [])],
cee-chen marked this conversation as resolved.
Show resolved Hide resolved
};

/**
* Setting the initial state of pushed based on the `type` prop
Expand Down Expand Up @@ -136,6 +145,7 @@ export const EuiCollapsibleNav: FunctionComponent<EuiCollapsibleNavProps> = ({
onMouseUpCapture: (e: React.MouseEvent<HTMLElement>) => {
e.nativeEvent.stopImmediatePropagation();
},
ref: combinedButtonRef,
});

const flyout = (
Expand All @@ -151,6 +161,7 @@ export const EuiCollapsibleNav: FunctionComponent<EuiCollapsibleNavProps> = ({
outsideClickCloses={outsideClickCloses}
closeButtonPosition={closeButtonPosition}
paddingSize={paddingSize}
focusTrapProps={focusTrapProps}
{...rest}
// Props dependent on internal docked status
type={navIsDocked ? 'push' : 'overlay'}
Expand Down
10 changes: 9 additions & 1 deletion src/components/flyout/flyout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {
} from '../../services';

import { CommonProps, keysOf, PropsOfElement } from '../common';
import { EuiFocusTrap } from '../focus_trap';
import { EuiFocusTrap, EuiFocusTrapProps } from '../focus_trap';
import { EuiOverlayMask, EuiOverlayMaskProps } from '../overlay_mask';
import { EuiButtonIcon, EuiButtonIconPropsForButton } from '../button';
import { EuiI18n } from '../i18n';
Expand Down Expand Up @@ -151,6 +151,12 @@ interface _EuiFlyoutProps {
*/
pushMinBreakpoint?: EuiBreakpointSize | number;
style?: CSSProperties;
/**
* Object of props passed to EuiFocusTrap.
* `shards` specifies an array of elements that will be considered part of the flyout, preventing the flyout from being closed when clicked.
* `closeOnMouseup` will delay the close callback, allowing time for external toggle buttons to handle close behavior.
*/
focusTrapProps?: Pick<EuiFocusTrapProps, 'closeOnMouseup' | 'shards'>;
thompsongl marked this conversation as resolved.
Show resolved Hide resolved
}

const defaultElement = 'div';
Expand Down Expand Up @@ -189,6 +195,7 @@ export const EuiFlyout = forwardRef(
outsideClickCloses,
role = 'dialog',
pushMinBreakpoint = 'l',
focusTrapProps,
...rest
}: EuiFlyoutProps<T>,
ref:
Expand Down Expand Up @@ -362,6 +369,7 @@ export const EuiFlyout = forwardRef(
disabled={isPushed}
clickOutsideDisables={!ownFocus}
onClickOutside={onClickOutside}
{...focusTrapProps}
>
<Element
{...(rest as ComponentPropsWithRef<T>)}
Expand Down
78 changes: 77 additions & 1 deletion src/components/focus_trap/focus_trap.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

/// <reference types="../../../cypress/support"/>

import React from 'react';
import React, { useRef } from 'react';
import { EuiFocusTrap } from './focus_trap';
import { EuiPortal } from '../portal';

Expand Down Expand Up @@ -157,4 +157,80 @@ describe('EuiFocusTrap', () => {
cy.get('[data-focus-lock-disabled=false]').should('not.exist');
});
});

describe('outside click handling', () => {
const Trap = ({
onClickOutside,
shards,
closeOnMouseup,
}: {
onClickOutside?: any;
shards?: boolean;
closeOnMouseup?: boolean;
}) => {
const buttonRef = useRef();
return (
<div>
<EuiFocusTrap
onClickOutside={onClickOutside}
shards={shards ? [buttonRef] : []}
closeOnMouseup={closeOnMouseup}
>
<div data-test-subj="container">
<input data-test-subj="input" />
<input data-test-subj="input2" />
</div>
</EuiFocusTrap>
<button ref={buttonRef} data-test-subj="outside">
outside the focus trap
</button>
<button data-test-subj="outside2">also outside the focus trap</button>
</div>
);
};

it('calls the callback on mousedown', () => {
const onClickOutside = cy.stub();
cy.mount(<Trap onClickOutside={onClickOutside} />);

cy.get('[data-test-subj=outside]')
.realMouseDown()
.then(() => {
expect(onClickOutside).to.be.called;
});
});

it('calls the callback on mouseup when using closeOnMouseup', () => {
const onClickOutside = cy.stub();
cy.mount(<Trap onClickOutside={onClickOutside} closeOnMouseup />);

cy.get('[data-test-subj=outside]')
.realMouseDown()
.then(() => {
expect(onClickOutside).to.not.be.called;
});
cy.get('[data-test-subj=outside]')
.click() // real events not working here
.then(() => {
expect(onClickOutside).to.be.called;
});
});

it('does not call the callback if the element is a shard', () => {
const onClickOutside = cy.stub();
cy.mount(<Trap onClickOutside={onClickOutside} shards />);

cy.get('[data-test-subj=outside]')
.realMouseDown()
.then(() => {
expect(onClickOutside).to.not.be.called;
});
// But still calls if the element is not a shard
cy.get('[data-test-subj=outside2]')
.realMouseDown()
.then(() => {
expect(onClickOutside).to.be.called;
});
});
});
});
30 changes: 28 additions & 2 deletions src/components/focus_trap/focus_trap.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ import { findElementBySelectorOrRef, ElementTarget } from '../../services';
export type FocusTarget = ElementTarget;

interface EuiFocusTrapInterface {
/**
* Whether `onClickOutside` should be called on mouseup instead of mousedown.
* This flag can be used to prevent conflicts with outside toggle buttons by delaying the closing click callback.
*/
closeOnMouseup?: boolean;
/**
* Clicking outside the trap area will disable the trap
*/
Expand Down Expand Up @@ -64,6 +69,10 @@ export class EuiFocusTrap extends Component<EuiFocusTrapProps, State> {
}
}

componentWillUnmount() {
this.removeMouseupListener();
}

// Programmatically sets focus on a nested DOM node; optional
setInitialFocus = (initialFocus?: FocusTarget) => {
const node = findElementBySelectorOrRef(initialFocus);
Expand All @@ -72,14 +81,31 @@ export class EuiFocusTrap extends Component<EuiFocusTrapProps, State> {
node.setAttribute('data-autofocus', 'true');
};

onMouseupOutside = (e: MouseEvent | TouchEvent) => {
this.removeMouseupListener();
// Timeout gives precedence to the consumer to initiate close if it has toggle behavior.
// Otherwise this event may occur first and the consumer toggle will reopen the flyout.
setTimeout(() => this.props.onClickOutside?.(e));
};

addMouseupListener = () => {
document.addEventListener('mouseup', this.onMouseupOutside);
document.addEventListener('touchend', this.onMouseupOutside);
};

removeMouseupListener = () => {
document.removeEventListener('mouseup', this.onMouseupOutside);
document.removeEventListener('touchend', this.onMouseupOutside);
};

handleOutsideClick: ReactFocusOnProps['onClickOutside'] = (...args) => {
const { onClickOutside, clickOutsideDisables } = this.props;
const { onClickOutside, clickOutsideDisables, closeOnMouseup } = this.props;
if (clickOutsideDisables) {
this.setState({ hasBeenDisabledByClick: true });
}

if (onClickOutside) {
onClickOutside(...args);
closeOnMouseup ? this.addMouseupListener() : onClickOutside(...args);
}
};

Expand Down
6 changes: 6 additions & 0 deletions upcoming_changelogs/5860.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
- Added the `focusTrapProps` prop to `EuiFlyout` to aid outside click detection and closing event

**Bug fixes**

- Fixed `EuiCollapsibleNav` failing to close when the button is clicked