Skip to content

Commit

Permalink
Feature: Drawing Annotations Undo Redo (#287)
Browse files Browse the repository at this point in the history 
* Fix: thread save incorrectly rejects when no dialogue exists
* Update: drawing annotations now rescale correctly
* Fix: annotator is now loaded with the correct initial scale
* Fix: update annotator tests for constructor change
* Update: tests for updating local annotations on AnnotationThreads
* Fix: clean up anonymous functions
* Fix: various cleanup for PR
* Fix: sentenced to 1 year of unit tests
* Update: undo redo drawing container
* Fix: pull request feedback
* Update: undo and redo grey out when applicable
* Fix: update tests with new fn calls
* Update: drawing annotation icon svgs
* Fix: bind cleanup listeners on drawing thread and fix resize
* Fix: scale annotations in redo stack
* Update: custom thread cleanup for drawing annotationevent
  • Loading branch information
@Minh-Ng
Minh-Ng committed Aug 21, 2017
1 parent 5900ece commit 4b5421c
Show file tree
Hide file tree
Showing 15 changed files with 869 additions and 171 deletions.
21 changes: 13 additions & 8 deletions src/lib/_boxui.scss
View file
Expand Up @@ -83,6 +83,19 @@
}
}

.bp-btn-plain {
background: transparent;
box-shadow: none;
outline: none;

&.is-disabled {
pointer-events: none;
color: #666;
opacity: .4;
background-color: $haze;
}
}

.bp-btn {
-webkit-appearance: none;
background-color: $white;
Expand Down Expand Up @@ -150,14 +163,6 @@
}
}

.bp-btn-plain,
.bp-btn-plain:hover,
.bp-btn-plain:active,
.bp-btn-plain:focus {
background: transparent;
box-shadow: none;
outline: none;
}

//------------------------------------------------------------------------------
// Forms
Expand Down
132 changes: 99 additions & 33 deletions src/lib/annotations/Annotator.js
View file
Expand Up @@ -17,6 +17,8 @@ import {
SELECTOR_ANNOTATION_BUTTON_DRAW_CANCEL,
SELECTOR_ANNOTATION_BUTTON_DRAW_ENTER,
SELECTOR_ANNOTATION_BUTTON_DRAW_POST,
SELECTOR_ANNOTATION_BUTTON_DRAW_UNDO,
SELECTOR_ANNOTATION_BUTTON_DRAW_REDO,
TYPES
} from './annotationConstants';

Expand Down Expand Up @@ -385,13 +387,18 @@ class Annotator extends EventEmitter {

if (mode === TYPES.draw) {
const drawEnterEl = buttonEl.querySelector(SELECTOR_ANNOTATION_BUTTON_DRAW_ENTER);
annotatorUtil.showElement(drawEnterEl);

const drawCancelEl = buttonEl.querySelector(SELECTOR_ANNOTATION_BUTTON_DRAW_CANCEL);
annotatorUtil.hideElement(drawCancelEl);

const postButtonEl = this.getAnnotateButton(SELECTOR_ANNOTATION_BUTTON_DRAW_POST);
const undoButtonEl = this.getAnnotateButton(SELECTOR_ANNOTATION_BUTTON_DRAW_UNDO);
const redoButtonEl = this.getAnnotateButton(SELECTOR_ANNOTATION_BUTTON_DRAW_REDO);

annotatorUtil.showElement(drawEnterEl);
annotatorUtil.hideElement(drawCancelEl);
annotatorUtil.hideElement(postButtonEl);
annotatorUtil.hideElement(undoButtonEl);
annotatorUtil.hideElement(redoButtonEl);
annotatorUtil.disableElement(undoButtonEl);
annotatorUtil.disableElement(redoButtonEl);
}
}

Expand All @@ -414,13 +421,16 @@ class Annotator extends EventEmitter {

if (mode === TYPES.draw) {
const drawEnterEl = buttonEl.querySelector(SELECTOR_ANNOTATION_BUTTON_DRAW_ENTER);
annotatorUtil.hideElement(drawEnterEl);

const drawCancelEl = buttonEl.querySelector(SELECTOR_ANNOTATION_BUTTON_DRAW_CANCEL);
annotatorUtil.showElement(drawCancelEl);

const postButtonEl = this.getAnnotateButton(SELECTOR_ANNOTATION_BUTTON_DRAW_POST);
const undoButtonEl = this.getAnnotateButton(SELECTOR_ANNOTATION_BUTTON_DRAW_UNDO);
const redoButtonEl = this.getAnnotateButton(SELECTOR_ANNOTATION_BUTTON_DRAW_REDO);

annotatorUtil.hideElement(drawEnterEl);
annotatorUtil.showElement(drawCancelEl);
annotatorUtil.showElement(postButtonEl);
annotatorUtil.showElement(undoButtonEl);
annotatorUtil.showElement(redoButtonEl);
}
}

Expand Down Expand Up @@ -657,6 +667,7 @@ class Annotator extends EventEmitter {
unbindCustomListenersOnThread(thread) {
thread.removeAllListeners('threaddeleted');
thread.removeAllListeners('threadcleanup');
thread.removeAllListeners('annotationevent');
}

/**
Expand All @@ -670,16 +681,18 @@ class Annotator extends EventEmitter {
const handlers = [];

if (mode === TYPES.point) {
handlers.push({
type: 'mousedown',
func: this.pointClickHandler,
eventObj: this.annotatedElement
});
handlers.push({
type: 'touchstart',
func: this.pointClickHandler,
eventObj: this.annotatedElement
});
handlers.push(
{
type: 'mousedown',
func: this.pointClickHandler,
eventObj: this.annotatedElement
},
{
type: 'touchstart',
func: this.pointClickHandler,
eventObj: this.annotatedElement
}
);
} else if (mode === TYPES.draw) {
const drawingThread = this.createAnnotationThread([], {}, TYPES.draw);
this.bindCustomListenersOnThread(drawingThread);
Expand All @@ -689,23 +702,56 @@ class Annotator extends EventEmitter {
/* eslint-enable require-jsdoc */

const postButtonEl = this.getAnnotateButton(SELECTOR_ANNOTATION_BUTTON_DRAW_POST);

handlers.push({
type: 'mousemove',
func: annotatorUtil.eventToLocationHandler(locationFunction, drawingThread.handleMove),
eventObj: this.annotatedElement
});
handlers.push({
type: 'mousedown',
func: annotatorUtil.eventToLocationHandler(locationFunction, drawingThread.handleStart),
eventObj: this.annotatedElement
});
handlers.push({
type: 'mouseup',
func: annotatorUtil.eventToLocationHandler(locationFunction, drawingThread.handleStop),
eventObj: this.annotatedElement
const undoButtonEl = this.getAnnotateButton(SELECTOR_ANNOTATION_BUTTON_DRAW_UNDO);
const redoButtonEl = this.getAnnotateButton(SELECTOR_ANNOTATION_BUTTON_DRAW_REDO);

// NOTE (@minhnguyen): Move this logic to a new controller class
const that = this;
drawingThread.addListener('annotationevent', (data = {}) => {
switch (data.type) {
case 'drawcommit':
drawingThread.removeAllListeners('annotationevent');
break;
case 'pagechanged':
drawingThread.saveAnnotation(TYPES.draw);
that.unbindModeListeners();
that.bindModeListeners(TYPES.draw);
break;
case 'availableactions':
if (data.undo === 1) {
annotatorUtil.enableElement(undoButtonEl);
} else if (data.undo === 0) {
annotatorUtil.disableElement(undoButtonEl);
}

if (data.redo === 1) {
annotatorUtil.enableElement(redoButtonEl);
} else if (data.redo === 0) {
annotatorUtil.disableElement(redoButtonEl);
}
break;
default:
}
});

handlers.push(
{
type: 'mousemove',
func: annotatorUtil.eventToLocationHandler(locationFunction, drawingThread.handleMove),
eventObj: this.annotatedElement
},
{
type: 'mousedown',
func: annotatorUtil.eventToLocationHandler(locationFunction, drawingThread.handleStart),
eventObj: this.annotatedElement
},
{
type: 'mouseup',
func: annotatorUtil.eventToLocationHandler(locationFunction, drawingThread.handleStop),
eventObj: this.annotatedElement
}
);

if (postButtonEl) {
handlers.push({
type: 'click',
Expand All @@ -716,6 +762,26 @@ class Annotator extends EventEmitter {
eventObj: postButtonEl
});
}

if (undoButtonEl) {
handlers.push({
type: 'click',
func: () => {
drawingThread.undo();
},
eventObj: undoButtonEl
});
}

if (redoButtonEl) {
handlers.push({
type: 'click',
func: () => {
drawingThread.redo();
},
eventObj: redoButtonEl
});
}
}

handlers.forEach((handler) => {
Expand Down
49 changes: 40 additions & 9 deletions src/lib/annotations/__tests__/Annotator-test.js
View file
Expand Up @@ -10,7 +10,9 @@ import {
CLASS_ANNOTATION_MODE,
CLASS_ACTIVE,
CLASS_HIDDEN,
SELECTOR_BOX_PREVIEW_BTN_ANNOTATE_POINT
SELECTOR_ANNOTATION_BUTTON_DRAW_POST,
SELECTOR_ANNOTATION_BUTTON_DRAW_UNDO,
SELECTOR_ANNOTATION_BUTTON_DRAW_REDO
} from '../annotationConstants';

let annotator;
Expand Down Expand Up @@ -369,7 +371,7 @@ describe('lib/annotations/Annotator', () => {
annotator.disableAnnotationMode(TYPES.draw, btn);
expect(btn).to.not.have.class(CLASS_ACTIVE);
expect(stubs.show).to.be.called;
expect(stubs.hide).to.be.calledTwice;
expect(stubs.hide).to.have.callCount(4);
});
});

Expand Down Expand Up @@ -401,7 +403,7 @@ describe('lib/annotations/Annotator', () => {
annotator.enableAnnotationMode(TYPES.draw, btn);
expect(btn).to.have.class(CLASS_ACTIVE);
expect(stubs.hide).to.be.called;
expect(stubs.show).to.be.calledTwice;
expect(stubs.show).to.have.callCount(4);
});
});

Expand Down Expand Up @@ -545,6 +547,7 @@ describe('lib/annotations/Annotator', () => {
it('should unbind custom listeners from the thread', () => {
stubs.threadMock.expects('removeAllListeners').withArgs('threaddeleted');
stubs.threadMock.expects('removeAllListeners').withArgs('threadcleanup');
stubs.threadMock.expects('removeAllListeners').withArgs('annotationevent');
annotator.unbindCustomListenersOnThread(stubs.thread);
});
});
Expand Down Expand Up @@ -579,14 +582,14 @@ describe('lib/annotations/Annotator', () => {
expect(annotator.annotationModeHandlers.length).equals(2);
});

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

const postButtonEl = {
addEventListener: sandbox.stub(),
removeEventListener: sandbox.stub()
};
sandbox.stub(annotator, 'getAnnotateButton').returns(null);
sandbox.stub(annotator, 'getAnnotateButton').returns(postButtonEl);
const locationHandler = (() => {});

sandbox.stub(annotatorUtil, 'eventToLocationHandler').returns(locationHandler);
Expand All @@ -597,22 +600,23 @@ describe('lib/annotations/Annotator', () => {
sinon.match.string,
locationHandler
).thrice;
expect(postButtonEl.addEventListener).to.not.be.calledWith(
expect(postButtonEl.addEventListener).to.be.calledWith(
'click',
sinon.match.func
);
expect(annotator.annotationModeHandlers.length).equals(3);
expect(annotator.annotationModeHandlers.length).equals(6);
});

it('should bind draw mode click handlers if post button exists', () => {
it('should successfully bind draw mode handlers if undo and redo buttons do not exist', () => {
sandbox.stub(annotator, 'createAnnotationThread').returns(drawingThread);

const postButtonEl = {
addEventListener: sandbox.stub(),
removeEventListener: sandbox.stub()
};
sandbox.stub(annotator, 'getAnnotateButton').returns(postButtonEl);
const locationHandler = (() => {});
const getAnnotateButton = sandbox.stub(annotator, 'getAnnotateButton');
getAnnotateButton.withArgs(SELECTOR_ANNOTATION_BUTTON_DRAW_POST).returns(postButtonEl);

sandbox.stub(annotatorUtil, 'eventToLocationHandler').returns(locationHandler);

Expand All @@ -628,6 +632,33 @@ describe('lib/annotations/Annotator', () => {
);
expect(annotator.annotationModeHandlers.length).equals(4);
});

it('should successfully bind draw mode handlers if post button does not exist', () => {
sandbox.stub(annotator, 'createAnnotationThread').returns(drawingThread);

const doButtonEl = {
addEventListener: sandbox.stub(),
removeEventListener: sandbox.stub()
};
const locationHandler = (() => {});
const getAnnotateButton = sandbox.stub(annotator, 'getAnnotateButton');
getAnnotateButton.withArgs(SELECTOR_ANNOTATION_BUTTON_DRAW_UNDO).returns(doButtonEl);
getAnnotateButton.withArgs(SELECTOR_ANNOTATION_BUTTON_DRAW_REDO).returns(doButtonEl);

sandbox.stub(annotatorUtil, 'eventToLocationHandler').returns(locationHandler);

annotator.bindModeListeners(TYPES.draw);

expect(annotator.annotatedElement.addEventListener).to.be.calledWith(
sinon.match.string,
locationHandler
).thrice;
expect(doButtonEl.addEventListener).to.be.calledWith(
'click',
sinon.match.func
);
expect(annotator.annotationModeHandlers.length).equals(5);
});
});

describe('unbindModeListeners()', () => {
Expand Down
30 changes: 30 additions & 0 deletions src/lib/annotations/__tests__/annotatorUtil-test.js
View file
Expand Up @@ -5,6 +5,8 @@ import {
getPageInfo,
showElement,
hideElement,
enableElement,
disableElement,
showInvisibleElement,
hideElementVisibility,
resetTextarea,
Expand Down Expand Up @@ -122,6 +124,34 @@ describe('lib/annotations/annotatorUtil', () => {
});
});

describe('enableElement()', () => {
it('should remove disabled class from element with matching selector', () => {
// Hide element before testing show function
childEl.classList.add('is-disabled');
enableElement('.child');
assert.ok(!childEl.classList.contains('is-disabled'));
});

it('should remove hidden class from provided element', () => {
// Hide element before testing show function
childEl.classList.add('is-disabled');
enableElement(childEl);
assert.ok(!childEl.classList.contains('is-disabled'));
});
});

describe('disableElement()', () => {
it('should add hidden class to matching element', () => {
disableElement('.child');
assert.ok(childEl.classList.contains('is-disabled'));
});

it('should add hidden class to provided element', () => {
disableElement(childEl);
assert.ok(childEl.classList.contains('is-disabled'));
});
});

describe('showInvisibleElement()', () => {
it('should remove invisible class from element with matching selector', () => {
// Hide element before testing show function
Expand Down

0 comments on commit 4b5421c

Please sign in to comment.