Skip to content

Commit 8ceec85

Browse files
mickrmickr
authored
feat(annotations): Move annotator event listener (#2087)
* feat(annotations): Move annotator event listener * Move event listener set up to withAnnotations * Update ContentPreview to only listen for annotator event * Update tests and types * Add eslint rule for explicit-any on rest args only
1 parent e132dd9 commit 8ceec85

File tree

5 files changed

+33
-20
lines changed

5 files changed

+33
-20
lines changed

‎src/elements/common/annotator-context/__tests__/withAnnotations.test.tsx‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import React, { Component } from 'react';
22
import { shallow, ShallowWrapper } from 'enzyme';
33

4-
import { AnnotatorContext, withAnnotations } from '../index';
4+
import { Annotator, AnnotatorContext, withAnnotations } from '../index';
55
import { WithAnnotationsProps, ComponentWithAnnotations } from '../withAnnotations';
66
import { AnnotatorContext as AnnotatorContextType, Action } from '../types';
77

@@ -16,14 +16,13 @@ type ContextProviderProps = {
1616
};
1717

1818
describe('elements/common/annotator-context/withAnnotations', () => {
19+
let mockAnnotator = {} as Annotator;
1920
const MockComponent = (props: ComponentProps) => <div {...props} />;
20-
2121
const WrappedComponent = withAnnotations(MockComponent);
2222

2323
const defaultProps = {
2424
className: 'foo',
2525
onAnnotator: jest.fn(),
26-
onAnnotatorEvent: jest.fn(),
2726
onPreviewDestroy: jest.fn(),
2827
};
2928

@@ -36,13 +35,21 @@ describe('elements/common/annotator-context/withAnnotations', () => {
3635
wrapper: ShallowWrapper<WrappedComponentProps, {}, Component & ComponentWithAnnotations>,
3736
) => wrapper.find<ContextProviderProps>(AnnotatorContext.Provider);
3837

39-
test('should pass onAnnotatorEvent and onPreviewDestroy as props on the wrapped component', () => {
38+
beforeEach(() => {
39+
mockAnnotator = {
40+
addListener: jest.fn(),
41+
emit: jest.fn(),
42+
removeAllListeners: jest.fn(),
43+
removeListener: jest.fn(),
44+
};
45+
});
46+
47+
test('should pass onAnnotator and onPreviewDestroy as props on the wrapped component', () => {
4048
const wrapper = getWrapper();
4149

4250
const wrappedComponent = wrapper.find<WrappedComponentProps>(MockComponent);
4351
expect(wrappedComponent.exists()).toBeTruthy();
4452
expect(wrappedComponent.props().onAnnotator).toBeTruthy();
45-
expect(wrappedComponent.props().onAnnotatorEvent).toBeTruthy();
4653
expect(wrappedComponent.props().onPreviewDestroy).toBeTruthy();
4754
});
4855

@@ -63,14 +70,11 @@ describe('elements/common/annotator-context/withAnnotations', () => {
6370

6471
describe('emitActiveChangeEvent', () => {
6572
test('should call annotator emit on action', () => {
66-
const mockAnnotator = {
67-
emit: jest.fn(),
68-
};
6973
const wrapper = getWrapper();
7074
const instance = wrapper.instance();
7175

7276
// Set the annotator on the withAnnotations instance
73-
instance.handleOnAnnotator(mockAnnotator);
77+
instance.handleAnnotator(mockAnnotator);
7478
instance.emitActiveChangeEvent('123');
7579

7680
expect(mockAnnotator.emit).toBeCalled();
@@ -153,13 +157,15 @@ describe('elements/common/annotator-context/withAnnotations', () => {
153157
test('should reset state and annotator', () => {
154158
const wrapper = getWrapper();
155159
const instance = wrapper.instance();
160+
instance.handleAnnotator(mockAnnotator);
156161
instance.handleActiveChange('123');
157162
let contextProvider = getContextProvider(wrapper);
158163
expect(contextProvider.prop('value').state.activeAnnotationId).toEqual('123');
159164

160165
wrapper.instance().handlePreviewDestroy();
161166
contextProvider = getContextProvider(wrapper);
162167
expect(contextProvider.prop('value').state.activeAnnotationId).toEqual(null);
168+
expect(mockAnnotator.removeListener).toBeCalledWith('annotatorevent', instance.handleAnnotatorEvent);
163169
});
164170
});
165171
});

‎src/elements/common/annotator-context/types.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,14 @@ export enum Action {
66
// Can extend to other actions: update_start, update_end, delete_start, delete_end
77
}
88

9+
/* eslint-disable @typescript-eslint/no-explicit-any */
910
export interface Annotator {
10-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
11+
addListener: (event: string | symbol, listener: (...args: any[]) => void) => void;
1112
emit: (event: string | symbol, ...args: any[]) => void;
13+
removeAllListeners: () => void;
14+
removeListener: (event: string | symbol, listener: (...args: any[]) => void) => void;
1215
}
16+
/* eslint-enable @typescript-eslint/no-explicit-any */
1317

1418
export interface AnnotatorState {
1519
activeAnnotationId?: string | null;

‎src/elements/common/annotator-context/withAnnotations.tsx‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import { Action, Annotator, AnnotationActionEvent, AnnotatorState, Status } from
44

55
export interface WithAnnotationsProps {
66
onAnnotator: (annotator: Annotator) => void;
7-
onAnnotatorEvent: ({ event, data }: { event: string; data: unknown }) => void;
87
onPreviewDestroy: () => void;
98
}
109

@@ -14,8 +13,8 @@ export interface ComponentWithAnnotations {
1413
handleActiveChange: (annotationId: string | null) => void;
1514
handleAnnotationChangeEvent: (id: string | null) => void;
1615
handleAnnotationCreate: (eventData: AnnotationActionEvent) => void;
16+
handleAnnotator: (annotator: Annotator) => void;
1717
handleAnnotatorEvent: ({ event, data }: { event: string; data?: unknown }) => void;
18-
handleOnAnnotator: (annotator: Annotator) => void;
1918
handlePreviewDestroy: () => void;
2019
}
2120

@@ -75,12 +74,18 @@ export default function withAnnotations<P extends object>(
7574
}
7675
};
7776

78-
handleOnAnnotator = (annotator: Annotator): void => {
77+
handleAnnotator = (annotator: Annotator): void => {
7978
this.annotator = annotator;
79+
this.annotator.addListener('annotatorevent', this.handleAnnotatorEvent);
8080
};
8181

8282
handlePreviewDestroy = (): void => {
8383
this.setState(defaultState);
84+
85+
if (this.annotator) {
86+
this.annotator.removeListener('annotatorevent', this.handleAnnotatorEvent);
87+
}
88+
8489
this.annotator = null;
8590
};
8691

@@ -91,8 +96,7 @@ export default function withAnnotations<P extends object>(
9196
>
9297
<WrappedComponent
9398
{...this.props}
94-
onAnnotator={this.handleOnAnnotator}
95-
onAnnotatorEvent={this.handleAnnotatorEvent}
99+
onAnnotator={this.handleAnnotator}
96100
onPreviewDestroy={this.handlePreviewDestroy}
97101
/>
98102
</AnnotatorContext.Provider>

‎src/elements/content-preview/ContentPreview.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -763,7 +763,6 @@ class ContentPreview extends React.PureComponent<Props, State> {
763763
this.preview.addListener('thumbnailsClose', () => this.setState({ isThumbnailSidebarOpen: false }));
764764

765765
if (showAnnotationsControls) {
766-
this.preview.addListener('annotatorevent', onAnnotatorEvent);
767766
this.preview.addListener('annotator', onAnnotator);
768767
}
769768

‎src/elements/content-preview/__tests__/ContentPreview.test.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -322,8 +322,8 @@ describe('elements/content-preview/ContentPreview', () => {
322322
`(
323323
'should call onAnnotationCreate $called if showAnnotationsControls is $showAnnotationsControls',
324324
async ({ called, showAnnotationsControls }) => {
325-
const onAnnotatorEvent = jest.fn();
326-
const wrapper = getWrapper({ ...props, showAnnotationsControls, onAnnotatorEvent });
325+
const onAnnotator = jest.fn();
326+
const wrapper = getWrapper({ ...props, showAnnotationsControls, onAnnotator });
327327

328328
wrapper.setState({ file });
329329

@@ -332,9 +332,9 @@ describe('elements/content-preview/ContentPreview', () => {
332332
await instance.loadPreview();
333333

334334
if (called) {
335-
expect(instance.preview.addListener).toHaveBeenCalledWith('annotatorevent', onAnnotatorEvent);
335+
expect(instance.preview.addListener).toHaveBeenCalledWith('annotator', onAnnotator);
336336
} else {
337-
expect(instance.preview.addListener).not.toHaveBeenCalledWith('annotatorevent', onAnnotatorEvent);
337+
expect(instance.preview.addListener).not.toHaveBeenCalledWith('annotator', onAnnotator);
338338
}
339339
},
340340
);

0 commit comments

Comments
 (0)