Skip to content

Commit

Permalink
refactor: useMountId hook for uuid (#659)
Browse files Browse the repository at this point in the history
  • Loading branch information
Conrad Chan authored Dec 17, 2020
1 parent f445cef commit d67fef3
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 49 deletions.
7 changes: 7 additions & 0 deletions src/common/__mocks__/useMountId.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function useMountId(callback?: (uuid: string) => void): string {
if (callback) {
callback('123');
}

return '123';
}
25 changes: 25 additions & 0 deletions src/common/__tests__/useMountId-test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import * as React from 'react';
import { mount, ReactWrapper } from 'enzyme';
import useMountId from '../useMountId';

jest.mock('uuid', () => ({
v4: () => '123',
}));

describe('useMountId', () => {
function TestComponent({ onMount }: { onMount: (uuid: string) => void }): JSX.Element {
const uuid = useMountId(onMount);

return <div data-testid="uuid" data-uuid={uuid} />;
}

const getWrapper = (onMount: (uuid: string) => void): ReactWrapper => mount(<TestComponent onMount={onMount} />);

test('should render the component with a generated uuid and call the callback with the generated uuid', () => {
const onMount = jest.fn();
const wrapper = getWrapper(onMount);

expect(wrapper.find('[data-testid="uuid"]').prop('data-uuid')).toEqual('123');
expect(onMount).toHaveBeenCalledWith('123');
});
});
14 changes: 14 additions & 0 deletions src/common/useMountId.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import React from 'react';
import * as uuid from 'uuid';

// Returns a generated uuid, then calls the provided callback via an effect
export default function useMountId(callback: (uuid: string) => void): string {
const uuidRef = React.useRef<string>(uuid.v4());

// The callback is only invoked after the parent component has rendered
React.useEffect(() => {
callback(uuidRef.current);
}, [callback]);

return uuidRef.current;
}
10 changes: 3 additions & 7 deletions src/drawing/DrawingSVGGroup.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';
import noop from 'lodash/noop';
import * as uuid from 'uuid';
import useMountId from '../common/useMountId';

export type Props = React.SVGAttributes<SVGGElement> & {
children?: React.ReactNode;
Expand All @@ -11,14 +11,10 @@ export type DrawingSVGGroup = SVGGElement;

export function DrawingSVGGroup(props: Props, ref: React.Ref<DrawingSVGGroup>): JSX.Element {
const { children, onMount = noop, ...rest } = props;
const uuidRef = React.useRef<string>(uuid.v4());

React.useEffect(() => {
onMount(uuidRef.current);
}, [onMount]);
const uuid = useMountId(onMount);

return (
<g ref={ref} data-ba-reference-id={uuidRef.current} {...rest}>
<g ref={ref} data-ba-reference-id={uuid} {...rest}>
{children}
</g>
);
Expand Down
8 changes: 2 additions & 6 deletions src/drawing/__tests__/DrawingSVGGroup-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,14 @@ import React from 'react';
import { shallow, ShallowWrapper } from 'enzyme';
import DrawingSVGGroup, { Props } from '../DrawingSVGGroup';

jest.mock('../../common/useMountId');

describe('DrawingSVGGroup', () => {
const getDefaults = (): Props => ({
onMount: jest.fn(),
});
const getWrapper = (props: Partial<Props>): ShallowWrapper =>
shallow(<DrawingSVGGroup {...getDefaults()} {...props} />);
const uuidRef = { current: '123' };

beforeEach(() => {
jest.spyOn(React, 'useRef').mockImplementationOnce(() => uuidRef);
jest.spyOn(React, 'useEffect').mockImplementation(f => f());
});

describe('render', () => {
test('should render with provided props', () => {
Expand Down
10 changes: 3 additions & 7 deletions src/highlight/HighlightTarget.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import * as uuid from 'uuid';
import classNames from 'classnames';
import noop from 'lodash/noop';
import useMountId from '../common/useMountId';
import { MOUSE_PRIMARY } from '../constants';
import { Rect } from '../@types/model';
import './HighlightTarget.scss';
Expand All @@ -20,7 +20,7 @@ export type HighlightTargetRef = HTMLAnchorElement;

const HighlightTarget = (props: Props, ref: React.Ref<HighlightTargetRef>): JSX.Element => {
const { annotationId, className, isActive, onHover = noop, onMount = noop, onSelect = noop, shapes } = props;
const uuidRef = React.useRef<string>(uuid.v4());
const uuid = useMountId(onMount);

const handleClick = (event: React.MouseEvent<HighlightTargetRef>): void => {
// These are needed to prevent the anchor link from being followed and updating the url location
Expand Down Expand Up @@ -65,16 +65,12 @@ const HighlightTarget = (props: Props, ref: React.Ref<HighlightTargetRef>): JSX.
}
};

React.useEffect(() => {
onMount(uuidRef.current);
}, [onMount]);

return (
// eslint-disable-next-line jsx-a11y/anchor-is-valid
<a
ref={ref}
className={classNames('ba-HighlightTarget', className, { 'is-active': isActive })}
data-ba-reference-id={uuidRef.current}
data-ba-reference-id={uuid}
data-resin-itemid={annotationId}
data-resin-target="highlightText"
data-testid={`ba-AnnotationTarget-${annotationId}`}
Expand Down
17 changes: 3 additions & 14 deletions src/highlight/__tests__/HighlightTarget-test.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
import React, { MutableRefObject } from 'react';
import * as ReactRedux from 'react-redux';
import React from 'react';
import { shallow, ShallowWrapper } from 'enzyme';
import HighlightTarget from '../HighlightTarget';

jest.mock('react', () => ({
...jest.requireActual('react'),
useState: jest.fn(),
}));
jest.mock('../../common/useMountId');

describe('HighlightTarget', () => {
const defaults = {
Expand All @@ -25,13 +21,6 @@ describe('HighlightTarget', () => {
};

const getWrapper = (props = {}): ShallowWrapper => shallow(<HighlightTarget {...defaults} {...props} />);
const mockInitalRef = { current: '123' } as MutableRefObject<string>;

beforeEach(() => {
jest.spyOn(React, 'useEffect').mockImplementation(f => f());
jest.spyOn(React, 'useRef').mockImplementation(() => mockInitalRef);
jest.spyOn(ReactRedux, 'useSelector').mockImplementation(() => true);
});

describe('render()', () => {
test('should render anchor with provided rects', () => {
Expand Down Expand Up @@ -160,7 +149,7 @@ describe('HighlightTarget', () => {
const handleMount = jest.fn();
getWrapper({ onMount: handleMount });

expect(handleMount).toHaveBeenCalledWith(expect.any(String));
expect(handleMount).toHaveBeenCalledWith('123');
});
});
});
10 changes: 3 additions & 7 deletions src/region/RegionRect.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import * as uuid from 'uuid';
import noop from 'lodash/noop';
import classNames from 'classnames';
import useMountId from '../common/useMountId';
import { Shape } from '../@types';
import { styleShape } from './regionUtil';
import './RegionRect.scss';
Expand All @@ -16,18 +16,14 @@ type Props = {
export type RegionRectRef = HTMLDivElement;

export function RegionRect(props: Props, ref: React.Ref<RegionRectRef>): JSX.Element {
const uuidRef = React.useRef<string>(uuid.v4());
const { className, isActive, onMount = noop, shape } = props;

React.useEffect(() => {
onMount(uuidRef.current);
}, [onMount]);
const uuid = useMountId(onMount);

return (
<div
ref={ref}
className={classNames('ba-RegionRect', className, { 'is-active': isActive })}
data-ba-reference-id={uuidRef.current}
data-ba-reference-id={uuid}
style={styleShape(shape)}
/>
);
Expand Down
12 changes: 4 additions & 8 deletions src/region/__tests__/RegionRect-test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { MutableRefObject } from 'react';
import React from 'react';
import { shallow, ShallowWrapper } from 'enzyme';
import RegionRect from '../RegionRect';
import { styleShape } from '../regionUtil';
Expand All @@ -7,14 +7,10 @@ jest.mock('../regionUtil', () => ({
styleShape: jest.fn(value => value),
}));

jest.mock('../../common/useMountId');

describe('RegionRect', () => {
const getWrapper = (props = {}): ShallowWrapper => shallow(<RegionRect {...props} />);
const mockInitalRef = { current: '123' } as MutableRefObject<string>;

beforeEach(() => {
jest.spyOn(React, 'useEffect').mockImplementation(f => f());
jest.spyOn(React, 'useRef').mockImplementation(() => mockInitalRef);
});

describe('render', () => {
test('should call styleShape with the provided shape prop value', () => {
Expand All @@ -39,7 +35,7 @@ describe('RegionRect', () => {
const handleMount = jest.fn();
getWrapper({ onMount: handleMount });

expect(handleMount).toHaveBeenCalledWith(expect.any(String));
expect(handleMount).toHaveBeenCalledWith('123');
});
});
});

0 comments on commit d67fef3

Please sign in to comment.