Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore: Store annotation threads by threadID in the thread map #316

Merged
merged 16 commits into from
Sep 5, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 56 additions & 34 deletions src/lib/annotations/Annotator.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ class Annotator extends EventEmitter {
this.unbindModeListeners();

if (this.threads) {
Object.keys(this.threads).forEach((page) => {
this.threads[page].forEach((thread) => {
Object.values(this.threads).forEach((pageThreads) => {
Object.values(pageThreads).forEach((thread) => {
this.unbindCustomListenersOnThread(thread);
});
});
Expand Down Expand Up @@ -250,10 +250,8 @@ class Annotator extends EventEmitter {
* @return {void}
*/
hideAnnotations() {
Object.keys(this.threads).forEach((page) => {
this.threads[page].forEach((thread) => {
thread.hide();
});
Object.keys(this.threads).forEach((pageNum) => {
this.hideAnnotationsOnPage(pageNum);
});
}

Expand All @@ -264,11 +262,14 @@ class Annotator extends EventEmitter {
* @return {void}
*/
hideAnnotationsOnPage(pageNum) {
if (this.threads[pageNum]) {
this.threads[pageNum].forEach((thread) => {
thread.hide();
});
if (!this.threads) {
return;
}

const pageThreads = this.getThreadsOnPage(pageNum);
Object.values(pageThreads).forEach((thread) => {
thread.hide();
});
}

/**
Expand All @@ -278,10 +279,8 @@ class Annotator extends EventEmitter {
* @return {void}
*/
renderAnnotations() {
Object.keys(this.threads).forEach((page) => {
this.threads[page].forEach((thread) => {
thread.show();
});
Object.keys(this.threads).forEach((pageNum) => {
this.renderAnnotationsOnPage(pageNum);
});
}

Expand All @@ -293,11 +292,14 @@ class Annotator extends EventEmitter {
* @return {void}
*/
renderAnnotationsOnPage(pageNum) {
if (this.threads && this.threads[pageNum]) {
this.threads[pageNum].forEach((thread) => {
thread.show();
});
if (!this.threads) {
return;
}

const pageThreads = this.getThreadsOnPage(pageNum);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be setup so that if it is given a certain value, it renders ALL annotations?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean? We are currently rendering all annotations on that page

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, nvm

Object.values(pageThreads).forEach((thread) => {
thread.show();
});
}

/**
Expand Down Expand Up @@ -525,7 +527,7 @@ class Annotator extends EventEmitter {
* @return {void}
*/
setupAnnotations() {
// Map of page => [threads on page]
// Map of page => {threads on page}
this.threads = {};
this.bindDOMListeners();
this.bindCustomListenersOnService(this.annotationService);
Expand Down Expand Up @@ -675,14 +677,7 @@ class Annotator extends EventEmitter {

// Thread was deleted, remove from thread map
thread.addListener('threaddeleted', () => {
const page = thread.location.page || 1;

// Remove from map
if (this.threads[page] instanceof Array) {
this.threads[page] = this.threads[page].filter(
(searchThread) => searchThread.threadID !== thread.threadID
);
}
this.removeThreadFromMap(thread);
});

// Thread should be cleaned up, unbind listeners - we don't do this
Expand Down Expand Up @@ -809,9 +804,21 @@ class Annotator extends EventEmitter {
*/
addThreadToMap(thread) {
// Add thread to in-memory map
const page = thread.location.page || 1; // Defaults to page 1 if thread has no page
this.threads[page] = this.threads[page] || [];
this.threads[page].push(thread);
const page = thread.location.page || 1; // Defaults to page 1 if thread has no page'
const pageThreads = this.getThreadsOnPage(page);
pageThreads[thread.threadID] = thread;
}

/**
* Removes thread to in-memory map.
*
* @protected
* @param {AnnotationThread} thread - Thread to bind events to
* @return {void}
*/
removeThreadFromMap(thread) {
const page = thread.location.page || 1;
delete this.threads[page][thread.threadID];
}

/**
Expand Down Expand Up @@ -840,6 +847,20 @@ class Annotator extends EventEmitter {
this.setScale(data.scale);
this.rotateAnnotations(data.rotationAngle, data.pageNum);
}
/**
* Gets threads on page
*
* @private
* @param {number} page - Current page number
* @return {Map|[]} Threads on page
*/
getThreadsOnPage(page) {
if (!(page in this.threads)) {
this.threads[page] = {};
}

return this.threads[page];
}

/**
* Destroys pending threads.
Expand All @@ -850,11 +871,12 @@ class Annotator extends EventEmitter {
*/
destroyPendingThreads() {
let hasPendingThreads = false;
Object.keys(this.threads).forEach((page) => {
this.threads[page].forEach((pendingThread) => {
if (annotatorUtil.isPending(pendingThread.state)) {

Object.values(this.threads).forEach((pageThreads) => {
Object.values(pageThreads).forEach((thread) => {
if (annotatorUtil.isPending(thread.state)) {
hasPendingThreads = true;
pendingThread.destroy();
thread.destroy();
}
});
});
Expand Down
Loading