Skip to content

Commit 5eb6337

Browse files
authored
feat(video-annotations): updating redux store for video annotations (#728)
* feat(video-annotatons): updating media annotator test * feat(video-annotations): addressing PR comments * feat(video-annotations): fixing linter issues * feat(video-annotations): adding video support to redux store * feat(video-annotations): updating comments and adding space * feat(video-annotations): fixing linter error
1 parent 6886574 commit 5eb6337

File tree

9 files changed

+115
-16
lines changed

9 files changed

+115
-16
lines changed
Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,27 @@
11
import { Annotation } from '../../../@types';
22
import { AnnotationsState } from '../types';
33

4-
const annotationState: AnnotationsState = {
4+
export const annotationState: AnnotationsState = {
55
activeId: null,
66
allIds: ['test1', 'test2', 'test3'],
77
byId: {
8-
test1: { id: 'test1', target: { location: { value: 1 } } } as Annotation,
9-
test2: { id: 'test2', target: { location: { value: 1 } } } as Annotation,
10-
test3: { id: 'test3', target: { location: { value: 2 } } } as Annotation,
8+
test1: { id: 'test1', target: { location: { type: 'page' as const, value: 1 } } } as Annotation,
9+
test2: { id: 'test2', target: { location: { type: 'page' as const, value: 1 } } } as Annotation,
10+
test3: { id: 'test3', target: { location: { type: 'page' as const, value: 2 } } } as Annotation,
1111
},
1212
isInitialized: false,
1313
};
1414

15-
export default annotationState;
15+
export const videoAnnotationState: AnnotationsState = {
16+
activeId: null,
17+
allIds: ['testVid1', 'testVid2', 'testVid3'],
18+
byId: {
19+
testVid1: { id: 'testVid1', target: { location: { type: 'frame' as const, value: 100 } } } as Annotation,
20+
testVid2: { id: 'testVid2', target: { location: { type: 'frame' as const, value: 100 } } } as Annotation,
21+
testVid3: { id: 'testVid3', target: { location: { type: 'frame' as const, value: 20 } } } as Annotation,
22+
},
23+
isInitialized: false,
24+
};
25+
26+
27+
export default { annotationState, videoAnnotationState };

src/store/annotations/__tests__/reducer-test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import reducer from '../reducer';
2-
import state from '../__mocks__/annotationsState';
2+
import {annotationState as state} from '../__mocks__/annotationsState';
33
import { Annotation, AnnotationDrawing, NewAnnotation, PathGroup } from '../../../@types';
44
import { APICollection } from '../../../api';
55
import {

src/store/annotations/__tests__/selectors-test.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import annotationsState from '../__mocks__/annotationsState';
1+
import {annotationState, videoAnnotationState } from '../__mocks__/annotationsState';
22
import {
33
getActiveAnnotationId,
44
getAnnotation,
@@ -8,7 +8,8 @@ import {
88
} from '../selectors';
99

1010
describe('store/annotations/selectors', () => {
11-
const state = { annotations: annotationsState };
11+
const state = { annotations: annotationState };
12+
const videoState = { annotations: videoAnnotationState };
1213

1314
describe('getAnnotation', () => {
1415
test('should return an annotation by the specified id', () => {
@@ -33,6 +34,14 @@ describe('store/annotations/selectors', () => {
3334
expect.objectContaining({ id: 'test2' }),
3435
]);
3536
});
37+
38+
test('should return all annotations for video content', () => {
39+
expect(getAnnotationsForLocation(videoState, -1)).toEqual([
40+
expect.objectContaining({ id: 'testVid1' }),
41+
expect.objectContaining({ id: 'testVid2' }),
42+
expect.objectContaining({ id: 'testVid3' }),
43+
]);
44+
});
3645
});
3746

3847
describe('getActiveAnnotationId', () => {
@@ -41,7 +50,7 @@ describe('store/annotations/selectors', () => {
4150
});
4251

4352
test('should get the active id', () => {
44-
const newState = { annotations: { ...annotationsState, activeId: '123' } };
53+
const newState = { annotations: { ...annotationState, activeId: '123' } };
4554
expect(getActiveAnnotationId(newState)).toEqual('123');
4655
});
4756
});

src/store/annotations/selectors.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,20 @@
11
import getProp from 'lodash/get';
22
import { Annotation } from '../../@types';
33
import { AppState } from '../types';
4+
import { MEDIA_LOCATION_INDEX } from '../../constants';
45

56
type State = Pick<AppState, 'annotations'>;
67

78
export const getActiveAnnotationId = ({ annotations }: State): string | null => annotations.activeId;
89
export const getAnnotation = ({ annotations }: State, id: string): Annotation | undefined => annotations.byId[id];
910
export const getAnnotations = ({ annotations }: State): Annotation[] => [...Object.values(annotations.byId)];
10-
export const getAnnotationsForLocation = (state: State, location: number): Annotation[] =>
11-
getAnnotations(state).filter(annotation => getProp(annotation, 'target.location.value') === location);
11+
export const getAnnotationsForLocation = (state: State, location: number): Annotation[] => {
12+
const annotations = getAnnotations(state);
13+
// For video annotations the location will be -1 by default as there is no page number for videos. We will return all of the annotations
14+
// and the component will filter them by the current play time of the video.
15+
if (location === MEDIA_LOCATION_INDEX) {
16+
return annotations;
17+
}
18+
return annotations.filter(annotation => getProp(annotation, 'target.location.value') === location);
19+
};
1220
export const getIsInitialized = ({ annotations }: State): boolean => annotations.isInitialized;

src/store/creator/__mocks__/creatorState.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { CreatorStatus } from '../types';
22

3-
export default {
3+
export const pageCreatorState = {
44
cursor: 0,
55
error: null,
66
message: 'test',
@@ -15,6 +15,28 @@ export default {
1515
y: 10,
1616
},
1717
type: 'region' as const,
18+
targetType: 'page' as const,
19+
},
20+
status: CreatorStatus.init,
21+
};
22+
23+
24+
export const videoCreatorState = {
25+
cursor: 0,
26+
error: null,
27+
message: 'test',
28+
referenceId: '20001',
29+
staged: {
30+
location: 120,
31+
shape: {
32+
height: 100,
33+
width: 100,
34+
type: 'rect' as const,
35+
x: 10,
36+
y: 10,
37+
},
38+
type: 'region' as const,
39+
targetType: 'frame' as const,
1840
},
1941
status: CreatorStatus.init,
2042
};

src/store/creator/__tests__/reducer-test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import reducer from '../reducer';
2-
import state from '../__mocks__/creatorState';
2+
import {pageCreatorState as state} from '../__mocks__/creatorState';
33
import { createAnnotationAction } from '../../annotations';
44
import { CreatorStatus } from '../types';
55
import { NewAnnotation } from '../../../@types';

src/store/creator/__tests__/selectors-test.ts

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import creatorState from '../__mocks__/creatorState';
1+
import {pageCreatorState, videoCreatorState} from '../__mocks__/creatorState';
22
import { CreatorStatus } from '../types';
33
import {
44
getCreatorMessage,
@@ -9,7 +9,8 @@ import {
99
} from '../selectors';
1010

1111
describe('store/annotations/selectors', () => {
12-
const state = { creator: creatorState };
12+
const state = { creator: pageCreatorState };
13+
const videoState = { creator: videoCreatorState };
1314

1415
describe('getCreatorStatus', () => {
1516
test('should return the current creator status', () => {
@@ -35,6 +36,24 @@ describe('store/annotations/selectors', () => {
3536
"x": 10,
3637
"y": 10,
3738
},
39+
"targetType": "page",
40+
"type": "region",
41+
}
42+
`);
43+
});
44+
45+
test('should return the current creator staged item for video content', () => {
46+
expect(getCreatorStaged(videoState)).toMatchInlineSnapshot(`
47+
Object {
48+
"location": 120,
49+
"shape": Object {
50+
"height": 100,
51+
"type": "rect",
52+
"width": 100,
53+
"x": 10,
54+
"y": 10,
55+
},
56+
"targetType": "frame",
3857
"type": "region",
3958
}
4059
`);
@@ -56,6 +75,22 @@ describe('store/annotations/selectors', () => {
5675
});
5776
expect(getCreatorStagedForLocation(state, 2)).toEqual(null);
5877
});
78+
79+
test('should return the current creator staged item for video content', () => {
80+
expect(getCreatorStagedForLocation(videoState, -1)).toMatchObject({
81+
location: 120,
82+
shape: {
83+
height: 100,
84+
type: 'rect',
85+
width: 100,
86+
x: 10,
87+
y: 10,
88+
},
89+
type: 'region',
90+
});
91+
92+
expect(getCreatorStagedForLocation(videoState, 1)).toEqual(null);
93+
});
5994
});
6095

6196
describe('getCreatorMessage', () => {

src/store/creator/selectors.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { AppState } from '../types';
2+
import { MEDIA_LOCATION_INDEX } from '../../constants';
23
import { CreatorItem, CreatorItemDrawing, CreatorItemHighlight, CreatorItemRegion, CreatorStatus } from './types';
34

45
type State = Pick<AppState, 'creator'>;
@@ -9,6 +10,18 @@ export const getCreatorReferenceId = (state: State): string | null => state.crea
910
export const getCreatorStaged = (state: State): CreatorItem | null => state.creator.staged;
1011
export const getCreatorStagedForLocation = (state: State, location: number): CreatorItem | null => {
1112
const staged = getCreatorStaged(state);
13+
// The location is typically a page and this selector was originally meant to only return the staged annotation
14+
// (those that have been drawn but not saved) that belong to the page specified by the location. For documents,
15+
// the annotator creates new sets of managers(highlights, drawings, regions) for each page with the current
16+
// page number being the location e.g. page 4 of 10. This selector prevents a staged annotation from staying on
17+
// the screen when a user navigates to a different page. By default, the location value for managers is 1 which is
18+
// why this selector works for images. Video annotations will not work this way because they reference a timestamp, not a page,
19+
// and the drawing canvas is the video player which has no pages. In this case, we just need to get the staged annotation and do the
20+
// locaton check for MEDIA_LOCATION_INDEX. The MEDIA_LOCATION_INDEX is a special value, set in the manager, that is used to indicate that
21+
// the this is actually a staged video annotation with a timestamp as its location.
22+
if (location === MEDIA_LOCATION_INDEX) {
23+
return staged;
24+
}
1225
return staged && staged.location === location ? staged : null;
1326
};
1427
export const getCreatorStatus = (state: State): CreatorStatus => state.creator.status;

src/store/eventing/__tests__/init-test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import eventManager from '../../../common/EventManager';
2-
import annotationState from '../../annotations/__mocks__/annotationsState';
2+
import {annotationState} from '../../annotations/__mocks__/annotationsState';
33
import { AppState } from '../../types';
44
import { handleAnnotationsInitialized } from '../init';
55

0 commit comments

Comments
 (0)