From bfa50e581ac19933a9782fea932664942d8a0abd Mon Sep 17 00:00:00 2001 From: Sumedha Pramod Date: Mon, 11 Sep 2017 13:09:14 -0700 Subject: [PATCH] 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 --- src/lib/annotations/Annotator.scss | 1 + src/lib/annotations/doc/DocHighlightDialog.js | 12 ++++++++++-- .../doc/__tests__/DocHighlightDialog-test.js | 18 ++++++++++-------- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/lib/annotations/Annotator.scss b/src/lib/annotations/Annotator.scss index 7fee4644d..2d6a51ee6 100644 --- 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 index 0c7f5fec9..d96cc3c2b 100644 --- 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 index 632f526da..7f882973a 100644 --- 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'); }); });