feat(image): Add support for region annotations on image files (#508)

package.json

@@ -34,6 +34,7 @@
         "react-tether": "1.0.5",
         "react-textarea-autosize": "^7.1.2",
         "redux": "^4.0.5",
+        "regenerator-runtime": "^0.13.5",
         "scroll-into-view-if-needed": "^2.2.24"
     },
     "devDependencies": {
@@ -100,7 +101,6 @@
         "postcss-loader": "^3.0.0",
         "prettier": "^1.19.1",
         "raw-loader": "^4.0.1",
-        "regenerator-runtime": "^0.13.5",
         "sass-loader": "^8.0.2",
         "style-loader": "^1.1.4",
         "stylelint": "^12.0.0",

src/BoxAnnotations.ts

@@ -1,9 +1,12 @@
+import 'regenerator-runtime/runtime';
 import getProp from 'lodash/get';
+import BaseAnnotator from './common/BaseAnnotator';
+import ImageAnnotator from './image/ImageAnnotator';
 import DocumentAnnotator from './document/DocumentAnnotator';
 import { IntlOptions, Permissions, PERMISSIONS, Type } from './@types';
 
 type Annotator = {
-    CONSTRUCTOR: typeof DocumentAnnotator;
+    CONSTRUCTOR: typeof BaseAnnotator;
     NAME: string;
     TYPES: string[];
     VIEWERS: string[];
@@ -46,6 +49,12 @@ const ANNOTATORS: Annotator[] = [
         TYPES: [Type.region],
         VIEWERS: ['AutoCAD', 'Document', 'Presentation'],
     },
+    {
+        CONSTRUCTOR: ImageAnnotator,
+        NAME: 'Image',
+        TYPES: [Type.region],
+        VIEWERS: ['Image'],
+    },
 ];
 
 class BoxAnnotations {

src/common/BaseAnnotator.ts

@@ -5,7 +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 { setIsInitialized } from '../store';
 import './BaseAnnotator.scss';
 
 export type Container = string | HTMLElement;
@@ -36,14 +36,17 @@ export type Options = {
     token: string;
 };
 
+export const CSS_CONTAINER_CLASS = 'ba';
+export const CSS_LOADED_CLASS = 'ba-annotations-loaded';
+
 export default class BaseAnnotator extends EventEmitter {
-    container: Container;
+    annotatedEl?: HTMLElement | null;
 
-    intl: IntlShape;
+    containerEl?: HTMLElement | null;
 
-    rootEl?: HTMLElement | null;
+    container: Container;
 
-    scale = 1;
+    intl: IntlShape;
 
     store: store.AppStore;
 
@@ -79,8 +82,12 @@ export default class BaseAnnotator extends EventEmitter {
     }
 
     public destroy(): void {
-        if (this.rootEl) {
-            this.rootEl.classList.remove('ba');
+        if (this.containerEl) {
+            this.containerEl.classList.remove(CSS_CONTAINER_CLASS);
+        }
+
+        if (this.annotatedEl) {
+            this.annotatedEl.classList.remove(CSS_LOADED_CLASS);
         }
 
         this.removeListener(LegacyEvent.SCALE, this.handleScale);
@@ -89,16 +96,28 @@ export default class BaseAnnotator extends EventEmitter {
         this.removeListener(Event.VISIBLE_SET, this.handleSetVisible);
     }
 
-    public init(scale: number): void {
-        this.rootEl = this.getElement(this.container);
-        this.scale = scale;
+    public init(scale = 1, rotation = 0): void {
+        this.containerEl = this.getElement(this.container);
+        this.annotatedEl = this.getAnnotatedElement();
 
-        if (!this.rootEl) {
+        if (!this.annotatedEl || !this.containerEl) {
             this.emit(LegacyEvent.ERROR, this.intl.formatMessage(messages.annotationsLoadError));
             return;
         }
 
-        this.rootEl.classList.add('ba');
+        // Add classes to the parent elements to support CSS scoping
+        this.annotatedEl.classList.add(CSS_LOADED_CLASS);
+        this.containerEl.classList.add(CSS_CONTAINER_CLASS);
+
+        // Update the store with the options provided by preview
+        this.store.dispatch(store.setRotationAction(rotation));
+        this.store.dispatch(store.setScaleAction(scale));
+
+        // Defer to the child class to render annotations
+        this.render();
+
+        // Update the store now that annotations have been rendered
+        this.store.dispatch(setIsInitialized());
     }
 
     public removeAnnotation = (annotationId: string): void => {
@@ -115,20 +134,25 @@ export default class BaseAnnotator extends EventEmitter {
     }
 
     public setVisibility(visibility: boolean): void {
-        if (!this.rootEl) {
+        if (!this.containerEl) {
             return;
         }
+
         if (visibility) {
-            this.rootEl.classList.remove('is-hidden');
+            this.containerEl.classList.remove('is-hidden');
         } else {
-            this.rootEl.classList.add('is-hidden');
+            this.containerEl.classList.add('is-hidden');
         }
     }
 
     public toggleAnnotationMode(mode: store.Mode): void {
         this.store.dispatch(store.toggleAnnotationModeAction(mode));
     }
 
+    protected getAnnotatedElement(): HTMLElement | null | undefined {
+        return undefined; // Must be implemented in child class
+    }
+
     protected getElement(selector: HTMLElement | string): HTMLElement | null {
         return typeof selector === 'string' ? document.querySelector(selector) : selector;
     }
@@ -137,8 +161,8 @@ export default class BaseAnnotator extends EventEmitter {
         this.removeAnnotation(annotationId);
     };
 
-    protected handleScale = ({ scale }: { scale: number }): void => {
-        this.init(scale);
+    protected handleScale = ({ rotationAngle, scale }: { rotationAngle: number; scale: number }): void => {
+        this.init(scale, rotationAngle);
     };
 
     protected handleSetActive = (annotationId: string | null): void => {
@@ -155,9 +179,7 @@ export default class BaseAnnotator extends EventEmitter {
         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());
-        }
+    protected render(): void {
+        // Must be implemented in child class
     }
 }

src/common/BaseManager.ts

@@ -2,8 +2,7 @@ import { IntlShape } from 'react-intl';
 import { Store } from 'redux';
 
 export type Options = {
-    page: number;
-    pageEl: HTMLElement;
+    location?: number;
     referenceEl: HTMLElement;
 };
 
@@ -14,6 +13,7 @@ export type Props = {
 
 export default interface BaseManager {
     destroy(): void;
-    exists(pageEl: HTMLElement): boolean;
+    exists(parentEl: HTMLElement): boolean;
     render(props: Props): void;
+    style(styles: Partial<CSSStyleDeclaration>): CSSStyleDeclaration;
 }

src/common/__tests__/BaseAnnotator-test.ts

@@ -1,25 +1,27 @@
 import * as store from '../../store';
 import APIFactory from '../../api';
-import BaseAnnotator from '../BaseAnnotator';
+import BaseAnnotator, { CSS_CONTAINER_CLASS, CSS_LOADED_CLASS } from '../BaseAnnotator';
 import { ANNOTATOR_EVENT } from '../../constants';
 import { Event, LegacyEvent } from '../../@types';
 import { Mode } from '../../store/common';
+import { setIsInitialized } from '../../store';
 
 jest.mock('../../api');
-jest.mock('../../store', () => ({
-    createStore: jest.fn(() => ({ dispatch: jest.fn() })),
-    removeAnnotationAction: jest.fn(),
-    fetchAnnotationsAction: jest.fn(),
-    fetchCollaboratorsAction: jest.fn(),
-    setActiveAnnotationIdAction: jest.fn(),
-    setVisibilityAction: jest.fn(),
-    toggleAnnotationModeAction: jest.fn(),
-}));
+jest.mock('../../store/createStore');
+
+class MockAnnotator extends BaseAnnotator {
+    protected getAnnotatedElement(): HTMLElement | null | undefined {
+        return this.containerEl?.querySelector('.inner');
+    }
+}
 
 describe('BaseAnnotator', () => {
+    const container = document.createElement('div');
+    container.innerHTML = `<div class="inner" />`;
+
     const defaults = {
         apiHost: 'https://api.box.com',
-        container: document.createElement('div'),
+        container,
         file: {
             id: '12345',
             file_version: { id: '98765' },
@@ -34,7 +36,7 @@ describe('BaseAnnotator', () => {
         locale: 'en-US',
         token: '1234567890',
     };
-    const getAnnotator = (options = {}): BaseAnnotator => new BaseAnnotator({ ...defaults, ...options });
+    const getAnnotator = (options = {}): MockAnnotator => new MockAnnotator({ ...defaults, ...options });
     let annotator = getAnnotator();
 
     beforeEach(() => {
@@ -107,13 +109,13 @@ describe('BaseAnnotator', () => {
 
     describe('destroy()', () => {
         test('should remove the base class name from the root element', () => {
-            const rootEl = document.createElement('div');
-            rootEl.classList.add('ba');
+            const containerEl = document.createElement('div');
+            containerEl.classList.add(CSS_CONTAINER_CLASS);
 
-            annotator.rootEl = rootEl;
+            annotator.containerEl = containerEl;
             annotator.destroy();
 
-            expect(annotator.rootEl.classList).not.toContain('ba');
+            expect(annotator.containerEl.classList).not.toContain(CSS_CONTAINER_CLASS);
         });
 
         test('should remove proper event handlers', () => {
@@ -129,11 +131,20 @@ describe('BaseAnnotator', () => {
     });
 
     describe('init()', () => {
-        test('should set the root element based on class selector', () => {
+        test('should set its reference elements based on class selector', () => {
             annotator.init(5);
 
-            expect(annotator.rootEl).toBe(defaults.container);
-            expect(annotator.rootEl && annotator.rootEl.classList).toContain('ba');
+            expect(annotator.containerEl).toBeDefined();
+            expect(annotator.containerEl?.classList).toContain(CSS_CONTAINER_CLASS);
+            expect(annotator.annotatedEl?.classList).toContain(CSS_LOADED_CLASS);
+        });
+
+        test('should dispatch all necessary actions', () => {
+            annotator.init(1, 180);
+
+            expect(annotator.store.dispatch).toHaveBeenCalledWith(store.setRotationAction(180));
+            expect(annotator.store.dispatch).toHaveBeenCalledWith(store.setScaleAction(1));
+            expect(annotator.store.dispatch).toHaveBeenCalledWith(setIsInitialized());
         });
 
         test('should emit error if no root element exists', () => {
@@ -142,7 +153,7 @@ describe('BaseAnnotator', () => {
             annotator.init(5);
 
             expect(annotator.emit).toBeCalledWith(ANNOTATOR_EVENT.error, expect.any(String));
-            expect(annotator.rootEl).toBeNull();
+            expect(annotator.containerEl).toBeNull();
         });
     });
 
@@ -155,8 +166,8 @@ describe('BaseAnnotator', () => {
         });
 
         test('should call their underlying methods', () => {
-            annotator.emit(LegacyEvent.SCALE, { scale: 1 });
-            expect(annotator.init).toHaveBeenCalledWith(1);
+            annotator.emit(LegacyEvent.SCALE, { rotationAngle: 0, scale: 1 });
+            expect(annotator.init).toHaveBeenCalledWith(1, 0);
 
             annotator.emit(Event.ACTIVE_SET, 12345);
             expect(annotator.setActiveId).toHaveBeenCalledWith(12345);
@@ -177,36 +188,37 @@ describe('BaseAnnotator', () => {
 
     describe('setVisibility()', () => {
         test.each([true, false])('should hide/show annotations if visibility is %p', visibility => {
-            annotator.rootEl = defaults.container;
+            annotator.init(1);
             annotator.setVisibility(visibility);
-            expect(annotator.rootEl.classList.contains('is-hidden')).toEqual(!visibility);
+            expect(annotator.containerEl?.classList.contains('is-hidden')).toEqual(!visibility);
+        });
+
+        test('should do nothing if the root element is not defined', () => {
+            annotator.containerEl = document.querySelector('nonsense') as HTMLElement;
+            annotator.setVisibility(true);
+            expect(annotator.containerEl?.classList).toBeFalsy();
         });
     });
 
     describe('setActiveAnnotationId()', () => {
         test.each([null, '12345'])('should dispatch setActiveAnnotationIdAction with id %s', id => {
             annotator.setActiveId(id);
-            expect(annotator.store.dispatch).toBeCalled();
-            expect(store.setActiveAnnotationIdAction).toBeCalledWith(id);
+            expect(annotator.store.dispatch).toBeCalledWith(store.setActiveAnnotationIdAction(id));
         });
     });
 
     describe('removeAnnotation', () => {
-        test('should dispatch deleteActiveAnnotationAction', () => {
+        test('should dispatch removeActiveAnnotationAction with the specified id', () => {
             const id = '123';
             annotator.removeAnnotation(id);
-
-            expect(annotator.store.dispatch).toBeCalled();
-            expect(store.removeAnnotationAction).toBeCalledWith(id);
+            expect(annotator.store.dispatch).toBeCalledWith(store.removeAnnotationAction(id));
         });
     });
 
     describe('toggleAnnotationMode()', () => {
         test('should dispatch toggleAnnotationModeAction with specified mode', () => {
             annotator.toggleAnnotationMode('region' as Mode);
-
-            expect(annotator.store.dispatch).toBeCalled();
-            expect(store.toggleAnnotationModeAction).toBeCalledWith('region');
+            expect(annotator.store.dispatch).toBeCalledWith(store.toggleAnnotationModeAction('region' as Mode));
         });
     });
 });

src/constants.js

@@ -1,15 +1,6 @@
-// Annotation CSS constants
-export const CLASS_ANNOTATIONS_LOADED = 'ba-annotations-loaded';
-
 export const ANNOTATOR_EVENT = {
     fetch: 'annotationsfetched',
     error: 'annotationerror',
     scale: 'scaleannotations',
     setVisibility: 'annotationsetvisibility',
 };
-
-export const PLACEHOLDER_USER = {
-    type: 'user',
-    id: '0',
-    email: '',
-};