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

feat(react): update to support react 17 #1563

Closed
wants to merge 15 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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@
"prettier": "^2",
"puppeteer": "^22.12.1",
"react": "^17",
"react-dom": "^16.13.1",
"react-dom": "^17",
"react-helmet": "^5.2.1",
"react-router-dom": "^5.3.0",
"remark-frontmatter": "^5.0.0",
Expand Down
7 changes: 4 additions & 3 deletions packages/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"dependencies": {
"@popperjs/core": "^2.5.4",
"classnames": "^2.2.6",
"focus-trap-react": "8",
"focus-trap-react": "^10.2.3",
"focusable": "^2.3.0",
"keyname": "^0.1.0",
"react-id-generator": "^3.0.1",
Expand Down Expand Up @@ -65,8 +65,9 @@
"postcss-cli": "^7.1.1",
"postcss-import": "^12.0.1",
"postcss-loader": "^3.0.0",
"react": "^16.13.1",
"react-dom": "^16.13.1",
"prop-types": "^15.8.1",
"react": "^17",
"react-dom": "^17",
"rollup": "^2.23.0",
"sinon": "^10.0.0",
"ts-node": "^10.9.2",
Expand Down
75 changes: 56 additions & 19 deletions packages/react/src/components/ClickOutsideListener/index.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import { render, screen, fireEvent } from '@testing-library/react';
import { userEvent } from '@testing-library/user-event';
import ClickOutsideListener from './';

let wrapperNode: HTMLDivElement | null;
Expand Down Expand Up @@ -35,16 +36,20 @@ test('should render children with the text when using ClickOutsideListener', ()
expect(renderedChild).toBeInTheDocument();
});

test('should call onClickOutside when clicked outside', () => {
test('should call onClickOutside when clicked outside', async () => {
const onClickOutside = jest.fn();
const user = userEvent.setup();

render(
<ClickOutsideListener onClickOutside={onClickOutside}>
<div>bar</div>
</ClickOutsideListener>
</ClickOutsideListener>,
{
container: mountNode as HTMLElement
}
);

fireEvent.click(screen.getByRole('link', { name: 'Click Me!' }));
await user.click(screen.getByRole('link', { name: 'Click Me!' }));
expect(onClickOutside).toBeCalled();
});

Expand All @@ -54,10 +59,13 @@ test('should call onClickOutside with event', () => {
render(
<ClickOutsideListener onClickOutside={onClickOutside}>
<div>bar</div>
</ClickOutsideListener>
</ClickOutsideListener>,
{
container: mountNode as HTMLElement
}
);

const event = new MouseEvent('click', { bubbles: true });
const event = new MouseEvent('mouseup', { bubbles: true });
fireEvent(screen.getByTestId('link'), event);
expect(onClickOutside).toHaveBeenCalledWith(event);
});
Expand All @@ -68,24 +76,31 @@ test('should call onClickOutside when touched outside', () => {
render(
<ClickOutsideListener onClickOutside={onClickOutside}>
<div>bar</div>
</ClickOutsideListener>
</ClickOutsideListener>,
{
container: mountNode as HTMLElement
}
);

const event = new TouchEvent('touchend', { bubbles: true });
fireEvent(screen.getByTestId('link'), event);
expect(onClickOutside).toHaveBeenCalledTimes(1);
});

test('should not call onClickOutside when clicked inside', () => {
test('should not call onClickOutside when clicked inside', async () => {
const onClickOutside = jest.fn();
const user = userEvent.setup();

render(
<ClickOutsideListener onClickOutside={onClickOutside}>
<div>Click me!</div>
</ClickOutsideListener>
</ClickOutsideListener>,
{
container: mountNode as HTMLElement
}
);

fireEvent.click(screen.getByText('Click me!'));
await user.click(screen.getByText('Click me!'));
expect(onClickOutside).not.toBeCalled();
});

Expand All @@ -95,7 +110,10 @@ test('should not call onClickOutside when touched inside', () => {
render(
<ClickOutsideListener onClickOutside={onClickOutside}>
<div data-testid="test">Touch me!</div>
</ClickOutsideListener>
</ClickOutsideListener>,
{
container: mountNode as HTMLElement
}
);

const event = new TouchEvent('touchend');
Expand All @@ -112,7 +130,10 @@ test('should allow mouseEvent to be changed', () => {
mouseEvent="mousedown"
>
<div>bar</div>
</ClickOutsideListener>
</ClickOutsideListener>,
{
container: mountNode as HTMLElement
}
);

const event = new MouseEvent('mousedown', { bubbles: true });
Expand All @@ -126,7 +147,10 @@ test('should allow mouseEvent to be false', () => {
render(
<ClickOutsideListener onClickOutside={onClickOutside} mouseEvent={false}>
<div>bar</div>
</ClickOutsideListener>
</ClickOutsideListener>,
{
container: mountNode as HTMLElement
}
);

const event = new MouseEvent('click', { bubbles: true });
Expand All @@ -143,7 +167,10 @@ test('should allow touchEvent to be changed', () => {
touchEvent="touchstart"
>
<div>div</div>
</ClickOutsideListener>
</ClickOutsideListener>,
{
container: mountNode as HTMLElement
}
);

const event = new TouchEvent('touchstart', { bubbles: true });
Expand All @@ -157,24 +184,31 @@ test('should allow touchEvent to be false', () => {
render(
<ClickOutsideListener onClickOutside={onClickOutside} touchEvent={false}>
<div>div</div>
</ClickOutsideListener>
</ClickOutsideListener>,
{
container: mountNode as HTMLElement
}
);

const event = new TouchEvent('touchend', { bubbles: true });
fireEvent(screen.getByTestId('link'), event);
expect(onClickOutside).not.toBeCalled();
});

test('should remove event listeners when props change', () => {
test('should remove event listeners when props change', async () => {
const onClickOutside = jest.fn();
const user = userEvent.setup();

const { rerender } = render(
<ClickOutsideListener onClickOutside={onClickOutside}>
<div>bar</div>
</ClickOutsideListener>
</ClickOutsideListener>,
{
container: mountNode as HTMLElement
}
);

fireEvent.click(screen.getByTestId('link'));
await user.click(screen.getByTestId('link'));
expect(onClickOutside).toHaveBeenCalledTimes(1);

rerender(
Expand All @@ -183,7 +217,7 @@ test('should remove event listeners when props change', () => {
</ClickOutsideListener>
);

fireEvent.click(screen.getByTestId('link'));
await user.click(screen.getByTestId('link'));
expect(onClickOutside).toHaveBeenCalledTimes(1);
});

Expand All @@ -194,7 +228,10 @@ test('should not remove event listeners when event props do not change', () => {
const { getByTestId } = render(
<ClickOutsideListener onClickOutside={onClickOutside} mouseEvent="click">
<div>bar</div>
</ClickOutsideListener>
</ClickOutsideListener>,
{
container: mountNode as HTMLElement
}
);

fireEvent.click(getByTestId('link'));
Expand Down
109 changes: 42 additions & 67 deletions packages/react/src/components/ClickOutsideListener/index.tsx
Original file line number Diff line number Diff line change
@@ -1,97 +1,72 @@
import React from 'react';
import React, { useRef, useEffect } from 'react';
import setRef from '../../utils/setRef';

export interface ClickOutsideListenerProps<
T extends HTMLElement = HTMLElement
> {
children?: React.ReactNode;
children?: React.ReactElement;
onClickOutside: (e: MouseEvent | TouchEvent) => void;
mouseEvent?: 'mousedown' | 'click' | 'mouseup' | false;
touchEvent?: 'touchstart' | 'touchend' | false;
target?: T;
}

export default class ClickOutsideListener extends React.Component<ClickOutsideListenerProps> {
static displayName = 'ClickOutsideListener';

static defaultProps = {
mouseEvent: 'click',
touchEvent: 'touchend'
};

private nodeRef: HTMLElement | null;

handleEvent = (event: MouseEvent | TouchEvent) => {
const { nodeRef, props } = this;
const { onClickOutside, target } = props;
function ClickOutsideListener(
{
children,
mouseEvent = 'mouseup',
touchEvent = 'touchend',
target,
onClickOutside = () => null
}: ClickOutsideListenerProps,
ref: React.ForwardedRef<HTMLElement>
): JSX.Element | null {
const childElementRef = useRef<HTMLElement>();

const handleEvent = (event: MouseEvent | TouchEvent) => {
if (event.defaultPrevented) {
return;
}

const eventTarget = event.target as HTMLElement;
if (
(target && !target.contains(eventTarget)) ||
(nodeRef && !nodeRef.contains(eventTarget))
(!!target && !target.contains(eventTarget)) ||
(childElementRef.current &&
!childElementRef.current.contains(eventTarget))
) {
onClickOutside(event);
}
};

componentDidMount() {
this.attachEventListeners();
}

componentDidUpdate(prevProps: ClickOutsideListenerProps) {
const { mouseEvent, touchEvent } = this.props;
if (
prevProps.mouseEvent !== mouseEvent ||
prevProps.touchEvent !== touchEvent
) {
this.removeEventListeners(prevProps.mouseEvent, prevProps.touchEvent);
this.attachEventListeners();
}
}

componentWillUnmount() {
const { mouseEvent, touchEvent } = this.props;
this.removeEventListeners(mouseEvent, touchEvent);
}

private attachEventListeners = () => {
const { mouseEvent, touchEvent } = this.props;
typeof mouseEvent === 'string' &&
document.addEventListener(mouseEvent, this.handleEvent);
typeof touchEvent === 'string' &&
document.addEventListener(touchEvent, this.handleEvent);
const resolveRef = (node: HTMLElement) => {
childElementRef.current = node;
// Ref for this component should pass-through to the child node
setRef(ref, node);
// If child has its own ref, we want to update
// its ref with the newly cloned node
const { ref: childRef } = children as any;
setRef(childRef, node);
};

private removeEventListeners = (
mouseEvent: ClickOutsideListenerProps['mouseEvent'],
touchEvent: ClickOutsideListenerProps['touchEvent']
) => {
useEffect(() => {
typeof mouseEvent === 'string' &&
document.removeEventListener(mouseEvent, this.handleEvent);
document.addEventListener(mouseEvent, handleEvent);
typeof touchEvent === 'string' &&
document.removeEventListener(touchEvent, this.handleEvent);
};
document.addEventListener(touchEvent, handleEvent);

resolveRef = (node: HTMLElement) => {
this.nodeRef = node;
return () => {
typeof mouseEvent === 'string' &&
document.removeEventListener(mouseEvent, handleEvent);
typeof touchEvent === 'string' &&
document.removeEventListener(touchEvent, handleEvent);
};
}, [mouseEvent, touchEvent]);

setRef;
// If child has its own ref, we want to update
// its ref with the newly cloned node
const { ref } = this.props.children as any;
setRef(ref, node);
};

render() {
const { props, resolveRef } = this;
return !props.children
? null
: React.cloneElement(props.children as any, {
ref: resolveRef
});
}
return !children
? null
: React.cloneElement(children as React.ReactElement, { ref: resolveRef });
}

ClickOutsideListener.displayName = 'ClickOutsideListener';

export default React.forwardRef(ClickOutsideListener);
6 changes: 6 additions & 0 deletions packages/react/src/components/Combobox/Combobox.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,9 @@ test('should prevent default with enter keypress and open listbox', () => {
// rtl doesn't let us mock preventDefault
// see: https://github.com/testing-library/react-testing-library/issues/572
event.preventDefault = preventDefault;
if (type === 'focus') {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes here I suspect are related to testing-library/user-event#592, but we're not using user-event but are calling fireEvent due to specific limitations with testing library.

fireEvent.focusIn(combobox);
}
fireEvent(combobox, event);
};

Expand Down Expand Up @@ -545,6 +548,9 @@ test('should not prevent default with enter keypress and closed listbox', () =>
// rtl doesn't let us mock preventDefault
// see: https://github.com/testing-library/react-testing-library/issues/572
event.preventDefault = preventDefault;
if (type === 'focus') {
fireEvent.focusIn(combobox);
}
fireEvent(combobox, event);
};

Expand Down
Loading
Loading