Skip to content

Commit

Permalink
feat(scroll): emit 'annotations_initialized' when annotations first i…
Browse files Browse the repository at this point in the history
…nitializes. (#515)

* feat(scroll): add method to return annotation by Id

* feat(scroll): add method to return annotation by Id

* accommodate changes in content-preview to send ready event with annotations once annotations are loaded.

* feat(scroll): add method to return annotation by Id

* PR Feedback

* feat(scroll): add method to return annotation by Id

* Refactor to emit 'annotations_initialized' using eventing middleware

* feat(scroll): add method to return annotation by Id

* Remove un-used class variable

* feat(scroll): add method to return annotation by Id

* PR Feedback

* feat(scroll): add method to return annotation by Id

* PR Feedback

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
mickr and mergify[bot] authored Jun 16, 2020
1 parent 28e2e1d commit 6021585
Show file tree
Hide file tree
Showing 14 changed files with 95 additions and 13 deletions.
5 changes: 3 additions & 2 deletions src/@types/events.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
enum Event {
ACTIVE_CHANGE = 'annotations_active_change',
ACTIVE_SET = 'annotations_active_set',
ANNOTATION_CREATE = 'annotations_create',
ANNOTATION_FETCH_ERROR = 'annotations_fetch_error',
ANNOTATIONS_INITIALIZED = 'annotations_initialized',
ANNOTATION_REMOVE = 'annotations_remove',
ACTIVE_CHANGE = 'annotations_active_change',
ACTIVE_SET = 'annotations_active_set',
VISIBLE_SET = 'annotations_visible_set',
}

Expand Down
7 changes: 7 additions & 0 deletions src/common/BaseAnnotator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import EventEmitter from './EventEmitter';
import i18n from '../utils/i18n';
import messages from '../messages';
import { Event, IntlOptions, LegacyEvent, Permissions } from '../@types';
import { getIsInitialized, setIsInitialized } from '../store';
import './BaseAnnotator.scss';

export type Container = string | HTMLElement;
Expand Down Expand Up @@ -151,4 +152,10 @@ export default class BaseAnnotator extends EventEmitter {
this.store.dispatch<any>(store.fetchAnnotationsAction()); // eslint-disable-line @typescript-eslint/no-explicit-any
this.store.dispatch<any>(store.fetchCollaboratorsAction()); // eslint-disable-line @typescript-eslint/no-explicit-any
}

protected handleInitialized(): void {
if (!getIsInitialized(this.store.getState())) {
this.store.dispatch(setIsInitialized());
}
}
}
3 changes: 2 additions & 1 deletion src/document/DocumentAnnotator.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import BaseAnnotator from '../common/BaseAnnotator';
import BaseManager from '../common/BaseManager';
import { CLASS_ANNOTATIONS_LOADED } from '../constants';
import { getAnnotation } from '../store/annotations';
import { centerRegion, isRegion, RegionManager } from '../region';
import { getAnnotation } from '../store/annotations';
import './DocumentAnnotator.scss';

export const SCROLL_THRESHOLD = 1000; // pixels
Expand Down Expand Up @@ -63,6 +63,7 @@ export default class DocumentAnnotator extends BaseAnnotator {
this.annotatedEl.classList.add(CLASS_ANNOTATIONS_LOADED);

this.render();
this.handleInitialized();
}

render(): void {
Expand Down
25 changes: 18 additions & 7 deletions src/document/__tests__/DocumentAnnotator-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ describe('DocumentAnnotator', () => {
const getPage = (pageNumber = 1): HTMLElement => {
return container.querySelector(`[data-page-number="${pageNumber}"]`) as HTMLElement;
};

const payload = {
entries: regions as Annotation[],
limit: 1000,
next_marker: null,
previous_marker: null,
};

let annotator = getAnnotator();

beforeEach(() => {
Expand Down Expand Up @@ -135,6 +143,16 @@ describe('DocumentAnnotator', () => {
expect(annotator.annotatedEl).toBe(container.querySelector('.bp-doc'));
expect(annotator.annotatedEl!.className).toContain(CLASS_ANNOTATIONS_LOADED); // eslint-disable-line @typescript-eslint/no-non-null-assertion
});

test('should dispatch once on first init', () => {
const mockDispatch = jest.spyOn(annotator.store, 'dispatch');

annotator.init(1);
annotator.init(1);
annotator.init(1);

expect(mockDispatch).toBeCalledTimes(1);
});
});

describe('render()', () => {
Expand Down Expand Up @@ -168,13 +186,6 @@ describe('DocumentAnnotator', () => {

describe('scrollToAnnotation()', () => {
beforeEach(() => {
const payload = {
entries: regions as Annotation[],
limit: 1000,
next_marker: null,
previous_marker: null,
};

annotator.scrollToLocation = jest.fn();
annotator.store.dispatch(fetchAnnotationsAction.fulfilled(payload, 'test', undefined));
});
Expand Down
1 change: 1 addition & 0 deletions src/store/annotations/__mocks__/annotationsState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const annotationState: AnnotationsState = {
test2: { id: 'test2', target: { location: { value: 1 } } } as Annotation,
test3: { id: 'test3', target: { location: { value: 2 } } } as Annotation,
},
isInitialized: false,
};

export default annotationState;
9 changes: 9 additions & 0 deletions src/store/annotations/__tests__/reducer-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,18 @@ import {
fetchAnnotationsAction,
removeAnnotationAction,
setActiveAnnotationIdAction,
setIsInitialized,
} from '../actions';

describe('store/annotations/reducer', () => {
describe('setIsInitialized', () => {
test('should set isInitialized', () => {
const newState = reducer(state, setIsInitialized());

expect(newState.isInitialized).toEqual(true);
});
});

describe('createAnnotationAction', () => {
test('should set state when fulfilled', () => {
const arg = {} as NewAnnotation;
Expand Down
14 changes: 13 additions & 1 deletion src/store/annotations/__tests__/selectors-test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import annotationsState from '../__mocks__/annotationsState';
import { getActiveAnnotationId, getAnnotation, getAnnotations, getAnnotationsForLocation } from '../selectors';
import {
getActiveAnnotationId,
getAnnotation,
getAnnotations,
getAnnotationsForLocation,
getIsInitialized,
} from '../selectors';

describe('store/annotations/selectors', () => {
const state = { annotations: annotationsState };
Expand Down Expand Up @@ -39,4 +45,10 @@ describe('store/annotations/selectors', () => {
expect(getActiveAnnotationId(newState)).toEqual('123');
});
});

describe('getIsInitialized', () => {
test('should return isInitialized status', () => {
expect(getIsInitialized(state)).toBe(false);
});
});
});
1 change: 1 addition & 0 deletions src/store/annotations/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,4 @@ export const fetchAnnotationsAction = createAsyncThunk<APICollection<Annotation>

export const removeAnnotationAction = createAction<string>('REMOVE_ANNOTATION');
export const setActiveAnnotationIdAction = createAction<string | null>('SET_ACTIVE_ANNOTATION_ID');
export const setIsInitialized = createAction('SET_IS_INITIALIZED');
6 changes: 6 additions & 0 deletions src/store/annotations/reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
fetchAnnotationsAction,
removeAnnotationAction,
setActiveAnnotationIdAction,
setIsInitialized,
} from './actions';

const activeAnnotationId = createReducer<AnnotationsState['activeId']>(null, builder =>
Expand Down Expand Up @@ -41,8 +42,13 @@ const annotationsById = createReducer<AnnotationsState['byId']>({}, builder =>
}),
);

const setAnnotationsIsInitialized = createReducer(false, builder => {
builder.addCase(setIsInitialized, () => true);
});

export default combineReducers({
activeId: activeAnnotationId,
allIds: annotationsAllIds,
byId: annotationsById,
isInitialized: setAnnotationsIsInitialized,
});
1 change: 1 addition & 0 deletions src/store/annotations/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ export const getAnnotation = ({ annotations }: State, id: string): Annotation |
export const getAnnotations = ({ annotations }: State): Annotation[] => [...Object.values(annotations.byId)];
export const getAnnotationsForLocation = (state: State, location: number): Annotation[] =>
getAnnotations(state).filter(annotation => getProp(annotation, 'target.location.value') === location);
export const getIsInitialized = ({ annotations }: State): boolean => annotations.isInitialized;
1 change: 1 addition & 0 deletions src/store/annotations/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@ export type AnnotationsState = {
activeId: string | null;
allIds: string[];
byId: Record<string, Annotation>;
isInitialized: boolean;
};
16 changes: 16 additions & 0 deletions src/store/eventing/__tests__/init-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import eventManager from '../../../common/EventManager';
import annotationState from '../../annotations/__mocks__/annotationsState';
import { AppState } from '../../types';
import { handleAnnotationsInitialized } from '../init';

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

describe('store/eventing/init', () => {
test('should emit annotations_initialized event with annotations', () => {
handleAnnotationsInitialized({} as AppState, { annotations: annotationState } as AppState);

expect(eventManager.emit).toBeCalledWith('annotations_initialized', {
annotations: [annotationState.byId.test1, annotationState.byId.test2, annotationState.byId.test3],
});
});
});
8 changes: 8 additions & 0 deletions src/store/eventing/init.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import eventManager from '../../common/EventManager';
import { AppState } from '../types';
import { Event } from '../../@types';
import { getAnnotations } from '../annotations';

export const handleAnnotationsInitialized = (prevState: AppState, nextState: AppState): void => {
eventManager.emit(Event.ANNOTATIONS_INITIALIZED, { annotations: getAnnotations(nextState) });
};
11 changes: 9 additions & 2 deletions src/store/eventing/middleware.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,25 @@
import noop from 'lodash/noop';
import { Action, Dispatch, Middleware, MiddlewareAPI } from '@reduxjs/toolkit';
import { createAnnotationAction, fetchAnnotationsAction, setActiveAnnotationIdAction } from '../annotations/actions';
import {
createAnnotationAction,
fetchAnnotationsAction,
setActiveAnnotationIdAction,
setIsInitialized,
} from '../annotations/actions';
import { handleActiveAnnotationEvents } from './active';
import { handleCreateErrorEvents, handleCreatePendingEvents, handleCreateSuccessEvents } from './create';
import { handleFetchErrorEvents } from './fetch';
import { handleAnnotationsInitialized } from './init';
import { EventHandlerMap } from './types';

// Array of event handlers based on redux action. To add handling for new events add an entry keyed by action
const eventHandlers: EventHandlerMap = {
[createAnnotationAction.pending.toString()]: handleCreatePendingEvents,
[createAnnotationAction.fulfilled.toString()]: handleCreateSuccessEvents,
[createAnnotationAction.pending.toString()]: handleCreatePendingEvents,
[createAnnotationAction.rejected.toString()]: handleCreateErrorEvents,
[fetchAnnotationsAction.rejected.toString()]: handleFetchErrorEvents,
[setActiveAnnotationIdAction.toString()]: handleActiveAnnotationEvents,
[setIsInitialized.toString()]: handleAnnotationsInitialized,
};

function getEventingMiddleware(handlers: EventHandlerMap = eventHandlers): Middleware {
Expand Down

0 comments on commit 6021585

Please sign in to comment.