refactor: Move deselect and isListening logic (#662)

src/common/BaseAnnotator.ts

@@ -120,6 +120,8 @@ export default class BaseAnnotator extends EventEmitter {
             this.annotatedEl.classList.remove(CSS_LOADED_CLASS);
         }
 
+        document.removeEventListener('mousedown', this.handleMouseDown);
+
         this.removeAnnotationClasses();
 
         this.removeListener(LegacyEvent.SCALE, this.handleScale);
@@ -129,6 +131,12 @@ export default class BaseAnnotator extends EventEmitter {
     }
 
     public init(scale = 1, rotation = 0): void {
+        // Check for containerEl prevents listener from being added on subsequent calls to init
+        if (!this.containerEl) {
+            // Add document listener to handle setting active annotation to null on mousedown
+            document.addEventListener('mousedown', this.handleMouseDown);
+        }
+
         this.containerEl = this.getElement(this.container);
         this.annotatedEl = this.getAnnotatedElement();
 
@@ -195,6 +203,10 @@ export default class BaseAnnotator extends EventEmitter {
         return typeof selector === 'string' ? document.querySelector(selector) : selector;
     }
 
+    protected handleMouseDown = (): void => {
+        this.setActiveId(null);
+    };
+
     protected handleRemove = (annotationId: string): void => {
         this.removeAnnotation(annotationId);
     };

src/common/__mocks__/useIsListInteractive.ts

@@ -0,0 +1 @@
+export default jest.fn().mockReturnValue(true);

src/common/__tests__/BaseAnnotator-test.ts

@@ -195,6 +195,14 @@ describe('BaseAnnotator', () => {
             expect(annotator.removeListener).toBeCalledWith(Event.VISIBLE_SET, expect.any(Function));
             expect(annotator.removeListener).toBeCalledWith(LegacyEvent.SCALE, expect.any(Function));
         });
+
+        test('should remove mousedown event listener', () => {
+            const removeEventListenerSpy = jest.spyOn(document, 'removeEventListener');
+
+            annotator.destroy();
+
+            expect(removeEventListenerSpy).toHaveBeenCalledWith('mousedown', expect.any(Function));
+        });
     });
 
     describe('init()', () => {
@@ -222,6 +230,13 @@ describe('BaseAnnotator', () => {
             expect(annotator.emit).toBeCalledWith(ANNOTATOR_EVENT.error, expect.any(String));
             expect(annotator.containerEl).toBeNull();
         });
+
+        test('should add a mousedown handler for deselecting the active id', () => {
+            const addEventListenerSpy = jest.spyOn(document, 'addEventListener');
+            annotator.init();
+
+            expect(addEventListenerSpy).toHaveBeenCalledWith('mousedown', expect.any(Function));
+        });
     });
 
     describe('event handlers', () => {

src/common/__tests__/useIsListInteractive-test.tsx

@@ -0,0 +1,40 @@
+import * as React from 'react';
+import { mount, ReactWrapper } from 'enzyme';
+import { Provider } from 'react-redux';
+import useIsListInteractive from '../useIsListInteractive';
+import { createStore, CreatorStatus } from '../../store';
+
+describe('useIsListInteractive', () => {
+    function TestComponent(): JSX.Element {
+        const isListening = useIsListInteractive();
+
+        return <div data-islistening={isListening} data-testid="islistening" />;
+    }
+
+    const getWrapper = (store = createStore()): ReactWrapper =>
+        mount(
+            <Provider store={store}>
+                <TestComponent />
+            </Provider>,
+        );
+
+    test('should be listening by default', () => {
+        const wrapper = getWrapper();
+
+        expect(wrapper.find('[data-testid="islistening"]').prop('data-islistening')).toEqual(true);
+    });
+
+    test('should not be listening if CreatorStatus is not init', () => {
+        const store = createStore({ creator: { status: CreatorStatus.started } });
+        const wrapper = getWrapper(store);
+
+        expect(wrapper.find('[data-testid="islistening"]').prop('data-islistening')).toEqual(false);
+    });
+
+    test('should not be listening if isSelecting is true', () => {
+        const store = createStore({ highlight: { isSelecting: true } });
+        const wrapper = getWrapper(store);
+
+        expect(wrapper.find('[data-testid="islistening"]').prop('data-islistening')).toEqual(false);
+    });
+});

src/common/useIsListInteractive.ts

@@ -0,0 +1,10 @@
+import * as ReactRedux from 'react-redux';
+import { getCreatorStatus, getIsSelecting, CreatorStatus } from '../store';
+
+// Returns whether rendered annotations should be interactive
+export default function useIsListInteractive(): boolean {
+    const status = ReactRedux.useSelector(getCreatorStatus);
+    const isSelecting = ReactRedux.useSelector(getIsSelecting);
+
+    return status === CreatorStatus.init && !isSelecting;
+}

src/drawing/DrawingList.tsx

@@ -3,7 +3,7 @@ import classNames from 'classnames';
 import noop from 'lodash/noop';
 import DrawingSVG, { DrawingSVGRef } from './DrawingSVG';
 import DrawingTarget from './DrawingTarget';
-import useOutsideEvent from '../common/useOutsideEvent';
+import useIsListInteractive from '../common/useIsListInteractive';
 import { AnnotationDrawing } from '../@types';
 import { checkValue } from '../utils/util';
 import { getShape } from './drawingUtil';
@@ -31,15 +31,8 @@ export function sortDrawing({ target: targetA }: AnnotationDrawing, { target: ta
 }
 
 export function DrawingList({ activeId = null, annotations, className, onSelect = noop }: Props): JSX.Element {
-    const [isListening, setIsListening] = React.useState(true);
     const [rootEl, setRootEl] = React.useState<DrawingSVGRef | null>(null);
-
-    // Document-level event handlers for focus and pointer control
-    useOutsideEvent('mousedown', rootEl, (): void => {
-        onSelect(null);
-        setIsListening(false);
-    });
-    useOutsideEvent('mouseup', rootEl, (): void => setIsListening(true));
+    const isListening = useIsListInteractive();
 
     return (
         <DrawingSVG

src/drawing/__tests__/DrawingAnnotationsContainer-test.tsx

@@ -6,6 +6,7 @@ import DrawingAnnotations from '../DrawingAnnotations';
 import DrawingAnnotationsContainer, { Props } from '../DrawingAnnotationsContainer';
 import { createStore, CreatorStatus, Mode } from '../../store';
 
+jest.mock('../../common/useIsListInteractive');
 jest.mock('../../common/withProviders');
 
 describe('DrawingAnnotationsContainer', () => {

src/drawing/__tests__/DrawingList-test.tsx

@@ -2,44 +2,25 @@ import React from 'react';
 import { shallow, ShallowWrapper } from 'enzyme';
 import DrawingTarget from '../DrawingTarget';
 import DrawingList, { Props } from '../DrawingList';
-import useOutsideEvent from '../../common/useOutsideEvent';
+import useIsListInteractive from '../../common/useIsListInteractive';
 import { AnnotationDrawing } from '../../@types';
 import { annotations } from '../__mocks__/drawingData';
 
-jest.mock('../../common/useOutsideEvent', () => jest.fn((name, ref, cb) => cb()));
+jest.mock('../../common/useIsListInteractive');
 
 describe('DrawingList', () => {
     const getDefaults = (): Props => ({
         annotations: annotations as AnnotationDrawing[],
     });
     const getWrapper = (props = {}): ShallowWrapper => shallow(<DrawingList {...getDefaults()} {...props} />);
-    const setIsListeningValue: { current: unknown } = { current: null };
-    const setIsListening = jest.fn((value: unknown): void => {
-        setIsListeningValue.current = value;
-    });
 
     beforeEach(() => {
-        setIsListeningValue.current = null; // Reset the mocked state
-
-        jest.spyOn(React, 'useState').mockImplementation(() => [setIsListeningValue.current, setIsListening]);
-    });
-
-    describe('event handlers', () => {
-        test('should call setIsListening and onSelect based on outside mouse events', () => {
-            const onSelect = jest.fn();
-
-            getWrapper({ onSelect });
-
-            expect(onSelect).toHaveBeenCalledWith(null); // mousedown
-            expect(setIsListening).toHaveBeenNthCalledWith(1, false); // mousedown
-            expect(setIsListening).toHaveBeenNthCalledWith(2, true); // mouseup
-            expect(useOutsideEvent).toHaveBeenCalledTimes(2);
-        });
+        jest.spyOn(React, 'useState').mockImplementation(() => [null, jest.fn()]);
     });
 
     describe('render', () => {
         test.each([true, false])('should render the class based on isListening %s', isListening => {
-            setIsListening(isListening);
+            (useIsListInteractive as jest.Mock).mockReturnValue(isListening);
 
             const wrapper = getWrapper();
 

src/highlight/HighlightList.tsx

@@ -3,7 +3,7 @@ import classNames from 'classnames';
 import HighlightCanvas, { CanvasShape } from './HighlightCanvas';
 import HighlightSvg from './HighlightSvg';
 import HighlightTarget from './HighlightTarget';
-import useOutsideEvent from '../common/useOutsideEvent';
+import useIsListInteractive from '../common/useIsListInteractive';
 import { AnnotationHighlight, Rect, TargetHighlight } from '../@types';
 import { checkValue } from '../utils/util';
 import './HighlightList.scss';
@@ -60,20 +60,16 @@ export function getHighlightShapesFromAnnotations(
 }
 
 export function HighlightList({ activeId = null, annotations, className, onSelect }: Props): JSX.Element {
-    const [isListening, setIsListening] = React.useState(true);
     const [hoverId, setHoverId] = React.useState<string | null>(null);
     const svgElRef = React.createRef<SVGSVGElement>();
+    const isListening = useIsListInteractive();
     const sortedAnnotations = annotations.filter(filterHighlight).sort(sortHighlight);
     const canvasShapes = getHighlightShapesFromAnnotations(sortedAnnotations, activeId, hoverId);
 
     const handleTargetHover = (annotationId: string | null): void => {
         setHoverId(annotationId);
     };
 
-    // Document-level event handlers for focus and pointer control
-    useOutsideEvent('mousedown', svgElRef, (): void => setIsListening(false));
-    useOutsideEvent('mouseup', svgElRef, (): void => setIsListening(true));
-
     return (
         <div className={classNames('ba-HighlightList', className)} data-resin-component="highlightList">
             <HighlightCanvas shapes={canvasShapes} />

src/highlight/__tests__/HighlightList-test.tsx

@@ -4,9 +4,11 @@ import { mount, ReactWrapper } from 'enzyme';
 import HighlightCanvas, { CanvasShape } from '../HighlightCanvas';
 import HighlightSvg from '../HighlightSvg';
 import HighlightTarget from '../HighlightTarget';
+import useIsListInteractive from '../../common/useIsListInteractive';
 import { AnnotationHighlight } from '../../@types';
 import { HighlightList, Props } from '../HighlightList';
 
+jest.mock('../../common/useIsListInteractive');
 jest.mock('../HighlightCanvas');
 jest.mock('../HighlightTarget');
 
@@ -32,11 +34,10 @@ describe('HighlightList', () => {
         });
 
         test('should not have is-listening class if isListening state is false', async () => {
+            (useIsListInteractive as jest.Mock).mockReturnValue(false);
+
             const wrapper = getWrapper();
-            act(() => {
-                document.dispatchEvent(new MouseEvent('mousedown'));
-            });
-            wrapper.update();
+
             expect(wrapper.find(HighlightSvg).hasClass('is-listening')).toBe(false);
         });
 

src/region/RegionList.tsx

@@ -2,7 +2,7 @@ import React from 'react';
 import classNames from 'classnames';
 import noop from 'lodash/noop';
 import RegionAnnotation from './RegionAnnotation';
-import useOutsideEvent from '../common/useOutsideEvent';
+import useIsListInteractive from '../common/useIsListInteractive';
 import { AnnotationRegion } from '../@types';
 import { checkValue } from '../utils/util';
 
@@ -29,16 +29,9 @@ export function sortRegion({ target: targetA }: AnnotationRegion, { target: targ
 }
 
 export function RegionList({ activeId, annotations, className, onSelect = noop }: Props): JSX.Element {
-    const [isListening, setIsListening] = React.useState(true);
+    const isListening = useIsListInteractive();
     const rootElRef = React.createRef<HTMLDivElement>();
 
-    // Document-level event handlers for focus and pointer control
-    useOutsideEvent('mousedown', rootElRef, (): void => {
-        onSelect(null);
-        setIsListening(false);
-    });
-    useOutsideEvent('mouseup', rootElRef, (): void => setIsListening(true));
-
     return (
         <div
             ref={rootElRef}

src/region/__tests__/RegionAnnotationsContainer-test.tsx

@@ -5,6 +5,7 @@ import RegionAnnotations from '../RegionAnnotations';
 import RegionAnnotationsContainer, { Props } from '../RegionAnnotationsContainer';
 import { createStore } from '../../store';
 
+jest.mock('../../common/useIsListInteractive');
 jest.mock('../../common/withProviders');
 
 describe('RegionAnnotationsContainer', () => {

src/region/__tests__/RegionList-test.tsx

@@ -2,10 +2,10 @@ import React from 'react';
 import { shallow, ShallowWrapper } from 'enzyme';
 import RegionAnnotation from '../RegionAnnotation';
 import RegionList from '../RegionList';
-import useOutsideEvent from '../../common/useOutsideEvent';
+import useIsListInteractive from '../../common/useIsListInteractive';
 import { AnnotationRegion } from '../../@types';
 
-jest.mock('../../common/useOutsideEvent', () => jest.fn((name, ref, cb) => cb()));
+jest.mock('../../common/useIsListInteractive');
 
 describe('RegionList', () => {
     const defaults = {
@@ -18,33 +18,14 @@ describe('RegionList', () => {
         onSelect: jest.fn(),
     };
     const getWrapper = (props = {}): ShallowWrapper => shallow(<RegionList {...defaults} {...props} />);
-    const setIsListeningValue: { current: unknown } = { current: null };
-    const setIsListening = jest.fn((value: unknown): void => {
-        setIsListeningValue.current = value;
-    });
 
     beforeEach(() => {
-        setIsListeningValue.current = null; // Reset the mocked state
-
-        jest.spyOn(React, 'useState').mockImplementation(() => [setIsListeningValue.current, setIsListening]);
-    });
-
-    describe('event handlers', () => {
-        test('should call setIsListening and onSelect based on outside mouse events', () => {
-            const onSelect = jest.fn();
-
-            getWrapper({ onSelect });
-
-            expect(onSelect).toHaveBeenCalledWith(null); // mousedown
-            expect(setIsListening).toHaveBeenNthCalledWith(1, false); // mousedown
-            expect(setIsListening).toHaveBeenNthCalledWith(2, true); // mouseup
-            expect(useOutsideEvent).toHaveBeenCalledTimes(2);
-        });
+        jest.spyOn(React, 'useState').mockImplementation(() => [null, jest.fn()]);
     });
 
     describe('render', () => {
         test.each([true, false])('should render the class based on isListening %s', isListening => {
-            setIsListening(isListening);
+            (useIsListInteractive as jest.Mock).mockReturnValue(isListening);
 
             const wrapper = getWrapper();