Skip to content

Commit 907e14b

Browse files
author
Conrad Chan
authored
refactor(resin): Move iscurrent and fileid to BaseManager (#642)
1 parent 8fc39c5 commit 907e14b

File tree

17 files changed

+154
-71
lines changed

17 files changed

+154
-71
lines changed

src/common/BaseManager.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
import * as ReactDOM from 'react-dom';
22
import { IntlShape } from 'react-intl';
33
import { Store } from 'redux';
4+
import { applyResinTags } from '../utils/resin';
5+
6+
export type ResinTags = Record<string, unknown>;
47

58
export type Options = {
69
location?: number;
710
referenceEl: HTMLElement;
11+
resinTags?: ResinTags;
812
};
913

1014
export type Props = {
@@ -24,9 +28,12 @@ export default class BaseManager implements Manager {
2428

2529
reactEl: HTMLElement;
2630

27-
constructor({ location = 1, referenceEl }: Options) {
31+
constructor({ location = 1, referenceEl, resinTags = {} }: Options) {
2832
this.location = location;
29-
this.reactEl = this.insert(referenceEl);
33+
this.reactEl = this.insert(referenceEl, {
34+
...resinTags,
35+
feature: 'annotations',
36+
});
3037

3138
this.decorate();
3239
}
@@ -45,15 +52,17 @@ export default class BaseManager implements Manager {
4552
return parentEl.contains(this.reactEl);
4653
}
4754

48-
insert(referenceEl: HTMLElement): HTMLElement {
55+
insert(referenceEl: HTMLElement, resinTags: ResinTags = {}): HTMLElement {
4956
// Find the nearest applicable reference and document elements
5057
const documentEl = referenceEl.ownerDocument || document;
5158
const parentEl = referenceEl.parentNode || documentEl;
5259

5360
// Construct a layer element where we can inject a root React component
5461
const rootLayerEl = documentEl.createElement('div');
5562
rootLayerEl.classList.add('ba-Layer');
56-
rootLayerEl.setAttribute('data-resin-feature', 'annotations');
63+
64+
// Apply any resin tags
65+
applyResinTags(rootLayerEl, resinTags);
5766

5867
// Insert the new layer element immediately after the reference element
5968
return parentEl.insertBefore(rootLayerEl, referenceEl.nextSibling);
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import BaseManager, { Options, Props } from '../BaseManager';
2+
3+
class TestBaseManager extends BaseManager {
4+
// eslint-disable-next-line @typescript-eslint/no-empty-function
5+
decorate(): void {}
6+
7+
// eslint-disable-next-line @typescript-eslint/no-unused-vars
8+
render(props: Props): void {} // eslint-disable-line @typescript-eslint/no-empty-function
9+
}
10+
11+
describe('BaseManager', () => {
12+
const rootEl = document.createElement('div');
13+
const getOptions = (options: Partial<Options> = {}): Options => ({
14+
referenceEl: rootEl.querySelector('.reference') as HTMLElement,
15+
...options,
16+
});
17+
const getWrapper = (options?: Partial<Options>): TestBaseManager => new TestBaseManager(getOptions(options));
18+
19+
beforeEach(() => {
20+
rootEl.classList.add('root');
21+
rootEl.innerHTML = '<div class="reference" />'; // referenceEl
22+
});
23+
24+
describe('constructor()', () => {
25+
test('should add rootLayerEl', () => {
26+
const wrapper = getWrapper();
27+
expect(wrapper.reactEl.classList.contains('ba-Layer')).toBe(true);
28+
expect(wrapper.reactEl.getAttribute('data-resin-feature')).toBe('annotations');
29+
});
30+
31+
test('should add rootLayerEl with custom resin tags', () => {
32+
const resinTags = {
33+
foo: 'bar',
34+
};
35+
const wrapper = getWrapper({ resinTags });
36+
expect(wrapper.reactEl.classList.contains('ba-Layer')).toBe(true);
37+
expect(wrapper.reactEl.getAttribute('data-resin-feature')).toBe('annotations');
38+
expect(wrapper.reactEl.getAttribute('data-resin-foo')).toBe('bar');
39+
});
40+
});
41+
});

src/components/ItemList/ItemRow.tsx

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
import * as React from 'react';
2-
import * as ReactRedux from 'react-redux';
32
import classNames from 'classnames';
43
import { Collaborator } from '../../@types';
5-
import { getFileId, getIsCurrentFileVersion } from '../../store';
64
import './ItemRow.scss';
75

86
export type Props = {
@@ -18,8 +16,6 @@ export type ItemRowRef = HTMLLIElement;
1816

1917
const ItemRow = (props: Props, ref: React.Ref<ItemRowRef>): JSX.Element | null => {
2018
const { className, isActive, item: collaborator, onClick, onMouseDown, onMouseEnter } = props;
21-
const fileId = ReactRedux.useSelector(getFileId);
22-
const isCurrentFileVersion = ReactRedux.useSelector(getIsCurrentFileVersion);
2319

2420
if (!collaborator || !collaborator.item || !collaborator.item.name) {
2521
return null;
@@ -33,8 +29,6 @@ const ItemRow = (props: Props, ref: React.Ref<ItemRowRef>): JSX.Element | null =
3329
ref={ref}
3430
aria-selected={isActive}
3531
className={classNames(className, 'ba-ItemRow', { 'is-active': isActive })}
36-
data-resin-fileid={fileId}
37-
data-resin-iscurrent={isCurrentFileVersion}
3832
data-resin-target="atMention"
3933
data-testid="ba-ItemRow"
4034
onClick={onClick}

src/components/ItemList/__tests__/ItemRow-test.tsx

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import * as React from 'react';
2-
import * as ReactRedux from 'react-redux';
32
import { shallow, ShallowWrapper } from 'enzyme';
43
import { UserMini } from '../../../@types';
54
import ItemRow, { Props } from '../ItemRow';
@@ -16,12 +15,6 @@ describe('ItemRow', () => {
1615

1716
const getWrapper = (props = {}): ShallowWrapper => shallow(<ItemRow {...defaults} {...props} />);
1817

19-
let reduxSpy: jest.SpyInstance;
20-
21-
beforeEach(() => {
22-
reduxSpy = jest.spyOn(ReactRedux, 'useSelector').mockImplementation(() => true);
23-
});
24-
2518
describe('render()', () => {
2619
test('should not render anything if no item name', () => {
2720
const wrapper = getWrapper({ item: {} });
@@ -43,16 +36,9 @@ describe('ItemRow', () => {
4336
});
4437

4538
test('should add resin tags', () => {
46-
// mock fileId
47-
reduxSpy.mockReturnValueOnce('0');
48-
// mock isCurrentFileVersion
49-
reduxSpy.mockReturnValueOnce(true);
50-
5139
const wrapper = getWrapper();
5240

5341
expect(wrapper.props()).toMatchObject({
54-
'data-resin-fileid': '0',
55-
'data-resin-iscurrent': true,
5642
'data-resin-target': 'atMention',
5743
});
5844
});

src/components/ReplyForm/ReplyForm.tsx

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export type Props = ReplyFormProps &
2323
Pick<FormikProps<FormValues>, 'errors' | 'setFieldValue' | 'values'>;
2424

2525
const ReplyForm = (props: Props): JSX.Element => {
26-
const { errors, fileId, isCurrentFileVersion, isPending, onCancel, onChange, setFieldValue, values } = props;
26+
const { errors, isPending, onCancel, onChange, setFieldValue, values } = props;
2727

2828
const formRef = React.useRef<HTMLFormElement>(null);
2929
const intl = useIntl();
@@ -83,8 +83,6 @@ const ReplyForm = (props: Props): JSX.Element => {
8383
</div>
8484
<div className="ba-Popup-footer">
8585
<ReplyButton
86-
data-resin-fileid={fileId}
87-
data-resin-iscurrent={isCurrentFileVersion}
8886
data-resin-target="cancel"
8987
data-testid="ba-Popup-cancel"
9088
isDisabled={isPending}
@@ -94,8 +92,6 @@ const ReplyForm = (props: Props): JSX.Element => {
9492
<FormattedMessage {...messages.buttonCancel} />
9593
</ReplyButton>
9694
<ReplyButton
97-
data-resin-fileid={fileId}
98-
data-resin-iscurrent={isCurrentFileVersion}
9995
data-resin-target="post"
10096
data-testid="ba-Popup-submit"
10197
isDisabled={hasErrors || isPending}

src/components/ReplyForm/ReplyFormContainer.tsx

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,10 @@ import { FormikBag, withFormik } from 'formik';
55
import { connect } from 'react-redux';
66
import ReplyForm, { ReplyFormProps } from './ReplyForm';
77
import withMentionDecorator from '../ReplyField/withMentionDecorator';
8-
import { AppState, getCreatorCursor, getFileId, getIsCurrentFileVersion } from '../../store';
8+
import { AppState, getCreatorCursor } from '../../store';
99

1010
export type PropsFromState = {
1111
cursorPosition: number;
12-
fileId: string | null;
13-
isCurrentFileVersion: boolean;
1412
};
1513

1614
type Props = ReplyFormProps & PropsFromState;
@@ -27,8 +25,6 @@ const MAX_LENGTH = 10000;
2725

2826
export const mapStateToProps = (state: AppState): PropsFromState => ({
2927
cursorPosition: getCreatorCursor(state),
30-
fileId: getFileId(state),
31-
isCurrentFileVersion: getIsCurrentFileVersion(state),
3228
});
3329

3430
export const mapPropsToErrors = (): FormErrors => ({ editorState: 'initial' });

src/components/ReplyForm/__tests__/ReplyForm-test.tsx

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@ describe('ReplyForm', () => {
1616
const defaults: Props = {
1717
cursorPosition: 0,
1818
errors: {},
19-
fileId: '0',
20-
isCurrentFileVersion: true,
2119
isPending: false,
2220
onCancel: jest.fn(),
2321
onChange: jest.fn(),
@@ -61,17 +59,10 @@ describe('ReplyForm', () => {
6159
const cancelButton = wrapper.find('[data-testid="ba-Popup-cancel"]');
6260
const postButton = wrapper.find('[data-testid="ba-Popup-submit"]');
6361

64-
const resinTags = {
65-
'data-resin-fileid': defaults.fileId,
66-
'data-resin-iscurrent': defaults.isCurrentFileVersion,
67-
};
68-
6962
expect(cancelButton.props()).toMatchObject({
70-
...resinTags,
7163
'data-resin-target': 'cancel',
7264
});
7365
expect(postButton.props()).toMatchObject({
74-
...resinTags,
7566
'data-resin-target': 'post',
7667
});
7768
});

src/components/ReplyForm/__tests__/ReplyFormContainer-test.tsx

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ jest.mock('../../../store');
77
describe('ReplyFormContainer', () => {
88
const defaults = {
99
cursorPosition: 0,
10-
fileId: '0',
11-
isCurrentFileVersion: true,
1210
isPending: false,
1311
onCancel: jest.fn(),
1412
onChange: jest.fn(),
@@ -19,8 +17,6 @@ describe('ReplyFormContainer', () => {
1917
test('should set props', () => {
2018
expect(mapStateToProps({} as AppState)).toEqual({
2119
cursorPosition: 1,
22-
fileId: '0',
23-
isCurrentFileVersion: true,
2420
});
2521
});
2622
});

src/document/DocumentAnnotator.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { Event } from '../@types';
1313
import { getAnnotation } from '../store/annotations';
1414
import { getSelection } from './docUtil';
1515
import { Manager } from '../common/BaseManager';
16-
import { Mode } from '../store';
16+
import { getFileId, getIsCurrentFileVersion, Mode } from '../store';
1717
import { scrollToLocation } from '../utils/scroll';
1818
import './DocumentAnnotator.scss';
1919

@@ -54,9 +54,12 @@ export default class DocumentAnnotator extends BaseAnnotator {
5454
}
5555

5656
getPageManagers(pageEl: HTMLElement): Set<Manager> {
57+
const fileId = getFileId(this.store.getState());
58+
const isCurrentFileVersion = getIsCurrentFileVersion(this.store.getState());
5759
const pageNumber = this.getPageNumber(pageEl);
5860
const pageReferenceEl = this.getPageReference(pageEl);
5961
const managers = this.managers.get(pageNumber) || new Set();
62+
const resinTags = { fileid: fileId, iscurrent: isCurrentFileVersion };
6063

6164
// Destroy any managers that were attached to page elements that no longer exist
6265
managers.forEach(manager => {
@@ -68,10 +71,10 @@ export default class DocumentAnnotator extends BaseAnnotator {
6871

6972
// Lazily instantiate managers as pages are added or re-rendered
7073
if (managers.size === 0) {
71-
managers.add(new PopupManager({ location: pageNumber, referenceEl: pageReferenceEl }));
74+
managers.add(new PopupManager({ location: pageNumber, referenceEl: pageReferenceEl, resinTags }));
7275

7376
if (this.isFeatureEnabled('drawing')) {
74-
managers.add(new DrawingManager({ location: pageNumber, referenceEl: pageReferenceEl }));
77+
managers.add(new DrawingManager({ location: pageNumber, referenceEl: pageReferenceEl, resinTags }));
7578
}
7679

7780
const textLayer = pageEl.querySelector('.textLayer') as HTMLElement;
@@ -87,15 +90,15 @@ export default class DocumentAnnotator extends BaseAnnotator {
8790
);
8891
}
8992

90-
managers.add(new HighlightManager({ location: pageNumber, referenceEl: pageReferenceEl }));
93+
managers.add(new HighlightManager({ location: pageNumber, referenceEl: pageReferenceEl, resinTags }));
9194

92-
managers.add(new RegionManager({ location: pageNumber, referenceEl: pageReferenceEl }));
95+
managers.add(new RegionManager({ location: pageNumber, referenceEl: pageReferenceEl, resinTags }));
9396

9497
const canvasLayerEl = pageEl.querySelector<HTMLElement>('.canvasWrapper');
9598
const referenceEl =
9699
this.isFeatureEnabled('discoverability') && canvasLayerEl ? canvasLayerEl : pageReferenceEl;
97100

98-
managers.add(new RegionCreationManager({ location: pageNumber, referenceEl }));
101+
managers.add(new RegionCreationManager({ location: pageNumber, referenceEl, resinTags }));
99102
}
100103

101104
return managers;

src/drawing/DrawingAnnotations.tsx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,11 @@ import './DrawingAnnotations.scss';
66
export type Props = {
77
activeAnnotationId: string | null;
88
annotations: AnnotationDrawing[];
9-
isCurrentFileVersion: boolean;
109
setActiveAnnotationId: (annotationId: string | null) => void;
1110
};
1211

1312
const DrawingAnnotations = (props: Props): JSX.Element => {
14-
const { activeAnnotationId, annotations, isCurrentFileVersion, setActiveAnnotationId } = props;
13+
const { activeAnnotationId, annotations, setActiveAnnotationId } = props;
1514

1615
const handleAnnotationActive = (annotationId: string | null): void => {
1716
setActiveAnnotationId(annotationId);
@@ -22,7 +21,6 @@ const DrawingAnnotations = (props: Props): JSX.Element => {
2221
activeId={activeAnnotationId}
2322
annotations={annotations}
2423
className="ba-DrawingAnnotations-list"
25-
data-resin-iscurrent={isCurrentFileVersion}
2624
onSelect={handleAnnotationActive}
2725
/>
2826
);

0 commit comments

Comments
 (0)