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

Feature: Drawing Annotations Undo Redo #287

Merged
merged 91 commits into from Aug 21, 2017
Merged
Show file tree
Hide file tree
Changes from 72 commits
Commits
Show all changes
91 commits
Select commit Hold shift + click to select a range
12cc4ed
Fix: thread save incorrectly rejects when no dialogue exists
Jul 26, 2017
1872d82
Fix: error attempting to set dialogue threadnum when no dialogue exists
Jul 26, 2017
6db4184
Merge branch 'master' into branch/rebaseToRemote
Minh-Ng Jul 26, 2017
48af9ac
Merge branch 'master' into branch/rebaseToRemote
Minh-Ng Jul 27, 2017
533030c
Update: drawing annotations now rescale correctly
Jul 27, 2017
7f661ea
Fix: annotator is now loaded with the correct initial scale
Jul 28, 2017
7230510
Fix: update annotator tests for constructor change
Jul 28, 2017
b90659a
Update: drawingannotation rescaling
Jul 28, 2017
000b503
Merge branch 'master' into branch/rebaseToRemote
Jul 28, 2017
48b5589
Merge remote-tracking branch 'upstream/master' into feature/drawingAn…
Jul 28, 2017
8c88200
Update: tests for updating local annotations on annotationthreads
Jul 28, 2017
f3b557c
Merge branch 'branch/rebaseToRemote' of https://github.com/MinhHNguye…
Jul 28, 2017
ed403b8
Fix: typo
Jul 28, 2017
22d4faa
Update: merge initial scaling changes
Jul 28, 2017
4be971e
Merge branch 'branch/rebaseToRemote' into feature/drawingAnnotationSc…
Jul 28, 2017
51b0201
Update: fix tests
Jul 29, 2017
0757c0b
Fix: clean up anonymous functions
Jul 29, 2017
e4a097b
Fix: clean up anonymous functions
Jul 29, 2017
1075f78
Update: merge from master
Jul 31, 2017
f25c34c
Update: tests for docdrawingthread
Jul 31, 2017
70d619c
Update: change page change method
Aug 1, 2017
833a42a
Fix: various cleanup for PR
Aug 1, 2017
fe3587d
Fix: removing fixes coming in a different PR
Aug 1, 2017
924cd25
Fix: sentenced to 1 year of unit tests
Aug 1, 2017
3549473
Merge branch 'master' into feature/drawingAnnotationScaling
Minh-Ng Aug 1, 2017
1857b90
Update: docdrawingthread documentation
Aug 1, 2017
cb87a9e
Merge branch 'feature/drawingAnnotationScaling' of https://github.com…
Aug 1, 2017
5402f45
Merge branch 'master' into feature/drawingAnnotationScaling
Minh-Ng Aug 1, 2017
67e250f
Merge branch 'master' into feature/drawingAnnotationScaling
Minh-Ng Aug 1, 2017
bf29d9f
Update: merge from upstream master
Aug 2, 2017
7bd6fdb
Merge branch 'feature/drawingAnnotationScaling' of https://github.com…
Aug 2, 2017
74c85fc
Merge branch 'master' into feature/drawingAnnotationScaling
Minh-Ng Aug 2, 2017
10d1616
Update: undo redo drawing container
Aug 3, 2017
18f0444
Merge branch 'master' into feature/drawingAnnotationScaling
pramodsum Aug 4, 2017
60e259b
Update: resolve merge conflicts with master
Aug 7, 2017
034e37a
Fix: pull request feedback
Aug 7, 2017
8edb906
Update: remove todo
Aug 7, 2017
f79244b
New: drawing annotation undo and redo container
Aug 7, 2017
1fea19d
Merge branch 'master' into feature/drawingAnnotationScaling
Minh-Ng Aug 7, 2017
a1909fe
Merge branch 'master' into feature/drawingAnnotationScaling
Minh-Ng Aug 8, 2017
bb56ac4
Merge branch 'master' into feature/drawingAnnotationScaling
Minh-Ng Aug 9, 2017
d225a4a
Update: undo and redo grey out when applicable
Aug 9, 2017
6d0b4cc
Fix: update tests with new fn calls
Aug 9, 2017
b216523
Update: merge upstream master
Aug 9, 2017
bd25a6e
Chore: update variable name
Aug 9, 2017
08f44c1
Merge branch 'feature/drawingAnnotationScaling' of https://github.com…
Aug 9, 2017
41a5d51
Chore: actually update the draw states variable
Aug 9, 2017
5d05d52
Merge branch 'master' into feature/drawingAnnotationScaling
Minh-Ng Aug 10, 2017
06e4f24
Merge branch 'master' into feature/undoredo
Minh-Ng Aug 10, 2017
abdc1ac
Merge branch 'master' into feature/drawingAnnotationScaling
Minh-Ng Aug 10, 2017
55817f4
Update: drawing annotation svgs
Aug 11, 2017
1e31e26
Merge branch 'master' into feature/drawingAnnotationScaling
Minh-Ng Aug 11, 2017
c827c3e
Update: remove draw from drawstates
Aug 11, 2017
bf550bd
Merge branch 'master' into feature/drawingAnnotationScaling
Minh-Ng Aug 11, 2017
e44a151
Update: resolve conflicts from upstream
Aug 11, 2017
52642bf
Update: handle page change with two toggles
Aug 11, 2017
746d291
Update: merge from scaling pr
Aug 11, 2017
ee25fa4
Update: merge refactor from master
Aug 11, 2017
cffba79
Update: resolve merge
Aug 12, 2017
7f31e8c
Update: merge conflicts again please
Aug 12, 2017
0046949
Merge branch 'feature/drawingAnnotationScaling' into feature/undoredo
Aug 12, 2017
41287af
Fix: bind cleanup listeners on drawing thread and fix resize
Aug 12, 2017
1df105b
Fix: remove global test variable
Aug 12, 2017
94bf934
Merge branch 'feature/drawingAnnotationScaling' into feature/undoredo
Aug 12, 2017
2a39df7
Fix: scale annotations in redo stack
Aug 12, 2017
455549f
Update: drawing method cleanup
Aug 12, 2017
98d8e3b
Merge branch 'feature/undoredo' of https://github.com/MinhHNguyen/box…
Aug 12, 2017
fb0f5cb
Update: remove unused class
Aug 12, 2017
d8f9f80
Update: undo redo unit tests
Aug 13, 2017
6ac7878
Merge branch 'master' into feature/undoredo
Minh-Ng Aug 14, 2017
bad8368
Merge branch 'master' into feature/drawingAnnotationScaling
Minh-Ng Aug 14, 2017
2509bd6
Merge remote-tracking branch 'upstream/master' into feature/undoredo
Aug 14, 2017
08c24bc
Merge branch 'feature/undoredo' of https://github.com/MinhHNguyen/box…
Aug 14, 2017
94e92c0
Merge branch 'master' into feature/drawingAnnotationScaling
Minh-Ng Aug 14, 2017
ab8358b
Update: remove isTypeEnabled
Aug 14, 2017
ae4e82c
Update: merge from master
Aug 15, 2017
73aaa87
Merge branch 'feature/drawingAnnotationScaling' into feature/undoredo
Aug 15, 2017
cdf8b6b
Update: fix tests again
Aug 15, 2017
15e0e72
Merge branch 'feature/drawingAnnotationScaling' into feature/undoredo
Aug 15, 2017
2612395
Merge branch 'master' into feature/undoredo
Minh-Ng Aug 15, 2017
5bcc4a4
Update: resolve merge conflicts
Aug 15, 2017
d02ac11
Update: added test for DocDrawingThreadShow
Aug 15, 2017
c3d3ee5
Merge branch 'master' into feature/undoredo
Minh-Ng Aug 15, 2017
82012d1
Update: PR changes
Aug 15, 2017
a8a9be5
Update: custom thread cleanup for drawing annotationevent
Aug 15, 2017
e6e77fd
Merge branch 'master' into feature/undoredo
Minh-Ng Aug 15, 2017
53aed0e
Merge branch 'feature/undoredo' of https://github.com/MinhHNguyen/box…
Aug 15, 2017
91bffcc
Update: resolve merge conflict
Aug 17, 2017
fea659b
Merge branch 'master' into feature/undoredo
Minh-Ng Aug 17, 2017
7ba6bc6
Merge branch 'master' into feature/undoredo
Minh-Ng Aug 19, 2017
d08bd4f
Update: fix test from merging master
Aug 20, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 13 additions & 8 deletions src/lib/_boxui.scss
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
144 changes: 109 additions & 35 deletions src/lib/annotations/Annotator.js
Expand Up @@ -16,6 +16,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 @@ -117,8 +119,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 @@ -204,6 +205,17 @@ class Annotator extends EventEmitter {
this.container.appendChild(mobileDialogEl);
}

/**
* Returns true if the annotator has an annotation type enabled
*
* @param {string} type - Annotation type to check
* @return {boolean} Whether or not the annotation type is enabled
*/
isTypeEnabled(type) {
const { annotator } = this.options || {};
return annotator.TYPE && annotator.TYPE.includes(type);
}

/**
* Fetches and shows saved annotations.
*
Expand Down Expand Up @@ -366,13 +378,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.previewUI.getAnnotateButton(SELECTOR_ANNOTATION_BUTTON_DRAW_POST);
const undoButtonEl = this.previewUI.getAnnotateButton(SELECTOR_ANNOTATION_BUTTON_DRAW_UNDO);
const redoButtonEl = this.previewUI.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 @@ -395,13 +412,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.previewUI.getAnnotateButton(SELECTOR_ANNOTATION_BUTTON_DRAW_POST);
const undoButtonEl = this.previewUI.getAnnotateButton(SELECTOR_ANNOTATION_BUTTON_DRAW_UNDO);
const redoButtonEl = this.previewUI.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 @@ -638,6 +658,7 @@ class Annotator extends EventEmitter {
unbindCustomListenersOnThread(thread) {
thread.removeAllListeners('threaddeleted');
thread.removeAllListeners('threadcleanup');
thread.removeAllListeners('annotationevent');
}

/**
Expand All @@ -651,41 +672,74 @@ 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);

/* eslint-disable require-jsdoc */
const locationFunction = (event) => this.getLocationFromEvent(event, TYPES.point);
/* eslint-enable require-jsdoc */

const postButtonEl = this.previewUI.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.previewUI.getAnnotateButton(SELECTOR_ANNOTATION_BUTTON_DRAW_UNDO);
const redoButtonEl = this.previewUI.getAnnotateButton(SELECTOR_ANNOTATION_BUTTON_DRAW_REDO);

// NOTE (@minhnguyen): Move this logic to a new controller class
const that = this;
drawingThread.addListener('annotationevent', (data = {}) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

move this into a separate method + tests

Copy link
Contributor Author

@Minh-Ng Minh-Ng Aug 15, 2017

Choose a reason for hiding this comment

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

I'm planning on doing this in a separate PR (the code is actually in the process of being written on a different branch). I don't think it makes sense to separate and test it at the moment when it will be moved and changed in a coming class change.

switch (data.type) {
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 @@ -696,6 +750,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
1 change: 1 addition & 0 deletions src/lib/annotations/Annotator.scss
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
66 changes: 29 additions & 37 deletions src/lib/annotations/__tests__/Annotator-test.js
Expand Up @@ -285,6 +285,23 @@ describe('lib/annotations/Annotator', () => {
});
});

describe('isTypeEnabled()', () => {
it('should return true when option.TYPE includes the type being checked', () => {
annotator.options.TYPE = [TYPES.point, TYPES.draw];
expect(annotator.isTypeEnabled(TYPES.draw)).to.be.truthy;
});

it('should return false when option.TYPE does not include the type being checked', () => {
annotator.options.TYPE = [TYPES.point, TYPES.highlight];
expect(annotator.isTypeEnabled(TYPES.draw)).to.be.falsy;
});

it('should return false when option.TYPE does not exist', () => {
annotator.options.TYPE = undefined;
expect(annotator.isTypeEnabled(TYPES.draw)).to.be.falsy;
});
});

describe('toggleAnnotationHandler()', () => {
beforeEach(() => {
stubs.destroyStub = sandbox.stub(annotator, 'destroyPendingThreads');
Expand All @@ -308,7 +325,7 @@ describe('lib/annotations/Annotator', () => {
annotator.toggleAnnotationHandler(TYPES.highlight);
expect(stubs.destroyStub).to.be.called;
expect(stubs.exitAnnotationModes)
})
});

it('should turn annotation mode on if it is off', () => {
stubs.annotationMode.returns(false);
Expand Down Expand Up @@ -471,16 +488,26 @@ 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);
});
});

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 @@ -496,42 +523,7 @@ describe('lib/annotations/Annotator', () => {
expect(annotator.annotationModeHandlers.length).equals(2);
});

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

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

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

annotator.bindModeListeners(TYPES.draw);

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

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 All @@ -553,7 +545,7 @@ describe('lib/annotations/Annotator', () => {
'click',
sinon.match.func
);
expect(annotator.annotationModeHandlers.length).equals(4);
expect(annotator.annotationModeHandlers.length).equals(6);
Copy link
Contributor

Choose a reason for hiding this comment

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

Add tests for when only some of the buttons exist i.e postButtonEl, undoButton...etc

});
});

Expand Down