Chore: Disable dialog actions until annotation is saved on the server (…

…#48)
box · Nov 27, 2017 · 6dbcda2 · 6dbcda2
1 parent eea7dcf
commit 6dbcda2
Show file tree

Hide file tree

Showing 5 changed files with 121 additions and 0 deletions.
diff --git a/src/AnnotationDialog.js b/src/AnnotationDialog.js
@@ -438,6 +438,44 @@ class AnnotationDialog extends EventEmitter {
         }
     }
 
+    /**
+     * Enable all buttons for the temporary annotation element
+     *
+     * @protected
+     * @param {string} annotationID Annotation of saved annotation element
+     * @return {void}
+     */
+    enable(annotationID) {
+        const annotationEl = this.element.querySelector(`[data-annotation-id="${annotationID}"]`);
+        if (!annotationEl) {
+            return;
+        }
+
+        const btns = annotationEl.querySelectorAll('button');
+        btns.forEach((btn) => {
+            btn.classList.remove(constants.CLASS_DISABLED);
+        });
+    }
+
+    /**
+     * Disable all buttons for the temporary annotation element
+     *
+     * @protected
+     * @param {string} tempAnnotationID Annotation of temporary annotation element
+     * @return {void}
+     */
+    disable(tempAnnotationID) {
+        const annotationEl = this.element.querySelector(`[data-annotation-id="${tempAnnotationID}"]`);
+        if (!annotationEl) {
+            return;
+        }
+
+        const btns = annotationEl.querySelectorAll('button');
+        btns.forEach((btn) => {
+            btn.classList.add(constants.CLASS_DISABLED);
+        });
+    }
+
     //--------------------------------------------------------------------------
     // Private
     //--------------------------------------------------------------------------

diff --git a/src/AnnotationThread.js b/src/AnnotationThread.js
@@ -162,6 +162,10 @@ class AnnotationThread extends EventEmitter {
         this.saveAnnotationToThread(tempAnnotation);
         this.state = STATES.inactive;
 
+        if (this.dialog) {
+            this.dialog.disable(tempAnnotation.annotationID);
+        }
+
         // Save annotation on server
         return this.annotationService
             .create(annotationData)
@@ -508,6 +512,7 @@ class AnnotationThread extends EventEmitter {
             // Remove temporary annotation and replace it with the saved annotation
             this.dialog.addAnnotation(savedAnnotation);
             this.dialog.removeAnnotation(tempAnnotation.annotationID);
+            this.dialog.enable(savedAnnotation.annotationID);
         }
 
         if (this.isMobile) {

diff --git a/src/__tests__/AnnotationDialog-test.js b/src/__tests__/AnnotationDialog-test.js
@@ -557,6 +557,55 @@ describe('AnnotationDialog', () => {
         });
     });
 
+    describe('enable()', () => {
+        it('should enable all buttons in specified annotation element', () => {
+            dialog.element = document.createElement('div');
+
+            const annotationEl = document.createElement('div');
+            annotationEl.setAttribute('data-annotation-id', '123');
+            dialog.element.appendChild(annotationEl);
+
+            // Add buttons
+            const btn = document.createElement('button');
+            btn.classList.add(constants.CLASS_DISABLED);
+            sandbox.stub(annotationEl, 'querySelectorAll').returns([btn, btn]);
+
+            const wrongEl = document.createElement('div');
+            wrongEl.setAttribute('data-annotation-id', 'invalid');
+            sandbox.stub(wrongEl, 'querySelectorAll');
+            dialog.element.appendChild(wrongEl);
+
+            dialog.enable('123');
+            expect(annotationEl.querySelectorAll).to.be.called;
+            expect(btn).to.not.have.class(constants.CLASS_DISABLED)
+            expect(wrongEl.querySelectorAll).to.not.be.called;
+        });
+    });
+
+    describe('disable()', () => {
+        it('should disable all buttons in specified annotation element', () => {
+            dialog.element = document.createElement('div');
+
+            const annotationEl = document.createElement('div');
+            annotationEl.setAttribute('data-annotation-id', '123');
+            dialog.element.appendChild(annotationEl);
+
+            // Add buttons
+            const btn = document.createElement('button');
+            sandbox.stub(annotationEl, 'querySelectorAll').returns([btn, btn]);
+
+            const wrongEl = document.createElement('div');
+            wrongEl.setAttribute('data-annotation-id', 'invalid');
+            sandbox.stub(wrongEl, 'querySelectorAll');
+            dialog.element.appendChild(wrongEl);
+
+            dialog.disable('123');
+            expect(annotationEl.querySelectorAll).to.be.called;
+            expect(btn).to.have.class(constants.CLASS_DISABLED)
+            expect(wrongEl.querySelectorAll).to.not.be.called;
+        });
+    });
+
     describe('clickHandler()', () => {
         beforeEach(() => {
             stubs.event = {

diff --git a/src/__tests__/AnnotationThread-test.js b/src/__tests__/AnnotationThread-test.js
@@ -147,6 +147,13 @@ describe('AnnotationThread', () => {
 
             sandbox.stub(thread, 'getThreadEventData').returns({});
             stubs.create = sandbox.stub(annotationService, 'create');
+            thread.dialog = {
+                addAnnotation: () => {},
+                activateReply: () => {},
+                disable: () => {}
+            };
+            const dialogMock = sandbox.mock(thread.dialog);
+            dialogMock.expects('disable');
         });
 
         it('should save an annotation with the specified type and text', (done) => {
@@ -243,6 +250,27 @@ describe('AnnotationThread', () => {
             thread.updateTemporaryAnnotation(tempAnnotation, serverAnnotation);
         });
 
+        it('should update thread number and replace temporary annotation if dialog exists', () => {
+            const serverAnnotation = { annotationID: 123 };
+            const tempAnnotation = { annotationID: 1 };
+            thread.threadNumber = 'something';
+            thread.dialog = {
+                addAnnotation: () => {},
+                removeAnnotation: () => {},
+                enable: () => {},
+                element: {
+                    dataset: { threadNumber: undefined }
+                }
+            };
+            const dialogMock = sandbox.mock(thread.dialog);
+
+            dialogMock.expects('enable').withArgs(serverAnnotation.annotationID);
+            dialogMock.expects('addAnnotation').withArgs(serverAnnotation);
+            dialogMock.expects('removeAnnotation').withArgs(tempAnnotation.annotationID);
+            thread.updateTemporaryAnnotation(tempAnnotation, serverAnnotation);
+            expect(thread.dialog.element.dataset.threadNumber).to.not.be.undefined;
+        });
+
         it('should only show dialog immediately on mobile devices', () => {
             const serverAnnotation = { threadNumber: 1 };
             const tempAnnotation = serverAnnotation;

diff --git a/src/doc/__tests__/DocHighlightThread-test.js b/src/doc/__tests__/DocHighlightThread-test.js
@@ -60,6 +60,7 @@ describe('doc/DocHighlightThread', () => {
             // Adding a plain highlight annotation to the thread
             sandbox.stub(thread.annotationService, 'create').returns(Promise.resolve({}));
             sandbox.stub(thread.dialog, 'position');
+            sandbox.stub(thread.dialog, 'disable');
             thread.saveAnnotation('highlight', '');
 
             // Cancel first comment on existing annotation