Skip to content

Commit

Permalink
Use center rather than top position of anchor for bucket-on-screen test
Browse files Browse the repository at this point in the history
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
  • Loading branch information
robertknight committed Jun 8, 2023
1 parent bcd622a commit fbea415
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 11 deletions.
9 changes: 5 additions & 4 deletions src/annotator/util/buckets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
55 changes: 48 additions & 7 deletions src/annotator/util/test/buckets-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -161,6 +167,7 @@ describe('annotator/util/buckets', () => {

beforeEach(() => {
fakeAnchorPositions = [
// Above-screen anchors.
{
tag: 't0',
top: 1,
Expand All @@ -171,6 +178,7 @@ describe('annotator/util/buckets', () => {
top: 101,
bottom: 151,
},
// On-screen anchors.
{
tag: 't2',
top: 201,
Expand All @@ -181,6 +189,7 @@ describe('annotator/util/buckets', () => {
top: 301,
bottom: 351,
},
// Below-screen anchors.
{
tag: 't4',
top: 401,
Expand All @@ -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', () => {
Expand Down

0 comments on commit fbea415

Please sign in to comment.