Skip to content

Commit

Permalink
fix(highlight): Remove selection when clicking anywhere outside of po…
Browse files Browse the repository at this point in the history
…pup (#615)

* fix(highlight): Remove selection when clicking anywhere outside of popup

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
Mingze and mergify[bot] authored Oct 6, 2020
1 parent 044b0f0 commit 403d962
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 14 deletions.
5 changes: 5 additions & 0 deletions src/common/__mocks__/useOutsideEvent.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default jest.fn((_type: string, _ref?: React.RefObject<Element>, callback?: () => void): void => {
if (callback) {
callback();
}
});
2 changes: 1 addition & 1 deletion src/common/useOutsideEvent.ts
Original file line number Diff line number Diff line change
@@ -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);
Expand Down
6 changes: 5 additions & 1 deletion src/components/Popups/PopupHighlight.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down Expand Up @@ -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;

Expand All @@ -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
Expand Down
10 changes: 8 additions & 2 deletions src/components/Popups/PopupHighlightError.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import React from 'react';
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';

export type Props = {
onCancel?: () => void;
shape: Shape;
};

Expand Down Expand Up @@ -40,7 +43,8 @@ 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 popupRef = React.useRef<PopupBase>(null);
const { height, width, x, y } = shape;

const reference = {
Expand All @@ -54,8 +58,10 @@ export default function PopupHighlightError({ shape }: Props): JSX.Element {
}),
};

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>
);
Expand Down
22 changes: 15 additions & 7 deletions src/components/Popups/__tests__/PopupHighlight-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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', () => {
Expand Down
23 changes: 23 additions & 0 deletions src/components/Popups/__tests__/PopupHighlightError-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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();
Expand Down
16 changes: 13 additions & 3 deletions src/highlight/HighlightAnnotations.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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;
Expand All @@ -40,6 +41,7 @@ const HighlightAnnotations = (props: Props): JSX.Element => {
setActiveAnnotationId,
setIsPromoting,
setReferenceShape,
setSelection,
setStaged,
setStatus,
staged,
Expand Down Expand Up @@ -76,6 +78,10 @@ const HighlightAnnotations = (props: Props): JSX.Element => {
setIsPromoting(true);
};

const handleCancel = (): void => {
setSelection(null);
};

React.useEffect(() => {
if (!isSelecting) {
return;
Expand Down Expand Up @@ -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>
)}
</>
Expand Down
2 changes: 2 additions & 0 deletions src/highlight/HighlightContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
setActiveAnnotationIdAction,
setIsPromotingAction,
setReferenceShapeAction,
setSelectionAction,
setStagedAction,
setStatusAction,
} from '../store';
Expand Down Expand Up @@ -51,6 +52,7 @@ export const mapDispatchToProps = {
setActiveAnnotationId: setActiveAnnotationIdAction,
setIsPromoting: setIsPromotingAction,
setReferenceShape: setReferenceShapeAction,
setSelection: setSelectionAction,
setStaged: setStagedAction,
setStatus: setStatusAction,
};
Expand Down
10 changes: 10 additions & 0 deletions src/highlight/__tests__/HighlightAnnotations-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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', () => {
Expand Down

0 comments on commit 403d962

Please sign in to comment.