Skip to content

Commit

Permalink
Chore: Paging draw threads in the controller's internal thread map (#6)
Browse files Browse the repository at this point in the history
  • Loading branch information
@pramodsum
pramodsum authored Oct 31, 2017
1 parent 080c0f3 commit fc52e9e
Show file tree
Hide file tree
Showing 4 changed files with 222 additions and 217 deletions.
14 changes: 10 additions & 4 deletions src/AnnotationModeController.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import EventEmitter from 'events';
import { insertTemplate } from './annotatorUtil';

class AnnotationModeController extends EventEmitter {
/** @property {Array} - The array of annotation threads */
threads = [];
/** @property {Object} - Object of annotation threads indexed by threadID */
threads = {};

/** @property {Array} - The array of annotation handlers */
handlers = [];
Expand Down Expand Up @@ -64,7 +64,12 @@ class AnnotationModeController extends EventEmitter {
* @return {void}
*/
registerThread(thread) {
this.threads.push(thread);
const page = thread.location.page || 1; // Defaults to page 1 if thread has no page'
if (!(page in this.threads)) {
this.threads[page] = {};
}
const pageThreads = this.threads[page];
pageThreads[thread.threadID] = thread;
}

/**
Expand All @@ -75,7 +80,8 @@ class AnnotationModeController extends EventEmitter {
* @return {void}
*/
unregisterThread(thread) {
this.threads = this.threads.filter((item) => item !== thread);
const page = thread.location.page || 1;
delete this.threads[page][thread.threadID];
}

/**
Expand Down
96 changes: 53 additions & 43 deletions src/__tests__/AnnotationModeController-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,29 @@ import AnnotationModeController from '../AnnotationModeController';
import DocDrawingThread from '../doc/DocDrawingThread';
import * as util from '../annotatorUtil';

let annotationModeController;
let controller;
let stubs;
const sandbox = sinon.sandbox.create();

describe('AnnotationModeController', () => {
beforeEach(() => {
annotationModeController = new AnnotationModeController();
controller = new AnnotationModeController();
stubs = {};
});

afterEach(() => {
sandbox.verifyAndRestore();
stubs = null;
annotationModeController = null;
controller = null;
});

describe('registerAnnotator()', () => {
it('should internally keep track of the registered annotator', () => {
const annotator = 'I am an annotator';
expect(annotationModeController.annotator).to.be.undefined;
expect(controller.annotator).to.be.undefined;

annotationModeController.registerAnnotator(annotator);
expect(annotationModeController.annotator).to.equal(annotator);
controller.registerAnnotator(annotator);
expect(controller.annotator).to.equal(annotator);
});
});

Expand All @@ -37,14 +37,14 @@ describe('AnnotationModeController', () => {
addEventListener: sandbox.stub()
}
};
sandbox.stub(annotationModeController, 'setupHandlers', () => {
annotationModeController.handlers = [handlerObj];
sandbox.stub(controller, 'setupHandlers', () => {
controller.handlers = [handlerObj];
});
expect(annotationModeController.handlers.length).to.equal(0);
expect(controller.handlers.length).to.equal(0);

annotationModeController.bindModeListeners();
controller.bindModeListeners();
expect(handlerObj.eventObj.addEventListener).to.be.calledWith(handlerObj.type, handlerObj.func);
expect(annotationModeController.handlers.length).to.equal(1);
expect(controller.handlers.length).to.equal(1);
});
});

Expand All @@ -58,56 +58,66 @@ describe('AnnotationModeController', () => {
}
};

annotationModeController.handlers = [handlerObj];
expect(annotationModeController.handlers.length).to.equal(1);
controller.handlers = [handlerObj];
expect(controller.handlers.length).to.equal(1);

annotationModeController.unbindModeListeners();
controller.unbindModeListeners();
expect(handlerObj.eventObj.removeEventListener).to.be.calledWith(handlerObj.type, handlerObj.func);
expect(annotationModeController.handlers.length).to.equal(0);
expect(controller.handlers.length).to.equal(0);
});
});

describe('registerThread()', () => {
it('should internally keep track of the registered thread', () => {
const thread = 'I am a thread';
expect(annotationModeController.threads.includes(thread)).to.be.falsy;
controller.threads = { 1: {} };
const pageThreads = controller.threads[1];
const thread = {
threadID: '123abc',
location: { page: 1 }
};
expect(thread.threadID in pageThreads).to.be.falsy;

annotationModeController.registerThread(thread);
expect(annotationModeController.threads.includes(thread)).to.be.truthy;
controller.registerThread(thread);
expect(pageThreads[thread.threadID]).equals(thread);
});
});

describe('unregisterThread()', () => {
it('should internally keep track of the registered thread', () => {
const thread = 'I am a thread';
annotationModeController.threads = [thread, 'other'];
expect(annotationModeController.threads.includes(thread)).to.be.truthy;
controller.threads = { 1: {} };
const pageThreads = controller.threads[1];
const thread = {
threadID: '123abc',
location: { page: 1 }
};
controller.registerThread(thread);
expect(thread.threadID in pageThreads).to.be.truthy;

annotationModeController.unregisterThread(thread);
expect(annotationModeController.threads.includes(thread)).to.be.falsy;
controller.unregisterThread(thread);
expect(thread.threadID in pageThreads).to.be.falsy;
});
});

describe('bindCustomListenersOnThread()', () => {
it('should do nothing when the input is empty', () => {
annotationModeController.annotator = {
controller.annotator = {
bindCustomListenersOnThread: sandbox.stub()
};

annotationModeController.bindCustomListenersOnThread(undefined);
expect(annotationModeController.annotator.bindCustomListenersOnThread).to.not.be.called;
controller.bindCustomListenersOnThread(undefined);
expect(controller.annotator.bindCustomListenersOnThread).to.not.be.called;
});

it('should bind custom listeners on thread', () => {
const thread = {
addListener: sandbox.stub()
};
annotationModeController.annotator = {
controller.annotator = {
bindCustomListenersOnThread: sandbox.stub()
};

annotationModeController.bindCustomListenersOnThread(thread);
expect(annotationModeController.annotator.bindCustomListenersOnThread).to.be.called;
controller.bindCustomListenersOnThread(thread);
expect(controller.annotator.bindCustomListenersOnThread).to.be.called;
expect(thread.addListener).to.be.called;
});

Expand All @@ -119,14 +129,14 @@ describe('AnnotationModeController', () => {
Object.defineProperty(DocDrawingThread.prototype, 'getThreadEventData', { value: sandbox.stub() });
const thread = new DocDrawingThread({ threadID: 123 });

annotationModeController.handleAnnotationEvent = () => {
expect(annotationModeController.annotator).to.not.be.undefined;
controller.handleAnnotationEvent = () => {
expect(controller.annotator).to.not.be.undefined;
};
annotationModeController.annotator = {
controller.annotator = {
bindCustomListenersOnThread: sandbox.stub()
};

annotationModeController.bindCustomListenersOnThread(thread);
controller.bindCustomListenersOnThread(thread);
thread.emit('threadevent', {});
});
});
Expand All @@ -137,7 +147,7 @@ describe('AnnotationModeController', () => {
removeAllListeners: sandbox.stub()
};

annotationModeController.unbindCustomListenersOnThread(undefined);
controller.unbindCustomListenersOnThread(undefined);
expect(thread.removeAllListeners).to.not.be.called;
});

Expand All @@ -146,28 +156,28 @@ describe('AnnotationModeController', () => {
removeAllListeners: sandbox.stub()
};

annotationModeController.unbindCustomListenersOnThread(thread);
controller.unbindCustomListenersOnThread(thread);
expect(thread.removeAllListeners).to.be.calledWith('threadevent');
});
});

describe('pushElementHandler()', () => {
it('should do nothing when the element is invalid', () => {
const lengthBefore = annotationModeController.handlers.length;
const lengthBefore = controller.handlers.length;

annotationModeController.pushElementHandler(undefined, 'type', () => {});
const lengthAfter = annotationModeController.handlers.length;
controller.pushElementHandler(undefined, 'type', () => {});
const lengthAfter = controller.handlers.length;
expect(lengthAfter).to.equal(lengthBefore);
});

it('should add a handler descriptor to the handlers array', () => {
const lengthBefore = annotationModeController.handlers.length;
const lengthBefore = controller.handlers.length;
const element = 'element';
const type = ['type1', 'type2'];
const fn = 'fn';

annotationModeController.pushElementHandler(element, type, fn);
const handlers = annotationModeController.handlers;
controller.pushElementHandler(element, type, fn);
const handlers = controller.handlers;
const lengthAfter = handlers.length;
expect(lengthAfter).to.equal(lengthBefore+1);
expect(handlers[handlers.length - 1]).to.deep.equal({
Expand All @@ -186,7 +196,7 @@ describe('AnnotationModeController', () => {
};
const header = document.createElement('div');

annotationModeController.setupHeader(container, header);
controller.setupHeader(container, header);

expect(stubs.insertTemplate).to.be.calledWith(container, header);
});
Expand Down
23 changes: 12 additions & 11 deletions src/drawing/DrawingModeController.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,6 @@ import {
} from '../annotationConstants';

class DrawingModeController extends AnnotationModeController {
/* eslint-disable new-cap */
/** @property {Array} - The array of annotation threads */
threads = new rbush();
/* eslint-enable new-cap */

/** @property {DrawingThread} - The currently selected DrawingThread */
selectedThread;

Expand Down Expand Up @@ -72,7 +67,13 @@ class DrawingModeController extends AnnotationModeController {
return;
}

this.threads.insert(thread);
const page = thread.location.page || 1; // Defaults to page 1 if thread has no page'
if (!(page in this.threads)) {
/* eslint-disable new-cap */
this.threads[page] = new rbush();
/* eslint-enable new-cap */
}
this.threads[page].insert(thread);
}

/**
Expand All @@ -88,7 +89,8 @@ class DrawingModeController extends AnnotationModeController {
return;
}

this.threads.remove(thread);
const page = thread.location.page || 1;
this.threads[page].remove(thread);
}

/**
Expand Down Expand Up @@ -233,7 +235,8 @@ class DrawingModeController extends AnnotationModeController {
this.unregisterThread(thread);

// Redraw any threads that the deleted thread could have been overlapping
this.threads.search(thread).forEach((drawingThread) => drawingThread.show());
const page = thread.location.page;
this.threads[page].all().forEach((drawingThread) => drawingThread.show());
}

break;
Expand Down Expand Up @@ -270,9 +273,7 @@ class DrawingModeController extends AnnotationModeController {
};

// Get the threads that correspond to the point that was clicked on
const intersectingThreads = this.threads
.search(eventBoundary)
.filter((drawingThread) => drawingThread.location.page === location.page);
const intersectingThreads = this.threads[location.page].search(eventBoundary);

// Clear boundary on previously selected thread
this.removeSelection();
Expand Down
Loading

0 comments on commit fc52e9e

Please sign in to comment.