Chore: Paging draw threads in the controller's internal thread map #6
Conversation
pramodsum
force-pushed
the
master
branch
18 times, most recently
from
b5efd2a
to
3521db1
Compare
| @@ -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' | |||
There was a problem hiding this comment.
why would we default to page 1, shouldn't we throw an error if there is no page?
There was a problem hiding this comment.
As far as I remember, this was done so that we could allow for annotations on viewers w/o actual pages such as the media viewer, text viewer, etc. We defaulted to 1 page for those cases
There was a problem hiding this comment.
Do we want to make a change so that we default to null, in the case that we have an annotation that doesn't require pages? It might want to do it in another PR, though. Or do it now...
ie) video, audio, 3D models
There was a problem hiding this comment.
I'm ok with re-visiting this in another PR. I'll make a ticket for myself 👍
src/AnnotationModeController.js
Outdated
| @@ -3,7 +3,7 @@ import { insertTemplate } from './annotatorUtil'; | |||
|
|
|||
| class AnnotationModeController extends EventEmitter { | |||
| /** @property {Array} - The array of annotation threads */ | |||
There was a problem hiding this comment.
Update comment, maybe also add what the key and values should be?
| @@ -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' | |||
There was a problem hiding this comment.
Do we want to make a change so that we default to null, in the case that we have an annotation that doesn't require pages? It might want to do it in another PR, though. Or do it now...
ie) video, audio, 3D models
Original PR box/box-content-preview#450