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

Fix React@17 event delegation #5940

Closed
wants to merge 8 commits into from
Closed
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
19 changes: 11 additions & 8 deletions src-docs/src/views/overlay_mask/overlay_mask.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React, { useState } from 'react';

import {
EuiFocusTrap,
EuiOverlayMask,
EuiButton,
EuiSpacer,
Expand All @@ -13,14 +14,16 @@ export default () => {

const modal = (
<React.Fragment>
<EuiOverlayMask
onClick={() => {
changeMask(false);
}}
>
<EuiTitle>
<h2> Click anywhere to close overlay. </h2>
</EuiTitle>
<EuiOverlayMask>
<EuiFocusTrap
onClickOutside={() => {
changeMask(false);
}}
>
<EuiTitle>
<h2> Click anywhere to close overlay. </h2>
</EuiTitle>
</EuiFocusTrap>
</EuiOverlayMask>
</React.Fragment>
);
Expand Down
17 changes: 9 additions & 8 deletions src-docs/src/views/overlay_mask/overlay_mask_example.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ export const OverlayMaskExample = {
<strong>EuiOverlayMask</strong> is simply a display component used
to obscure the main content to bring attention to its children or
other content. It is best used in conjunction with hyper-focus
content areas like <Link to="/layout/modal">modals</Link> and{' '}
content areas like{' '}
<Link to="/utilities/focus-trap">EuiFocusTrap</Link>,{' '}
<Link to="/layout/modal">modals</Link> and{' '}
<Link to="/layout/flyout">flyouts</Link>.
</p>
<p>
Expand All @@ -38,15 +40,15 @@ export const OverlayMaskExample = {
many considerations
</a>{' '}
to make before choosing to use an overlay. At the very least, you
must provide a visible button to close the overlay. You can also
pass an <EuiCode>onClick</EuiCode> handler to handle closing the
overlay.
must provide a visible button to close the overlay.
</p>
</>
),
props: { EuiOverlayMask: EuiOverlayMaskProps },
snippet: `<EuiOverlayMask onClick={() => {}}>
<!-- Content goes here -->
snippet: `<EuiOverlayMask>
<EuiFocusTrap onClickOutside={() => {}}
<!-- Content goes here -->
</EuiFocusTrap>
</EuiOverlayMask>`,
demo: <OverlayMask />,
},
Expand Down Expand Up @@ -84,8 +86,7 @@ export const OverlayMaskExample = {
</>
),
props: { EuiOverlayMask: EuiOverlayMaskProps },
snippet: `<EuiOverlayMask onClick={toggleFlyOut} headerZindexLocation="below" />
<EuiFlyout onClose={toggleFlyOut}></EuiFlyout>`,
snippet: '<EuiFlyout ownFocus onClose={toggleFlyOut}></EuiFlyout>',
demo: <OverlayMaskHeader />,
},
],
Expand Down
4 changes: 1 addition & 3 deletions src-docs/src/views/overlay_mask/overlay_mask_header.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React, { useState } from 'react';

import {
EuiOverlayMask,
EuiButton,
EuiFlyout,
EuiTitle,
Expand All @@ -19,8 +18,7 @@ export default () => {
if (flyOut) {
flyout = (
<React.Fragment>
<EuiOverlayMask onClick={toggleFlyOut} headerZindexLocation="below" />
<EuiFlyout size="s" onClose={toggleFlyOut}>
<EuiFlyout size="s" onClose={toggleFlyOut} outsideClickCloses>
<EuiFlyoutHeader>
<EuiTitle>
<h1>Click outside this flyout to close overlay. </h1>
Expand Down
4 changes: 0 additions & 4 deletions src-docs/src/views/overlay_mask/props.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,6 @@ import React, { FunctionComponent, ReactNode } from 'react';
import { CommonProps } from '../../../../src/components/common';

interface EuiOverlayMaskInterface extends CommonProps {
/**
* Function that applies to clicking the mask itself and not the children
*/
onClick?: () => void;
/**
* ReactNode to render as this component's children
*/
Expand Down
8 changes: 4 additions & 4 deletions src-docs/src/views/selectable/search.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,24 +47,24 @@ export default () => {
* Hook up the keyboard shortcut for command+k to initiate focus into search input
*/
useEffect(() => {
window.addEventListener('keydown', onWindowKeyDown);
window.addEventListener('keydown', onWindowKeyDown, { capture: true });

return function cleanup() {
window.removeEventListener('resize', onWindowKeyDown);
window.removeEventListener('resize', onWindowKeyDown, { capture: true });
};
});

const onWindowKeyDown = (e: any) => {
if (e.metaKey && e.key.toLowerCase() === 'k') {
e.preventDefault();
window.addEventListener('keyup', onWindowKeyUp);
window.addEventListener('keyup', onWindowKeyUp, { capture: true });
}
};

const onWindowKeyUp = () => {
searchRef && searchRef.focus();
setLoading(true);
window.removeEventListener('keyup', onWindowKeyUp);
window.removeEventListener('keyup', onWindowKeyUp, { capture: true });
};

const onKeyUpCapture = (e: any) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ exports[`CollapsedItemActions render with href and _target provided 1`] = `
display="inlineBlock"
hasArrow={true}
id="id-actions"
isCapture={false}
isOpen={true}
ownFocus={true}
panelPaddingSize="none"
Expand Down
2 changes: 1 addition & 1 deletion src/components/context_menu/context_menu_panel.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ describe('EuiContextMenuPanel', () => {
},
];

const FLAKY_WAIT = 100; // For some reason CI is flaking on these two tests in way that is hard to repro locally
const FLAKY_WAIT = 300; // For some reason CI is flaking on these two tests in way that is hard to repro locally

it('does not lose focus while using left/right arrow navigation between panels', () => {
cy.mount(<EuiContextMenu panels={panels} initialPanelId={0} />);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ exports[`EuiDataGridHeaderCell renders 1`] = `
closePopover={[Function]}
display="inlineBlock"
hasArrow={true}
isCapture={false}
isOpen={false}
offset={7}
ownFocus={true}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ exports[`useDataGridDisplaySelector displaySelector renders a toolbar button/pop
data-test-subj="dataGridDisplaySelectorPopover"
display="inlineBlock"
hasArrow={true}
isCapture={false}
isOpen={false}
ownFocus={true}
panelClassName="euiDataGrid__displayPopoverPanel"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -964,12 +964,14 @@ exports[`EuiDatePicker popoverPlacement upRight is rendered 1`] = `
closePopover={[Function]}
display="block"
hasArrow={false}
isCapture={false}
isOpen={false}
ownFocus={false}
panelPaddingSize="none"
panelRef={[Function]}
>
<EuiOutsideClickDetector
isCapture={false}
onOutsideClick={[Function]}
>
<div
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ exports[`EuiQuickSelectPopover is rendered 1`] = `
closePopover={[Function]}
display="inlineBlock"
hasArrow={true}
isCapture={false}
isOpen={false}
ownFocus={true}
panelPaddingSize="m"
Expand Down
19 changes: 15 additions & 4 deletions src/components/focus_trap/focus_trap.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ interface EuiFocusTrapInterface {
* Reference to element that will get focus when the trap is initiated
*/
initialFocus?: FocusTarget;
isCapture?: boolean;
style?: CSSProperties;
disabled?: boolean;
}
Expand All @@ -49,6 +50,7 @@ export class EuiFocusTrap extends Component<EuiFocusTrapProps, State> {
returnFocus: true,
noIsolation: true,
scrollLock: false,
isCapture: false,
};

state: State = {
Expand Down Expand Up @@ -89,13 +91,21 @@ export class EuiFocusTrap extends Component<EuiFocusTrapProps, State> {
};

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

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

handleOutsideClick: ReactFocusOnProps['onClickOutside'] = (event) => {
Expand All @@ -117,6 +127,7 @@ export class EuiFocusTrap extends Component<EuiFocusTrapProps, State> {
returnFocus,
noIsolation,
scrollLock,
isCapture,
...rest
} = this.props;
const isDisabled = disabled || this.state.hasBeenDisabledByClick;
Expand Down
4 changes: 3 additions & 1 deletion src/components/image/image.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,9 @@ describe('EuiImage', () => {
expect(overlayMask.length).toBe(1);

act(() => {
(overlayMask[0] as HTMLElement).click();
(overlayMask[0] as HTMLElement).dispatchEvent(
new Event('mousedown', { bubbles: true })
);
});

overlayMask = document.querySelectorAll(
Expand Down
10 changes: 5 additions & 5 deletions src/components/image/image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,11 @@ export const EuiImage: FunctionComponent<EuiImageProps> = ({
);

const fullScreenDisplay = (
<EuiOverlayMask
data-test-subj="fullScreenOverlayMask"
onClick={closeFullScreen}
>
<EuiFocusTrap clickOutsideDisables={true}>
<EuiOverlayMask data-test-subj="fullScreenOverlayMask">
<EuiFocusTrap
onClickOutside={closeFullScreen}
data-test-subj="fullScreenFocusTrap"
>
<>
<figure
className="euiImage euiImage-isFullScreen"
Expand Down
17 changes: 13 additions & 4 deletions src/components/outside_click_detector/outside_click_detector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export interface EuiOutsideClickDetectorProps {
*/
children: ReactElement<any>;
onOutsideClick: (event: Event) => void;
isCapture?: boolean;
isDisabled?: boolean;
onMouseDown?: (event: ReactMouseEvent) => void;
onMouseUp?: (event: ReactMouseEvent) => void;
Expand Down Expand Up @@ -98,13 +99,21 @@ export class EuiOutsideClickDetector extends Component<
};

componentDidMount() {
document.addEventListener('mouseup', this.onClickOutside);
document.addEventListener('touchend', this.onClickOutside);
document.addEventListener('mouseup', this.onClickOutside, {
capture: this.props.isCapture,
});
document.addEventListener('touchend', this.onClickOutside, {
capture: this.props.isCapture,
});
}

componentWillUnmount() {
document.removeEventListener('mouseup', this.onClickOutside);
document.removeEventListener('touchend', this.onClickOutside);
document.removeEventListener('mouseup', this.onClickOutside, {
capture: this.props.isCapture,
});
document.removeEventListener('touchend', this.onClickOutside, {
capture: this.props.isCapture,
});
}

onChildClick = (
Expand Down
21 changes: 0 additions & 21 deletions src/components/overlay_mask/overlay_mask.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ import { CommonProps, keysOf } from '../common';
import { useCombinedRefs } from '../../services';

export interface EuiOverlayMaskInterface {
/**
* Function that applies to clicking the mask itself and not the children
*/
onClick?: () => void;
/**
* ReactNode to render as this component's content
*/
Expand All @@ -55,7 +51,6 @@ export type EuiOverlayMaskProps = CommonProps &
export const EuiOverlayMask: FunctionComponent<EuiOverlayMaskProps> = ({
className,
children,
onClick,
headerZindexLocation = 'above',
maskRef,
...rest
Expand Down Expand Up @@ -117,22 +112,6 @@ export const EuiOverlayMask: FunctionComponent<EuiOverlayMaskProps> = ({
);
}, [className, headerZindexLocation]);

useEffect(() => {
const portalTarget = overlayMaskNode.current;
if (!portalTarget || !onClick) return;

const listener = (e: Event) => {
if (e.target === portalTarget) {
onClick();
}
};
portalTarget.addEventListener('click', listener);

return () => {
portalTarget.removeEventListener('click', listener);
};
}, [onClick]);

return isPortalTargetReady ? (
<>{createPortal(children, overlayMaskNode.current!)}</>
) : null;
Expand Down
Loading