diff --git a/src/annotator/plugin/bucket-bar.js b/src/annotator/plugin/bucket-bar.js index e17e626013d..2945fbe8ae6 100644 --- a/src/annotator/plugin/bucket-bar.js +++ b/src/annotator/plugin/bucket-bar.js @@ -2,21 +2,12 @@ import Delegator from '../delegator'; import scrollIntoView from 'scroll-into-view'; import { setHighlightsFocused } from '../highlighter'; -import { - findClosestOffscreenAnchor, - constructPositionPoints, - buildBuckets, -} from '../util/buckets'; +import { findClosestOffscreenAnchor, anchorBuckets } from '../util/buckets'; /** * @typedef {import('../util/buckets').Bucket} Bucket - * @typedef {import('../util/buckets').PositionPoints} PositionPoints */ -const BUCKET_SIZE = 16; // Regular bucket size -const BUCKET_NAV_SIZE = BUCKET_SIZE + 6; // Bucket plus arrow (up/down) -const BUCKET_TOP_THRESHOLD = 115 + BUCKET_NAV_SIZE; // Toolbar - // Scroll to the next closest anchor off screen in the given direction. function scrollToClosest(anchors, direction) { const closest = findClosestOffscreenAnchor(anchors, direction); @@ -117,30 +108,7 @@ export default class BucketBar extends Delegator { } _update() { - /** @type {PositionPoints} */ - const { above, below, points } = constructPositionPoints( - this.annotator.anchors - ); - - this.buckets = buildBuckets(points); - - // Add a bucket to the top of the bar that, when clicked, will scroll up - // to the nearest bucket offscreen above, an upper navigation bucket - // TODO: This should be part of building the buckets - this.buckets.unshift( - { anchors: [], position: 0 }, - { anchors: above, position: BUCKET_TOP_THRESHOLD - 1 }, - { anchors: [], position: BUCKET_TOP_THRESHOLD } - ); - - // Add a bucket to the bottom of the bar that, when clicked, will scroll down - // to the nearest bucket offscreen below, a lower navigation bucket - // TODO: This should be part of building the buckets - this.buckets.push( - { anchors: [], position: window.innerHeight - BUCKET_NAV_SIZE }, - { anchors: below, position: window.innerHeight - BUCKET_NAV_SIZE + 1 }, - { anchors: [], position: window.innerHeight } - ); + this.buckets = anchorBuckets(this.annotator.anchors); // The following affordances attempt to reuse existing DOM elements // when reconstructing bucket "tabs" to cut down on the number of elements @@ -205,17 +173,9 @@ export default class BucketBar extends Delegator { _buildTabs() { this.tabs.forEach((tabEl, index) => { - let bucketHeight; const anchorCount = this.buckets[index].anchors.length; - // Positioning logic currently _relies_ on their being interstitial - // buckets that have no anchors but do have positions. Positioning - // is averaged between this bucket's position and the _next_ bucket's - // position. For now. TODO: Fix this - const pos = - (this.buckets[index].position + this.buckets[index + 1]?.position) / 2; - tabEl.className = 'annotator-bucket-indicator'; - tabEl.style.top = `${pos}px`; + tabEl.style.top = `${this.buckets[index].position}px`; tabEl.style.display = ''; if (anchorCount) { @@ -229,34 +189,20 @@ export default class BucketBar extends Delegator { tabEl.style.display = 'none'; } - if (this.isNavigationBucket(index)) { - bucketHeight = BUCKET_NAV_SIZE; - tabEl.classList.toggle('upper', this.isUpper(index)); - tabEl.classList.toggle('lower', this.isLower(index)); - } else { - bucketHeight = BUCKET_SIZE; - tabEl.classList.remove('upper'); - tabEl.classList.remove('lower'); - } - - tabEl.style.marginTop = (-1 * bucketHeight) / 2 + 'px'; + tabEl.classList.toggle('upper', this.isUpper(index)); + tabEl.classList.toggle('lower', this.isLower(index)); }); } isUpper(i) { - return i === 1; + return i === 0; } isLower(i) { - return i === this.buckets.length - 2; + return i === this.buckets.length - 1; } isNavigationBucket(i) { return this.isUpper(i) || this.isLower(i); } } - -// Export constants -BucketBar.BUCKET_SIZE = BUCKET_SIZE; -BucketBar.BUCKET_NAV_SIZE = BUCKET_NAV_SIZE; -BucketBar.BUCKET_TOP_THRESHOLD = BUCKET_TOP_THRESHOLD; diff --git a/src/annotator/plugin/test/bucket-bar-test.js b/src/annotator/plugin/test/bucket-bar-test.js index de41998d23a..88528dafc12 100644 --- a/src/annotator/plugin/test/bucket-bar-test.js +++ b/src/annotator/plugin/test/bucket-bar-test.js @@ -46,11 +46,8 @@ describe('BucketBar', () => { }; fakeBucketUtil = { + anchorBuckets: sinon.stub().returns([]), findClosestOffscreenAnchor: sinon.stub(), - constructPositionPoints: sinon - .stub() - .returns({ above: [], below: [], points: [] }), - buildBuckets: sinon.stub().returns([]), }; fakeHighlighter = { @@ -105,16 +102,16 @@ describe('BucketBar', () => { describe('updating buckets', () => { it('should update buckets when the window is resized', () => { bucketBar = createBucketBar(); - assert.notCalled(fakeBucketUtil.buildBuckets); + assert.notCalled(fakeBucketUtil.anchorBuckets); window.dispatchEvent(new Event('resize')); - assert.calledOnce(fakeBucketUtil.buildBuckets); + assert.calledOnce(fakeBucketUtil.anchorBuckets); }); it('should update buckets when the window is scrolled', () => { bucketBar = createBucketBar(); - assert.notCalled(fakeBucketUtil.buildBuckets); + assert.notCalled(fakeBucketUtil.anchorBuckets); window.dispatchEvent(new Event('scroll')); - assert.calledOnce(fakeBucketUtil.buildBuckets); + assert.calledOnce(fakeBucketUtil.anchorBuckets); }); context('when scrollables provided', () => { @@ -144,11 +141,11 @@ describe('BucketBar', () => { bucketBar = createBucketBar({ scrollables: ['.scrollable-1', '.scrollable-2'], }); - assert.notCalled(fakeBucketUtil.buildBuckets); + assert.notCalled(fakeBucketUtil.anchorBuckets); scrollableEls[0].dispatchEvent(new Event('scroll')); - assert.calledOnce(fakeBucketUtil.buildBuckets); + assert.calledOnce(fakeBucketUtil.anchorBuckets); scrollableEls[1].dispatchEvent(new Event('scroll')); - assert.calledTwice(fakeBucketUtil.buildBuckets); + assert.calledTwice(fakeBucketUtil.anchorBuckets); }); }); @@ -165,8 +162,10 @@ describe('BucketBar', () => { // Create fake anchors and render buckets. const anchors = [createAnchor()]; - fakeBucketUtil.buildBuckets.returns([ + fakeBucketUtil.anchorBuckets.returns([ + { anchors: [], position: 137 }, // Upper navigation { anchors: [anchors[0]], position: 250 }, + { anchors: [], position: 400 }, // Lower navigation ]); bucketBar.annotator.anchors = anchors; @@ -256,21 +255,17 @@ describe('BucketBar', () => { ]; // These two anchors are considered to be offscreen upwards fakeAbove = [fakeAnchors[0], fakeAnchors[1]]; + fakeBelow = [fakeAnchors[5]]; // These buckets are on-screen fakeBuckets = [ + { anchors: fakeAbove, position: 137 }, { anchors: [fakeAnchors[2], fakeAnchors[3]], position: 350 }, - { anchors: [], position: 450 }, // This is an empty bucket { anchors: [fakeAnchors[4]], position: 550 }, + { anchors: fakeBelow, position: 600 }, ]; // This anchor is offscreen below - fakeBelow = [fakeAnchors[5]]; - fakeBucketUtil.constructPositionPoints.returns({ - above: fakeAbove, - below: fakeBelow, - points: [], - }); - fakeBucketUtil.buildBuckets.returns(fakeBuckets.slice()); + fakeBucketUtil.anchorBuckets.returns(fakeBuckets.slice()); }); describe('navigation bucket tabs', () => { @@ -302,7 +297,7 @@ describe('BucketBar', () => { // Resetting this return is necessary to return a fresh array reference // on next update - fakeBucketUtil.buildBuckets.returns(fakeBuckets.slice()); + fakeBucketUtil.anchorBuckets.returns(fakeBuckets.slice()); bucketBar.update(); assert.equal(bucketBar.tabs.length, bucketBar.buckets.length); assert.notExists(bucketBar.element.querySelector('.extraTab')); @@ -370,12 +365,7 @@ describe('BucketBar', () => { }); it('does not display empty bucket tabs', () => { - fakeBucketUtil.buildBuckets.returns([]); - fakeBucketUtil.constructPositionPoints.returns({ - above: [], - below: [], - points: [], - }); + fakeBucketUtil.anchorBuckets.returns([]); bucketBar.update(); const allBuckets = bucketBar.element.querySelectorAll( diff --git a/src/annotator/util/buckets.js b/src/annotator/util/buckets.js index ff608af86a6..defc6174992 100644 --- a/src/annotator/util/buckets.js +++ b/src/annotator/util/buckets.js @@ -5,36 +5,43 @@ import { getBoundingClientRect } from '../highlighter'; */ /** - * A tuple representing either the top (`startOrEnd` = 1) or the - * bottom (`startOrEnd` = -1) of an anchor's highlight bounding box. - * - * @typedef {[pixelPosition: number, startOrEnd: (-1 | 1), anchor: Anchor]} PositionPoint + * @typedef Bucket + * @prop {Anchor[]} anchors - The anchors in this bucket + * @prop {number} position - The vertical pixel offset where this bucket should + * appear in the bucket bar. */ /** - * An object containing information about anchor highlight positions - * - * @typedef PositionPoints - * @prop {Anchor[]} above - Anchors that are offscreen above - * @prop {Anchor[]} below - Anchors that are offscreen below - * @prop {PositionPoint[]} points - PositionPoints for on-screen anchor - * highlights. Each highlight box has 2 PositionPoints (one for top edge - * and one for bottom edge). + * @typedef WorkingBucket + * @prop {Anchor[]} anchors - The anchors in this bucket + * @prop {number} position - The computed position (offset) for this bucket, + * based on the current anchors. This is centered between `top` and `bottom` + * @prop {number} top - The uppermost (lowest) vertical offset for the anchors + * in this bucket — the lowest `top` position value, akin to the top offest of + * a theoretical box drawn around all of the anchor highlights in this bucket + * @prop {number} bottom - The bottommost (highest) vertical offset for the + * anchors in this bucket — the highest `top` position value, akin to the + * bottom of a theoretical box drawn around all of the anchor highlights in + * this bucket */ /** - * @typedef Bucket - * @prop {Anchor[]} anchors - The anchors in this bucket - * @prop {number} position - The vertical pixel offset where this bucket should - * appear in the bucket bar + * @typedef AnchorPosition + * @prop {Anchor} anchor + * @prop {number} top - The vertical offset, in pixels, of the top of this + * anchor's highlight(s) bounding box + * @prop {number} bottom - The vertical offset, in pixels, of the bottom of this + * anchor's highlight(s) bounding box */ -// FIXME: Temporary duplication of size constants between here and BucketBar -const BUCKET_SIZE = 16; // Regular bucket size -const BUCKET_NAV_SIZE = BUCKET_SIZE + 6; // Bucket plus arrow (up/down) -const BUCKET_TOP_THRESHOLD = 115 + BUCKET_NAV_SIZE; // Toolbar - -// TODO!! This is an option in the plugin right now +// Only anchors with top offsets between `BUCKET_TOP_THRESHOLD` and +// `BUCKET_BOTTOM_THRESHOLD` are considered "on-screen" and will be bucketed. +// This is to account for bucket-bar tool buttons (top) and the height of +// the bottom navigation bucket (bottom) +const BUCKET_TOP_THRESHOLD = 137; +const BUCKET_BOTTOM_THRESHOLD = 22; +// Generated buckets of annotation anchor highlights should be spaced by +// at least this amount, in pixels const BUCKET_GAP_SIZE = 60; /** @@ -63,7 +70,7 @@ export function findClosestOffscreenAnchor(anchors, direction) { continue; } else if ( direction === 'down' && - top <= window.innerHeight - BUCKET_NAV_SIZE + top <= window.innerHeight - BUCKET_BOTTOM_THRESHOLD ) { // We're headed down but this anchor is already above // the usable bottom of the screen: it's not our guy @@ -88,161 +95,132 @@ export function findClosestOffscreenAnchor(anchors, direction) { } /** - * Construct an Array of points representing the positional tops and bottoms - * of current anchor highlights. Each anchor whose highlight(s)' bounding - * box is onscreen will result in two entries in the `points` Array: one - * for the top of the highlight box and one for the bottom + * Compute the AnchorPositions for the set of anchors provided, sorted + * by top offset * * @param {Anchor[]} anchors - * @return {PositionPoints} + * @return {AnchorPosition[]} */ -export function constructPositionPoints(anchors) { - const aboveScreenAnchors = new Set(); - const belowScreenAnchors = new Set(); - const points = /** @type {PositionPoint[]} */ ([]); +function getAnchorPositions(anchors) { + const anchorPositions = []; - for (let anchor of anchors) { + anchors.forEach(anchor => { if (!anchor.highlights?.length) { - continue; + return; } + const anchorBox = getBoundingClientRect(anchor.highlights); + anchorPositions.push({ + top: anchorBox.top, + bottom: anchorBox.bottom, + anchor, + }); + }); - const rect = getBoundingClientRect(anchor.highlights); - - if (rect.top < BUCKET_TOP_THRESHOLD) { - aboveScreenAnchors.add(anchor); - } else if (rect.top > window.innerHeight - BUCKET_NAV_SIZE) { - belowScreenAnchors.add(anchor); - } else { - // Add a point for the top of this anchor's highlight box - points.push([rect.top, 1, anchor]); - // Add a point for the bottom of this anchor's highlight box - points.push([rect.bottom, -1, anchor]); - } - } - - // Sort onscreen points by pixel position, secondarily by position "type" - // (top or bottom of higlight box) - points.sort((a, b) => { - for (let i = 0; i < a.length; i++) { - if (a[i] < b[i]) { - return -1; - } else if (a[i] > b[i]) { - return 1; - } + // Now sort by top position + anchorPositions.sort((a, b) => { + if (a.top < b.top) { + return -1; } - return 0; + return 1; }); - return { - above: Array.from(aboveScreenAnchors), - below: Array.from(belowScreenAnchors), - points, - }; + return anchorPositions; } /** - * Take a sorted set of `points` representing top and bottom positions of anchor - * highlights and group them into a collection of "buckets". + * Compute buckets * - * @param {PositionPoint[]} points + * @param {Anchor[]} anchors * @return {Bucket[]} */ -export function buildBuckets(points) { +export function anchorBuckets(anchors) { + const anchorPositions = getAnchorPositions(anchors); + const aboveScreen = new Set(); + const belowScreen = new Set(); const buckets = /** @type {Bucket[]} */ ([]); - // Anchors that are part of the currently-being-built bucket, and a correspon- - // ding count of unclosed top edges seen for that anchor - const current = /** @type {{anchors: Anchor[], counts: number[] }} */ ({ - anchors: [], - counts: [], - }); + // Hold current working anchors and positions as we build each bucket + /** @type {WorkingBucket|null} */ + let currentBucket = null; + + /** + * Create a new working bucket based on the provided `AnchorPosition` + * + * @param {AnchorPosition} anchorPosition + * @return {WorkingBucket} + */ + function _newBucket(anchorPosition) { + const anchorHeight = anchorPosition.bottom - anchorPosition.top; + const bucketPosition = anchorPosition.top + anchorHeight / 2; + const newBucket = /** @type WorkingBucket */ ({ + anchors: [anchorPosition.anchor], + top: anchorPosition.top, + bottom: anchorPosition.bottom, + position: bucketPosition, + }); + return newBucket; + } - points.forEach((point, index) => { - const [position, delta, anchor] = point; - // Does this point represent the top or the bottom of an anchor's highlight - // box? - const positionType = delta > 0 ? 'start' : 'end'; - - // See if this point's anchor is already in our working set of open anchors - const anchorIndex = current.anchors.indexOf(anchor); - - if (positionType === 'start') { - if (anchorIndex === -1) { - // Add an entry for this anchor to our current set of "open" anchors - current.anchors.unshift(anchor); - current.counts.unshift(1); - } else { - // Increment the number of times we've seen a start/top edge for this - // anchor - current.counts[anchorIndex]++; - } - } else { - // positionType = 'end' - // This is the bottom/end of an anchor that we should have already seen - // a top edge for. Decrement the count, representing that we've found an - // end point to balance a previously-seen start point - current.counts[anchorIndex]--; - if (current.counts[anchorIndex] === 0) { - // All start points for this anchor have been balanced by end point(s) - // So we can remove this anchor from our collection of open anchors - current.anchors.splice(anchorIndex, 1); - current.counts.splice(anchorIndex, 1); - } + // Build buckets from position information + anchorPositions.forEach(aPos => { + if (aPos.top < BUCKET_TOP_THRESHOLD) { + aboveScreen.add(aPos.anchor); + return; + } else if (aPos.top > window.innerHeight - BUCKET_BOTTOM_THRESHOLD) { + belowScreen.add(aPos.anchor); + return; } - // For each point, we'll either: - // * create a new bucket: Add a new bucket (w/corresponding bucket position) - // and add the working anchors to the new bucket. This, of course, has - // the effect of making the buckets collection larger. OR: - // * merge buckets: In most cases, merge the anchors from the last bucket - // into the penultimate (previous) bucket and remove the last bucket (and - // its corresponding `bucketPosition` entry). Also add the working anchors - // to the previous bucket. Note that this decreases the size of the - // buckets collection. - // The ultimate set of buckets is defined by the pattern of creating and - // merging/removing buckets as we iterate over points. - const isFirstOrLastPoint = - buckets.length === 0 || index === points.length - 1; - - const isLargeGap = - buckets.length && - position - buckets[buckets.length - 1].position > BUCKET_GAP_SIZE; - - if (current.anchors.length === 0 || isFirstOrLastPoint || isLargeGap) { - // Create a new bucket, because: - // - There are no more open/working anchors, OR - // - This is the first or last point, OR - // - There's been a large dimensional gap since the last bucket's position - buckets.push({ anchors: current.anchors.slice(), position }); + if (!currentBucket) { + // We've encountered our first on-screen anchor position: + // We'll need a bucket! + currentBucket = _newBucket(aPos); + return; + } + // We want to contain overlapping highlights and those near each other + // within a shared bucket + const isContainedWithin = + aPos.top > currentBucket.top && aPos.bottom < currentBucket.bottom; + + // The new anchor's position is far enough below the bottom of the current + // bucket to justify starting a new bucket + const isLargeGap = aPos.top - currentBucket.bottom > BUCKET_GAP_SIZE; + + if (isLargeGap && !isContainedWithin) { + // We need to start a new bucket; push the working bucket and create + // a new bucket + buckets.push(currentBucket); + currentBucket = _newBucket(aPos); } else { - // Merge bucket contents - - // Always remove the last bucket - const ultimateBucket = buckets.pop() || /** @type Bucket */ ({}); - - // Merge working anchors into the last bucket's anchors - let mergedAnchors = [...ultimateBucket.anchors, ...current.anchors]; - let mergedPosition = ultimateBucket.position; - - // If there is a previous bucket (penultimate bucket) and it has anchors - // in it (is not empty) - if (buckets[buckets.length - 1]?.anchors.length) { - // Remove the previous bucket, too - const penultimateBucket = /** @type Bucket */ (buckets.pop() || {}); - // Merge the penultimate bucket's anchors into our working set of - // merged anchors - mergedAnchors = [...penultimateBucket.anchors, ...mergedAnchors]; - // We'll use the penultimate bucket's position as the position for - // the merged bucket - mergedPosition = penultimateBucket.position; - } - - // Push the now-merged bucket onto the buckets collection - buckets.push({ - anchors: Array.from(new Set(mergedAnchors)), // De-dupe anchors - position: mergedPosition, - }); + // We'll add this anchor to the current working bucket and update + // offset properties accordingly + const updatedTop = + aPos.top < currentBucket.top ? aPos.top : currentBucket.top; + const updatedBottom = + aPos.bottom > currentBucket.bottom ? aPos.bottom : currentBucket.bottom; + const updatedHeight = updatedBottom - updatedTop; + + currentBucket.anchors.push(aPos.anchor); + currentBucket.top = updatedTop; + currentBucket.bottom = updatedBottom; + currentBucket.position = updatedTop + updatedHeight / 2; } }); + + if (currentBucket) { + buckets.push(currentBucket); + } + + // Add an upper "navigation" bucket with offscreen-above anchors + buckets.unshift({ + anchors: Array.from(aboveScreen), + position: BUCKET_TOP_THRESHOLD, + }); + + // Add a lower "navigation" bucket with offscreen-below anchors + buckets.push({ + anchors: Array.from(belowScreen), + position: window.innerHeight - BUCKET_BOTTOM_THRESHOLD, + }); return buckets; } diff --git a/src/annotator/util/test/buckets-test.js b/src/annotator/util/test/buckets-test.js index 93361f2798a..2c7eb24a584 100644 --- a/src/annotator/util/test/buckets-test.js +++ b/src/annotator/util/test/buckets-test.js @@ -1,29 +1,48 @@ -import { - findClosestOffscreenAnchor, - constructPositionPoints, - buildBuckets, -} from '../buckets'; +import { findClosestOffscreenAnchor, anchorBuckets } from '../buckets'; import { $imports } from '../buckets'; -function fakeAnchorFactory() { - let highlightIndex = 0; +function fakeAnchorFactory( + offsetStart = 1, + offsetIncrement = 100, + boxHeight = 50 +) { + let highlightIndex = offsetStart; return () => { - // This incrementing array-item value allows for differing - // `top` results; see fakeGetBoundingClientRect - return { highlights: [highlightIndex++] }; + // In a normal `Anchor` object, `highlights` would be an array of + // DOM elements. Here, `highlights[0]` is the vertical offset (top) of the + // fake anchor's highlight box and `highlights[1]` is the height of the + // box. This is in used in conjunction with the mock for + // `getBoundingClientRect`, below + const anchor = { highlights: [highlightIndex, boxHeight] }; + highlightIndex = highlightIndex + offsetIncrement; + return anchor; }; } describe('annotator/util/buckets', () => { let fakeGetBoundingClientRect; + let fakeAnchors; + let stubbedInnerHeight; + beforeEach(() => { + const fakeAnchor = fakeAnchorFactory(); + fakeAnchors = [ + fakeAnchor(), // top: 1, bottom: 51 — above screen + fakeAnchor(), // top: 101, bottom: 151 — above screen + fakeAnchor(), // top: 201, bottom: 251 — on screen + fakeAnchor(), // top: 301, bottom: 351 — on screen + fakeAnchor(), // top: 401, bottom: 451 — below screen + fakeAnchor(), // top: 501, bottom: 551 - below screen + ]; + stubbedInnerHeight = sinon.stub(window, 'innerHeight').value(410); + fakeGetBoundingClientRect = sinon.stub().callsFake(highlights => { - // Return a `top` value based on the first item in the array - const top = highlights[0] * 100 + 1; + // Use the entries of the faked anchor's `highlights` array to + // determine this anchor's "position" return { - top, - bottom: top + 50, + top: highlights[0], + bottom: highlights[0] + highlights[1], }; }); @@ -36,29 +55,10 @@ describe('annotator/util/buckets', () => { afterEach(() => { $imports.$restore(); + stubbedInnerHeight.restore(); }); describe('findClosestOffscreenAnchor', () => { - let fakeAnchors; - let stubbedInnerHeight; - - beforeEach(() => { - const fakeAnchor = fakeAnchorFactory(); - fakeAnchors = [ - fakeAnchor(), // top: 1 - fakeAnchor(), // top: 101 - fakeAnchor(), // top: 201 - fakeAnchor(), // top: 301 - fakeAnchor(), // top: 401 - fakeAnchor(), // top: 501 - ]; - stubbedInnerHeight = sinon.stub(window, 'innerHeight').value(410); - }); - - afterEach(() => { - stubbedInnerHeight.restore(); - }); - it('finds the closest anchor above screen when headed up', () => { // fakeAnchors [0] and [1] are offscreen upwards, having `top` values // < BUCKET_TOP_THRESHOLD. [1] is closer so wins out. [3] and [4] are @@ -113,163 +113,87 @@ describe('annotator/util/buckets', () => { }); }); - describe('constructPositionPoints', () => { - let fakeAnchors; - let stubbedInnerHeight; - - beforeEach(() => { - const fakeAnchor = fakeAnchorFactory(); - fakeAnchors = [ - fakeAnchor(), // top: 1 - fakeAnchor(), // top: 101 - fakeAnchor(), // top: 201 - fakeAnchor(), // top: 301 - fakeAnchor(), // top: 401 - fakeAnchor(), // top: 501 - ]; - stubbedInnerHeight = sinon.stub(window, 'innerHeight').value(410); - }); - - afterEach(() => { - stubbedInnerHeight.restore(); - }); - - it('returns an Array of anchors that are offscreen above', () => { - const positionPoints = constructPositionPoints(fakeAnchors); - - assert.deepEqual(positionPoints.above, [fakeAnchors[0], fakeAnchors[1]]); - }); - - it('returns an Array of anchors that are offscreen below', () => { - const positionPoints = constructPositionPoints(fakeAnchors); - - assert.deepEqual(positionPoints.below, [fakeAnchors[4], fakeAnchors[5]]); + describe('anchorBuckets', () => { + it('puts anchors that are above the screen into the first bucket', () => { + const buckets = anchorBuckets(fakeAnchors); + assert.deepEqual(buckets[0].anchors, [fakeAnchors[0], fakeAnchors[1]]); }); - it('does not return duplicate anchors', () => { - const positionPoints = constructPositionPoints([ - fakeAnchors[0], - fakeAnchors[0], - fakeAnchors[5], + it('puts anchors that are below the screen into the last bucket', () => { + const buckets = anchorBuckets(fakeAnchors); + assert.deepEqual(buckets[buckets.length - 1].anchors, [ + fakeAnchors[4], fakeAnchors[5], ]); - - assert.deepEqual(positionPoints.above, [fakeAnchors[0]]); - assert.deepEqual(positionPoints.below, [fakeAnchors[5]]); }); - it('returns an Array of position points for on-screen anchors', () => { - const positionPoints = constructPositionPoints(fakeAnchors); - - // It should return two "point" positions for each on-screen anchor, - // one representing the top of the anchor's highlight box, one representing - // the bottom position - assert.equal(positionPoints.points.length, 4); - // The top position of the first on-screen anchor - assert.deepEqual(positionPoints.points[0], [201, 1, fakeAnchors[2]]); - // The bottom position of the first on-screen anchor - assert.deepEqual(positionPoints.points[1], [251, -1, fakeAnchors[2]]); - // The top position of the second on-screen anchor - assert.deepEqual(positionPoints.points[2], [301, 1, fakeAnchors[3]]); - // The bottom position of the second on-screen anchor - assert.deepEqual(positionPoints.points[3], [351, -1, fakeAnchors[3]]); + it('puts on-screen anchors into a bucket', () => { + const buckets = anchorBuckets(fakeAnchors); + assert.deepEqual(buckets[1].anchors, [fakeAnchors[2], fakeAnchors[3]]); }); - it('sorts on-screen points based on position primarily, type secondarily', () => { - fakeGetBoundingClientRect.callsFake(() => { - return { - top: 250, - bottom: 250, - }; - }); - const positionPoints = constructPositionPoints(fakeAnchors); - for (let i = 0; i < fakeAnchors.length; i++) { - // The bottom position for all of the fake anchors is the same, so - // those points will all be at the top of the list - assert.equal(positionPoints.points[i][2], fakeAnchors[i]); - // This point is a "bottom" point - assert.equal(positionPoints.points[i][1], -1); - // The top position for all of the fake anchors is the same, so - // they'll be sorted to the end of the list - assert.equal( - positionPoints.points[i + fakeAnchors.length][2], - fakeAnchors[i] - ); - // This point is a "top" point - assert.equal(positionPoints.points[i + fakeAnchors.length][1], 1); - } + it('puts anchors into separate buckets if more than 60px separates their boxes', () => { + fakeAnchors[2].highlights = [201, 15]; // bottom 216 + fakeAnchors[3].highlights = [301, 15]; // top 301 - more than 60px from 216 + const buckets = anchorBuckets(fakeAnchors); + assert.deepEqual(buckets[1].anchors, [fakeAnchors[2]]); + assert.deepEqual(buckets[2].anchors, [fakeAnchors[3]]); }); - }); - describe('buildBuckets', () => { - it('should return empty buckets if points array is empty', () => { - const buckets = buildBuckets([]); - assert.isArray(buckets); - assert.isEmpty(buckets); + it('puts overlapping anchors into a shared bucket', () => { + fakeAnchors[2].highlights = [201, 200]; // Bottom 401 + fakeAnchors[3].highlights = [285, 100]; // Bottom 385 + const buckets = anchorBuckets(fakeAnchors); + assert.deepEqual(buckets[1].anchors, [fakeAnchors[2], fakeAnchors[3]]); }); - it('should group overlapping anchor highlights into shared buckets', () => { - const anchors = [{}, {}, {}, {}]; - const points = []; - // Represents points for 4 anchors that all have a top of 150px and bottom - // of 200 - anchors.forEach(anchor => { - points.push([150, 1, anchor]); - points.push([200, -1, anchor]); - }); - - const buckets = buildBuckets(points); - assert.equal(buckets.length, 2); - assert.isEmpty(buckets[1].anchors); - // All anchors are in a single bucket - assert.deepEqual(buckets[0].anchors, anchors); - // Because this is the first bucket, it will be aligned top - assert.equal(buckets[0].position, 150); + it('positions the bucket at v. midpoint of the box containing all bucket anchors', () => { + fakeAnchors[2].highlights = [200, 50]; // Top 200 + fakeAnchors[3].highlights = [225, 75]; // Bottom 300 + const buckets = anchorBuckets(fakeAnchors); + assert.equal(buckets[1].position, 250); }); - it('should group nearby anchor highlights into shared buckets', () => { - let increment = 25; - const anchors = [{}, {}, {}, {}]; - const points = []; - // Represents points for 4 anchors that all have different start and - // end positions, but only differing by 25px - anchors.forEach(anchor => { - points.push([150 + increment, 1, anchor]); - points.push([200 + increment, -1, anchor]); - increment += 25; - }); - - const buckets = buildBuckets(points); + it('only buckets annotations that have highlights', () => { + const badAnchor = { highlights: [] }; + fakeAnchors.push(badAnchor); + const buckets = anchorBuckets([badAnchor]); assert.equal(buckets.length, 2); - assert.isEmpty(buckets[1].anchors); - // All anchors are in a single bucket - assert.deepEqual(buckets[0].anchors, anchors); - // Because this is the first bucket, it will be aligned top - assert.equal(buckets[0].position, 175); + assert.isEmpty(buckets[0].anchors); // Holder for above-screen anchors + assert.isEmpty(buckets[1].anchors); // Holder for below-screen anchors }); - it('should put anchors that are not near each other in separate buckets', () => { - let position = 100; - const anchors = [{}, {}, {}, {}]; - const points = []; - // Represents points for 4 anchors that all have different start and - // end positions, but only differing by 25px - anchors.forEach(anchor => { - points.push([position, 1, anchor]); - points.push([position + 20, -1, anchor]); - position += 100; - }); - const buckets = buildBuckets(points); - assert.equal(buckets.length, 8); - // Legacy of previous implementation, shrug? - assert.isEmpty(buckets[1].anchors); - assert.isEmpty(buckets[3].anchors); - assert.isEmpty(buckets[5].anchors); - assert.isEmpty(buckets[7].anchors); - assert.deepEqual(buckets[0].anchors, [anchors[0]]); - assert.deepEqual(buckets[2].anchors, [anchors[1]]); - assert.deepEqual(buckets[4].anchors, [anchors[2]]); - assert.deepEqual(buckets[6].anchors, [anchors[3]]); + it('handles sorting anchors for position', () => { + const buckets = anchorBuckets([ + fakeAnchors[3], + fakeAnchors[2], + fakeAnchors[5], + fakeAnchors[4], + fakeAnchors[0], + fakeAnchors[1], + ]); + assert.deepEqual(buckets[0].anchors, [fakeAnchors[0], fakeAnchors[1]]); + assert.deepEqual(buckets[1].anchors, [fakeAnchors[2], fakeAnchors[3]]); + assert.deepEqual(buckets[2].anchors, [fakeAnchors[4], fakeAnchors[5]]); + }); + + it('returns only above- and below-screen anchors if none are on-screen', () => { + // Push these anchors below screen + fakeAnchors[2].highlights = [1000, 100]; + fakeAnchors[3].highlights = [1100, 75]; + fakeAnchors[4].highlights = [1200, 100]; + fakeAnchors[5].highlights = [1300, 75]; + const buckets = anchorBuckets(fakeAnchors); + assert.equal(buckets.length, 2); + // Above-screen + assert.deepEqual(buckets[0].anchors, [fakeAnchors[0], fakeAnchors[1]]); + // Below-screen + assert.deepEqual(buckets[1].anchors, [ + fakeAnchors[2], + fakeAnchors[3], + fakeAnchors[4], + fakeAnchors[5], + ]); }); }); }); diff --git a/src/styles/annotator/bucket-bar.scss b/src/styles/annotator/bucket-bar.scss index 1f3655e7ef4..59771a7dd4f 100644 --- a/src/styles/annotator/bucket-bar.scss +++ b/src/styles/annotator/bucket-bar.scss @@ -28,6 +28,7 @@ right: 0; pointer-events: all; position: absolute; + // Vertically center the element, which is 16px high margin-top: -8px; line-height: 1; height: 16px; @@ -35,6 +36,7 @@ -webkit-tap-highlight-color: rgba(255, 255, 255, 0); text-align: center; cursor: pointer; + margin-top: -8px; .label { @include reset.reset-box-model; @@ -86,10 +88,17 @@ border-right: solid transparent; margin-top: 0; } + & .label { + // Vertical alignment tweak to better center the label in the indicator + margin-top: -1px; + } } &.upper { border-radius: 2px 2px 4px 4px; + // Vertically center the element (which is 22px high) in the + // intended position + margin-top: -11px; &:before, &:after { @@ -111,6 +120,7 @@ } &.lower { + margin-top: 0; border-radius: 4px 4px 2px 2px; &:before,