Fix: Scrolling on highlight dialogs in powerpoints (#24)

box · Nov 10, 2017 · b21ed0e · b21ed0e
1 parent 8bd12a5
commit b21ed0e
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 3 deletions.
diff --git a/src/AnnotationDialog.js b/src/AnnotationDialog.js
@@ -791,7 +791,13 @@ class AnnotationDialog extends EventEmitter {
      * @return {void}
      */
     fitDialogHeightInPage() {
-        this.dialogEl.style.maxHeight = `${this.container.clientHeight / 2}px`;
+        const maxHeight = this.container.clientHeight / 2 - constants.PAGE_PADDING_TOP - constants.PAGE_PADDING_BOTTOM;
+        this.dialogEl.style.maxHeight = `${maxHeight}px`;
+
+        const commentsEl = this.dialogEl.querySelector(`.${constants.CLASS_ANNOTATION_CONTAINER}`);
+        if (commentsEl) {
+            commentsEl.style.maxHeight = `${maxHeight}px`;
+        }
     }
 }
 

diff --git a/src/__tests__/AnnotationDialog-test.js b/src/__tests__/AnnotationDialog-test.js
@@ -1025,10 +1025,25 @@ describe('AnnotationDialog', () => {
 
     describe('fitDialogHeightInPage()', () => {
         it('should allow scrolling on annotations dialog if file is a powerpoint', () => {
-            dialog.dialogEl = { style: {} };
+            dialog.dialogEl = {
+                style: {},
+                querySelector: sandbox.stub().returns(null)
+            };
+            dialog.container = { clientHeight: 100 };
+            dialog.fitDialogHeightInPage();
+            expect(dialog.dialogEl.style.maxHeight).equals('20px');
+        });
+
+        it('should allow scrolling on annotations dialog if file is a powerpoint', () => {
+            const commentsEl = document.createElement('div');
+            dialog.dialogEl = {
+                style: {},
+                querySelector: sandbox.stub().returns(commentsEl)
+            };
             dialog.container = { clientHeight: 100 };
             dialog.fitDialogHeightInPage();
-            expect(dialog.dialogEl.style.maxHeight).equals('50px');
+            expect(dialog.dialogEl.style.maxHeight).equals('20px');
+            expect(commentsEl.style.maxHeight).equals('20px');
         });
     });
 });
diff --git a/src/doc/DocHighlightDialog.js b/src/doc/DocHighlightDialog.js
@@ -364,6 +364,8 @@ class DocHighlightDialog extends AnnotationDialog {
     bindDOMListeners() {
         this.element.addEventListener('mousedown', this.mousedownHandler);
         this.element.addEventListener('keydown', this.keydownHandler);
+        this.element.addEventListener('mouseup', this.stopPropagation);
+        this.element.addEventListener('wheel', this.stopPropagation);
 
         if (!this.isMobile) {
             this.element.addEventListener('mouseenter', this.mouseenterHandler);
@@ -381,6 +383,8 @@ class DocHighlightDialog extends AnnotationDialog {
     unbindDOMListeners() {
         this.element.removeEventListener('mousedown', this.mousedownHandler);
         this.element.removeEventListener('keydown', this.keydownHandler);
+        this.element.removeEventListener('mouseup', this.stopPropagation);
+        this.element.removeEventListener('wheel', this.stopPropagation);
 
         if (!this.isMobile) {
             this.element.removeEventListener('mouseenter', this.mouseenterHandler);

diff --git a/src/doc/__tests__/DocHighlightDialog-test.js b/src/doc/__tests__/DocHighlightDialog-test.js
@@ -509,6 +509,8 @@ describe('doc/DocHighlightDialog', () => {
             dialog.bindDOMListeners();
             expect(stubs.add).to.be.calledWith('mousedown', sinon.match.func);
             expect(stubs.add).to.be.calledWith('keydown', sinon.match.func);
+            expect(stubs.add).to.be.calledWith('mouseup', sinon.match.func);
+            expect(stubs.add).to.be.calledWith('wheel', sinon.match.func);
             expect(stubs.add).to.be.calledWith('mouseleave', sinon.match.func);
             expect(stubs.add).to.be.calledWith('mouseenter', sinon.match.func);
         });
@@ -520,6 +522,8 @@ describe('doc/DocHighlightDialog', () => {
             dialog.bindDOMListeners();
             expect(stubs.add).to.be.calledWith('mousedown', sinon.match.func);
             expect(stubs.add).to.be.calledWith('keydown', sinon.match.func);
+            expect(stubs.add).to.be.calledWith('mouseup', sinon.match.func);
+            expect(stubs.add).to.be.calledWith('wheel', sinon.match.func);
             expect(stubs.add).to.not.be.calledWith('mouseenter', sinon.match.func);
             expect(stubs.add).to.not.be.calledWith('mouseleave', sinon.match.func);
         });
@@ -532,6 +536,8 @@ describe('doc/DocHighlightDialog', () => {
             dialog.unbindDOMListeners();
             expect(stubs.remove).to.be.calledWith('mousedown', sinon.match.func);
             expect(stubs.remove).to.be.calledWith('keydown', sinon.match.func);
+            expect(stubs.remove).to.be.calledWith('mouseup', sinon.match.func);
+            expect(stubs.remove).to.be.calledWith('wheel', sinon.match.func);
             expect(stubs.remove).to.be.calledWith('mouseleave', sinon.match.func);
             expect(stubs.remove).to.be.calledWith('mouseenter', sinon.match.func);
         });
@@ -543,6 +549,8 @@ describe('doc/DocHighlightDialog', () => {
             dialog.unbindDOMListeners();
             expect(stubs.remove).to.be.calledWith('mousedown', sinon.match.func);
             expect(stubs.remove).to.be.calledWith('keydown', sinon.match.func);
+            expect(stubs.remove).to.be.calledWith('mouseup', sinon.match.func);
+            expect(stubs.remove).to.be.calledWith('wheel', sinon.match.func);
             expect(stubs.remove).to.not.be.calledWith('mouseenter', sinon.match.func);
             expect(stubs.remove).to.not.be.calledWith('mouseleave', sinon.match.func);
         });