diff --git a/src/Annotator.js b/src/Annotator.js index 6177e54eb..44fee6c10 100644 --- a/src/Annotator.js +++ b/src/Annotator.js @@ -293,19 +293,19 @@ class Annotator extends EventEmitter { */ setupMobileDialog() { // Generate HTML of dialog - const mobileDialogEl = document.createElement('div'); - mobileDialogEl.setAttribute('data-type', DATA_TYPE_ANNOTATION_DIALOG); - mobileDialogEl.classList.add(CLASS_MOBILE_ANNOTATION_DIALOG); - mobileDialogEl.classList.add(CLASS_ANNOTATION_DIALOG); - mobileDialogEl.classList.add(CLASS_HIDDEN); - mobileDialogEl.id = ID_MOBILE_ANNOTATION_DIALOG; - - mobileDialogEl.innerHTML = ` + this.mobileDialogEl = document.createElement('div'); + this.mobileDialogEl.setAttribute('data-type', DATA_TYPE_ANNOTATION_DIALOG); + this.mobileDialogEl.classList.add(CLASS_MOBILE_ANNOTATION_DIALOG); + this.mobileDialogEl.classList.add(CLASS_ANNOTATION_DIALOG); + this.mobileDialogEl.classList.add(CLASS_HIDDEN); + this.mobileDialogEl.id = ID_MOBILE_ANNOTATION_DIALOG; + + this.mobileDialogEl.innerHTML = `
`.trim(); - this.container.appendChild(mobileDialogEl); + this.container.appendChild(this.mobileDialogEl); const pointController = this.modeControllers[TYPES.point]; if (pointController) { diff --git a/src/doc/DocAnnotator.js b/src/doc/DocAnnotator.js index 3f3d262dc..1cff97435 100644 --- a/src/doc/DocAnnotator.js +++ b/src/doc/DocAnnotator.js @@ -22,6 +22,7 @@ import { PAGE_PADDING_BOTTOM, CLASS_ANNOTATION_LAYER_HIGHLIGHT, CLASS_ANNOTATION_LAYER_DRAW, + CLASS_HIDDEN, THREAD_EVENT, ANNOTATOR_EVENT, CONTROLLER_EVENT, @@ -572,8 +573,11 @@ class DocAnnotator extends Annotator { * @return {void} */ onSelectionChange(event) { - // Do nothing if in a text area - if (document.activeElement.nodeName.toLowerCase() === 'textarea') { + // Do nothing if in a text area or mobile dialog or mobile create dialog is already open + const isHidden = this.mobileDialogEl && this.mobileDialogEl.classList.contains(CLASS_HIDDEN); + const pointController = this.modeControllers[TYPES.point]; + const isCreatingPoint = pointController && pointController.pendingThreadID !== null; + if (isCreatingPoint || !isHidden || document.activeElement.nodeName.toLowerCase() === 'textarea') { return; } diff --git a/src/doc/__tests__/DocAnnotator-test.js b/src/doc/__tests__/DocAnnotator-test.js index c4a175c86..2c5c365f3 100644 --- a/src/doc/__tests__/DocAnnotator-test.js +++ b/src/doc/__tests__/DocAnnotator-test.js @@ -14,6 +14,7 @@ import { ANNOTATOR_EVENT, STATES, TYPES, + CLASS_HIDDEN, CLASS_ANNOTATION_LAYER_HIGHLIGHT, DATA_TYPE_ANNOTATION_DIALOG, THREAD_EVENT, @@ -1101,22 +1102,39 @@ describe('doc/DocAnnotator', () => { describe('onSelectionChange()', () => { beforeEach(() => { annotator.setupAnnotations(); + annotator.mobileDialogEl = document.createElement('div'); + annotator.mobileDialogEl.classList.add(CLASS_HIDDEN); - annotator.highlighter = { - removeAllHighlights: sandbox.stub() + annotator.highlighter = { removeAllHighlights: sandbox.stub() }; + annotator.modeControllers = { + 'point': { pendingThreadID: null } }; + + stubs.getSelStub = sandbox.stub(window, 'getSelection'); }); it('should do nothing if focus is on a text input element', () => { const textAreaEl = document.createElement('textarea'); annotator.annotatedElement.appendChild(textAreaEl); textAreaEl.focus(); - const getSelStub = sandbox.stub(window, 'getSelection'); annotator.onSelectionChange({ nodeName: 'textarea' }); - expect(getSelStub).to.not.be.called; + expect(stubs.getSelStub).to.not.be.called; + }); + + it('should do nothing the shared mobile dialog is visible', () => { + annotator.mobileDialogEl.classList.remove(CLASS_HIDDEN); + + annotator.onSelectionChange({}); + expect(stubs.getSelStub).to.not.be.called; + }); + + it('should do nothing the the user is currently creating a point annotation', () => { + annotator.modeControllers['point'].pendingThreadID = 'something'; + annotator.onSelectionChange({}); + expect(stubs.getSelStub).to.not.be.called; }); it('should clear out previous highlights if a new highlight range is being created', () => { @@ -1128,7 +1146,7 @@ describe('doc/DocAnnotator', () => { anchorNode: 'not_derp' }; annotator.lastSelection = lastSelection; - sandbox.stub(window, 'getSelection').returns(selection); + stubs.getSelStub.returns(selection); annotator.onSelectionChange({}); expect(annotator.highlighter.removeAllHighlights).to.be.called; @@ -1138,7 +1156,7 @@ describe('doc/DocAnnotator', () => { const selection = { toString: () => '' // Causes invalid selection }; - sandbox.stub(window, 'getSelection').returns(selection); + stubs.getSelStub.returns(selection); annotator.lastSelection = selection; annotator.lastHighlightEvent = {}; @@ -1155,7 +1173,7 @@ describe('doc/DocAnnotator', () => { isCollapsed: false, toString: () => 'asdf' }; - sandbox.stub(window, 'getSelection').returns(selection); + stubs.getSelStub.returns(selection); annotator.lastSelection = selection; annotator.lastHighlightEvent = {}; annotator.createHighlightDialog.isVisible = false; @@ -1181,7 +1199,7 @@ describe('doc/DocAnnotator', () => { annotator.lastHighlightEvent = {}; annotator.threads = { 1: { '123abc': stubs.thread } }; - sandbox.stub(window, 'getSelection').returns(selection); + stubs.getSelStub.returns(selection); sandbox.stub(annotator.createHighlightDialog, 'show'); sandbox.stub(annotator, 'getHighlightThreadsOnPage').returns([thread]);