From 6021585f994c6e71b18278e63d0f361f1d662bd0 Mon Sep 17 00:00:00 2001 From: Mick Ryan Date: Tue, 16 Jun 2020 11:29:42 -0700 Subject: [PATCH] feat(scroll): emit 'annotations_initialized' when annotations first initializes. (#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> --- src/@types/events.ts | 5 ++-- src/common/BaseAnnotator.ts | 7 ++++++ src/document/DocumentAnnotator.ts | 3 ++- .../__tests__/DocumentAnnotator-test.ts | 25 +++++++++++++------ .../annotations/__mocks__/annotationsState.ts | 1 + .../annotations/__tests__/reducer-test.ts | 9 +++++++ .../annotations/__tests__/selectors-test.ts | 14 ++++++++++- src/store/annotations/actions.ts | 1 + src/store/annotations/reducer.ts | 6 +++++ src/store/annotations/selectors.ts | 1 + src/store/annotations/types.ts | 1 + src/store/eventing/__tests__/init-test.ts | 16 ++++++++++++ src/store/eventing/init.ts | 8 ++++++ src/store/eventing/middleware.ts | 11 ++++++-- 14 files changed, 95 insertions(+), 13 deletions(-) create mode 100644 src/store/eventing/__tests__/init-test.ts create mode 100644 src/store/eventing/init.ts diff --git a/src/@types/events.ts b/src/@types/events.ts index 39aa66887..9b9a06b9a 100644 --- a/src/@types/events.ts +++ b/src/@types/events.ts @@ -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', } diff --git a/src/common/BaseAnnotator.ts b/src/common/BaseAnnotator.ts index 84c4cc463..567e8c7b5 100644 --- a/src/common/BaseAnnotator.ts +++ b/src/common/BaseAnnotator.ts @@ -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; @@ -151,4 +152,10 @@ export default class BaseAnnotator extends EventEmitter { this.store.dispatch(store.fetchAnnotationsAction()); // eslint-disable-line @typescript-eslint/no-explicit-any this.store.dispatch(store.fetchCollaboratorsAction()); // eslint-disable-line @typescript-eslint/no-explicit-any } + + protected handleInitialized(): void { + if (!getIsInitialized(this.store.getState())) { + this.store.dispatch(setIsInitialized()); + } + } } diff --git a/src/document/DocumentAnnotator.ts b/src/document/DocumentAnnotator.ts index 3013a151c..5f60d62de 100644 --- a/src/document/DocumentAnnotator.ts +++ b/src/document/DocumentAnnotator.ts @@ -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 @@ -63,6 +63,7 @@ export default class DocumentAnnotator extends BaseAnnotator { this.annotatedEl.classList.add(CLASS_ANNOTATIONS_LOADED); this.render(); + this.handleInitialized(); } render(): void { diff --git a/src/document/__tests__/DocumentAnnotator-test.ts b/src/document/__tests__/DocumentAnnotator-test.ts index 57baf359f..a5720008a 100644 --- a/src/document/__tests__/DocumentAnnotator-test.ts +++ b/src/document/__tests__/DocumentAnnotator-test.ts @@ -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(() => { @@ -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()', () => { @@ -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)); }); diff --git a/src/store/annotations/__mocks__/annotationsState.ts b/src/store/annotations/__mocks__/annotationsState.ts index 6ef8e49dd..7638046e8 100644 --- a/src/store/annotations/__mocks__/annotationsState.ts +++ b/src/store/annotations/__mocks__/annotationsState.ts @@ -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; diff --git a/src/store/annotations/__tests__/reducer-test.ts b/src/store/annotations/__tests__/reducer-test.ts index 9baa7634b..470dc079e 100644 --- a/src/store/annotations/__tests__/reducer-test.ts +++ b/src/store/annotations/__tests__/reducer-test.ts @@ -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; diff --git a/src/store/annotations/__tests__/selectors-test.ts b/src/store/annotations/__tests__/selectors-test.ts index cf8f56325..5e8c2e59a 100644 --- a/src/store/annotations/__tests__/selectors-test.ts +++ b/src/store/annotations/__tests__/selectors-test.ts @@ -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 }; @@ -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); + }); + }); }); diff --git a/src/store/annotations/actions.ts b/src/store/annotations/actions.ts index f79906687..2b7bbdd78 100644 --- a/src/store/annotations/actions.ts +++ b/src/store/annotations/actions.ts @@ -48,3 +48,4 @@ export const fetchAnnotationsAction = createAsyncThunk export const removeAnnotationAction = createAction('REMOVE_ANNOTATION'); export const setActiveAnnotationIdAction = createAction('SET_ACTIVE_ANNOTATION_ID'); +export const setIsInitialized = createAction('SET_IS_INITIALIZED'); diff --git a/src/store/annotations/reducer.ts b/src/store/annotations/reducer.ts index faad9e5c5..9beae3e54 100644 --- a/src/store/annotations/reducer.ts +++ b/src/store/annotations/reducer.ts @@ -5,6 +5,7 @@ import { fetchAnnotationsAction, removeAnnotationAction, setActiveAnnotationIdAction, + setIsInitialized, } from './actions'; const activeAnnotationId = createReducer(null, builder => @@ -41,8 +42,13 @@ const annotationsById = createReducer({}, builder => }), ); +const setAnnotationsIsInitialized = createReducer(false, builder => { + builder.addCase(setIsInitialized, () => true); +}); + export default combineReducers({ activeId: activeAnnotationId, allIds: annotationsAllIds, byId: annotationsById, + isInitialized: setAnnotationsIsInitialized, }); diff --git a/src/store/annotations/selectors.ts b/src/store/annotations/selectors.ts index 4f57ecf74..07509b436 100644 --- a/src/store/annotations/selectors.ts +++ b/src/store/annotations/selectors.ts @@ -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; diff --git a/src/store/annotations/types.ts b/src/store/annotations/types.ts index 0a33c1071..ee5cf852f 100644 --- a/src/store/annotations/types.ts +++ b/src/store/annotations/types.ts @@ -4,4 +4,5 @@ export type AnnotationsState = { activeId: string | null; allIds: string[]; byId: Record; + isInitialized: boolean; }; diff --git a/src/store/eventing/__tests__/init-test.ts b/src/store/eventing/__tests__/init-test.ts new file mode 100644 index 000000000..5bd20920e --- /dev/null +++ b/src/store/eventing/__tests__/init-test.ts @@ -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], + }); + }); +}); diff --git a/src/store/eventing/init.ts b/src/store/eventing/init.ts new file mode 100644 index 000000000..f5eb82098 --- /dev/null +++ b/src/store/eventing/init.ts @@ -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) }); +}; diff --git a/src/store/eventing/middleware.ts b/src/store/eventing/middleware.ts index 956f9891b..f9f4334c0 100644 --- a/src/store/eventing/middleware.ts +++ b/src/store/eventing/middleware.ts @@ -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 {