Remove anchors from the `BucketSet` data structure

This PR makes the `Buckets` component to accept a simplified data
structure that doesn't contain `anchor`s. The `anchor` objects are no
longer needed and the can't be serialised (because they contain DOM
elements).

src/annotator/components/Buckets.js

@@ -14,17 +14,15 @@ import classnames from 'classnames';
  *   @param {(tags: string[], toggle: boolean) => void} props.onSelectAnnotations
  */
 function BucketButton({ bucket, onFocusAnnotations, onSelectAnnotations }) {
-  const buttonTitle = `Select nearby annotations (${bucket.anchors.length})`;
+  const buttonTitle = `Select nearby annotations (${bucket.tags.size})`;
 
   function selectAnnotations(event) {
-    const tags = bucket.anchors.map(anchor => anchor.annotation.$tag);
-    onSelectAnnotations(tags, event.metaKey || event.ctrlKey);
+    onSelectAnnotations([...bucket.tags], event.metaKey || event.ctrlKey);
   }
 
   function setFocus(hasFocus) {
     if (hasFocus) {
-      const tags = bucket.anchors.map(anchor => anchor.annotation.$tag);
-      onFocusAnnotations(tags);
+      onFocusAnnotations([...bucket.tags]);
     } else {
       onFocusAnnotations([]);
     }
@@ -40,7 +38,7 @@ function BucketButton({ bucket, onFocusAnnotations, onSelectAnnotations }) {
       title={buttonTitle}
       aria-label={buttonTitle}
     >
-      {bucket.anchors.length}
+      {bucket.tags.size}
     </button>
   );
 }
@@ -59,21 +57,18 @@ function NavigationBucketButton({
   direction,
   onScrollToClosestOffScreenAnchor,
 }) {
-  const buttonTitle = `Go ${direction} to next annotations (${bucket.anchors.length})`;
-
-  function scrollToClosest() {
-    const tags = bucket.anchors.map(anchor => anchor.annotation.$tag);
-    onScrollToClosestOffScreenAnchor(tags, direction);
-  }
+  const buttonTitle = `Go ${direction} to next annotations (${bucket.tags.size})`;
 
   return (
     <button
       className={classnames('Buckets__button', `Buckets__button--${direction}`)}
-      onClick={scrollToClosest}
+      onClick={() =>
+        onScrollToClosestOffScreenAnchor([...bucket.tags], direction)
+      }
       title={buttonTitle}
       aria-label={buttonTitle}
     >
-      {bucket.anchors.length}
+      {bucket.tags.size}
     </button>
   );
 }
@@ -98,8 +93,8 @@ export default function Buckets({
   onScrollToClosestOffScreenAnchor,
   onSelectAnnotations,
 }) {
-  const showUpNavigation = above.anchors.length > 0;
-  const showDownNavigation = below.anchors.length > 0;
+  const showUpNavigation = above.tags.size > 0;
+  const showDownNavigation = below.tags.size > 0;
 
   return (
     <ul className="Buckets__list">

src/annotator/components/test/Buckets-test.js

@@ -13,28 +13,19 @@ describe('Buckets', () => {
 
   beforeEach(() => {
     fakeAbove = {
-      anchors: [
-        { annotation: { $tag: 'a1' }, highlights: ['hi'] },
-        { annotation: { $tag: 'a2' }, highlights: ['there'] },
-      ],
+      tags: new Set(['a1', 'a2']),
       position: 150,
     };
     fakeBelow = {
-      anchors: [
-        { annotation: { $tag: 'b1' }, highlights: ['ho'] },
-        { annotation: { $tag: 'b2' }, highlights: ['there'] },
-      ],
+      tags: new Set(['b1', 'b2']),
       position: 550,
     };
     fakeBuckets = [
       {
-        anchors: [
-          { annotation: { $tag: 't1' }, highlights: ['hi'] },
-          { annotation: { $tag: 't2' }, highlights: ['yay'] },
-        ],
+        tags: new Set(['t1', 't2']),
         position: 250,
       },
-      { anchors: ['you', 'also', 'are', 'welcome'], position: 350 },
+      { tags: new Set(['t3', 't4', 't5', 't6']), position: 350 },
     ];
     fakeOnFocusAnnotations = sinon.stub();
     fakeOnScrollToClosestOffScreenAnchor = sinon.stub();
@@ -68,7 +59,7 @@ describe('Buckets', () => {
     });
 
     it('does not render an up navigation button if there are no above-screen anchors', () => {
-      fakeAbove = { anchors: [], position: 150 };
+      fakeAbove = { tags: new Set(), position: 150 };
       const wrapper = createComponent();
       assert.isFalse(wrapper.find('.Buckets__button--up').exists());
     });
@@ -88,7 +79,7 @@ describe('Buckets', () => {
     });
 
     it('does not render a down navigation button if there are no below-screen anchors', () => {
-      fakeBelow = { anchors: [], position: 550 };
+      fakeBelow = { tags: new Set(), position: 550 };
       const wrapper = createComponent();
       assert.isFalse(wrapper.find('.Buckets__button--down').exists());
     });
@@ -164,10 +155,7 @@ describe('Buckets', () => {
 
       assert.calledOnce(fakeOnSelectAnnotations);
       const call = fakeOnSelectAnnotations.getCall(0);
-      assert.deepEqual(call.args[0], [
-        fakeBuckets[0].anchors[0].annotation.$tag,
-        fakeBuckets[0].anchors[1].annotation.$tag,
-      ]);
+      assert.deepEqual(call.args[0], [...fakeBuckets[0].tags]);
       assert.equal(call.args[1], false);
     });
 

src/annotator/util/buckets.js

@@ -6,27 +6,27 @@ import { getBoundingClientRect } from '../highlighter';
 
 /**
  * @typedef Bucket
- * @prop {Anchor[]} anchors - The anchors in this bucket
+ * @prop {Set<string>} tags - The annotation tags in this bucket
  * @prop {number} position - The vertical pixel offset where this bucket should
- *                           appear in the bucket bar.
+ *   appear in the bucket bar
  */
 
 /**
  * @typedef BucketSet
- * @prop {Bucket} above - A single bucket containing all of the anchors that
- *                        are offscreen upwards
- * @prop {Bucket} below - A single bucket containing all of the anchors that are
- *                        offscreen downwards
+ * @prop {Bucket} above - A single bucket containing all the annotation
+ *   tags which anchors are offscreen upwards
+ * @prop {Bucket} below - A single bucket containing all the annotation
+ *   tags which anchors are offscreen downwards
  * @prop {Bucket[]} buckets - On-screen buckets
  */
 
 /**
  * @typedef WorkingBucket
- * @prop {Anchor[]} anchors - The anchors in this bucket
+ * @prop {Set<string>} tags - The annotation tags 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
+ *   in this bucket — the lowest `top` position value, akin to the top offset 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
@@ -148,9 +148,12 @@ function getAnchorPositions(anchors) {
  */
 export function anchorBuckets(anchors) {
   const anchorPositions = getAnchorPositions(anchors);
-  const aboveScreen = new Set();
-  const belowScreen = new Set();
-  const buckets = /** @type {Bucket[]} */ ([]);
+  /** @type {Set<string>} */
+  const aboveTags = new Set();
+  /** @type {Set<string>} */
+  const belowTags = new Set();
+  /** @type {Bucket[]} */
+  const buckets = [];
 
   // Hold current working anchors and positions as we build each bucket
   /** @type {WorkingBucket|null} */
@@ -166,7 +169,7 @@ export function anchorBuckets(anchors) {
     const anchorHeight = anchorPosition.bottom - anchorPosition.top;
     const bucketPosition = anchorPosition.top + anchorHeight / 2;
     const bucket = /** @type WorkingBucket */ ({
-      anchors: [anchorPosition.anchor],
+      tags: new Set([anchorPosition.anchor.annotation.$tag]),
       top: anchorPosition.top,
       bottom: anchorPosition.bottom,
       position: bucketPosition,
@@ -177,10 +180,10 @@ export function anchorBuckets(anchors) {
   // Build buckets from position information
   anchorPositions.forEach(aPos => {
     if (aPos.top < BUCKET_TOP_THRESHOLD) {
-      aboveScreen.add(aPos.anchor);
+      aboveTags.add(aPos.anchor.annotation.$tag);
       return;
     } else if (aPos.top > window.innerHeight - BUCKET_BOTTOM_THRESHOLD) {
-      belowScreen.add(aPos.anchor);
+      belowTags.add(aPos.anchor.annotation.$tag);
       return;
     }
 
@@ -216,7 +219,7 @@ export function anchorBuckets(anchors) {
         aPos.bottom > currentBucket.bottom ? aPos.bottom : currentBucket.bottom;
       const updatedHeight = updatedBottom - currentBucket.top;
 
-      currentBucket.anchors.push(aPos.anchor);
+      currentBucket.tags.add(aPos.anchor.annotation.$tag);
       currentBucket.bottom = updatedBottom;
       currentBucket.position = currentBucket.top + updatedHeight / 2;
     }
@@ -228,13 +231,13 @@ export function anchorBuckets(anchors) {
 
   // Add an upper "navigation" bucket with offscreen-above anchors
   const above = {
-    anchors: Array.from(aboveScreen),
+    tags: aboveTags,
     position: BUCKET_TOP_THRESHOLD,
   };
 
   // Add a lower "navigation" bucket with offscreen-below anchors
   const below = {
-    anchors: Array.from(belowScreen),
+    tags: belowTags,
     position: window.innerHeight - BUCKET_BOTTOM_THRESHOLD,
   };
 

src/annotator/util/test/buckets-test.js

 import { findClosestOffscreenAnchor, anchorBuckets } from '../buckets';
 import { $imports } from '../buckets';
 
+let tagIndex = 0;
 function fakeAnchorFactory(
   offsetStart = 1,
   offsetIncrement = 100,
@@ -13,7 +14,10 @@ function fakeAnchorFactory(
     // 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] };
+    const anchor = {
+      annotation: { $tag: `t${tagIndex++}` },
+      highlights: [highlightIndex, boxHeight],
+    };
     highlightIndex = highlightIndex + offsetIncrement;
     return anchor;
   };
@@ -58,6 +62,11 @@ describe('annotator/util/buckets', () => {
     stubbedInnerHeight.restore();
   });
 
+  /** @param {number} index */
+  const getTag = index => {
+    return fakeAnchors[index].annotation.$tag;
+  };
+
   describe('findClosestOffscreenAnchor', () => {
     it('finds the closest anchor above screen when headed up', () => {
       // fakeAnchors [0] and [1] are offscreen upwards, having `top` values
@@ -116,44 +125,32 @@ describe('annotator/util/buckets', () => {
   describe('anchorBuckets', () => {
     it('puts anchors that are above the screen into the `above` bucket', () => {
       const bucketSet = anchorBuckets(fakeAnchors);
-      assert.deepEqual(bucketSet.above.anchors, [
-        fakeAnchors[0],
-        fakeAnchors[1],
-      ]);
+      assert.deepEqual([...bucketSet.above.tags], [getTag(0), getTag(1)]);
     });
 
     it('puts anchors that are below the screen into the `below` bucket', () => {
       const bucketSet = anchorBuckets(fakeAnchors);
-      assert.deepEqual(bucketSet.below.anchors, [
-        fakeAnchors[4],
-        fakeAnchors[5],
-      ]);
+      assert.deepEqual([...bucketSet.below.tags], [getTag(4), getTag(5)]);
     });
 
     it('puts on-screen anchors into a buckets', () => {
       const bucketSet = anchorBuckets(fakeAnchors);
-      assert.deepEqual(bucketSet.buckets[0].anchors, [
-        fakeAnchors[2],
-        fakeAnchors[3],
-      ]);
+      assert.deepEqual([...bucketSet.buckets[0].tags], [getTag(2), getTag(3)]);
     });
 
     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 bucketSet = anchorBuckets(fakeAnchors);
-      assert.deepEqual(bucketSet.buckets[0].anchors, [fakeAnchors[2]]);
-      assert.deepEqual(bucketSet.buckets[1].anchors, [fakeAnchors[3]]);
+      assert.deepEqual([...bucketSet.buckets[0].tags], [getTag(2)]);
+      assert.deepEqual([...bucketSet.buckets[1].tags], [getTag(3)]);
     });
 
     it('puts overlapping anchors into a shared bucket', () => {
       fakeAnchors[2].highlights = [201, 200]; // Bottom 401
       fakeAnchors[3].highlights = [285, 100]; // Bottom 385
       const bucketSet = anchorBuckets(fakeAnchors);
-      assert.deepEqual(bucketSet.buckets[0].anchors, [
-        fakeAnchors[2],
-        fakeAnchors[3],
-      ]);
+      assert.deepEqual([...bucketSet.buckets[0].tags], [getTag(2), getTag(3)]);
     });
 
     it('positions the bucket at vertical midpoint of the box containing all bucket anchors', () => {
@@ -167,16 +164,16 @@ describe('annotator/util/buckets', () => {
       const badAnchor = { highlights: [] };
       const bucketSet = anchorBuckets([badAnchor]);
       assert.equal(bucketSet.buckets.length, 0);
-      assert.isEmpty(bucketSet.above.anchors); // Holder for above-screen anchors
-      assert.isEmpty(bucketSet.below.anchors); // Holder for below-screen anchors
+      assert.isEmpty(bucketSet.above.tags); // Holder for above-screen anchors
+      assert.isEmpty(bucketSet.below.tags); // Holder for below-screen anchors
     });
 
     it('does not bucket annotations whose highlights have zero area', () => {
       const badAnchor = { highlights: [0, 0] };
       const bucketSet = anchorBuckets([badAnchor]);
       assert.equal(bucketSet.buckets.length, 0);
-      assert.isEmpty(bucketSet.above.anchors);
-      assert.isEmpty(bucketSet.below.anchors);
+      assert.isEmpty(bucketSet.above.tags);
+      assert.isEmpty(bucketSet.below.tags);
     });
 
     it('sorts anchors by top position', () => {
@@ -188,18 +185,9 @@ describe('annotator/util/buckets', () => {
         fakeAnchors[0],
         fakeAnchors[1],
       ]);
-      assert.deepEqual(bucketSet.above.anchors, [
-        fakeAnchors[0],
-        fakeAnchors[1],
-      ]);
-      assert.deepEqual(bucketSet.buckets[0].anchors, [
-        fakeAnchors[2],
-        fakeAnchors[3],
-      ]);
-      assert.deepEqual(bucketSet.below.anchors, [
-        fakeAnchors[4],
-        fakeAnchors[5],
-      ]);
+      assert.deepEqual([...bucketSet.above.tags], [getTag(0), getTag(1)]);
+      assert.deepEqual([...bucketSet.buckets[0].tags], [getTag(2), getTag(3)]);
+      assert.deepEqual([...bucketSet.below.tags], [getTag(4), getTag(5)]);
     });
 
     it('returns only above- and below-screen anchors if none are on-screen', () => {
@@ -211,17 +199,12 @@ describe('annotator/util/buckets', () => {
       const bucketSet = anchorBuckets(fakeAnchors);
       assert.equal(bucketSet.buckets.length, 0);
       // Above-screen
-      assert.deepEqual(bucketSet.above.anchors, [
-        fakeAnchors[0],
-        fakeAnchors[1],
-      ]);
+      assert.deepEqual([...bucketSet.above.tags], [getTag(0), getTag(1)]);
       // Below-screen
-      assert.deepEqual(bucketSet.below.anchors, [
-        fakeAnchors[2],
-        fakeAnchors[3],
-        fakeAnchors[4],
-        fakeAnchors[5],
-      ]);
+      assert.deepEqual(
+        [...bucketSet.below.tags],
+        [getTag(2), getTag(3), getTag(4), getTag(5)]
+      );
     });
   });
 });