Commit 7d824030 authored by Robert Knight's avatar Robert Knight

Use center rather than top position of anchor for bucket-on-screen test

Use the position of the center of an anchor instead of the top to determine
whether it should be counted as off-screen or not. Since buckets in the bucket
bar are drawn where the center is, this ensures that on-screen buckets merge
into off-screen ones at a natural point, instead of when there is still a gap
between the two (for the "above screen" bucket) or after the on-screen bucket
has gone below the "below screen" bucket.

Fixes #5525
parent a2fb5e26
......@@ -54,8 +54,8 @@ export type WorkingBucket = {
// `window.innerHeight - 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;
export const BUCKET_TOP_THRESHOLD = 137;
export 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;
......@@ -167,10 +167,11 @@ export function computeBuckets(anchorPositions: AnchorPosition[]): BucketSet {
// Build buckets from position information
anchorPositions.forEach(aPos => {
if (aPos.top < BUCKET_TOP_THRESHOLD) {
const center = (aPos.top + aPos.bottom) / 2;
if (center < BUCKET_TOP_THRESHOLD) {
aboveTags.add(aPos.tag);
return;
} else if (aPos.top > window.innerHeight - BUCKET_BOTTOM_THRESHOLD) {
} else if (center > window.innerHeight - BUCKET_BOTTOM_THRESHOLD) {
belowTags.add(aPos.tag);
return;
}
......
......@@ -3,8 +3,12 @@ import {
computeAnchorPositions,
computeBuckets,
$imports,
BUCKET_TOP_THRESHOLD,
BUCKET_BOTTOM_THRESHOLD,
} from '../buckets';
const FAKE_WINDOW_HEIGHT = 410;
describe('annotator/util/buckets', () => {
let fakeAnchors;
let fakeGetBoundingClientRect;
......@@ -31,7 +35,9 @@ describe('annotator/util/buckets', () => {
{ annotation: { $tag: 't5' }, highlights: [501, 50] },
];
stubbedInnerHeight = sinon.stub(window, 'innerHeight').value(410);
stubbedInnerHeight = sinon
.stub(window, 'innerHeight')
.value(FAKE_WINDOW_HEIGHT);
fakeGetBoundingClientRect = sinon.stub().callsFake(highlights => {
// Use the entries of the faked anchor's `highlights` array to
......@@ -161,6 +167,7 @@ describe('annotator/util/buckets', () => {
beforeEach(() => {
fakeAnchorPositions = [
// Above-screen anchors.
{
tag: 't0',
top: 1,
......@@ -171,6 +178,7 @@ describe('annotator/util/buckets', () => {
top: 101,
bottom: 151,
},
// On-screen anchors.
{
tag: 't2',
top: 201,
......@@ -181,6 +189,7 @@ describe('annotator/util/buckets', () => {
top: 301,
bottom: 351,
},
// Below-screen anchors.
{
tag: 't4',
top: 401,
......@@ -194,14 +203,46 @@ describe('annotator/util/buckets', () => {
];
});
it('puts anchors that are above the screen into the `above` bucket', () => {
const bucketSet = computeBuckets(fakeAnchorPositions);
assert.deepEqual([...bucketSet.above.tags], ['t0', 't1']);
it('puts anchors whose center is above the screen into the `above` bucket', () => {
const thresholdPos = BUCKET_TOP_THRESHOLD;
const bucketSet = computeBuckets([
...fakeAnchorPositions,
{
tag: 'just-on-screen',
top: thresholdPos - 10,
bottom: thresholdPos + 11,
},
{
tag: 'just-off-screen',
top: thresholdPos - 11,
bottom: thresholdPos + 10,
},
]);
assert.deepEqual(
[...bucketSet.above.tags],
['t0', 't1', 'just-off-screen']
);
});
it('puts anchors that are below the screen into the `below` bucket', () => {
const bucketSet = computeBuckets(fakeAnchorPositions);
assert.deepEqual([...bucketSet.below.tags], ['t4', 't5']);
it('puts anchors whose center is below the screen into the `below` bucket', () => {
const thresholdPos = FAKE_WINDOW_HEIGHT - BUCKET_BOTTOM_THRESHOLD;
const bucketSet = computeBuckets([
...fakeAnchorPositions,
{
tag: 'just-on-screen',
top: thresholdPos - 11,
bottom: thresholdPos + 10,
},
{
tag: 'just-off-screen',
top: thresholdPos - 10,
bottom: thresholdPos + 11,
},
]);
assert.deepEqual(
[...bucketSet.below.tags],
['t4', 't5', 'just-off-screen']
);
});
it('puts on-screen anchors into a buckets', () => {
......
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