Skip to content

Commit

Permalink
Fix: Point annotation cleanup (#325)
Browse files Browse the repository at this point in the history
- Ensures that dialog.position doesn't get called for image point annotations 
- Added styling to the create annotation text area container so that it fits inside the tablet annotations dialog that slides out from the right 
- Removes 'zoomable' and 'pannable' classes from image when the image viewer controls are disabled
- Added extra padding below reply-container on mobile annotation dialogs so that the post/cancel buttons are displayed properly irrespective of whether or not the container can scroll
  • Loading branch information
pramodsum committed Aug 21, 2017
1 parent 4b5421c commit e27c18a
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 2 deletions.
3 changes: 2 additions & 1 deletion src/lib/annotations/Annotator.scss
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,8 @@ $avatar-color-9: #f22c44;
.bp-annotation-mode .page,
.bp-annotation-mode .bp-annotation-layer-highlight,
.bp-annotation-mode .textLayer > div,
.bp-annotation-mode > img {
.bp-annotation-mode > img,
.bp-annotation-mode img {
cursor: crosshair;
}

Expand Down
14 changes: 14 additions & 0 deletions src/lib/annotations/MobileAnnotator.scss
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,14 @@ $tablet: "(min-width: 768px)";
width: 24px;
}
}

.reply-container {
padding-bottom: 15px;

@media #{$tablet} {
padding-bottom: 45px;
}
}
}

/* Highlight dialog */
Expand All @@ -186,6 +194,12 @@ $tablet: "(min-width: 768px)";
width: 100%;

@include border-top-bottom;

@media #{$tablet} {
left: auto;
padding: 15px 0;
width: 415px;
}
}

.bp-mobile-annotation-dialog .bp-annotation-highlight-dialog {
Expand Down
4 changes: 3 additions & 1 deletion src/lib/annotations/image/ImagePointThread.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ class ImagePointThread extends AnnotationThread {
this.showDialog();

// Force dialogs to reposition on re-render
this.dialog.position();
if (!this.isMobile) {
this.dialog.position();
}
}
}

Expand Down
21 changes: 21 additions & 0 deletions src/lib/annotations/image/__tests__/ImagePointThread-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ describe('lib/annotations/image/ImagePointThread', () => {
thread.dialog = {
position: sandbox.stub()
};
thread.isMobile = false;
});

afterEach(() => {
Expand Down Expand Up @@ -72,6 +73,26 @@ describe('lib/annotations/image/ImagePointThread', () => {

expect(thread.showDialog).to.not.have.been.called;
});

it('should not re-position the dialog if pending but on a mobile device', () => {
thread.isMobile = true;
thread.state = STATES.pending;

sandbox.stub(imageAnnotatorUtil, 'getBrowserCoordinatesFromLocation').returns([1, 2]);
sandbox.stub(annotatorUtil, 'showElement');
sandbox.stub(thread, 'showDialog');

thread.show();

expect(imageAnnotatorUtil.getBrowserCoordinatesFromLocation).to.have.been.calledWith(
thread.location,
thread.annotatedElement
);

expect(thread.dialog.position).to.not.have.been.called;
expect(annotatorUtil.showElement).to.have.been.calledWith(thread.element);
expect()
});
});

describe('createDialog', () => {
Expand Down
4 changes: 4 additions & 0 deletions src/lib/viewers/image/ImageBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,10 @@ class ImageBaseViewer extends BaseViewer {
disableViewerControls() {
super.disableViewerControls();
this.unbindDOMListeners();

// Ensure zoom/pan classes are removed when controls are disabled
this.imageEl.classList.remove(CSS_CLASS_ZOOMABLE);
this.imageEl.classList.remove(CSS_CLASS_PANNABLE);
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/lib/viewers/image/__tests__/ImageBaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,8 @@ describe('lib/viewers/image/ImageBaseViewer', () => {
imageBase.disableViewerControls();
expect(imageBase.controls.disable).to.be.called;
expect(imageBase.unbindDOMListeners).to.be.called;
expect(imageBase.imageEl).to.not.have.class(CSS_CLASS_ZOOMABLE);
expect(imageBase.imageEl).to.not.have.class(CSS_CLASS_PANNABLE);
});
});

Expand Down

0 comments on commit e27c18a

Please sign in to comment.