Skip to content

Commit

Permalink
Feature: drawing annotation scaling (#267)
Browse files Browse the repository at this point in the history
* Fix: AnnotationThread.save incorrectly rejects when no dialogue exists
* Update: DrawingAnnotations now rescale correctly
* Fix: annotator is now loaded with the correct initial scale
* Fix: update annotator tests for constructor change
* Update: tests for DocDrawingThread
* Fix: sentenced to 1 year of unit tests
* Fix: pull request feedback
* Update: remove todo
* Chore: update the draw states variable
* Fix: bind cleanup listeners on drawing thread and fix resize
  • Loading branch information
Minh-Ng authored Aug 15, 2017
1 parent 18daaf9 commit 2fd0655
Show file tree
Hide file tree
Showing 16 changed files with 613 additions and 173 deletions.
4 changes: 2 additions & 2 deletions src/lib/annotations/Annotator.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,7 @@ class Annotator extends EventEmitter {
this.showModeAnnotateButton(type);
});

const scale = initialScale;
this.setScale(scale);
this.setScale(initialScale);
this.setupAnnotations();
this.showAnnotations();
}
Expand Down Expand Up @@ -671,6 +670,7 @@ class Annotator extends EventEmitter {
});
} else if (mode === TYPES.draw) {
const drawingThread = this.createAnnotationThread([], {}, TYPES.draw);
this.bindCustomListenersOnThread(drawingThread);

/* eslint-disable require-jsdoc */
const locationFunction = (event) => this.getLocationFromEvent(event, TYPES.point);
Expand Down
1 change: 1 addition & 0 deletions src/lib/annotations/Annotator.scss
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ $avatar-color-9: #f22c44;
// CSS for highlights
//------------------------------------------------------------------------------
.bp-annotation-layer-draw,
.bp-annotation-layer-draw-in-progress,
.bp-annotation-layer-highlight {
cursor: text;
left: 0;
Expand Down
19 changes: 9 additions & 10 deletions src/lib/annotations/__tests__/Annotator-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -484,11 +484,20 @@ describe('lib/annotations/Annotator', () => {
});

describe('bindModeListeners()', () => {
let drawingThread;

beforeEach(() => {
annotator.annotatedElement = {
addEventListener: sandbox.stub(),
removeEventListener: sandbox.stub()
};

drawingThread = {
handleStart: () => {},
handleStop: () => {},
handleMove: () => {},
addListener: sandbox.stub()
};
});

it('should get event handlers for point annotation mode', () => {
Expand All @@ -505,11 +514,6 @@ describe('lib/annotations/Annotator', () => {
});

it('should bind draw mode handlers', () => {
const drawingThread = {
handleStart: () => {},
handleStop: () => {},
handleMove: () => {}
};
sandbox.stub(annotator, 'createAnnotationThread').returns(drawingThread);

const postButtonEl = {
Expand All @@ -535,11 +539,6 @@ describe('lib/annotations/Annotator', () => {
});

it('should bind draw mode click handlers if post button exists', () => {
const drawingThread = {
handleStart: () => {},
handleStop: () => {},
handleMove: () => {}
};
sandbox.stub(annotator, 'createAnnotationThread').returns(drawingThread);

const postButtonEl = {
Expand Down
34 changes: 33 additions & 1 deletion src/lib/annotations/__tests__/annotatorUtil-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ import {
eventToLocationHandler,
decodeKeydown,
getHeaders,
replacePlaceholders
replacePlaceholders,
createLocation,
round
} from '../annotatorUtil';
import {
STATES,
Expand Down Expand Up @@ -404,6 +406,26 @@ describe('lib/annotations/annotatorUtil', () => {
});
});

describe('createLocation()', () => {
it('should create a location object without dimensions', () => {
const location = createLocation(1,2, undefined);
expect(location).to.deep.equal({
x: 1,
y: 2
});
});

it('should create a location object with dimensions', () => {
const dimensionalObj = 'dimensional object';
const location = createLocation(1,2, dimensionalObj);
expect(location).to.deep.equal({
x: 1,
y: 2,
dimensions: dimensionalObj
});
});
});

describe('decodeKeydown()', () => {
it('should return empty when no key', () => {
assert.equal(
Expand Down Expand Up @@ -531,6 +553,16 @@ describe('lib/annotations/annotatorUtil', () => {
});
});

describe('round()', () => {
it('should round to the correct decimal precision', () => {
const floatNum = 123456789.887654321;
expect(round(floatNum, 0)).to.equal(Math.ceil(floatNum));
expect(round(floatNum, 1)).to.equal(123456789.9);
expect(round(floatNum, 2)).to.equal(123456789.89);
expect(round(floatNum, 3)).to.equal(123456789.888);
expect(round(floatNum, 4)).to.equal(123456789.8877);
});
});
describe('replacePlaceholders()', () => {
it('should replace only the placeholder with the custom value in the given string', () => {
expect(replacePlaceholders('{1} highlighted', ['Bob'])).to.equal('Bob highlighted');
Expand Down
7 changes: 4 additions & 3 deletions src/lib/annotations/annotationConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export const CLASS_ADD_HIGHLIGHT_BTN = 'bp-add-highlight-btn';
export const CLASS_ADD_HIGHLIGHT_COMMENT_BTN = 'bp-highlight-comment-btn';
export const CLASS_ANNOTATION_LAYER_HIGHLIGHT = 'bp-annotation-layer-highlight';
export const CLASS_ANNOTATION_LAYER_DRAW = 'bp-annotation-layer-draw';
export const CLASS_ANNOTATION_LAYER_DRAW_IN_PROGRESS = 'bp-annotation-layer-draw-in-progress';
export const CLASS_ANNOTATION_BUTTON_POINT = 'bp-btn-annotate-point';
export const CLASS_ANNOTATION_BUTTON_DRAW_POST = 'bp-btn-annotate-draw-post';
export const CLASS_ANNOTATION_BUTTON_DRAW_CANCEL = 'bp-btn-annotate-draw-cancel';
Expand Down Expand Up @@ -64,10 +65,10 @@ export const SELECTOR_DIALOG_CLOSE = `.${CLASS_DIALOG_CLOSE}`;
export const SELECTOR_HIGHLIGHT_BTNS = `.${CLASS_HIGHLIGHT_BTNS}`;
export const SELECTOR_ADD_HIGHLIGHT_BTN = `.${CLASS_ADD_HIGHLIGHT_BTN}`;

export const STATES_DRAW = {
draw: 'draw',
export const DRAW_STATES = {
idle: 'idle',
erase: 'erase'
drawing: 'drawing',
erasing: 'erasing'
};

export const STATES = {
Expand Down
33 changes: 33 additions & 0 deletions src/lib/annotations/annotatorUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,23 @@ export function eventToLocationHandler(locationFunction, callback) {
};
}

/**
* Create a JSON object containing x/y coordinates and optionally dimensional information
*
* @param {number} x - The x position of the location object
* @param {number} y - The y position of the location object
* @param {Object} [dimensions] - The dimensional information of the location object
* @return {Object} - A location object with x/y position information as well as provided dimensional information
*/
export function createLocation(x, y, dimensions) {
const loc = { x, y };
if (dimensions) {
loc.dimensions = dimensions;
}

return loc;
}

//------------------------------------------------------------------------------
// General Util Methods
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -502,6 +519,22 @@ export function getHeaders(headers = {}, token = '', sharedLink = '', password =
return headers;
}

/**
* Round a number to a certain decimal place by concatenating an exponential factor. Credits to lodash library.
*
* @param {number} number - The number to be rounded
* @param {number} precision - The amount of decimal places to keep
* @return {number} The rounded number
*/
export function round(number, precision) {
/* eslint-disable prefer-template */
let pair = (number + 'e').split('e');
const value = Math.round(pair[0] + 'e' + (+pair[1] + precision));
pair = (value + 'e').split('e');
return +(pair[0] + 'e' + (+pair[1] - precision));
/* eslint-enable prefer-template */
}

/**
* Replaces variable place holders specified between {} in the string with
* specified custom value. Localizes strings that include variables.
Expand Down
9 changes: 6 additions & 3 deletions src/lib/annotations/doc/DocAnnotator.js
Original file line number Diff line number Diff line change
Expand Up @@ -308,11 +308,14 @@ class DocAnnotator extends Annotator {
thread = new DocDrawingThread(threadParams);
} else if (type === TYPES.point) {
thread = new DocPointThread(threadParams);
} else {
throw new Error(`Unhandled document annotation type: ${type}`);
}

this.addThreadToMap(thread);
if (!thread && this.notification) {
this.emit('annotationerror', __('annotations_create_error'));
} else if (thread) {
this.addThreadToMap(thread);
}

return thread;
}

Expand Down
Loading

0 comments on commit 2fd0655

Please sign in to comment.