Skip to content

Commit

Permalink
fix(controls): Prevent controls from hiding when already unmounted (#…
Browse files Browse the repository at this point in the history
…1316)

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
jstoffan and mergify[bot] authored Jan 12, 2021
1 parent 39bff92 commit c695342
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 3 deletions.
10 changes: 8 additions & 2 deletions src/lib/viewers/controls/controls-layer/ControlsLayer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,11 @@ export default function ControlsLayer({ children, onMount = noop }: Props): JSX.

// Visibility helpers
const helpersRef = React.useRef({
hide() {
clean() {
window.clearTimeout(hideTimeoutRef.current);
},
hide() {
helpersRef.current.clean();

hideTimeoutRef.current = window.setTimeout(() => {
if (hasCursorRef.current || hasFocusRef.current) {
Expand All @@ -40,7 +43,7 @@ export default function ControlsLayer({ children, onMount = noop }: Props): JSX.
hasFocusRef.current = false;
},
show() {
window.clearTimeout(hideTimeoutRef.current);
helpersRef.current.clean();
setIsShown(true);
},
});
Expand Down Expand Up @@ -71,6 +74,9 @@ export default function ControlsLayer({ children, onMount = noop }: Props): JSX.
onMount(helpersRef.current);
}, [onMount]);

// Destroy timeouts on unmount
React.useEffect(() => helpersRef.current.clean, []);

return (
<div
className={`bp-ControlsLayer ${isShown ? SHOW_CLASSNAME : ''}`}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import { act } from 'react-dom/test-utils';
import { mount, ReactWrapper } from 'enzyme';
import ControlsLayer, { HIDE_DELAY_MS, SHOW_CLASSNAME } from '../ControlsLayer';
import ControlsLayer, { Helpers, HIDE_DELAY_MS, SHOW_CLASSNAME } from '../ControlsLayer';

describe('ControlsLayer', () => {
const children = <div className="TestControls">Controls</div>;
Expand Down Expand Up @@ -88,6 +88,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
};
const wrapper = getWrapper({ onMount });

wrapper.unmount();

expect(window.clearTimeout).toBeCalledTimes(2); // Once on hide, once on unmount
});
});

describe('render', () => {
test('should invoke the onMount callback once with the visibility helpers', () => {
const onMount = jest.fn();
Expand All @@ -99,6 +114,7 @@ describe('ControlsLayer', () => {

expect(onMount).toBeCalledTimes(1);
expect(onMount).toBeCalledWith({
clean: expect.any(Function),
hide: expect.any(Function),
reset: expect.any(Function),
show: expect.any(Function),
Expand Down

0 comments on commit c695342

Please sign in to comment.