Skip to content

C
coopwire-hypothesis
Commit 845eef5f authored by Robert Knight's avatar Robert Knight Committed by GitHub
Browse files
Options

Merge pull request #493 from hypothesis/adder-positioning-fixes 

Fix adder position when document or body position is offset.
parents 921cad75 fded65c6
Hide whitespace changes
Inline Side-by-side
Showing with 158 additions and 39 deletions
src/annotator/adder.js
View file @ 845eef5f
...@@ -9,6 +9,13 @@ var ANNOTATE_BTN_SELECTOR = '.js-annotate-btn'; ...@@ -9,6 +9,13 @@ var ANNOTATE_BTN_SELECTOR = '.js-annotate-btn';
var HIGHLIGHT_BTN_SELECTOR = '.js-highlight-btn'; var HIGHLIGHT_BTN_SELECTOR = '.js-highlight-btn';
/**
* @typedef Target
* @prop {number} left - Offset from left edge of viewport.
* @prop {number} top - Offset from top edge of viewport.
* @prop {number} arrowDirection - Direction of the adder's arrow.
*/
/** /**
* Show the adder above the selection with an arrow pointing down at the * Show the adder above the selection with an arrow pointing down at the
* selected text. * selected text.
...@@ -43,6 +50,25 @@ function attachShadow(element) { ...@@ -43,6 +50,25 @@ function attachShadow(element) {
} }
} }
/**
* Return the closest ancestor of `el` which has been positioned.
*
* If no ancestor has been positioned, returns the root element.
*
* @param {Element} el
* @return {Element}
*/
function nearestPositionedAncestor(el) {
var parentEl = el.parentElement;
while (parentEl.parentElement) {
if (getComputedStyle(parentEl).position !== 'static') {
break;
}
parentEl = parentEl.parentElement;
}
return parentEl;
}
/** /**
* Create the DOM structure for the Adder. * Create the DOM structure for the Adder.
* *
...@@ -162,11 +188,12 @@ function Adder(container, options) { ...@@ -162,11 +188,12 @@ function Adder(container, options) {
* Return the best position to show the adder in order to target the * Return the best position to show the adder in order to target the
* selected text in `targetRect`. * selected text in `targetRect`.
* *
* @param {Rect} targetRect - The rect of text to target, in document * @param {Rect} targetRect - The rect of text to target, in viewport
* coordinates. * coordinates.
* @param {boolean} isSelectionBackwards - True if the selection was made * @param {boolean} isSelectionBackwards - True if the selection was made
* backwards, such that the focus point is mosty likely at the top-left * backwards, such that the focus point is mosty likely at the top-left
* edge of `targetRect`. * edge of `targetRect`.
* @return {Target}
*/ */
this.target = function (targetRect, isSelectionBackwards) { this.target = function (targetRect, isSelectionBackwards) {
// Set the initial arrow direction based on whether the selection was made // Set the initial arrow direction based on whether the selection was made
...@@ -191,13 +218,10 @@ function Adder(container, options) { ...@@ -191,13 +218,10 @@ function Adder(container, options) {
// Flip arrow direction if adder would appear above the top or below the // Flip arrow direction if adder would appear above the top or below the
// bottom of the viewport. // bottom of the viewport.
// if (targetRect.top - height() < 0 &&
// Note: `pageYOffset` is used instead of `scrollY` here for IE
// compatibility
if (targetRect.top - height() < view.pageYOffset &&
arrowDirection === ARROW_POINTING_DOWN) { arrowDirection === ARROW_POINTING_DOWN) {
arrowDirection = ARROW_POINTING_UP; arrowDirection = ARROW_POINTING_UP;
} else if (targetRect.top + height() > view.pageYOffset + view.innerHeight) { } else if (targetRect.top + height() > view.innerHeight) {
arrowDirection = ARROW_POINTING_DOWN; arrowDirection = ARROW_POINTING_DOWN;
} }
...@@ -208,18 +232,21 @@ function Adder(container, options) { ...@@ -208,18 +232,21 @@ function Adder(container, options) {
} }
// Constrain the adder to the viewport. // Constrain the adder to the viewport.
left = Math.max(left, view.pageXOffset); left = Math.max(left, 0);
left = Math.min(left, view.pageXOffset + view.innerWidth - width()); left = Math.min(left, view.innerWidth - width());
top = Math.max(top, view.pageYOffset); top = Math.max(top, 0);
top = Math.min(top, view.pageYOffset + view.innerHeight - height()); top = Math.min(top, view.innerHeight - height());
return {top: top, left: left, arrowDirection: arrowDirection}; return {top, left, arrowDirection};
}; };
/** /**
* Show the adder at the given position and with the arrow pointing in * Show the adder at the given position and with the arrow pointing in
* `arrowDirection`. * `arrowDirection`.
*
* @param {number} left - Horizontal offset from left edge of viewport.
* @param {number} top - Vertical offset from top edge of viewport.
*/ */
this.showAt = function (left, top, arrowDirection) { this.showAt = function (left, top, arrowDirection) {
self.element.className = classnames({ self.element.className = classnames({
...@@ -236,9 +263,18 @@ function Adder(container, options) { ...@@ -236,9 +263,18 @@ function Adder(container, options) {
self.element.querySelector(ANNOTATE_BTN_SELECTOR).style.display = ''; self.element.querySelector(ANNOTATE_BTN_SELECTOR).style.display = '';
self.element.querySelector(HIGHLIGHT_BTN_SELECTOR).style.display = ''; self.element.querySelector(HIGHLIGHT_BTN_SELECTOR).style.display = '';
// Translate the (left, top) viewport coordinates into positions relative to
// the adder's nearest positioned ancestor (NPA).
//
// Typically the adder is a child of the `<body>` and the NPA is the root
// `<html>` element. However page styling may make the `<body>` positioned.
// See https://github.com/hypothesis/client/issues/487.
var positionedAncestor = nearestPositionedAncestor(container);
var parentRect = positionedAncestor.getBoundingClientRect();
Object.assign(container.style, { Object.assign(container.style, {
top: toPx(top), top: toPx(top - parentRect.top),
left: toPx(left), left: toPx(left - parentRect.left),
}); });
self.element.style.visibility = 'visible'; self.element.style.visibility = 'visible';
......
src/annotator/range-util.js
View file @ 845eef5f
'use strict'; 'use strict';
function translate(rect, x, y) {
return {
left: rect.left + x,
top: rect.top + y,
width: rect.width,
height: rect.height,
};
}
function mapViewportRectToDocument(window, rect) {
// `pageXOffset` and `pageYOffset` are used rather than `scrollX`
// and `scrollY` for IE 10/11 compatibility.
return translate(rect, window.pageXOffset, window.pageYOffset);
}
/** /**
* Returns true if the start point of a selection occurs after the end point, * Returns true if the start point of a selection occurs after the end point,
* in document order. * in document order.
...@@ -79,7 +64,7 @@ function forEachNodeInRange(range, callback) { ...@@ -79,7 +64,7 @@ function forEachNodeInRange(range, callback) {
* Returns the bounding rectangles of non-whitespace text nodes in `range`. * Returns the bounding rectangles of non-whitespace text nodes in `range`.
* *
* @param {Range} range * @param {Range} range
* @return {Array<Rect>} Array of bounding rects in document coordinates. * @return {Array<Rect>} Array of bounding rects in viewport coordinates.
*/ */
function getTextBoundingBoxes(range) { function getTextBoundingBoxes(range) {
var whitespaceOnly = /^\s*$/; var whitespaceOnly = /^\s*$/;
...@@ -110,21 +95,19 @@ function getTextBoundingBoxes(range) { ...@@ -110,21 +95,19 @@ function getTextBoundingBoxes(range) {
// Measure the range and translate from viewport to document coordinates // Measure the range and translate from viewport to document coordinates
var viewportRects = Array.from(nodeRange.getClientRects()); var viewportRects = Array.from(nodeRange.getClientRects());
nodeRange.detach(); nodeRange.detach();
rects = rects.concat(viewportRects.map(function (rect) { rects = rects.concat(viewportRects);
return mapViewportRectToDocument(node.ownerDocument.defaultView, rect);
}));
}); });
return rects; return rects;
} }
/** /**
* Returns the rectangle, in document coordinates, for the line of text * Returns the rectangle, in viewport coordinates, for the line of text
* containing the focus point of a Selection. * containing the focus point of a Selection.
* *
* Returns null if the selection is empty. * Returns null if the selection is empty.
* *
* @param {Selection} selection * @param {Selection} selection
* @return {Rect?} * @return {Rect|null}
*/ */
function selectionFocusRect(selection) { function selectionFocusRect(selection) {
if (selection.isCollapsed) { if (selection.isCollapsed) {
......
src/annotator/test/adder-test.js
View file @ 845eef5f
...@@ -7,16 +7,36 @@ function rect(left, top, width, height) { ...@@ -7,16 +7,36 @@ function rect(left, top, width, height) {
return {left: left, top: top, width: width, height: height}; return {left: left, top: top, width: width, height: height};
} }
describe('adder', function () { /**
* Offset an `Element` from its default position.
*/
function offsetElement(el) {
el.style.position = 'relative';
el.style.left = '-200px';
el.style.top = '-200px';
}
/**
* Reset an element back to its default position.
*/
function revertOffsetElement(el) {
el.style.position = 'static';
el.style.left = '0';
el.style.top = '0';
}
describe('annotator.adder', function () {
var adderCtrl; var adderCtrl;
var adderCallbacks; var adderCallbacks;
var adderEl;
beforeEach(function () { beforeEach(function () {
adderCallbacks = { adderCallbacks = {
onAnnotate: sinon.stub(), onAnnotate: sinon.stub(),
onHighlight: sinon.stub(), onHighlight: sinon.stub(),
}; };
var adderEl = document.createElement('div'); adderEl = document.createElement('div');
document.body.appendChild(adderEl); document.body.appendChild(adderEl);
adderCtrl = new adder.Adder(adderEl, adderCallbacks); adderCtrl = new adder.Adder(adderEl, adderCallbacks);
...@@ -24,7 +44,7 @@ describe('adder', function () { ...@@ -24,7 +44,7 @@ describe('adder', function () {
afterEach(function () { afterEach(function () {
adderCtrl.hide(); adderCtrl.hide();
adderCtrl.element.parentNode.removeChild(adderCtrl.element); adderEl.remove();
}); });
function windowSize() { function windowSize() {
...@@ -111,4 +131,52 @@ describe('adder', function () { ...@@ -111,4 +131,52 @@ describe('adder', function () {
assert.isAtLeast(target.left, 0); assert.isAtLeast(target.left, 0);
}); });
}); });
describe('#showAt', () => {
context('when the document and body elements have no offset', () => {
it('shows adder at target position', () => {
adderCtrl.showAt(100, 100, adder.ARROW_POINTING_UP);
var { left, top } = adderEl.getBoundingClientRect();
assert.equal(left, 100);
assert.equal(top, 100);
});
});
context('when the body element is offset', () => {
beforeEach(() => {
offsetElement(document.body);
});
afterEach(() => {
revertOffsetElement(document.body);
});
it('shows adder at target position', () => {
adderCtrl.showAt(100, 100, adder.ARROW_POINTING_UP);
var { left, top } = adderEl.getBoundingClientRect();
assert.equal(left, 100);
assert.equal(top, 100);
});
});
context('when the document element is offset', () => {
beforeEach(() => {
offsetElement(document.documentElement);
});
afterEach(() => {
revertOffsetElement(document.documentElement);
});
it('shows adder at target position when document element is offset', () => {
adderCtrl.showAt(100, 100, adder.ARROW_POINTING_UP);
var { left, top } = adderEl.getBoundingClientRect();
assert.equal(left, 100);
assert.equal(top, 100);
});
});
});
}); });
src/annotator/test/range-util-test.js
View file @ 845eef5f
...@@ -9,7 +9,21 @@ function createRange(node, start, end) { ...@@ -9,7 +9,21 @@ function createRange(node, start, end) {
return range; return range;
} }
describe('range-util', function () { /**
* Round coordinates in `rect` to nearest integer values.
*/
function roundCoords(rect) {
return {
bottom: Math.round(rect.bottom),
height: Math.round(rect.height),
left: Math.round(rect.left),
right: Math.round(rect.right),
top: Math.round(rect.top),
width: Math.round(rect.width),
};
}
describe('annotator.range-util', function () {
var selection; var selection;
var testNode; var testNode;
...@@ -62,8 +76,26 @@ describe('range-util', function () { ...@@ -62,8 +76,26 @@ describe('range-util', function () {
it('gets the bounding box of a range containing a text node', function () { it('gets the bounding box of a range containing a text node', function () {
testNode.innerHTML = 'plain text'; testNode.innerHTML = 'plain text';
var rng = createRange(testNode, 0, 1); var rng = createRange(testNode, 0, 1);
var boxes = rangeUtil.getTextBoundingBoxes(rng); var boxes = rangeUtil.getTextBoundingBoxes(rng);
assert.ok(boxes.length);
assert.match(boxes, [sinon.match({
left: sinon.match.number,
top: sinon.match.number,
width: sinon.match.number,
height: sinon.match.number,
bottom: sinon.match.number,
right: sinon.match.number,
})]);
});
it('returns the bounding box in viewport coordinates', function () {
testNode.innerHTML = 'plain text';
var rng = createRange(testNode, 0, 1);
var [rect] = rangeUtil.getTextBoundingBoxes(rng);
assert.deepEqual(roundCoords(rect), roundCoords(testNode.getBoundingClientRect()));
}); });
}); });
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment