Skip to content

Commit

Permalink
Fix: Unable to select drawing annotation after saving (#350)
Browse files Browse the repository at this point in the history
  • Loading branch information
Conrad Chan authored Mar 1, 2019
1 parent 6c24c11 commit 8632777
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 9 deletions.
1 change: 0 additions & 1 deletion src/AnnotationThread.js
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,6 @@ class AnnotationThread extends EventEmitter {
}

this.show();
this.renderAnnotationPopover();
this.emit(THREAD_EVENT.save);
}

Expand Down
3 changes: 0 additions & 3 deletions src/__tests__/AnnotationThread-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,6 @@ describe('AnnotationThread', () => {
beforeEach(() => {
thread.api.create = jest.fn();
thread.getThreadEventData = jest.fn().mockReturnValue({});
thread.renderAnnotationPopover = jest.fn();
thread.comments = [tempAnnotation];
});

Expand All @@ -291,15 +290,13 @@ describe('AnnotationThread', () => {

it('should only render popover on desktop', () => {
thread.updateTemporaryAnnotation(tempAnnotation.id, serverAnnotation);
expect(thread.renderAnnotationPopover).toBeCalled();
expect(thread.state).toEqual(STATES.inactive);
});

it('should only render popover on mobile', () => {
util.shouldDisplayMobileUI = jest.fn().mockReturnValue(true);
thread.updateTemporaryAnnotation(tempAnnotation.id, serverAnnotation);
expect(thread.state).toEqual(STATES.active);
expect(thread.renderAnnotationPopover).toBeCalled();
});
});

Expand Down
10 changes: 9 additions & 1 deletion src/controllers/DrawingModeController.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ class DrawingModeController extends AnnotationModeController {
location.maxY = location.y;

// Create new thread with no annotations, show indicator, and show dialog
const thread = this.registerThread({
const threadParams = this.getThreadParams({
id: AnnotationAPI.generateID(),
type: this.mode,
location,
Expand All @@ -215,6 +215,12 @@ class DrawingModeController extends AnnotationModeController {
comments: []
});

if (!threadParams) {
return;
}

const thread = this.instantiateThread(threadParams);

if (!thread) {
return;
}
Expand Down Expand Up @@ -268,6 +274,8 @@ class DrawingModeController extends AnnotationModeController {
this.resetCurrentThread(thread);
this.unbindListeners();

this.registerThread(thread);

// Clear existing canvases
// eslint-disable-next-line
const pageEl = this.annotatedElement.querySelector(`[data-page-number="${thread.location.page}"]`);
Expand Down
8 changes: 4 additions & 4 deletions src/controllers/__tests__/DrawingModeController-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ describe('controllers/DrawingModeController', () => {
controller.getLocation = jest.fn();
controller.currentThread = undefined;
controller.locationFunction = jest.fn();
controller.registerThread = jest.fn().mockReturnValue(thread);
controller.getThreadParams = jest.fn().mockReturnValue({});
controller.instantiateThread = jest.fn().mockReturnValue(thread);
});

afterEach(() => {
Expand All @@ -203,7 +204,7 @@ describe('controllers/DrawingModeController', () => {
it('should do nothing if drawing start is invalid', () => {
controller.drawingStartHandler(event);
expect(controller.getLocation).toBeCalled();
expect(controller.registerThread).not.toBeCalled();
expect(controller.instantiateThread).not.toBeCalled();
});

it('should continue drawing if in the middle of creating a new drawing', () => {
Expand All @@ -212,13 +213,12 @@ describe('controllers/DrawingModeController', () => {
thread.getThreadEventData = jest.fn().mockReturnValue({});

controller.drawingStartHandler(event);
expect(controller.registerThread).not.toBeCalled();
expect(controller.instantiateThread).not.toBeCalled();
expect(thread.handleStart).toBeCalledWith(location);
});

it('should begin a new drawing thread if none exist already', () => {
controller.getLocation = jest.fn().mockReturnValue(location);
controller.registerThread = jest.fn().mockReturnValue(thread);
thread.getThreadEventData = jest.fn().mockReturnValue({});

controller.drawingStartHandler(event);
Expand Down
11 changes: 11 additions & 0 deletions src/doc/DocDrawingThread.js
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,17 @@ class DocDrawingThread extends DrawingThread {
popoverEl.style.left = `${popoverX}px`;
popoverEl.style.top = `${popoverY}px`;
};

/**
* Update annotation after successfully saved.
* @override
*/
updateTemporaryAnnotation(tempAnnotationID: string, savedAnnotation: Annotation) {
super.updateTemporaryAnnotation(tempAnnotationID, savedAnnotation);

// Unmount the drawing annotation popover after successful save
this.unmountPopover();
}
}

export default DocDrawingThread;

0 comments on commit 8632777

Please sign in to comment.