From f93f39f9201aa55b03bff150da37778687224691 Mon Sep 17 00:00:00 2001
From: Mingze Xiao <mxiao@box.com>
Date: Fri, 2 Oct 2020 17:04:12 -0700
Subject: [PATCH 1/3] fix(highlight): Remove selection when clicking anywhere
 outside of popup

---
 src/components/Popups/PopupHighlight.tsx      |  6 +++++-
 src/components/Popups/PopupHighlightError.tsx | 12 +++++++++++-
 src/highlight/HighlightAnnotations.tsx        | 16 +++++++++++++---
 src/highlight/HighlightContainer.tsx          |  2 ++
 4 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/src/components/Popups/PopupHighlight.tsx b/src/components/Popups/PopupHighlight.tsx
index 26160a0c7..8f1a7d2fb 100644
--- a/src/components/Popups/PopupHighlight.tsx
+++ b/src/components/Popups/PopupHighlight.tsx
@@ -4,11 +4,13 @@ import noop from 'lodash/noop';
 import { FormattedMessage } from 'react-intl';
 import messages from './messages';
 import PopupBase from './PopupBase';
+import useOutsideEvent from '../../common/useOutsideEvent';
 import { Options } from './Popper';
 import { Shape } from '../../@types/model';
 import './PopupHighlight.scss';
 
 export type Props = {
+    onCancel?: () => void;
     onClick?: (event: React.MouseEvent) => void;
     shape: Shape;
 };
@@ -43,7 +45,7 @@ const options: Partial<Options> = {
     placement: 'bottom',
 };
 
-export default function PopupHighlight({ onClick = noop, shape }: Props): JSX.Element {
+export default function PopupHighlight({ onCancel = noop, onClick = noop, shape }: Props): JSX.Element {
     const buttonRef = React.useRef<HTMLButtonElement>(null);
     const { height, width, x, y } = shape;
 
@@ -62,6 +64,8 @@ export default function PopupHighlight({ onClick = noop, shape }: Props): JSX.El
         onClick(event);
     };
 
+    useOutsideEvent('mousedown', buttonRef, onCancel);
+
     return (
         <PopupBase className="ba-PopupHighlight" options={options} reference={reference}>
             <button
diff --git a/src/components/Popups/PopupHighlightError.tsx b/src/components/Popups/PopupHighlightError.tsx
index 85eae5f36..dbdafc638 100644
--- a/src/components/Popups/PopupHighlightError.tsx
+++ b/src/components/Popups/PopupHighlightError.tsx
@@ -1,4 +1,5 @@
 import React from 'react';
+import noop from 'lodash/noop';
 import { FormattedMessage } from 'react-intl';
 import messages from './messages';
 import PopupBase from './PopupBase';
@@ -7,6 +8,7 @@ import { Shape } from '../../@types/model';
 import './PopupHighlightError.scss';
 
 export type Props = {
+    onCancel?: () => void;
     shape: Shape;
 };
 
@@ -40,7 +42,7 @@ const options: Partial<Options> = {
     placement: 'bottom',
 };
 
-export default function PopupHighlightError({ shape }: Props): JSX.Element {
+export default function PopupHighlightError({ onCancel = noop, shape }: Props): JSX.Element {
     const { height, width, x, y } = shape;
 
     const reference = {
@@ -54,6 +56,14 @@ export default function PopupHighlightError({ shape }: Props): JSX.Element {
         }),
     };
 
+    React.useEffect(() => {
+        document.addEventListener('mousedown', onCancel);
+
+        return () => {
+            document.removeEventListener('mousedown', onCancel);
+        };
+    }, [onCancel]);
+
     return (
         <PopupBase className="ba-PopupHighlightError" options={options} reference={reference}>
             <FormattedMessage {...messages.popupHighlightRestrictedPrompt} />
diff --git a/src/highlight/HighlightAnnotations.tsx b/src/highlight/HighlightAnnotations.tsx
index 20cd7d5bd..a4fe3ada0 100644
--- a/src/highlight/HighlightAnnotations.tsx
+++ b/src/highlight/HighlightAnnotations.tsx
@@ -8,7 +8,7 @@ import PopupHighlightError from '../components/Popups/PopupHighlightError';
 import useWindowSize from '../common/useWindowSize';
 import { AnnotationHighlight } from '../@types';
 import { CreateArg } from './actions';
-import { CreatorItemHighlight, CreatorStatus, SelectionItem } from '../store';
+import { CreatorItemHighlight, CreatorStatus, SelectionArg, SelectionItem } from '../store';
 import { getBoundingRect, getShapeRelativeToContainer } from './highlightUtil';
 import './HighlightAnnotations.scss';
 
@@ -24,6 +24,7 @@ type Props = {
     setActiveAnnotationId: (annotationId: string | null) => void;
     setIsPromoting: (isPromoting: boolean) => void;
     setReferenceShape: (rect: DOMRect) => void;
+    setSelection: (selection: SelectionArg | null) => void;
     setStaged: (staged: CreatorItemHighlight | null) => void;
     setStatus: (status: CreatorStatus) => void;
     staged?: CreatorItemHighlight | null;
@@ -40,6 +41,7 @@ const HighlightAnnotations = (props: Props): JSX.Element => {
         setActiveAnnotationId,
         setIsPromoting,
         setReferenceShape,
+        setSelection,
         setStaged,
         setStatus,
         staged,
@@ -76,6 +78,10 @@ const HighlightAnnotations = (props: Props): JSX.Element => {
         setIsPromoting(true);
     };
 
+    const handleCancel = (): void => {
+        setSelection(null);
+    };
+
     React.useEffect(() => {
         if (!isSelecting) {
             return;
@@ -119,14 +125,18 @@ const HighlightAnnotations = (props: Props): JSX.Element => {
             {/* Layer 3a: Annotations promoter to promote selection to staged */}
             {!isCreating && selection && !selection.hasError && (
                 <div className="ba-HighlightAnnotations-popup">
-                    <PopupHighlight onClick={handlePromote} shape={getBoundingRect(selection.rects)} />
+                    <PopupHighlight
+                        onCancel={handleCancel}
+                        onClick={handlePromote}
+                        shape={getBoundingRect(selection.rects)}
+                    />
                 </div>
             )}
 
             {/* Layer 3b: Highlight error popup */}
             {selection && selection.hasError && (
                 <div className="ba-HighlightAnnotations-popup">
-                    <PopupHighlightError shape={getBoundingRect(selection.rects)} />
+                    <PopupHighlightError onCancel={handleCancel} shape={getBoundingRect(selection.rects)} />
                 </div>
             )}
         </>
diff --git a/src/highlight/HighlightContainer.tsx b/src/highlight/HighlightContainer.tsx
index 8a93cc0f1..95d50520b 100644
--- a/src/highlight/HighlightContainer.tsx
+++ b/src/highlight/HighlightContainer.tsx
@@ -18,6 +18,7 @@ import {
     setActiveAnnotationIdAction,
     setIsPromotingAction,
     setReferenceShapeAction,
+    setSelectionAction,
     setStagedAction,
     setStatusAction,
 } from '../store';
@@ -51,6 +52,7 @@ export const mapDispatchToProps = {
     setActiveAnnotationId: setActiveAnnotationIdAction,
     setIsPromoting: setIsPromotingAction,
     setReferenceShape: setReferenceShapeAction,
+    setSelection: setSelectionAction,
     setStaged: setStagedAction,
     setStatus: setStatusAction,
 };

From 2c0313187ce23168536dd8288c9a15e062717c63 Mon Sep 17 00:00:00 2001
From: Mingze Xiao <mxiao@box.com>
Date: Tue, 6 Oct 2020 09:54:18 -0700
Subject: [PATCH 2/3] fix(highlight): Address comments

---
 src/common/useOutsideEvent.ts                 |  2 +-
 src/components/Popups/PopupHighlightError.tsx | 12 ++++--------
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/src/common/useOutsideEvent.ts b/src/common/useOutsideEvent.ts
index 0b60fc656..e033cde1d 100644
--- a/src/common/useOutsideEvent.ts
+++ b/src/common/useOutsideEvent.ts
@@ -1,7 +1,7 @@
 import * as React from 'react';
 
 // Utilizes the useEffect hook to handle when an event is fired on the document, but outside of the specified element
-export default function useOutsideEvent(type: string, ref: React.RefObject<Element>, callback: () => void): void {
+export default function useOutsideEvent(type: string, ref?: React.RefObject<Element>, callback?: () => void): void {
     React.useEffect(() => {
         function handleEvent(event: Event): void {
             const containsEventTarget = !!ref?.current?.contains(event.target as Node);
diff --git a/src/components/Popups/PopupHighlightError.tsx b/src/components/Popups/PopupHighlightError.tsx
index dbdafc638..2c6ee31fe 100644
--- a/src/components/Popups/PopupHighlightError.tsx
+++ b/src/components/Popups/PopupHighlightError.tsx
@@ -3,6 +3,7 @@ import noop from 'lodash/noop';
 import { FormattedMessage } from 'react-intl';
 import messages from './messages';
 import PopupBase from './PopupBase';
+import useOutsideEvent from '../../common/useOutsideEvent';
 import { Options } from './Popper';
 import { Shape } from '../../@types/model';
 import './PopupHighlightError.scss';
@@ -43,6 +44,7 @@ const options: Partial<Options> = {
 };
 
 export default function PopupHighlightError({ onCancel = noop, shape }: Props): JSX.Element {
+    const popupRef = React.useRef<PopupBase>(null);
     const { height, width, x, y } = shape;
 
     const reference = {
@@ -56,16 +58,10 @@ export default function PopupHighlightError({ onCancel = noop, shape }: Props):
         }),
     };
 
-    React.useEffect(() => {
-        document.addEventListener('mousedown', onCancel);
-
-        return () => {
-            document.removeEventListener('mousedown', onCancel);
-        };
-    }, [onCancel]);
+    useOutsideEvent('mousedown', popupRef.current?.popupRef, onCancel);
 
     return (
-        <PopupBase className="ba-PopupHighlightError" options={options} reference={reference}>
+        <PopupBase ref={popupRef} className="ba-PopupHighlightError" options={options} reference={reference}>
             <FormattedMessage {...messages.popupHighlightRestrictedPrompt} />
         </PopupBase>
     );

From 40d21d82d683b26a1a37432b5993377711828979 Mon Sep 17 00:00:00 2001
From: Mingze Xiao <mxiao@box.com>
Date: Tue, 6 Oct 2020 10:34:45 -0700
Subject: [PATCH 3/3] fix(highlight): Add tests

---
 src/common/__mocks__/useOutsideEvent.ts       |  5 ++++
 .../Popups/__tests__/PopupHighlight-test.tsx  | 22 ++++++++++++------
 .../__tests__/PopupHighlightError-test.tsx    | 23 +++++++++++++++++++
 .../__tests__/HighlightAnnotations-test.tsx   | 10 ++++++++
 4 files changed, 53 insertions(+), 7 deletions(-)
 create mode 100644 src/common/__mocks__/useOutsideEvent.ts

diff --git a/src/common/__mocks__/useOutsideEvent.ts b/src/common/__mocks__/useOutsideEvent.ts
new file mode 100644
index 000000000..36129d344
--- /dev/null
+++ b/src/common/__mocks__/useOutsideEvent.ts
@@ -0,0 +1,5 @@
+export default jest.fn((_type: string, _ref?: React.RefObject<Element>, callback?: () => void): void => {
+    if (callback) {
+        callback();
+    }
+});
diff --git a/src/components/Popups/__tests__/PopupHighlight-test.tsx b/src/components/Popups/__tests__/PopupHighlight-test.tsx
index a699020e0..23d04449b 100644
--- a/src/components/Popups/__tests__/PopupHighlight-test.tsx
+++ b/src/components/Popups/__tests__/PopupHighlight-test.tsx
@@ -2,9 +2,13 @@ import React from 'react';
 import { shallow, ShallowWrapper } from 'enzyme';
 import PopupBase from '../PopupBase';
 import PopupHighlight from '../PopupHighlight';
+import useOutsideEvent from '../../../common/useOutsideEvent';
+
+jest.mock('../../../common/useOutsideEvent');
 
 describe('PopupHighlight', () => {
     const defaults = {
+        onCancel: jest.fn(),
         onClick: jest.fn(),
         shape: {
             height: 10,
@@ -16,15 +20,19 @@ describe('PopupHighlight', () => {
 
     const getWrapper = (props = {}): ShallowWrapper => shallow(<PopupHighlight {...defaults} {...props} />);
 
-    const buttonEl = {
-        addEventListener: jest.fn(),
-        removeEventListener: jest.fn(),
-    };
+    const buttonRef = React.createRef();
 
     beforeEach(() => {
-        jest.spyOn(React, 'useRef').mockImplementationOnce(() => ({
-            current: buttonEl,
-        }));
+        jest.spyOn(React, 'useRef').mockImplementationOnce(() => buttonRef);
+    });
+
+    describe('event handlers', () => {
+        test('should call onCancel', () => {
+            getWrapper();
+
+            expect(defaults.onCancel).toHaveBeenCalled();
+            expect(useOutsideEvent).toHaveBeenCalledWith('mousedown', buttonRef, defaults.onCancel);
+        });
     });
 
     describe('mouse events', () => {
diff --git a/src/components/Popups/__tests__/PopupHighlightError-test.tsx b/src/components/Popups/__tests__/PopupHighlightError-test.tsx
index 55b1bfe03..726c155c8 100644
--- a/src/components/Popups/__tests__/PopupHighlightError-test.tsx
+++ b/src/components/Popups/__tests__/PopupHighlightError-test.tsx
@@ -3,9 +3,13 @@ import { FormattedMessage } from 'react-intl';
 import { shallow, ShallowWrapper } from 'enzyme';
 import PopupBase from '../PopupBase';
 import PopupHighlightError from '../PopupHighlightError';
+import useOutsideEvent from '../../../common/useOutsideEvent';
+
+jest.mock('../../../common/useOutsideEvent');
 
 describe('PopupHighlightError', () => {
     const defaults = {
+        onCancel: jest.fn(),
         shape: {
             height: 10,
             width: 0,
@@ -16,6 +20,25 @@ describe('PopupHighlightError', () => {
 
     const getWrapper = (props = {}): ShallowWrapper => shallow(<PopupHighlightError {...defaults} {...props} />);
 
+    const divRef = React.createRef();
+
+    beforeEach(() => {
+        jest.spyOn(React, 'useRef').mockImplementationOnce(() => ({
+            current: {
+                popupRef: divRef,
+            },
+        }));
+    });
+
+    describe('event handlers', () => {
+        test('should call onCancel', () => {
+            getWrapper();
+
+            expect(defaults.onCancel).toHaveBeenCalled();
+            expect(useOutsideEvent).toHaveBeenCalledWith('mousedown', divRef, defaults.onCancel);
+        });
+    });
+
     describe('render()', () => {
         test('should render correct rect and message', () => {
             const wrapper = getWrapper();
diff --git a/src/highlight/__tests__/HighlightAnnotations-test.tsx b/src/highlight/__tests__/HighlightAnnotations-test.tsx
index 612bcaa66..6767c7db8 100644
--- a/src/highlight/__tests__/HighlightAnnotations-test.tsx
+++ b/src/highlight/__tests__/HighlightAnnotations-test.tsx
@@ -29,6 +29,7 @@ describe('HighlightAnnotations', () => {
         setActiveAnnotationId: jest.fn(),
         setIsPromoting: jest.fn(),
         setReferenceShape: jest.fn(),
+        setSelection: jest.fn(),
         setStaged: jest.fn(),
         setStatus: jest.fn(),
         staged: null,
@@ -153,6 +154,15 @@ describe('HighlightAnnotations', () => {
         });
     });
 
+    describe('handleCancel()', () => {
+        test('should clear selection in store', () => {
+            const wrapper = getWrapper({ selection: selectionMock });
+            (wrapper.find(PopupHighlight).prop('onCancel') as Function)();
+
+            expect(defaults.setSelection).toHaveBeenCalledWith(null);
+        });
+    });
+
     describe('effects', () => {
         describe('Beginning a selection', () => {
             test('should reset staged and status if isSelecting is true', () => {