Skip to content

Commit

Permalink
Chore: Removing references to annotations from individual viewers (#271)
Browse files Browse the repository at this point in the history 
- Use touchstart and mousedown to trigger the creation of point annotations
- Swap out event with touchEvent if the user is on a mobile device
  • Loading branch information
@pramodsum
pramodsum authored Aug 2, 2017
1 parent 1a7c710 commit 2a1e6cd
Show file tree
Hide file tree
Showing 11 changed files with 206 additions and 215 deletions.
26 changes: 18 additions & 8 deletions src/lib/annotations/Annotator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -545,14 +545,23 @@ class Annotator extends EventEmitter {
*/
bindPointModeListeners() {
const pointFunc = this.pointClickHandler.bind(this.annotatedElement);
const handler = {
type: 'click',
func: pointFunc,
eventObj: this.annotatedElement
};

handler.eventObj.addEventListener(handler.type, handler.func);
this.annotationModeHandlers.push(handler);
const handlers = [
{
type: 'mousedown',
func: pointFunc,
eventObj: this.annotatedElement
},
{
type: 'touchstart',
func: pointFunc,
eventObj: this.annotatedElement
}
];

handlers.forEach((handler) => {
handler.eventObj.addEventListener(handler.type, handler.func);
this.annotationModeHandlers.push(handler);
});
}

/**
Expand All @@ -565,6 +574,7 @@ class Annotator extends EventEmitter {
*/
pointClickHandler(event) {
event.stopPropagation();
event.preventDefault();

// Determine if a point annotation dialog is already open and close the
// current open dialog
Expand Down
39 changes: 24 additions & 15 deletions src/lib/annotations/__tests__/Annotator-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ describe('lib/annotations/Annotator', () => {
});
});

describe('setupAnnotations', () => {
describe('setupAnnotations()', () => {
it('should initialize thread map and bind DOM listeners', () => {
sandbox.stub(annotator, 'bindDOMListeners');
sandbox.stub(annotator, 'bindCustomListenersOnService');
Expand Down Expand Up @@ -341,7 +341,7 @@ describe('lib/annotations/Annotator', () => {
});
});

describe('fetchAnnotations', () => {
describe('fetchAnnotations()', () => {
beforeEach(() => {
annotator.annotationService = {
getThreadMap: () => {}
Expand Down Expand Up @@ -380,7 +380,7 @@ describe('lib/annotations/Annotator', () => {
});
});

describe('bindCustomListenersOnService', () => {
describe('bindCustomListenersOnService()', () => {
it('should do nothing if the service does not exist', () => {
annotator.annotationService = {
addListener: sandbox.stub()
Expand Down Expand Up @@ -438,7 +438,7 @@ describe('lib/annotations/Annotator', () => {
});
});

describe('unbindCustomListenersOnService', () => {
describe('unbindCustomListenersOnService()', () => {
it('should do nothing if the service does not exist', () => {
annotator.annotationService = {
removeListener: sandbox.stub()
Expand All @@ -463,23 +463,23 @@ describe('lib/annotations/Annotator', () => {
});
});

describe('bindCustomListenersOnThread', () => {
describe('bindCustomListenersOnThread()', () => {
it('should bind custom listeners on the thread', () => {
stubs.threadMock.expects('addListener').withArgs('threaddeleted', sinon.match.func);
stubs.threadMock.expects('addListener').withArgs('threadcleanup', sinon.match.func);
annotator.bindCustomListenersOnThread(stubs.thread);
});
});

describe('unbindCustomListenersOnThread', () => {
describe('unbindCustomListenersOnThread()', () => {
it('should unbind custom listeners from the thread', () => {
stubs.threadMock.expects('removeAllListeners').withArgs('threaddeleted');
stubs.threadMock.expects('removeAllListeners').withArgs('threadcleanup');
annotator.unbindCustomListenersOnThread(stubs.thread);
});
});

describe('bindPointModeListeners', () => {
describe('bindPointModeListeners()', () => {
it('should bind point mode click handler', () => {
sandbox.stub(annotator.annotatedElement, 'addEventListener');
sandbox.stub(annotator.annotatedElement, 'removeEventListener');
Expand All @@ -488,7 +488,11 @@ describe('lib/annotations/Annotator', () => {
annotator.bindPointModeListeners();
expect(annotator.pointClickHandler.bind).to.be.called;
expect(annotator.annotatedElement.addEventListener).to.be.calledWith(
'click',
'mousedown',
annotator.pointClickHandler
);
expect(annotator.annotatedElement.addEventListener).to.be.calledWith(
'touchstart',
annotator.pointClickHandler
);
});
Expand All @@ -503,7 +507,11 @@ describe('lib/annotations/Annotator', () => {
annotator.unbindModeListeners();
expect(annotator.pointClickHandler.bind).to.be.called;
expect(annotator.annotatedElement.removeEventListener).to.be.calledWith(
'click',
'mousedown',
annotator.pointClickHandler
);
expect(annotator.annotatedElement.removeEventListener).to.be.calledWith(
'touchstart',
annotator.pointClickHandler
);
});
Expand Down Expand Up @@ -542,7 +550,8 @@ describe('lib/annotations/Annotator', () => {

describe('pointClickHandler()', () => {
const event = {
stopPropagation: () => {}
stopPropagation: () => {},
preventDefault: () => {}
};

beforeEach(() => {
Expand Down Expand Up @@ -603,7 +612,7 @@ describe('lib/annotations/Annotator', () => {
});
});

describe('bindDrawModeListeners', () => {
describe('bindDrawModeListeners()', () => {
it('should do nothing if neither a thread nor a post button is not provided', () => {
const drawingThread = {
handleStart: () => {},
Expand Down Expand Up @@ -661,7 +670,7 @@ describe('lib/annotations/Annotator', () => {
});
});

describe('addToThreadMap', () => {
describe('addToThreadMap()', () => {
it('should add valid threads to the thread map', () => {
stubs.thread.location = { page: 2 };
stubs.thread2.location = { page: 3 };
Expand All @@ -684,7 +693,7 @@ describe('lib/annotations/Annotator', () => {
});
});

describe('isInPointMode', () => {
describe('isInPointMode()', () => {
it('should return whether the annotator is in point mode or not', () => {
annotator.annotatedElement.classList.add(CLASS_ANNOTATION_POINT_MODE);
expect(annotator.isInPointMode()).to.be.true;
Expand All @@ -694,7 +703,7 @@ describe('lib/annotations/Annotator', () => {
});
});

describe('isInDrawMode', () => {
describe('isInDrawMode()', () => {
it('should return whether the annotator is in draw mode or not', () => {
annotator.annotatedElement.classList.add(CLASS_ANNOTATION_DRAW_MODE);
expect(annotator.isInDrawMode()).to.be.true;
Expand All @@ -704,7 +713,7 @@ describe('lib/annotations/Annotator', () => {
});
});

describe('destroyPendingThreads', () => {
describe('destroyPendingThreads()', () => {
beforeEach(() => {
stubs.thread = {
location: { page: 2 },
Expand Down
39 changes: 26 additions & 13 deletions src/lib/annotations/doc/DocAnnotator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,16 +174,24 @@ class DocAnnotator extends Annotator {
let location = null;
const zoomScale = annotatorUtil.getScale(this.annotatedElement);

if (annotationType === TYPES.point) {
// If there is a selection, ignore
if (docAnnotatorUtil.isSelectionPresent()) {
let clientEvent = event;
if (this.isMobile) {
if (!event.targetTouches || event.targetTouches.length === 0) {
return location;
}
clientEvent = event.targetTouches[0];
}

// If click isn't on a page, ignore
const eventTarget = event.target;
const { pageEl, page } = annotatorUtil.getPageInfo(eventTarget);
if (!pageEl) {
// If click isn't on a page, ignore
const eventTarget = clientEvent.target;
const { pageEl, page } = annotatorUtil.getPageInfo(eventTarget);
if (!pageEl) {
return location;
}

if (annotationType === TYPES.point) {
// If there is a selection, ignore
if (docAnnotatorUtil.isSelectionPresent()) {
return location;
}

Expand All @@ -198,15 +206,23 @@ class DocAnnotator extends Annotator {
const pageWidth = pageDimensions.width;
const pageHeight = pageDimensions.height - PAGE_PADDING_TOP - PAGE_PADDING_BOTTOM;
const browserCoordinates = [
event.clientX - pageDimensions.left,
event.clientY - pageDimensions.top - PAGE_PADDING_TOP
clientEvent.clientX - pageDimensions.left,
clientEvent.clientY - pageDimensions.top - PAGE_PADDING_TOP
];
let [x, y] = browserCoordinates;

// Do not create annotation if event doesn't have coordinates
if (isNaN(x) || isNaN(y)) {
this.emit('annotationerror', __('annotations_create_error'));
return location;
}

const pdfCoordinates = docAnnotatorUtil.convertDOMSpaceToPDFSpace(
browserCoordinates,
pageHeight,
zoomScale
);
const [x, y] = pdfCoordinates;
[x, y] = pdfCoordinates;

// We save the dimensions of the annotated element scaled to 100%
// so we can compare to the annotated element during render time
Expand All @@ -222,9 +238,6 @@ class DocAnnotator extends Annotator {
return location;
}

// Get correct page
const { pageEl, page } = annotatorUtil.getPageInfo(event.target);

// Use highlight module to calculate quad points
const { highlightEls } = docAnnotatorUtil.getHighlightAndHighlightEls(this.highlighter, pageEl);

Expand Down
Loading

0 comments on commit 2a1e6cd

Please sign in to comment.