Fix: Plain highlight is mispositioned when it runs off the side (#381)

- Ensure activated point annotation icon appears ABOVE dialog. This reduces the flicker that occurs when the mouse moves between icon and dialog
box · Sep 11, 2017 · bfa50e5 · bfa50e5
1 parent ec043b9
commit bfa50e5
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 10 deletions.
diff --git a/src/lib/annotations/Annotator.scss b/src/lib/annotations/Annotator.scss
@@ -264,6 +264,7 @@ $avatar-color-9: #f22c44;
     padding: 0;
     position: absolute;
     width: 24px;
+    z-index: 10000; // Ensure activated point annotation icon is above dialog
 
     svg {
         fill: fade-out($box-blue, .35);

diff --git a/src/lib/annotations/doc/DocHighlightDialog.js b/src/lib/annotations/doc/DocHighlightDialog.js
@@ -484,8 +484,13 @@ class DocHighlightDialog extends AnnotationDialog {
      * @return {number} Annotations dialog width
      */
     getDialogWidth() {
-        // Switches to 'visibility: hidden' to ensure that dialog takes up DOM
-        // space while still being invisible
+        // Ensure dialog will not be displayed off the page when
+        // calculating the dialog width
+        const prevDialogX = this.element.style.left;
+        this.element.style.left = 0;
+
+        // Switches to 'visibility: hidden' to ensure that dialog takes up
+        // DOM space while still being invisible
         annotatorUtil.hideElementVisibility(this.element);
         annotatorUtil.showElement(this.element);
 
@@ -496,6 +501,9 @@ class DocHighlightDialog extends AnnotationDialog {
         annotatorUtil.hideElement(this.element);
         annotatorUtil.showInvisibleElement(this.element);
 
+        // Reset dialog left positioning
+        this.element.style.left = prevDialogX;
+
         return this.highlightDialogWidth;
     }
 

diff --git a/src/lib/annotations/doc/__tests__/DocHighlightDialog-test.js b/src/lib/annotations/doc/__tests__/DocHighlightDialog-test.js
@@ -250,7 +250,7 @@ describe('lib/annotations/doc/DocHighlightDialog', () => {
             expect(stubs.width).to.have.been.called;
             expect(stubs.caret).to.have.been.called;
             expect(stubs.show).to.have.been.called;
-            expect(dialog.element.style.left).to.equal('10px');
+            expect(dialog.element.style.left).equals('10px');
         });
 
         it('should position the highlight comments dialog at the right place and show it', () => {
@@ -263,7 +263,7 @@ describe('lib/annotations/doc/DocHighlightDialog', () => {
             expect(stubs.width).to.have.been.called;
             expect(stubs.caret).to.have.been.called;
             expect(stubs.show).to.have.been.called;
-            expect(dialog.element.style.left).to.equal('10px');
+            expect(dialog.element.style.left).equals('10px');
         });
 
         it('should adjust the dialog if the mouse location is above the page', () => {
@@ -273,7 +273,7 @@ describe('lib/annotations/doc/DocHighlightDialog', () => {
             dialog.position();
 
             expect(stubs.scaled).to.have.been.called;
-            expect(dialog.element.style.top).to.equal(`${PAGE_PADDING_TOP}px`);
+            expect(dialog.element.style.top).equals(`${PAGE_PADDING_TOP}px`);
         });
 
         it('should adjust the dialog if the dialog will run below the page', () => {
@@ -282,7 +282,7 @@ describe('lib/annotations/doc/DocHighlightDialog', () => {
             dialog.position();
 
             expect(stubs.scaled).to.have.been.called;
-            expect(dialog.element.style.top).to.equal(`${PAGE_PADDING_TOP}px`);
+            expect(dialog.element.style.top).equals(`${PAGE_PADDING_TOP}px`);
         });
     });
 
@@ -407,7 +407,7 @@ describe('lib/annotations/doc/DocHighlightDialog', () => {
 
         it('should setup the dialog element and add thread number to the dialog', () => {
             dialog.setup([stubs.annotation]);
-            expect(dialog.element.dataset.threadNumber).to.equal('1');
+            expect(dialog.element.dataset.threadNumber).equals('1');
         });
 
         it('should not set the thread number when using a mobile browser', () => {
@@ -634,15 +634,17 @@ describe('lib/annotations/doc/DocHighlightDialog', () => {
             const highlightLabelEl = dialog.element.querySelector(`.${CLASS_HIGHLIGHT_LABEL}`);
             highlightLabelEl.innerHTML = 'Bob highlighted';
             dialog.element.style.width = '100px';
+            dialog.element.style.left = '30px';
 
             const width = dialog.getDialogWidth();
-            expect(width).to.equal(100);
+            expect(width).equals(100);
+            expect(dialog.element.style.left).equals('30px');
         });
 
         it('should return previously set dialog width if already calculated', () => {
             dialog.element.style.width = '252px';
             const width = dialog.getDialogWidth();
-            expect(width).to.equal(252); // Default comments dialog width
+            expect(width).equals(252); // Default comments dialog width
         });
     });
 
@@ -674,7 +676,7 @@ describe('lib/annotations/doc/DocHighlightDialog', () => {
             const comment = document.querySelector('.annotation-comment');
 
             expect(comment).to.be.null;
-            expect(highlight.dataset.annotationId).to.equal('1');
+            expect(highlight.dataset.annotationId).equals('1');
         });
     });