diff --git a/src/i18n/en-US.properties b/src/i18n/en-US.properties index 22f978826..a703d81d0 100644 --- a/src/i18n/en-US.properties +++ b/src/i18n/en-US.properties @@ -113,6 +113,8 @@ annotation_add_comment_placeholder=Add a comment here... annotation_reply_placeholder=Post a reply... # Text for cancel annotation button annotation_cancel=Cancel +# Text for save annotation button +annotation_save=Save # Text for post annotation button annotation_post=Post # Text for delete annotation button diff --git a/src/lib/annotations/AnnotationDialog.js b/src/lib/annotations/AnnotationDialog.js index 935d78325..bc2bc4df9 100644 --- a/src/lib/annotations/AnnotationDialog.js +++ b/src/lib/annotations/AnnotationDialog.js @@ -54,6 +54,7 @@ class AnnotationDialog extends EventEmitter { this.hasAnnotations = data.annotations.length > 0; this.canAnnotate = data.canAnnotate; this.locale = data.locale; + this.isMobile = data.isMobile; } /** diff --git a/src/lib/annotations/AnnotationThread.js b/src/lib/annotations/AnnotationThread.js index 3ad1c9d74..48ac0a399 100644 --- a/src/lib/annotations/AnnotationThread.js +++ b/src/lib/annotations/AnnotationThread.js @@ -50,6 +50,7 @@ class AnnotationThread extends EventEmitter { this.type = data.type; this.locale = data.locale; this.isMobile = data.isMobile; + this.hasTouch = data.hasTouch; this.permissions = data.permissions; this.setup(); diff --git a/src/lib/annotations/Annotator.scss b/src/lib/annotations/Annotator.scss index 500aa7cee..7fee4644d 100644 --- a/src/lib/annotations/Annotator.scss +++ b/src/lib/annotations/Annotator.scss @@ -57,6 +57,7 @@ $avatar-color-9: #f22c44; } } +.bp-annotation-drawing-dialog, .bp-annotation-highlight-dialog .bp-btn-plain, .bp-annotation-highlight-dialog .bp-btn-plain:hover { padding-left: 5px; @@ -362,6 +363,7 @@ $avatar-color-9: #f22c44; } } +.bp-annotation-drawing-dialog, .bp-annotation-highlight-dialog, .bp-annotation-highlight-dialog:hover { background-color: $white; @@ -374,6 +376,15 @@ $avatar-color-9: #f22c44; padding-top: 1px; } + .bp-btn-annotate-draw-add { + padding-bottom: 3px; + padding-right: 6px; + } + + .bp-btn-annotate-draw-delete { + padding-bottom: 3px; + } + .bp-highlight-comment-btn { padding-top: 3px; } @@ -430,6 +441,7 @@ $avatar-color-9: #f22c44; } } +.bp-annotation-drawing-label, .bp-annotation-highlight-label { padding-left: 5px; padding-right: 5px; diff --git a/src/lib/annotations/__tests__/annotatorUtil-test.js b/src/lib/annotations/__tests__/annotatorUtil-test.js index 9bbe3ba7c..5bc1b3f37 100644 --- a/src/lib/annotations/__tests__/annotatorUtil-test.js +++ b/src/lib/annotations/__tests__/annotatorUtil-test.js @@ -435,7 +435,7 @@ describe('lib/annotations/annotatorUtil', () => { it('should do nothing when the target exists and it is not the textLayer', () => { event.target = { - className: 'button' + nodeName: 'BUTTON' }; locationHandler(event); expect(callback).to.not.be.called; diff --git a/src/lib/annotations/annotationConstants.js b/src/lib/annotations/annotationConstants.js index 4b140c90b..5390eb7ac 100644 --- a/src/lib/annotations/annotationConstants.js +++ b/src/lib/annotations/annotationConstants.js @@ -35,6 +35,8 @@ export const CLASS_ANNOTATION_BUTTON_DRAW_POST = 'bp-btn-annotate-draw-post'; export const CLASS_ANNOTATION_BUTTON_DRAW_CANCEL = 'bp-btn-annotate-draw-cancel'; export const CLASS_ANNOTATION_BUTTON_DRAW_ENTER = 'bp-btn-annotate-draw-enter'; export const CLASS_ANNOTATION_DRAWING_LABEL = 'bp-annotation-drawing-label'; +export const CLASS_ANNOTATION_DRAWING_DIALOG = 'bp-annotation-drawing-dialog'; +export const CLASS_ANNOTATION_DRAWING_BTNS = 'bp-annotation-drawing-btns'; export const CLASS_ADD_DRAWING_BTN = 'bp-btn-annotate-draw-add'; export const CLASS_DELETE_DRAWING_BTN = 'bp-btn-annotate-draw-delete'; diff --git a/src/lib/annotations/annotatorUtil.js b/src/lib/annotations/annotatorUtil.js index b5cf46bb7..cd33bfeb3 100644 --- a/src/lib/annotations/annotatorUtil.js +++ b/src/lib/annotations/annotatorUtil.js @@ -449,8 +449,9 @@ export function eventToLocationHandler(locationFunction, callback) { return (event) => { const evt = event || window.event; // Do nothing when the target isn't the text layer in case the text layer receives event precedence over buttons - // NOTE: Currently only applicable to documents. Need to find a better way to ensure button event precedence. - if (!evt || (event.target && event.target.className !== 'textLayer')) { + // NOTE: @jpress Currently only applicable to documents. + // Need to find a better way to ensure button event precedence. + if (!evt || (evt.target && evt.target.nodeName === 'BUTTON')) { return; } @@ -475,6 +476,7 @@ export function prevDefAndStopProp(event) { event.preventDefault(); event.stopPropagation(); } + /** * Create a JSON object containing x/y coordinates and optionally dimensional information * diff --git a/src/lib/annotations/doc/DocAnnotator.js b/src/lib/annotations/doc/DocAnnotator.js index 99cbe3a18..08134ed5b 100644 --- a/src/lib/annotations/doc/DocAnnotator.js +++ b/src/lib/annotations/doc/DocAnnotator.js @@ -325,6 +325,7 @@ class DocAnnotator extends Annotator { container: this.container, fileVersionId: this.fileVersionId, isMobile: this.isMobile, + hasTouch: this.hasTouch, locale: this.locale, location, type, @@ -548,10 +549,6 @@ class DocAnnotator extends Annotator { this.highlightThrottleHandle = null; } - if (!this.permissions.canAnnotate) { - return; - } - Object.keys(this.modeControllers).forEach((mode) => { const controller = this.modeControllers[mode]; controller.removeSelection(); diff --git a/src/lib/annotations/doc/DocDrawingDialog.js b/src/lib/annotations/doc/DocDrawingDialog.js index 940228c94..856b8b813 100644 --- a/src/lib/annotations/doc/DocDrawingDialog.js +++ b/src/lib/annotations/doc/DocDrawingDialog.js @@ -1,46 +1,276 @@ import AnnotationDialog from '../AnnotationDialog'; +import * as annotatorUtil from '../annotatorUtil'; +import * as constants from '../annotationConstants'; +import { ICON_DRAW_SAVE, ICON_DRAW_DELETE } from '../../icons/icons'; + +const LABEL_TEMPLATE = ``; +const COMMIT_TEMPLATE = ` + `.trim(); +const DELETE_TEMPLATE = ` + `.trim(); class DocDrawingDialog extends AnnotationDialog { + /** @property {boolean} Whether or not the dialog is visible */ visible = false; /** - * Empty stub to avoid unexpected behavior. + * [constructor] + * + * @param {AnnotationDialogData} data - Data for constructing drawing dialog + * @return {DocDrawingDialog} Drawing dialog instance + */ + constructor(data) { + super(data); + + this.postDrawing = this.postDrawing.bind(this); + this.deleteAnnotation = this.deleteAnnotation.bind(this); + } + + /** + * [destructor] + * + * @return {void} + */ + destroy() { + this.unbindDOMListeners(); + this.removeAllListeners(); + + if (!this.element) { + return; + } + + this.element.removeEventListener('click', annotatorUtil.prevDefAndStopProp); + if (this.pageEl && this.pageEl.contains(this.element)) { + this.pageEl.removeChild(this.element); + } + } + + /** + * Returns whether or not the dialog is able to be seen + * + * @public + * @return {boolean} Whether or not the dialog is able to be seen + */ + isVisible() { + return this.visible; + } + + /** + * Save the drawing thread upon clicking save. Will cause a soft commit. * * @override * @protected + * @return {void} */ addAnnotation() {} /** - * Empty stub to avoid unexpected behavior. + * Empty stub to avoid unexpected behavior. Removing a drawing annotation can only be done by deleting the thread. * * @override * @protected + * @return {void} */ removeAnnotation() {} + /** + * Bind dialog button listeners + * + * @protected + * @return {void} + */ + bindDOMListeners() { + if (this.commitButtonEl) { + this.commitButtonEl.addEventListener('click', this.postDrawing); + + if (this.hasTouch) { + this.commitButtonEl.addEventListener('touchend', this.postDrawing); + } + } + + if (this.deleteButtonEl) { + this.deleteButtonEl.addEventListener('click', this.deleteAnnotation); + + if (this.hasTouch) { + this.deleteButtonEl.addEventListener('touchend', this.deleteAnnotation); + } + } + } + + /** + * Unbind dialog button listeners + * + * @protected + * @return {void} + */ + unbindDOMListeners() { + if (this.commitButtonEl) { + this.commitButtonEl.removeEventListener('click', this.postDrawing); + this.commitButtonEl.removeEventListener('touchend', this.postDrawing); + } + + if (this.deleteButtonEl) { + this.deleteButtonEl.removeEventListener('click', this.deleteAnnotation); + this.deleteButtonEl.removeEventListener('touchend', this.deleteAnnotation); + } + } + /** * Sets up the drawing dialog element. * + * @protected * @param {Annotation[]} annotations - Annotations to show in the dialog * @param {HTMLElement} threadEl - Annotation icon element * @return {void} + */ + setup(annotations) { + // Create outermost element container + this.element = document.createElement('div'); + this.element.addEventListener('click', annotatorUtil.prevDefAndStopProp); + this.element.classList.add(constants.CLASS_ANNOTATION_DIALOG); + + // Create the dialog element consisting of a label, save, and delete button + this.drawingDialogEl = this.generateDialogEl(annotations); + + // Set the newly created buttons from the dialog element + this.commitButtonEl = this.drawingDialogEl.querySelector(`.${constants.CLASS_ADD_DRAWING_BTN}`); + this.deleteButtonEl = this.drawingDialogEl.querySelector(`.${constants.CLASS_DELETE_DRAWING_BTN}`); + + this.bindDOMListeners(); + + if (annotations.length > 0) { + this.assignDrawingLabel(annotations[0]); + } + + this.element.appendChild(this.drawingDialogEl); + } + + /** + * Position the drawing dialog with an x,y browser coordinate + * + * @protected + * @param {number} x - The x position to position the dialog with + * @param {number} y - The y position to position the dialog with + * @return {void} + */ + position(x, y) { + if (!this.pageEl) { + this.pageEl = this.annotatedElement.querySelector(`[data-page-number="${this.location.page}"]`); + } + + // Reinsert when the dialog is removed from the page + if (!this.pageEl.contains(this.element)) { + this.pageEl.appendChild(this.element); + } + + // NOTE: (@pramodsum) Add the annotationDialog.flipDialog implementation here + // Show dialog so we can get width + const clientRect = this.element.getBoundingClientRect(); + this.element.style.left = `${x - clientRect.width}px`; + this.element.style.top = `${y}px`; + } + + /** + * Hide the dialog in the browser + * * @protected + * @return {void} */ - /* eslint-disable no-unused-vars */ - setup(annotations, threadEl) {} - /* eslint-enable no-unused-vars */ + hide() { + annotatorUtil.hideElement(this.element); + this.visible = false; + } /** * Display the dialog in the browser * - * @protected + * @protected * @return {void} */ - show() {} + show() { + annotatorUtil.showElement(this.element); + this.visible = true; + } - isVisible() { - return this.visible; + /** + * Generate the dialog HTMLElement consisting of a label, save, and delete button + * + * @private + * @param {Array} annotations - Array of annotations. A non-empty array means there are saved drawings. + * @return {HTMLElement} The drawing dialog element + */ + generateDialogEl(annotations) { + const canCommit = annotations.length === 0; + const canDelete = canCommit || (annotations[0].permissions && annotations[0].permissions.can_delete); + + const drawingDialogEl = document.createElement('div'); + + let includedButtonsHTML = canCommit ? COMMIT_TEMPLATE : ''; + if (canDelete) { + includedButtonsHTML += DELETE_TEMPLATE; + } + + drawingDialogEl.classList.add(constants.CLASS_ANNOTATION_DRAWING_DIALOG); + drawingDialogEl.innerHTML = ` + + ${LABEL_TEMPLATE} + ${includedButtonsHTML} + `.trim(); + return drawingDialogEl; + } + + /** + * Fill out the drawing dialog label with the name of the user who drew the drawing. Will use anonymous if + * the username does not exist. + * + * @private + * @param {Annotation} savedAnnotation - The annotation data to populate the label with. + * @return {void} + */ + assignDrawingLabel(savedAnnotation) { + if (!savedAnnotation || !this.drawingDialogEl) { + return; + } + + const drawingLabelEl = this.drawingDialogEl.querySelector(`.${constants.CLASS_ANNOTATION_DRAWING_LABEL}`); + const username = savedAnnotation.user ? savedAnnotation.user.name : constants.USER_ANONYMOUS; + drawingLabelEl.textContent = annotatorUtil.replacePlaceholders(__('annotation_who_drew'), [username]); + + annotatorUtil.showElement(drawingLabelEl); + } + + /** + * Broadcasts message to save the drawing in progress + * + * @private + * @param {event} event - The event object from an event emitter + * @return {void} + */ + postDrawing(event) { + event.stopPropagation(); + event.preventDefault(); + this.emit('annotationcreate'); + } + + /** + * Broadcasts message to delete a drawing. + * + * @private + * @param {event} event - The event object from an event emitter + * @return {void} + */ + deleteAnnotation(event) { + event.stopPropagation(); + event.preventDefault(); + this.emit('annotationdelete'); } } diff --git a/src/lib/annotations/doc/DocDrawingThread.js b/src/lib/annotations/doc/DocDrawingThread.js index f402475ed..6ea80b600 100644 --- a/src/lib/annotations/doc/DocDrawingThread.js +++ b/src/lib/annotations/doc/DocDrawingThread.js @@ -100,23 +100,26 @@ class DocDrawingThread extends DrawingThread { * @return {void} */ handleStop() { + // Stop the render loop and finish drawing + window.cancelAnimationFrame(this.lastAnimationRequestId); + this.lastAnimationRequestId = 0; + this.drawingFlag = DRAW_STATES.idle; if (!this.pendingPath || this.pendingPath.isEmpty()) { return; } + if (!this.dialog) { + this.createDialog(); + } + this.pathContainer.insert(this.pendingPath); this.updateBoundary(this.pendingPath); this.setBoundary(); + this.drawBoundary(); this.emitAvailableActions(); this.pendingPath = null; - - if (this.dialog) { - return; - } - - this.createDialog(); } /** @@ -142,7 +145,7 @@ class DocDrawingThread extends DrawingThread { this.reset(); // Only make save request to server if there exist paths to save - if (this.pathContainer.isUndoEmpty()) { + if (this.pathContainer.isEmpty()) { return; } @@ -169,6 +172,7 @@ class DocDrawingThread extends DrawingThread { const context = this.selectContext(); if (this.dialog && this.dialog.isVisible()) { this.drawBoundary(); + this.dialog.show(); } // Generate the paths and draw to the annotation layer canvas @@ -224,7 +228,9 @@ class DocDrawingThread extends DrawingThread { annotations: this.annotations, locale: this.locale, location: this.location, - canAnnotate: this.annotationService.canAnnotate + canAnnotate: this.annotationService.canAnnotate, + isMobile: this.isMobile, + hasTouch: this.hasTouch }); this.bindCustomListenersOnDialog(); diff --git a/src/lib/annotations/doc/__tests__/DocAnnotator-test.js b/src/lib/annotations/doc/__tests__/DocAnnotator-test.js index ffb851a50..9a1c6010f 100644 --- a/src/lib/annotations/doc/__tests__/DocAnnotator-test.js +++ b/src/lib/annotations/doc/__tests__/DocAnnotator-test.js @@ -633,7 +633,7 @@ describe('lib/annotations/doc/DocAnnotator', () => { annotator.permissions.canAnnotate = true; annotator.isMobile = true; annotator.hasTouch = true; - + const docListen = sandbox.spy(document, 'addEventListener'); const annotatedElementListen = sandbox.spy(annotator.annotatedElement, 'addEventListener'); diff --git a/src/lib/annotations/doc/__tests__/DocDrawingDialog-test.html b/src/lib/annotations/doc/__tests__/DocDrawingDialog-test.html new file mode 100644 index 000000000..689a70afc --- /dev/null +++ b/src/lib/annotations/doc/__tests__/DocDrawingDialog-test.html @@ -0,0 +1,13 @@ +
+
+
+ + +
+ Yao Ming drew + + +
diff --git a/src/lib/annotations/doc/__tests__/DocDrawingDialog-test.js b/src/lib/annotations/doc/__tests__/DocDrawingDialog-test.js new file mode 100644 index 000000000..4ed2247b4 --- /dev/null +++ b/src/lib/annotations/doc/__tests__/DocDrawingDialog-test.js @@ -0,0 +1,294 @@ +import * as annotatorUtil from '../../annotatorUtil'; +import DocDrawingDialog from '../DocDrawingDialog'; + +let docDrawingDialog; +let stubs; +const sandbox = sinon.sandbox.create(); + +describe('lib/annotations/doc/DocDrawingDialog', () => { + before(() => { + fixture.setBase('src/lib'); + }); + + beforeEach(() => { + fixture.load('annotations/doc/__tests__/DocDrawingDialog-test.html'); + docDrawingDialog = new DocDrawingDialog({ + annotatedElement: document.querySelector('.annotated-element'), + location: {}, + annotations: [], + canAnnotate: true + }); + stubs = {}; + }); + + afterEach(() => { + sandbox.verifyAndRestore(); + docDrawingDialog = null; + stubs = null; + }); + + describe('isVisible()', () => { + it('should return true if the dialog is visible', () => { + docDrawingDialog.visible = true; + expect(docDrawingDialog.isVisible()).to.be.truthy; + }); + + it('should return false if the dialog is not visible', () => { + docDrawingDialog.visible = false; + expect(docDrawingDialog.isVisible()).to.be.falsy; + }); + }); + + describe('postDrawing()', () => { + it('should emit annotation create to indicate that the save button was pressed', () => { + const event = { + stopPropagation: sandbox.stub(), + preventDefault: sandbox.stub() + } + sandbox.stub(docDrawingDialog, 'emit'); + + docDrawingDialog.postDrawing(event); + expect(docDrawingDialog.emit).to.be.calledWith('annotationcreate'); + expect(event.stopPropagation).to.be.called; + expect(event.preventDefault).to.be.called; + }); + }); + + describe('bindDOMListeners()', () => { + it('should bind listeners to a commit button element', () => { + docDrawingDialog.hasTouch = true; + docDrawingDialog.commitButtonEl = { + addEventListener: sandbox.stub() + }; + + docDrawingDialog.bindDOMListeners(); + expect(docDrawingDialog.commitButtonEl.addEventListener).to.be.calledWith( + 'click', + docDrawingDialog.postDrawing + ); + expect(docDrawingDialog.commitButtonEl.addEventListener).to.be.calledWith( + 'touchend', + docDrawingDialog.postDrawing + ); + }); + + it('should bind listeners to a delete button element', () => { + docDrawingDialog.hasTouch = true; + docDrawingDialog.deleteButtonEl = { + addEventListener: sandbox.stub() + }; + + docDrawingDialog.bindDOMListeners(); + expect(docDrawingDialog.deleteButtonEl.addEventListener).to.be.calledWith( + 'click', + docDrawingDialog.deleteAnnotation + ); + expect(docDrawingDialog.deleteButtonEl.addEventListener).to.be.calledWith( + 'touchend', + docDrawingDialog.deleteAnnotation + ); + }) + }); + + describe('unbindDOMListeners()', () => { + it('should unbind listeners on a commit button element', () => { + docDrawingDialog.hasTouch = true; + docDrawingDialog.commitButtonEl = { + removeEventListener: sandbox.stub() + }; + + docDrawingDialog.unbindDOMListeners(); + expect(docDrawingDialog.commitButtonEl.removeEventListener).to.be.calledWith( + 'click', + docDrawingDialog.postDrawing + ); + expect(docDrawingDialog.commitButtonEl.removeEventListener).to.be.calledWith( + 'touchend', + docDrawingDialog.postDrawing + ); + }); + + it('should unbind listeners on a delete button element', () => { + docDrawingDialog.hasTouch = true; + docDrawingDialog.deleteButtonEl = { + removeEventListener: sandbox.stub() + }; + + docDrawingDialog.unbindDOMListeners(); + expect(docDrawingDialog.deleteButtonEl.removeEventListener).to.be.calledWith( + 'click', + docDrawingDialog.deleteAnnotation + ); + expect(docDrawingDialog.deleteButtonEl.removeEventListener).to.be.calledWith( + 'touchend', + docDrawingDialog.deleteAnnotation + ); + }) + }); + + describe('setup()', () => { + let drawingDialogEl; + + beforeEach(() => { + drawingDialogEl = document.querySelector('.bp-annotation-drawing-dialog'); + + sandbox.stub(docDrawingDialog, 'generateDialogEl').returns(drawingDialogEl); + sandbox.stub(docDrawingDialog, 'bindDOMListeners'); + sandbox.stub(docDrawingDialog, 'assignDrawingLabel'); + }); + + it('should setup the dialog element without a commit button when given an annotation', () => { + const annotation = { + user: { + name: 'Yao Ming' + } + }; + + expect(docDrawingDialog.element).to.be.undefined; + docDrawingDialog.setup([annotation]); + expect(docDrawingDialog.generateDialogEl).to.be.called; + expect(docDrawingDialog.bindDOMListeners).to.be.called; + expect(docDrawingDialog.assignDrawingLabel).to.be.called; + expect(docDrawingDialog.element.contains(docDrawingDialog.drawingDialogEl)); + expect(docDrawingDialog.commitButtonEl).to.be.null; + }); + + it('should setup the dialog element with a commit button when not given an annotation', () => { + docDrawingDialog.setup([]); + expect(docDrawingDialog.generateDialogEl).to.be.called; + expect(docDrawingDialog.bindDOMListeners).to.be.called; + expect(docDrawingDialog.assignDrawingLabel).to.not.be.called; + expect(docDrawingDialog.element.contains(docDrawingDialog.drawingDialogEl)); + expect(docDrawingDialog.commitButtonEl).to.not.be.undefined; + expect(docDrawingDialog.element.contains(docDrawingDialog.commitButtonEl)); + }); + }); + + describe('position()', () => { + it('should insert the element into the page element and set the position', () => { + const rect = { + width: 1 + }; + const pageEl = { + contains: sandbox.stub().returns(false), + appendChild: sandbox.stub() + }; + + docDrawingDialog.location = { + page: 1 + }; + docDrawingDialog.annotatedElement = { + querySelector: sandbox.stub().returns(pageEl) + }; + docDrawingDialog.element = { + style: { + left: 0, + top: 0 + }, + getBoundingClientRect: sandbox.stub().returns(rect) + } + + docDrawingDialog.position(5, 10); + expect(docDrawingDialog.element.getBoundingClientRect).to.be.called; + expect(pageEl.contains).to.be.called; + expect(pageEl.appendChild).to.be.calledWith(docDrawingDialog.element); + expect(docDrawingDialog.annotatedElement.querySelector).to.be.called; + expect(docDrawingDialog.element.style.left).to.equal(`4px`, `10px`); + }); + }); + + describe('hide()', () => { + it('should hide the element with css', () => { + const element = 'e'; + + sandbox.stub(annotatorUtil, 'hideElement'); + docDrawingDialog.element = element; + expect(docDrawingDialog.visible).to.be.truthy; + + docDrawingDialog.hide(); + expect(annotatorUtil.hideElement).to.be.calledWith(element); + expect(docDrawingDialog.visible).to.be.falsy; + }); + }); + + describe('show()', () => { + it('should show the element with css', () => { + const element = 'e'; + + sandbox.stub(annotatorUtil, 'showElement'); + docDrawingDialog.element = element; + expect(docDrawingDialog.visible).to.be.falsy; + + docDrawingDialog.show(); + expect(annotatorUtil.showElement).to.be.calledWith(element); + expect(docDrawingDialog.visible).to.be.truthy; + }); + }); + + describe('generateDialogEl()', () => { + let annotation; + + beforeEach(() => { + annotation = { + user: { + name: 'Yao Ming' + }, + permissions: { + can_delete: true + } + } + }); + + it('should generate the correctly formatted label dialog element', () => { + annotation.permissions.can_delete = false; + + const returnValue = docDrawingDialog.generateDialogEl([annotation]); + expect(returnValue.classList.contains('bp-annotation-drawing-dialog')).to.be.truthy; + expect(returnValue.querySelector('.bp-btn-annotate-draw-delete')).to.be.null; + expect(returnValue.querySelector('.bp-btn-annotate-draw-add')).to.be.null; + expect(returnValue.querySelector('.bp-annotation-drawing-label')).to.not.be.null; + }); + + it('should generate without a save button', () => { + const returnValue = docDrawingDialog.generateDialogEl([annotation]); + expect(returnValue.classList.contains('bp-annotation-drawing-dialog')).to.be.truthy; + expect(returnValue.querySelector('.bp-btn-annotate-draw-delete')).to.not.be.null; + expect(returnValue.querySelector('.bp-btn-annotate-draw-add')).to.be.null; + expect(returnValue.querySelector('.bp-annotation-drawing-label')).to.not.be.null; + }); + + it('should generate a save and delete button', () => { + const returnValue = docDrawingDialog.generateDialogEl([]); + expect(returnValue.classList.contains('bp-annotation-drawing-dialog')).to.be.truthy; + expect(returnValue.querySelector('.bp-btn-annotate-draw-delete')).to.not.be.null; + expect(returnValue.querySelector('.bp-btn-annotate-draw-add')).to.not.be.null; + expect(returnValue.querySelector('.bp-annotation-drawing-label')).to.not.be.null; + }); + }); + + describe('assignDrawingLabel()', () => { + it('should assign a name to a drawing label', () => { + const drawingLabelEl = {}; + const notYaoMing = 'not yao ming'; + docDrawingDialog.drawingDialogEl = { + querySelector: sandbox.stub().returns(drawingLabelEl) + }; + sandbox.stub(annotatorUtil, 'replacePlaceholders').returns(notYaoMing); + sandbox.stub(annotatorUtil, 'showElement'); + + docDrawingDialog.assignDrawingLabel('non empty'); + expect(drawingLabelEl.textContent).to.equal(notYaoMing); + expect(docDrawingDialog.drawingDialogEl.querySelector).to.be.called; + expect(annotatorUtil.replacePlaceholders).to.be.called; + expect(annotatorUtil.showElement).to.be.called; + }); + + it('should do nothing when given an invalid annotation or does not have a dialog', () => { + expect(docDrawingDialog.assignDrawingLabel, 'not empty').to.not.throw(); + + docDrawingDialog.drawingDialogEl = 'not empty'; + expect(docDrawingDialog.assignDrawingLabel, undefined).to.not.throw(); + expect(docDrawingDialog.assignDrawingLabel, null).to.not.throw(); + }); + }); +}); diff --git a/src/lib/annotations/doc/__tests__/DocDrawingThread-test.js b/src/lib/annotations/doc/__tests__/DocDrawingThread-test.js index 04660f2f0..6b269a8ba 100644 --- a/src/lib/annotations/doc/__tests__/DocDrawingThread-test.js +++ b/src/lib/annotations/doc/__tests__/DocDrawingThread-test.js @@ -122,6 +122,7 @@ describe('lib/annotations/doc/DocDrawingThread', () => { stubs.emitAvailableActions = sandbox.stub(docDrawingThread, 'emitAvailableActions'); stubs.updateBoundary = sandbox.stub(docDrawingThread, 'updateBoundary'); stubs.setBoundary = sandbox.stub(docDrawingThread, 'setBoundary'); + stubs.drawBoundary = sandbox.stub(docDrawingThread, 'drawBoundary'); stubs.createDialog = sandbox.stub(docDrawingThread, 'createDialog'); docDrawingThread.drawingFlag = DRAW_STATES.drawing; docDrawingThread.pendingPath = { @@ -137,6 +138,7 @@ describe('lib/annotations/doc/DocDrawingThread', () => { expect(stubs.emitAvailableActions).to.be.called; expect(stubs.updateBoundary).to.be.called; expect(stubs.setBoundary).to.be.called; + expect(stubs.drawBoundary).to.be.called; expect(stubs.createDialog).to.be.called; expect(docDrawingThread.pathContainer.insert).to.be.called; expect(docDrawingThread.drawingFlag).to.equal(DRAW_STATES.idle); @@ -234,10 +236,10 @@ describe('lib/annotations/doc/DocDrawingThread', () => { it('should clean up without committing when there are no paths to be saved', () => { sandbox.stub(docDrawingThread, 'reset'); - sandbox.stub(docDrawingThread.pathContainer, 'isUndoEmpty').returns(true); + sandbox.stub(docDrawingThread.pathContainer, 'isEmpty').returns(true); docDrawingThread.saveAnnotation('draw'); - expect(docDrawingThread.pathContainer.isUndoEmpty).to.be.called; + expect(docDrawingThread.pathContainer.isEmpty).to.be.called; expect(AnnotationThread.prototype.saveAnnotation).to.not.be.called; expect(docDrawingThread.reset).to.be.called; expect(stubs.show).to.not.be.called; @@ -257,10 +259,10 @@ describe('lib/annotations/doc/DocDrawingThread', () => { clearRect: sandbox.stub() }; - sandbox.stub(docDrawingThread.pathContainer, 'isUndoEmpty').returns(false); + sandbox.stub(docDrawingThread.pathContainer, 'isEmpty').returns(false); docDrawingThread.saveAnnotation('draw'); - expect(docDrawingThread.pathContainer.isUndoEmpty).to.be.called; + expect(docDrawingThread.pathContainer.isEmpty).to.be.called; expect(docDrawingThread.drawingContext.clearRect).to.be.called; expect(AnnotationThread.prototype.saveAnnotation).to.be.called; expect(stubs.show).to.be.called; @@ -343,12 +345,14 @@ describe('lib/annotations/doc/DocDrawingThread', () => { isVisible: sandbox.stub().returns(true), destroy: () => {}, removeAllListeners: () => {}, - hide: () => {} + hide: () => {}, + show: sandbox.stub() } docDrawingThread.show(); expect(docDrawingThread.dialog.isVisible).to.be.called; expect(docDrawingThread.drawBoundary).to.be.called; + expect(docDrawingThread.dialog.show).to.be.called; }) }); diff --git a/src/lib/annotations/drawing/DrawingContainer.js b/src/lib/annotations/drawing/DrawingContainer.js index 55400db14..88bafe2e7 100644 --- a/src/lib/annotations/drawing/DrawingContainer.js +++ b/src/lib/annotations/drawing/DrawingContainer.js @@ -36,7 +36,7 @@ class DrawingContainer { * @return {boolean} Whether or not an undo was done. */ undo() { - if (this.isUndoEmpty()) { + if (this.isEmpty()) { return false; } @@ -65,7 +65,7 @@ class DrawingContainer { * * @return {boolean} Whether or not there exists committed items */ - isUndoEmpty() { + isEmpty() { return this.undoStack.length === 0; } diff --git a/src/lib/annotations/drawing/DrawingModeController.js b/src/lib/annotations/drawing/DrawingModeController.js index b5b69b596..8b56db147 100644 --- a/src/lib/annotations/drawing/DrawingModeController.js +++ b/src/lib/annotations/drawing/DrawingModeController.js @@ -223,7 +223,8 @@ class DrawingModeController extends AnnotationModeController { * @return {void} */ handleSelection(event) { - if (!event) { + // NOTE: @jpress This is a workaround when buttons are not given precedence in the event chain + if (!event || (event.target && event.target.nodeName === 'BUTTON')) { return; } diff --git a/src/lib/annotations/drawing/DrawingThread.js b/src/lib/annotations/drawing/DrawingThread.js index 2b220bcda..a2526920b 100644 --- a/src/lib/annotations/drawing/DrawingThread.js +++ b/src/lib/annotations/drawing/DrawingThread.js @@ -100,11 +100,17 @@ class DrawingThread extends AnnotationThread { this.emit('threadcleanup'); } + /** + * Reset the state of the thread and clear any drawn boundary + * + * @return {void} + */ reset() { super.reset(); this.clearBoundary(); } + /* eslint-disable no-unused-vars */ /** * Handle a pointer movement @@ -313,7 +319,7 @@ class DrawingThread extends AnnotationThread { this.drawingContext.restore(); if (this.dialog) { - if (!this.dialog.isVisible() && !this.pathContainer.isUndoEmpty()) { + if (!this.dialog.isVisible() && !this.pathContainer.isEmpty()) { this.showDialog(); } @@ -335,7 +341,6 @@ class DrawingThread extends AnnotationThread { const elapsed = timestamp - (this.lastRenderTimestamp || 0); if (elapsed >= DRAW_RENDER_THRESHOLD) { this.draw(this.drawingContext, true); - this.drawBoundary(); this.lastRenderTimestamp = timestamp; renderAgain = this.drawingFlag === DRAW_STATES.drawing; @@ -385,7 +390,7 @@ class DrawingThread extends AnnotationThread { this.minY = boundaryData.minY; this.maxY = boundaryData.maxY; - if (this.dialog && this.pathContainer.isUndoEmpty()) { + if (this.dialog && this.pathContainer.isEmpty()) { this.dialog.hide(); } } diff --git a/src/lib/annotations/drawing/__tests__/DrawingContainer-test.js b/src/lib/annotations/drawing/__tests__/DrawingContainer-test.js index 2dc79040c..dfaa845f9 100644 --- a/src/lib/annotations/drawing/__tests__/DrawingContainer-test.js +++ b/src/lib/annotations/drawing/__tests__/DrawingContainer-test.js @@ -168,15 +168,15 @@ describe('lib/annotations/drawing/DrawingContainer', () => { }); }); - describe('isUndoEmpty()', () => { + describe('isEmpty()', () => { it('should return true when no items are in the undoStack', () => { drawingContainer.undoStack = []; - expect(drawingContainer.isUndoEmpty()).to.be.truthy; + expect(drawingContainer.isEmpty()).to.be.truthy; }); it('should return false when there are items are in the undoStack', () => { drawingContainer.undoStack = ['one']; - expect(drawingContainer.isUndoEmpty()).to.be.falsy; + expect(drawingContainer.isEmpty()).to.be.falsy; }); }); });