Skip to content

Commit

Permalink
Update: Annotations with header (#233)
Browse files Browse the repository at this point in the history 
* Update: Adding headerElement to annotator options

* Update: headerElement fallback to container when using preview header

* Chore: hide the annotation button on destroy

* Chore: default to header container

* Chore: addressing comments

* Chore: Only hideButton if modeButton exists
  • Loading branch information
Conrad Chan authored Oct 11, 2018
1 parent 3a5cc86 commit 91cdce3
Show file tree
Hide file tree
Showing 8 changed files with 141 additions and 18 deletions.
18 changes: 16 additions & 2 deletions src/Annotator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ import {
THREAD_EVENT,
ANNOTATOR_EVENT,
CONTROLLER_EVENT,
CLASS_ANNOTATIONS_LOADED
CLASS_ANNOTATIONS_LOADED,
SELECTOR_BOX_PREVIEW_HEADER_CONTAINER
} from './constants';

class Annotator extends EventEmitter {
Expand Down Expand Up @@ -103,7 +104,19 @@ class Annotator extends EventEmitter {
// Get the container dom element if selector was passed, in tests
this.container = this.options.container;
if (typeof this.options.container === 'string') {
this.container = document.querySelector(this.options.container);
this.container = document.querySelector(this.container);
}

// Get the header dom element if selector was passed, in tests
this.headerElement = this.options.headerElement;
if (typeof this.headerElement === 'string') {
this.headerElement = document.querySelector(this.headerElement);
}

// If using box content preview header and no external header element was specified,
// fallback to the container element
if (this.options.header !== 'none' && !this.headerElement) {
this.headerElement = this.container.querySelector(SELECTOR_BOX_PREVIEW_HEADER_CONTAINER);
}

// Get annotated element from container
Expand Down Expand Up @@ -243,6 +256,7 @@ class Annotator extends EventEmitter {
const controller = this.modeControllers[type];
controller.init({
container: this.container,
headerElement: this.headerElement,
annotatedElement: this.annotatedElement,
mode: type,
modeButton: this.modeButtons[type],
Expand Down
12 changes: 10 additions & 2 deletions src/__tests__/Annotator-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ import {
ANNOTATOR_EVENT,
THREAD_EVENT,
CONTROLLER_EVENT,
SELECTOR_ANNOTATED_ELEMENT
SELECTOR_ANNOTATED_ELEMENT,
SELECTOR_BOX_PREVIEW_HEADER_CONTAINER
} from '../constants';

let annotator;
let controller;
let thread;
const html = `<button class="bp-btn-annotate"></button>
const html = `<div class="bp-header-container"></div>
<button class="bp-btn-annotate"></button>
<div class="annotated-element"></div>`;

describe('Annotator', () => {
Expand Down Expand Up @@ -112,6 +114,12 @@ describe('Annotator', () => {
expect(annotator.loadAnnotations).toBeCalled();
});

it('should set the headerElement to the container as a fallback', () => {
annotator.options.header = 'light';
annotator.init(5);
expect(annotator.headerElement).toEqual(document.querySelector(SELECTOR_BOX_PREVIEW_HEADER_CONTAINER));
});

it('should setup mobile dialog for mobile browsers', () => {
annotator.isMobile = true;
annotator.init();
Expand Down
2 changes: 2 additions & 0 deletions src/constants.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ export const CLASS_ANNOTATION_BUTTON_DRAW = 'bp-btn-annotate-draw';
export const SELECTOR_ANNOTATION_BUTTON_DRAW = `.${CLASS_ANNOTATION_BUTTON_DRAW}`;
export const CLASS_ANNOTATION_BUTTON_DRAW_ENTER = 'bp-btn-annotate-draw-enter';
export const SELECTOR_ANNOTATION_BUTTON_DRAW_ENTER = `.${CLASS_ANNOTATION_BUTTON_DRAW_ENTER}`;
export const CLASS_BOX_PREVIEW_HEADER_CONTAINER = 'bp-header-container';
export const SELECTOR_BOX_PREVIEW_HEADER_CONTAINER = `.${CLASS_BOX_PREVIEW_HEADER_CONTAINER}`;

export const CLASS_BOX_PREVIEW = 'bp';
export const SELECTOR_BOX_PREVIEW = `.${CLASS_BOX_PREVIEW}`;
Expand Down
31 changes: 28 additions & 3 deletions src/controllers/AnnotationModeController.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class AnnotationModeController extends EventEmitter {
*/
init(data) {
this.container = data.container;
this.headerElement = data.headerElement;
this.annotatedElement = data.annotatedElement;
this.mode = data.mode;
this.annotator = data.annotator;
Expand Down Expand Up @@ -69,6 +70,10 @@ class AnnotationModeController extends EventEmitter {
if (this.buttonEl) {
this.buttonEl.removeEventListener('click', this.toggleMode);
}

if (this.modeButton) {
this.hideButton();
}
}

/**
Expand All @@ -78,7 +83,11 @@ class AnnotationModeController extends EventEmitter {
* @return {HTMLElement|null} Annotate button element or null if the selector did not find an element.
*/
getButton(annotatorSelector) {
return this.container.querySelector(annotatorSelector);
if (!this.headerElement) {
return null;
}

return this.headerElement.querySelector(annotatorSelector);
}

/**
Expand All @@ -87,7 +96,7 @@ class AnnotationModeController extends EventEmitter {
* @return {void}
*/
showButton() {
if (!this.permissions.canAnnotate) {
if (!this.permissions.canAnnotate || !this.modeButton) {
return;
}

Expand All @@ -101,6 +110,22 @@ class AnnotationModeController extends EventEmitter {
}
}

/**
* Hides the annotate button for the specified mode
*
* @return {void}
*/
hideButton() {
if (!this.permissions.canAnnotate || !this.modeButton) {
return;
}

this.buttonEl = this.getButton(this.modeButton.selector);
if (this.buttonEl) {
this.buttonEl.classList.add(CLASS_HIDDEN);
}
}

/**
* Toggles annotation modes on and off. When an annotation mode is
* on, annotation threads will be created at that location.
Expand All @@ -126,7 +151,7 @@ class AnnotationModeController extends EventEmitter {
*/
exit() {
this.emit(CONTROLLER_EVENT.exit, { mode: this.mode });
replaceHeader(this.container, SELECTOR_BOX_PREVIEW_BASE_HEADER);
replaceHeader(this.headerElement, SELECTOR_BOX_PREVIEW_BASE_HEADER);

this.destroyPendingThreads();

Expand Down
6 changes: 3 additions & 3 deletions src/controllers/DrawingModeController.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ class DrawingModeController extends AnnotationModeController {
// light, dark, or no value given), then we want to use our draw
// header. Otherwise we expect header UI to be handled by Preview’s
// consumer
if (data.options.header !== 'none') {
this.setupHeader(this.container, shell);
if (data.options.header !== 'none' || this.headerElement) {
this.setupHeader(this.headerElement, shell);
}

this.handleSelection = this.handleSelection.bind(this);
Expand Down Expand Up @@ -250,7 +250,7 @@ class DrawingModeController extends AnnotationModeController {
*/
enter() {
super.enter();
replaceHeader(this.container, SELECTOR_DRAW_MODE_HEADER);
replaceHeader(this.headerElement, SELECTOR_DRAW_MODE_HEADER);
this.annotatedElement.classList.add(CLASS_ANNOTATION_DRAW_MODE);
}

Expand Down
6 changes: 3 additions & 3 deletions src/controllers/PointModeController.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ class PointModeController extends AnnotationModeController {
// light, dark, or no value given), then we want to use our draw
// header. Otherwise we expect header UI to be handled by Preview’s
// consumer
if (data.options.header !== 'none') {
this.setupHeader(this.container, shell);
if (data.options.header !== 'none' || this.headerElement) {
this.setupHeader(this.headerElement, shell);
}
}

Expand Down Expand Up @@ -138,7 +138,7 @@ class PointModeController extends AnnotationModeController {
/** @inheritdoc */
enter() {
super.enter();
replaceHeader(this.container, SELECTOR_POINT_MODE_HEADER);
replaceHeader(this.headerElement, SELECTOR_POINT_MODE_HEADER);
this.annotatedElement.classList.add(CLASS_ANNOTATION_POINT_MODE);

if (this.buttonEl) {
Expand Down
78 changes: 75 additions & 3 deletions src/controllers/__tests__/AnnotationModeController-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ describe('controllers/AnnotationModeController', () => {
});

describe('destroy()', () => {
beforeEach(() => {
controller.hideButton = jest.fn();
});

it('should destroy all the threads in controller', () => {
// eslint-disable-next-line new-cap
controller.threads = { 1: new rbush() };
Expand All @@ -89,17 +93,33 @@ describe('controllers/AnnotationModeController', () => {
controller.destroy();
expect(controller.buttonEl.removeEventListener).toBeCalled();
});

it('should hide the button if modeButton exists', () => {
controller.modeButton = {};
controller.destroy();
expect(controller.hideButton).toBeCalled();
});

it('should not hide the button if modeButton does not exist', () => {
controller.modeButton = undefined;
controller.destroy();
expect(controller.hideButton).not.toBeCalled();
});
});

describe('getButton', () => {
it('should return the annotation mode button', () => {
const buttonEl = document.createElement('button');
buttonEl.classList.add('class');
controller.container = document.createElement('div');
controller.container.appendChild(buttonEl);
controller.headerElement = document.createElement('div');
controller.headerElement.appendChild(buttonEl);

expect(controller.getButton('.class')).not.toBeNull();
});

it('should return null if no headerElement', () => {
expect(controller.getButton('.class')).toBeNull();
});
});

describe('showButton()', () => {
Expand Down Expand Up @@ -139,6 +159,58 @@ describe('controllers/AnnotationModeController', () => {
expect(buttonEl.classList).not.toContain(CLASS_HIDDEN);
expect(buttonEl.addEventListener).toBeCalledWith('click', controller.toggleMode);
});

it('should do nothing if no modeButton', () => {
controller.modeButton = undefined;
controller.permissions.canAnnotate = false;
controller.showButton();
expect(buttonEl.classList).toContain(CLASS_HIDDEN);
});
});

describe('hideButton()', () => {
let buttonEl;

beforeEach(() => {
controller.modeButton = {
type: {
title: 'Annotation Mode',
selector: '.selector'
}
};
buttonEl = document.createElement('button');
buttonEl.title = controller.modeButton.title;
// buttonEl.classList.add(CLASS_HIDDEN);
buttonEl.classList.add('selector');
buttonEl.addEventListener = jest.fn();

controller.permissions = { canAnnotate: true };
controller.getButton = jest.fn().mockReturnValue(buttonEl);
});

it('should do nothing if user cannot annotate', () => {
controller.permissions.canAnnotate = false;
controller.hideButton();
expect(buttonEl.classList).not.toContain(CLASS_HIDDEN);
});

it('should do nothing if button is not found', () => {
controller.getButton = jest.fn();
controller.hideButton();
expect(buttonEl.classList).not.toContain(CLASS_HIDDEN);
});

it('should add the bp-is-hidden class to the button', () => {
controller.hideButton();
expect(buttonEl.classList).toContain(CLASS_HIDDEN);
});

it('should do nothing if no modeButton', () => {
controller.modeButton = undefined;
controller.permissions.canAnnotate = false;
controller.hideButton();
expect(buttonEl.classList).not.toContain(CLASS_HIDDEN);
});
});

describe('toggleMode()', () => {
Expand Down Expand Up @@ -178,7 +250,7 @@ describe('controllers/AnnotationModeController', () => {
expect(controller.emit).toBeCalledWith(CONTROLLER_EVENT.exit, expect.any(Object));
expect(controller.unbindListeners).toBeCalled();
expect(controller.emit).toBeCalledWith('binddomlisteners');
expect(util.replaceHeader).toBeCalledWith(controller.container, SELECTOR_BOX_PREVIEW_BASE_HEADER);
expect(util.replaceHeader).toBeCalledWith(controller.headerElement, SELECTOR_BOX_PREVIEW_BASE_HEADER);
});
});

Expand Down
6 changes: 4 additions & 2 deletions src/controllers/__tests__/PointModeController-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ describe('controllers/PointModeController', () => {
// Set up annotation mode
controller.annotatedElement = document.createElement('div');
controller.annotatedElement.classList.add(CLASS_ANNOTATION_MODE);
controller.headerElement = document.createElement('div');

controller.buttonEl = document.createElement('button');
controller.buttonEl.classList.add(CLASS_ACTIVE);
Expand Down Expand Up @@ -180,6 +181,7 @@ describe('controllers/PointModeController', () => {

controller.annotatedElement = document.createElement('div');
controller.annotatedElement.classList.add(CLASS_ANNOTATION_MODE);
controller.headerElement = document.createElement('div');

controller.buttonEl = document.createElement('button');
controller.buttonEl.classList.add(CLASS_ACTIVE);
Expand All @@ -189,14 +191,14 @@ describe('controllers/PointModeController', () => {
controller.enter();
expect(controller.emit).toBeCalledWith(CONTROLLER_EVENT.enter, expect.any(Object));
expect(controller.bindListeners).toBeCalled();
expect(util.replaceHeader).toBeCalledWith(controller.container, SELECTOR_POINT_MODE_HEADER);
expect(util.replaceHeader).toBeCalledWith(controller.headerElement, SELECTOR_POINT_MODE_HEADER);
});

it('should activate mode button if available', () => {
controller.buttonEl = document.createElement('button');
controller.enter();
expect(controller.buttonEl.classList).toContain(CLASS_ACTIVE);
expect(util.replaceHeader).toBeCalledWith(controller.container, SELECTOR_POINT_MODE_HEADER);
expect(util.replaceHeader).toBeCalledWith(controller.headerElement, SELECTOR_POINT_MODE_HEADER);
});
});

Expand Down

0 comments on commit 91cdce3

Please sign in to comment.