Skip to content

Commit

Permalink
Fix: Plain highlight is mispositioned when it runs off the side (#381)
Browse files Browse the repository at this point in the history
- Ensure activated point annotation icon appears ABOVE dialog. This reduces the flicker that occurs when the mouse moves between icon
  and dialog
  • Loading branch information
pramodsum authored Sep 11, 2017
1 parent ec043b9 commit bfa50e5
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 10 deletions.
1 change: 1 addition & 0 deletions src/lib/annotations/Annotator.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
12 changes: 10 additions & 2 deletions src/lib/annotations/doc/DocHighlightDialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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;
}

Expand Down
18 changes: 10 additions & 8 deletions src/lib/annotations/doc/__tests__/DocHighlightDialog-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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`);
});
});

Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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
});
});

Expand Down Expand Up @@ -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');
});
});

Expand Down

0 comments on commit bfa50e5

Please sign in to comment.