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

chore(deps): Upgrade React dependencies to v17 #1334

Merged
merged 3 commits into from
Mar 12, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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 build/jest/envSetup.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import '@testing-library/jest-dom';
import Adapter from 'enzyme-adapter-react-16';
import Adapter from '@wojtekmaj/enzyme-adapter-react-17';
import Enzyme, { mount, shallow } from 'enzyme';

expect.extend({
Expand Down
12 changes: 6 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@
"@types/classnames": "^2.2.11",
"@types/enzyme": "^3.10.8",
"@types/lodash": "^4.14.149",
"@types/react": "^16.9.0",
"@types/react-dom": "^16.9.0",
"@types/react": "^17.0.2",
"@types/react-dom": "^17.0.1",
"@typescript-eslint/eslint-plugin": "^2.13.0",
"@typescript-eslint/parser": "^2.13.0",
"@wojtekmaj/enzyme-adapter-react-17": "^0.4.1",
"autoprefixer": "^9.6.1",
"axios": "^0.19.0",
"babel-eslint": "^10.0.3",
Expand All @@ -44,7 +45,6 @@
"cssnano-cli": "^1.0.5",
"cypress": "^3.8.1",
"enzyme": "^3.10.0",
"enzyme-adapter-react-16": "^1.15.1",
"eslint": "^6.7.2",
"eslint-config-airbnb": "^18.0.1",
"eslint-config-prettier": "^6.7.0",
Expand Down Expand Up @@ -82,11 +82,11 @@
"postcss-sass": "^0.4.1",
"prettier": "^1.19.1",
"raw-loader": "^3.1.0",
"react": "^16.9.0",
"react-dom": "^16.9.0",
"react": "^17.0.1",
"react-dom": "^17.0.1",
"react-intl": "^2.9.0",
"react-tether": "^1.0.0",
"react-virtualized": "^9.13.0",
"react-virtualized": "^9.22.3",
"sass-loader": "^7.1.0",
"sinon": "^9.0.3",
"sinon-chai": "^3.5.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,33 @@ describe('AnnotationsControls', () => {
});

describe('lifecycle', () => {
let unmount = (): void => {
// placeholder
};

beforeEach(() => {
jest.spyOn(React, 'useEffect').mockImplementation(cb => {
unmount = cb() as () => void; // Enzyme unmount helper does not currently invoke useEffect cleanup
});
});

test('should add and remove its event handlers on mount and unmount', () => {
const wrapper = getWrapper({
getWrapper({
annotationMode: AnnotationMode.REGION,
hasHighlight: true,
hasRegion: true,
});
expect(document.addEventListener).toBeCalledWith('keydown', expect.any(Function));

wrapper.unmount();
unmount();
expect(document.removeEventListener).toBeCalledWith('keydown', expect.any(Function));
});

test('should not add a handler if the annotation mode is set to none', () => {
const wrapper = getWrapper({ hasHighlight: true, hasRegion: true });
getWrapper({ hasHighlight: true, hasRegion: true });
expect(document.addEventListener).not.toBeCalledWith('keydown', expect.any(Function));

wrapper.unmount();
unmount();
expect(document.removeEventListener).toBeCalledWith('keydown', expect.any(Function));
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,21 @@ describe('ControlsLayer', () => {

describe('unmount', () => {
test('should destroy any existing hide timeout', () => {
jest.spyOn(window, 'clearTimeout');

const onMount = (helpers: Helpers): void => {
helpers.hide(); // Kick off the hide timeout
let unmount = (): void => {
// placeholder
};
const wrapper = getWrapper({ onMount });

wrapper.unmount();
jest.spyOn(window, 'clearTimeout');
jest.spyOn(React, 'useEffect').mockImplementation(cb => {
unmount = cb() as () => void; // Enzyme unmount helper does not currently invoke useEffect cleanup
});

getWrapper({
onMount: (helpers: Helpers): void => {
helpers.hide(); // Kick off the hide timeout
},
});
unmount();

expect(window.clearTimeout).toBeCalledTimes(2); // Once on hide, once on unmount
});
Expand Down
32 changes: 0 additions & 32 deletions src/lib/viewers/controls/hooks/__tests__/usePreventKey-test.tsx

This file was deleted.

1 change: 0 additions & 1 deletion src/lib/viewers/controls/hooks/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
export { default as useAttention } from './useAttention';
export { default as useFullscreen } from './useFullscreen';
export { default as usePreventKey } from './usePreventKey';
26 changes: 0 additions & 26 deletions src/lib/viewers/controls/hooks/usePreventKey.ts

This file was deleted.

12 changes: 8 additions & 4 deletions src/lib/viewers/controls/media/MediaToggle.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import React from 'react';
import usePreventKey from '../hooks/usePreventKey';
import { decodeKeydown } from '../../../util';

export type Props = React.ButtonHTMLAttributes<HTMLButtonElement>;

export default function MediaToggle(props: Props): JSX.Element {
const buttonElRef = React.useRef<HTMLButtonElement>(null);
const handleKeydown = (event: React.KeyboardEvent): void => {
const key = decodeKeydown(event);

usePreventKey(buttonElRef, ['Enter', 'Space']);
if (key === 'Enter' || key === 'Space') {
event.stopPropagation();
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

}
};

return <button ref={buttonElRef} type="button" {...props} />;
return <button onKeyDown={handleKeydown} type="button" {...props} />;
}
19 changes: 12 additions & 7 deletions src/lib/viewers/controls/media/VolumeControls.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import IconVolumeLow24 from '../icons/IconVolumeLow24';
import IconVolumeMedium24 from '../icons/IconVolumeMedium24';
import IconVolumeMute24 from '../icons/IconVolumeMute24';
import MediaToggle from './MediaToggle';
import SliderControl, { Ref as SliderControlRef } from '../slider';
import SliderControl from '../slider';
import useAttention from '../hooks/useAttention';
import usePreventKey from '../hooks/usePreventKey';
import { decodeKeydown } from '../../../util';
import './VolumeControls.scss';

export type Props = {
Expand All @@ -33,12 +33,20 @@ export function getIcon(volume: number): (props: React.SVGProps<SVGSVGElement>)

export default function VolumeControls({ onMuteChange, onVolumeChange, volume = 1 }: Props): JSX.Element {
const [isActive, handlers] = useAttention();
const inputElRef = React.useRef<SliderControlRef>(null);
const isMuted = !volume;
const Icon = isMuted ? IconVolumeMute24 : getIcon(volume);
const title = isMuted ? __('media_unmute') : __('media_mute');
const value = Math.round(volume * 100);

const handleKeydown = (event: React.KeyboardEvent): void => {
const key = decodeKeydown(event);

// Allow the range input to handle its own left/right keydown events
if (key === 'ArrowLeft' || key === 'ArrowRight') {
event.stopPropagation(); // Prevents global key handling
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this comment be applied above to MediaToggle as well?

}
};

const handleMute = (): void => {
onMuteChange(!isMuted);
};
Expand All @@ -47,9 +55,6 @@ export default function VolumeControls({ onMuteChange, onVolumeChange, volume =
onVolumeChange(newValue / 100);
};

// Allow the range input to handle its own left/right keydown events
usePreventKey(inputElRef, ['ArrowLeft', 'ArrowRight']);

return (
<div className="bp-VolumeControls">
<MediaToggle className="bp-VolumeControls-toggle" onClick={handleMute} title={title} {...handlers}>
Expand All @@ -58,9 +63,9 @@ export default function VolumeControls({ onMuteChange, onVolumeChange, volume =

<div className={classNames('bp-VolumeControls-flyout', { 'bp-is-open': isActive })}>
<SliderControl
ref={inputElRef}
className="bp-VolumeControls-slider"
onChange={handleVolume}
onKeyDown={handleKeydown}
title={__('media_volume_slider')}
track={`linear-gradient(to right, ${bdlBoxBlue} ${value}%, ${white} ${value}%)`}
value={value}
Expand Down
12 changes: 6 additions & 6 deletions src/lib/viewers/controls/media/__tests__/MediaToggle-test.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import React from 'react';
import { shallow, ShallowWrapper } from 'enzyme';
import MediaToggle from '../MediaToggle';
import usePreventKey from '../../hooks/usePreventKey';

jest.mock('../../hooks/usePreventKey');

describe('MediaToggle', () => {
const getWrapper = (props = {}): ShallowWrapper => shallow(<MediaToggle {...props} />);
Expand All @@ -18,10 +15,13 @@ describe('MediaToggle', () => {
});
});

test('should call the usePreventKey hook with specific keys', () => {
getWrapper();
test.each(['Enter', 'Space'])('should defer to the inner button for the %s key', key => {
const wrapper = getWrapper();
const event = { key, stopPropagation: jest.fn() };

wrapper.simulate('keydown', event);

expect(usePreventKey).toBeCalledWith({ current: expect.any(Object) }, ['Enter', 'Space']);
expect(event.stopPropagation).toBeCalled();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ import IconVolumeMute24 from '../../icons/IconVolumeMute24';
import MediaToggle from '../MediaToggle';
import SliderControl from '../../slider';
import VolumeControls from '../VolumeControls';
import usePreventKey from '../../hooks/usePreventKey';

jest.mock('../../hooks/usePreventKey');
jest.mock('../../slider');

describe('VolumeControls', () => {
Expand All @@ -31,10 +29,13 @@ describe('VolumeControls', () => {
expect(onMuteChange).toBeCalledWith(isMuted);
});

test('should defer to the inner input for left/right arrow keys', () => {
getWrapper();
test.each(['ArrowLeft', 'ArrowRight'])('should defer to the inner input for the %s key', key => {
const event = { key, stopPropagation: jest.fn() };
const wrapper = getWrapper();

wrapper.find(SliderControl).simulate('keydown', event);

expect(usePreventKey).toBeCalledWith(expect.any(Object), ['ArrowLeft', 'ArrowRight']);
expect(event.stopPropagation).toBeCalled();
});
});

Expand Down
Loading