feat(discoverability): put region layer under textLayer (#579)

* feat(discoverability): put region layer under textLayer
box · Sep 15, 2020 · 1a2fc85 · 1a2fc85
1 parent c782d49
commit 1a2fc85
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 15 deletions.
diff --git a/src/common/BaseAnnotator.ts b/src/common/BaseAnnotator.ts
@@ -41,6 +41,10 @@ export type Options = {
 
 export const CSS_CONTAINER_CLASS = 'ba';
 export const CSS_LOADED_CLASS = 'ba-annotations-loaded';
+export const ANNOTATION_CLASSES: { [M in store.Mode]?: string } = {
+    [store.Mode.HIGHLIGHT]: 'ba-is-create--highlight',
+    [store.Mode.REGION]: 'ba-is-create--region',
+};
 
 export default class BaseAnnotator extends EventEmitter {
     annotatedEl?: HTMLElement | null;
@@ -101,6 +105,8 @@ export default class BaseAnnotator extends EventEmitter {
             this.annotatedEl.classList.remove(CSS_LOADED_CLASS);
         }
 
+        this.removeAnnotationClasses();
+
         this.removeListener(LegacyEvent.SCALE, this.handleScale);
         this.removeListener(Event.ACTIVE_SET, this.handleSetActive);
         this.removeListener(Event.ANNOTATION_REMOVE, this.handleRemove);
@@ -191,6 +197,19 @@ export default class BaseAnnotator extends EventEmitter {
         this.store.dispatch<any>(store.fetchAnnotationsAction()); // eslint-disable-line @typescript-eslint/no-explicit-any
     }
 
+    protected removeAnnotationClasses = (): void => {
+        if (!this.annotatedEl) {
+            return;
+        }
+
+        const annotatedElement = this.annotatedEl;
+        Object.values(ANNOTATION_CLASSES).forEach(className => {
+            if (className) {
+                annotatedElement.classList.remove(className);
+            }
+        });
+    };
+
     protected render(): void {
         // Must be implemented in child class
     }

diff --git a/src/common/__tests__/BaseAnnotator-test.ts b/src/common/__tests__/BaseAnnotator-test.ts
@@ -1,6 +1,6 @@
 import * as store from '../../store';
 import APIFactory from '../../api';
-import BaseAnnotator, { CSS_CONTAINER_CLASS, CSS_LOADED_CLASS } from '../BaseAnnotator';
+import BaseAnnotator, { ANNOTATION_CLASSES, CSS_CONTAINER_CLASS, CSS_LOADED_CLASS } from '../BaseAnnotator';
 import { ANNOTATOR_EVENT } from '../../constants';
 import { Event, LegacyEvent } from '../../@types';
 import { Mode } from '../../store/common';
@@ -156,6 +156,18 @@ describe('BaseAnnotator', () => {
             expect(annotator.containerEl.classList).not.toContain(CSS_CONTAINER_CLASS);
         });
 
+        test('should remove all annotation classes from annotation layer', () => {
+            const annotatedEl = document.createElement('div');
+            annotatedEl.classList.add(CSS_LOADED_CLASS);
+
+            annotator.annotatedEl = annotatedEl;
+            annotator.destroy();
+
+            expect(annotator.annotatedEl.classList).not.toContain(CSS_LOADED_CLASS);
+            expect(annotator.annotatedEl.classList).not.toContain(ANNOTATION_CLASSES[Mode.HIGHLIGHT]);
+            expect(annotator.annotatedEl.classList).not.toContain(ANNOTATION_CLASSES[Mode.REGION]);
+        });
+
         test('should remove proper event handlers', () => {
             annotator.removeListener = jest.fn();
 

diff --git a/src/document/DocumentAnnotator.scss b/src/document/DocumentAnnotator.scss
@@ -11,7 +11,17 @@
         pointer-events: none;
     }
 
-    &.ba-is-highlighting {
+    &.ba-discoverability-enabled {
+        .textLayer {
+            pointer-events: none; // Ensures that click events are received on the create region layer
+
+            span {
+                pointer-events: auto; // Text layer still receives text highlight events
+            }
+        }
+    }
+
+    &.ba-is-create--highlight {
         .textLayer {
             > span {
                 @include ba-HighlightCreator-cursor;

diff --git a/src/document/DocumentAnnotator.ts b/src/document/DocumentAnnotator.ts
@@ -1,4 +1,4 @@
-import BaseAnnotator, { Options } from '../common/BaseAnnotator';
+import BaseAnnotator, { ANNOTATION_CLASSES, Options } from '../common/BaseAnnotator';
 import BaseManager from '../common/BaseManager';
 import { centerHighlight, isHighlight } from '../highlight/highlightUtil';
 import { centerRegion, isRegion, RegionManager } from '../region';
@@ -75,7 +75,12 @@ export default class DocumentAnnotator extends BaseAnnotator {
 
                 managers.add(new HighlightManager({ location: pageNumber, referenceEl: pageReferenceEl }));
             }
-            managers.add(new RegionManager({ location: pageNumber, referenceEl: pageReferenceEl }));
+
+            const canvasLayerEl = pageEl.querySelector<HTMLElement>('.canvasWrapper');
+            const referenceEl =
+                this.isFeatureEnabled('discoverability') && canvasLayerEl ? canvasLayerEl : pageReferenceEl;
+
+            managers.add(new RegionManager({ location: pageNumber, referenceEl }));
         }
 
         return managers;
@@ -106,10 +111,12 @@ export default class DocumentAnnotator extends BaseAnnotator {
             return;
         }
 
-        if (mode === Mode.HIGHLIGHT) {
-            this.annotatedEl.classList.add('ba-is-highlighting');
-        } else {
-            this.annotatedEl.classList.remove('ba-is-highlighting');
+        this.removeAnnotationClasses();
+
+        const annotatedElement = this.annotatedEl;
+        const className = ANNOTATION_CLASSES[mode];
+        if (className) {
+            annotatedElement.classList.add(className);
         }
     };
 

diff --git a/src/document/__tests__/DocumentAnnotator-test.ts b/src/document/__tests__/DocumentAnnotator-test.ts
@@ -3,6 +3,7 @@ import DocumentAnnotator from '../DocumentAnnotator';
 import HighlightListener from '../../highlight/HighlightListener';
 import RegionManager from '../../region/RegionManager';
 import { Annotation, Event } from '../../@types';
+import { ANNOTATION_CLASSES } from '../../common/BaseAnnotator';
 import { annotation as highlight } from '../../highlight/__mocks__/data';
 import { annotations as regions } from '../../region/__mocks__/data';
 import { fetchAnnotationsAction, Mode } from '../../store';
@@ -107,11 +108,11 @@ describe('DocumentAnnotator', () => {
         });
 
         test('should add/remove highlight class', () => {
-            annotator.emit(Event.ANNOTATIONS_MODE_CHANGE, { mode: 'highlight' });
-            expect(annotator.annotatedEl?.classList.add).toHaveBeenCalledWith('ba-is-highlighting');
+            annotator.emit(Event.ANNOTATIONS_MODE_CHANGE, { mode: Mode.HIGHLIGHT });
+            expect(annotator.annotatedEl?.classList.add).toHaveBeenCalledWith(ANNOTATION_CLASSES[Mode.HIGHLIGHT]);
 
-            annotator.emit(Event.ANNOTATIONS_MODE_CHANGE, { mode: 'region' });
-            expect(annotator.annotatedEl?.classList.remove).toHaveBeenCalledWith('ba-is-highlighting');
+            annotator.emit(Event.ANNOTATIONS_MODE_CHANGE, { mode: Mode.REGION });
+            expect(annotator.annotatedEl?.classList.remove).toHaveBeenCalledWith(ANNOTATION_CLASSES[Mode.REGION]);
         });
     });
 
@@ -286,15 +287,15 @@ describe('DocumentAnnotator', () => {
         });
 
         test('should add and remove is highlighting class if mode changes', () => {
-            expect(annotator.annotatedEl?.classList.contains('ba-is-highlighting')).toBe(false);
+            expect(annotator.annotatedEl?.classList.contains('ba-is-create--higlight')).toBe(false);
 
             annotator.toggleAnnotationMode(Mode.HIGHLIGHT);
 
-            expect(annotator.annotatedEl?.classList.contains('ba-is-highlighting')).toBe(true);
+            expect(annotator.annotatedEl?.classList.contains('ba-is-create--highlight')).toBe(true);
 
             annotator.toggleAnnotationMode(Mode.NONE);
 
-            expect(annotator.annotatedEl?.classList.contains('ba-is-highlighting')).toBe(false);
+            expect(annotator.annotatedEl?.classList.contains('ba-is-create--highlight')).toBe(false);
         });
     });
 });