Skip to content

Commit

Permalink
Fix: Activate annotation dialog on post (#42)
Browse files Browse the repository at this point in the history
- Doesn't immediately hide the annotation dialog when a comment is
  deleted
  • Loading branch information
pramodsum committed Nov 20, 2017
1 parent 97a91c3 commit bc001be
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 10 deletions.
7 changes: 3 additions & 4 deletions src/AnnotationDialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,6 @@ class AnnotationDialog extends EventEmitter {
}

this.addAnnotationElement(annotation);
this.deactivateReply(); // Deactivate reply area and focus
}

/**
Expand Down Expand Up @@ -418,10 +417,10 @@ class AnnotationDialog extends EventEmitter {
if (replyTextArea.textContent !== '' || commentsTextArea.textContent !== '') {
this.emit('annotationcommentpending');
}

// Ensure textarea stays open
this.activateReply();
}

// Ensure textarea stays open
this.activateReply();
}

/**
Expand Down
3 changes: 3 additions & 0 deletions src/AnnotationThread.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,8 @@ class AnnotationThread extends EventEmitter {
// Otherwise, remove deleted annotation from dialog
} else if (this.dialog) {
this.dialog.removeAnnotation(annotationID);
this.showDialog();
this.dialog.deactivateReply();
}

if (!useServer) {
Expand Down Expand Up @@ -553,6 +555,7 @@ class AnnotationThread extends EventEmitter {

if (this.dialog) {
this.dialog.addAnnotation(annotation);
this.dialog.activateReply();
}
}

Expand Down
6 changes: 4 additions & 2 deletions src/__tests__/AnnotationDialog-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,13 +241,11 @@ describe('AnnotationDialog', () => {
describe('addAnnotation()', () => {
beforeEach(() => {
stubs.addEl = sandbox.stub(dialog, 'addAnnotationElement');
stubs.deactivate = sandbox.stub(dialog, 'deactivateReply');
});

it('should add annotation to the dialog and deactivate the reply area', () => {
dialog.addAnnotation(new Annotation({}));
expect(stubs.addEl).to.be.called;
expect(stubs.deactivate).to.be.calledWithExactly();
});

it('should hide the create section and show the show section if there are no annotations', () => {
Expand Down Expand Up @@ -515,18 +513,21 @@ describe('AnnotationDialog', () => {
describe('mouseenterHandler()', () => {
beforeEach(() => {
stubs.show = sandbox.stub(annotatorUtil, 'showElement');
stubs.activate = sandbox.stub(dialog, 'activateReply');
});

it('should show the element only if the element is currently hidden', () => {
dialog.element.classList.add(constants.CLASS_HIDDEN);

dialog.mouseenterHandler();
expect(annotatorUtil.showElement).to.be.called;
expect(stubs.activate).to.be.called;
});

it('should do nothing if the element is already shown', () => {
dialog.mouseenterHandler();
expect(annotatorUtil.showElement).to.not.be.called;
expect(stubs.activate).to.be.called;
});

it('should emit \'annotationcommentpending\' when user hovers back into a dialog that has a pending comment', () => {
Expand All @@ -537,6 +538,7 @@ describe('AnnotationDialog', () => {
dialog.mouseenterHandler();
expect(stubs.show).to.be.called;
expect(stubs.emit).to.be.calledWith('annotationcommentpending');
expect(stubs.activate).to.be.called;
});
});

Expand Down
12 changes: 8 additions & 4 deletions src/__tests__/AnnotationThread-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ describe('AnnotationThread', () => {
});

thread.dialog = {
activateReply: () => {},
addListener: () => {},
addAnnotation: () => {},
destroy: () => {},
Expand Down Expand Up @@ -302,6 +303,7 @@ describe('AnnotationThread', () => {
thread.dialog = {
addListener: () => {},
addAnnotation: () => {},
deactivateReply: () => {},
destroy: () => {},
removeAllListeners: () => {},
show: () => {},
Expand All @@ -314,6 +316,7 @@ describe('AnnotationThread', () => {
stubs.isPlain = sandbox.stub(annotatorUtil, 'isPlainHighlight');
stubs.cancel = sandbox.stub(thread, 'cancelFirstComment');
stubs.destroy = sandbox.stub(thread, 'destroy');
sandbox.stub(thread, 'showDialog');
sandbox.stub(thread, 'getThreadEventData').returns({
threadNumber: 1
});
Expand Down Expand Up @@ -352,6 +355,7 @@ describe('AnnotationThread', () => {
// Add another annotation to thread so 'someID' isn't the only annotation
thread.annotations[stubs.annotation2.annotationID] = stubs.annotation2;
stubs.dialogMock.expects('removeAnnotation').withArgs('someID');
stubs.dialogMock.expects('deactivateReply');

const promise = thread.deleteAnnotation('someID', false);
promise.then(() => {
Expand Down Expand Up @@ -760,7 +764,6 @@ describe('AnnotationThread', () => {

describe('saveAnnotationToThread()', () => {
it('should add the annotation to the thread, and add to the dialog when the dialog exists', () => {
stubs.add = sandbox.stub(thread.dialog, 'addAnnotation');
stubs.push = sandbox.stub(thread.annotations, 'push');
const annotation = new Annotation({
fileVersionId: '2',
Expand All @@ -772,13 +775,13 @@ describe('AnnotationThread', () => {
created: Date.now()
});

stubs.dialogMock.expects('activateReply');
stubs.dialogMock.expects('addAnnotation').withArgs(annotation);
thread.saveAnnotationToThread(annotation);
expect(stubs.add).to.be.calledWith(annotation);
expect(thread.annotations[annotation.annotationID]).to.not.be.undefined;
});

it('should not try to push an annotation to the dialog if it doesn\'t exist', () => {
stubs.add = sandbox.stub(thread.dialog, 'addAnnotation');
const annotation = new Annotation({
fileVersionId: '2',
threadID: '1',
Expand All @@ -790,8 +793,9 @@ describe('AnnotationThread', () => {
});

thread.dialog = undefined;
stubs.dialogMock.expects('activateReply').never();
stubs.dialogMock.expects('addAnnotation').never();
thread.saveAnnotationToThread(annotation);
expect(stubs.add).to.not.be.called;
expect(thread.annotations[annotation.annotationID]).to.not.be.undefined;
});
});
Expand Down

0 comments on commit bc001be

Please sign in to comment.