Skip to content

Commit

Permalink
New: Multi image annotations post bundling refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
JustinHoldstock authored May 11, 2017
1 parent 699ed56 commit 93a5ba2
Show file tree
Hide file tree
Showing 23 changed files with 301 additions and 233 deletions.
2 changes: 1 addition & 1 deletion src/lib/annotations/BoxAnnotations.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const ANNOTATORS = [
{
NAME: 'Image',
CONSTRUCTOR: ImageAnnotator,
VIEWER: ['Image'],
VIEWER: ['Image', 'MultiImage'],
TYPE: ['point']
}
];
Expand Down
4 changes: 4 additions & 0 deletions src/lib/annotations/__tests__/annotatorUtil-test.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@

<textarea class="textarea"></textarea>

<div class="page" data-page-number="2">
<div class="foo"></div>
</div>

<div class="bp-annotation-dialog">
<div class="bp-annotation-caret"></div>
</div>
18 changes: 18 additions & 0 deletions src/lib/annotations/__tests__/annotatorUtil-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import {
findClosestElWithClass,
findClosestDataType,
getPageElAndPageNumber,
showElement,
hideElement,
showInvisibleElement,
Expand Down Expand Up @@ -65,6 +66,23 @@ describe('lib/annotations/annotatorUtil', () => {
});
});

describe('getPageElAndPageNumber()', () => {
it('should return page element and page number that the specified element is on', () => {
const fooEl = document.querySelector('.foo');
const pageEl = document.querySelector('.page');
const result = getPageElAndPageNumber(fooEl);
assert.equal(result.pageEl, pageEl, 'Page element should be equal');
assert.equal(result.page, 2, 'Page number should be equal');
});

it('should return no page element and -1 page number if no page is found', () => {
const barEl = document.querySelector('.bar');
const result = getPageElAndPageNumber(barEl);
assert.equal(result.pageEl, null, 'Page element should be null');
assert.equal(result.page, -1, 'Page number should be -1');
});
});

describe('showElement()', () => {
it('should remove hidden class from element with matching selector', () => {
// Hide element before testing show function
Expand Down
20 changes: 20 additions & 0 deletions src/lib/annotations/annotatorUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,26 @@ export function findClosestElWithClass(element, className) {
return null;
}

/**
* Returns the page element and page number that the element is on.
* @param {HTMLElement} element - Element to find page and page number for
* @return {Object} Page element/page number if found or null/-1 if not
*/
export function getPageElAndPageNumber(element) {
const pageEl = findClosestElWithClass(element, 'page');
if (pageEl) {
return {
pageEl,
page: parseInt(pageEl.getAttribute('data-page-number'), 10)
};
}

return {
pageEl: null,
page: -1
};
}

/**
* Finds the closest element with a data type and returns that data type. If
* an attributeName is provided, search for that data atttribute instead of
Expand Down
12 changes: 6 additions & 6 deletions src/lib/annotations/doc/DocAnnotator.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class DocAnnotator extends Annotator {

// If click isn't on a page, ignore
const eventTarget = event.target;
const { pageEl, page } = docAnnotatorUtil.getPageElAndPageNumber(eventTarget);
const { pageEl, page } = annotatorUtil.getPageElAndPageNumber(eventTarget);
if (!pageEl) {
return location;
}
Expand Down Expand Up @@ -96,10 +96,10 @@ class DocAnnotator extends Annotator {
}

// Get correct page
let { pageEl, page } = docAnnotatorUtil.getPageElAndPageNumber(event.target);
let { pageEl, page } = annotatorUtil.getPageElAndPageNumber(event.target);
if (page === -1) {
// The ( .. ) around assignment is required syntax
({ pageEl, page } = docAnnotatorUtil.getPageElAndPageNumber(window.getSelection().anchorNode));
({ pageEl, page } = annotatorUtil.getPageElAndPageNumber(window.getSelection().anchorNode));
}

// Use Rangy to save the current selection because using the
Expand Down Expand Up @@ -352,7 +352,7 @@ class DocAnnotator extends Annotator {

this.throttledHighlightMousemoveHandler = throttle((event) => {
// Only filter through highlight threads on the current page
const page = docAnnotatorUtil.getPageElAndPageNumber(event.target).page;
const page = annotatorUtil.getPageElAndPageNumber(event.target).page;
const pageThreads = this.getHighlightThreadsOnPage(page);
const delayThreads = [];

Expand Down Expand Up @@ -454,7 +454,7 @@ class DocAnnotator extends Annotator {

// Only filter through highlight threads on the current page
// Reset active highlight threads before creating new highlight
const page = docAnnotatorUtil.getPageElAndPageNumber(event.target).page;
const page = annotatorUtil.getPageElAndPageNumber(event.target).page;
const activeThreads = this.getHighlightThreadsOnPage(page).filter((thread) => constants.ACTIVE_STATES.indexOf(thread.state) > -1);
activeThreads.forEach((thread) => {
thread.reset();
Expand Down Expand Up @@ -499,7 +499,7 @@ class DocAnnotator extends Annotator {
});

// Only filter through highlight threads on the current page
const page = docAnnotatorUtil.getPageElAndPageNumber(event.target).page;
const page = annotatorUtil.getPageElAndPageNumber(event.target).page;
const pageThreads = this.getHighlightThreadsOnPage(page);
pageThreads.forEach((thread) => {
// We use this to prevent a mousedown from activating two different
Expand Down
2 changes: 1 addition & 1 deletion src/lib/annotations/doc/__tests__/DocAnnotator-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('lib/annotations/doc/DocAnnotator', () => {
annotator.annotatedElement = annotator.getAnnotatedEl(document);
annotator.annotationService = {};

stubs.getPage = sandbox.stub(docAnnotatorUtil, 'getPageElAndPageNumber');
stubs.getPage = sandbox.stub(annotatorUtil, 'getPageElAndPageNumber');
});

afterEach(() => {
Expand Down
18 changes: 0 additions & 18 deletions src/lib/annotations/doc/__tests__/docAnnotatorUtil-test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/* eslint-disable no-unused-expressions */
import {
isPresentation,
getPageElAndPageNumber,
isInDialog,
hasActiveDialog,
fitDialogHeightInPage,
Expand Down Expand Up @@ -43,23 +42,6 @@ describe('lib/annotations/doc/docAnnotatorUtil', () => {
});
});

describe('getPageElAndPageNumber()', () => {
it('should return page element and page number that the specified element is on', () => {
const fooEl = document.querySelector('.foo');
const pageEl = document.querySelector('.page');
const result = getPageElAndPageNumber(fooEl);
assert.equal(result.pageEl, pageEl, 'Page element should be equal');
assert.equal(result.page, 2, 'Page number should be equal');
});

it('should return no page element and -1 page number if no page is found', () => {
const barEl = document.querySelector('.bar');
const result = getPageElAndPageNumber(barEl);
assert.equal(result.pageEl, null, 'Page element should be null');
assert.equal(result.page, -1, 'Page number should be -1');
});
});

describe('isInDialog()', () => {
it('should return false if no dialog element exists', () => {
const result = isInDialog({ clientX: 8, clientY: 8 });
Expand Down
20 changes: 0 additions & 20 deletions src/lib/annotations/doc/docAnnotatorUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,6 @@ export function isPresentation(annotatedElement) {
// DOM Utils
//------------------------------------------------------------------------------

/**
* Returns the page element and page number that the element is on.
* @param {HTMLElement} element - Element to find page and page number for
* @return {Object} Page element/page number if found or null/-1 if not
*/
export function getPageElAndPageNumber(element) {
const pageEl = annotatorUtil.findClosestElWithClass(element, 'page');
if (pageEl) {
return {
pageEl,
page: parseInt(pageEl.getAttribute('data-page-number'), 10)
};
}

return {
pageEl: null,
page: -1
};
}

/**
* Checks whether mouse is inside the dialog represented by this thread.
*
Expand Down
54 changes: 30 additions & 24 deletions src/lib/annotations/image/ImageAnnotator.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ import * as annotatorUtil from '../annotatorUtil';
import * as imageAnnotatorUtil from './imageAnnotatorUtil';
import { SELECTOR_BOX_PREVIEW_BTN_ANNOTATE } from '../../constants';

const IMAGE_NODE_NAME = 'img';
// Selector for image container OR multi-image container
const ANNOTATED_ELEMENT_SELECTOR = '.bp-image, .bp-images-wrapper';

@autobind
class ImageAnnotator extends Annotator {

Expand All @@ -19,7 +23,7 @@ class ImageAnnotator extends Annotator {
* @return {HTMLElement} Annotated element in the viewer
*/
getAnnotatedEl(containerEl) {
return containerEl.querySelector('.bp-image');
return containerEl.querySelector(ANNOTATED_ELEMENT_SELECTOR);
}

/**
Expand All @@ -36,27 +40,21 @@ class ImageAnnotator extends Annotator {
let location = null;

// Get image tag inside viewer
const imageEl = this.annotatedElement.querySelector('img');
if (!imageEl) {
const imageEl = event.target;
if (imageEl.nodeName.toLowerCase() !== IMAGE_NODE_NAME) {
return location;
}

// If click is inside an annotation dialog, ignore
const dataType = annotatorUtil.findClosestDataType(this.annotatedElement);
if (dataType === 'annotation-dialog' || dataType === 'annotation-indicator') {
// If no image page was selected, ignore, as all images have a page number.
const { page } = annotatorUtil.getPageElAndPageNumber(imageEl);
if (!page) {
return location;
}

// Location based only on image position
const imageDimensions = imageEl.getBoundingClientRect();
let [x, y] = [(event.clientX - imageDimensions.left), (event.clientY - imageDimensions.top)];

// If click isn't in image area, ignore
if (event.clientX > imageDimensions.right || event.clientX < imageDimensions.left ||
event.clientY > imageDimensions.bottom || event.clientY < imageDimensions.top) {
return location;
}

// Scale location coordinates according to natural image size
const scale = annotatorUtil.getScale(this.annotatedElement);
const rotation = Number(imageEl.getAttribute('data-rotation-angle'));
Expand All @@ -69,7 +67,13 @@ class ImageAnnotator extends Annotator {
y: imageDimensions.height / scale
};

location = { x, y, imageEl, dimensions };
location = {
x,
y,
imageEl,
dimensions,
page
};

return location;
}
Expand Down Expand Up @@ -155,17 +159,19 @@ class ImageAnnotator extends Annotator {
// Only show/hide point annotation button if user has the appropriate
// permissions
if (this.annotationService.canAnnotate) {
// Hide create annotations button if image is rotated
// TODO(@spramod) actually adjust getLocationFromEvent method in
// annotator to get correct location rather than disabling the creation
// of annotations on rotated images
const annotateButton = document.querySelector(SELECTOR_BOX_PREVIEW_BTN_ANNOTATE);

if (rotationAngle !== 0) {
annotatorUtil.hideElement(annotateButton);
} else {
annotatorUtil.showElement(annotateButton);
}
return;
}

// Hide create annotations button if image is rotated
// TODO(@spramod) actually adjust getLocationFromEvent method in
// annotator to get correct location rather than disabling the creation
// of annotations on rotated images
const annotateButton = document.querySelector(SELECTOR_BOX_PREVIEW_BTN_ANNOTATE);

if (rotationAngle !== 0) {
annotatorUtil.hideElement(annotateButton);
} else {
annotatorUtil.showElement(annotateButton);
}
}
}
Expand Down
7 changes: 2 additions & 5 deletions src/lib/annotations/image/ImagePointDialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,8 @@ class ImagePointDialog extends AnnotationDialog {
const dialogDimensions = this.element.getBoundingClientRect();
const dialogWidth = dialogDimensions.width;

// Get image tag inside viewer
const imageEl = this.annotatedElement.querySelector('img');
if (!imageEl) {
return;
}
// Get image tag inside viewer, based on page number. All images are page 1 by default.
const imageEl = this.annotatedElement.querySelector(`[data-page-number="${this.location.page || 1}"]`) || this.annotatedElement.querySelector('img');

// Center middle of dialog with point - this coordinate is with respect to the page
let dialogLeftX = browserX - (dialogWidth / 2);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<div class="bp-image annotated-element">
<img width="100px" height="200px">
<img class="page" width="100px" height="200px" data-page-number="1">
<button class="bp-point-annotation-btn"></button>
</div>
34 changes: 8 additions & 26 deletions src/lib/annotations/image/__tests__/ImageAnnotator-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,30 +41,10 @@ describe('lib/annotations/image/ImageAnnotator', () => {
describe('getLocationFromEvent()', () => {
it('should not return a location if image isn\'t inside viewer', () => {
annotator.annotatedElement = document.createElement('div');
const location = annotator.getLocationFromEvent({});
expect(location).to.be.null;
});

it('should not return a location if click is on dialog', () => {
sandbox.stub(annotatorUtil, 'findClosestDataType').returns('annotation-dialog');

const location = annotator.getLocationFromEvent({});
expect(location).to.be.null;
});

it('should not return a location if click is on annotation indicator', () => {
sandbox.stub(annotatorUtil, 'findClosestDataType').returns('annotation-indicator');

const location = annotator.getLocationFromEvent({});
expect(location).to.be.null;
});

it('should not return a location if click isn\'t in image area', () => {
sandbox.stub(annotatorUtil, 'findClosestDataType').returns('not-a-dialog');

const location = annotator.getLocationFromEvent({
clientX: -1,
clientY: -1
target: {
nodeName: 'not-annotated'
}
});
expect(location).to.be.null;
});
Expand All @@ -77,16 +57,18 @@ describe('lib/annotations/image/ImageAnnotator', () => {
y: 200
};
const imageEl = annotator.annotatedElement.querySelector('img');
sandbox.stub(annotatorUtil, 'findClosestDataType').returns('not-a-dialog');
sandbox.stub(annotatorUtil, 'getScale').returns(1);
sandbox.stub(imageAnnotatorUtil, 'getLocationWithoutRotation').returns([x, y]);

const location = annotator.getLocationFromEvent({});
const location = annotator.getLocationFromEvent({
target: imageEl
});
expect(location).to.deep.equal({
x,
y,
imageEl,
dimensions
dimensions,
page: 1
});
});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<div class="annotated-element">
<img width="400px" height="600px">
<div class="annotated-element bp-annotated">
<img width="400px" height="600px" data-page-number="1">
<div data-type="annotation-dialog" class="bp-annotation-dialog">
<div class="bp-annotation-caret"></div>
</div>
</div>
</div>
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<div class="mock-header-bar" height="30px" width="100px"></div>
<div class="annotated-element">
<img width="100px" height="200px">
<div class="annotated-element bp-annotated">
<img width="100px" height="200px" data-page-number="1">
<button class="bp-point-annotation-btn"></button>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,8 @@ describe('lib/annotations/image/imageAnnotatorUtil', () => {
dimensions: {
x: 100,
y: 200
}
},
page: 1
};
const annotatedEl = document.querySelector('.annotated-element');
const coordinates = getBrowserCoordinatesFromLocation(location, annotatedEl);
Expand Down
Loading

0 comments on commit 93a5ba2

Please sign in to comment.