Rewrite selection buffering implementation

Rewrite the selection change observer in `src/annotator/selections` to
make it easier to understand how this functionality works and change it
in future.

The major change is to remove the dependency on the `Observable` class
provided by `zen-observable`, which adds cognitive overhead here, isn't used
elsewhere and is stuck on an old version due to subtle breaking changes in newer releases.

Replace it instead with a `SelectionObserver` class which is modeled
after DOM APIs like `MutationObserver`. It takes a callback in the
constructor that is invoked with the selected `Range` or `null` when the
selection changes and provides a `disconnect` method which stops
watching for future changes. The implementation only uses DOM APIs.

package.json

@@ -91,8 +91,7 @@
     "typescript": "^4.0.2",
     "vinyl": "^2.2.0",
     "watchify": "^3.7.0",
-    "wrap-text": "^1.0.7",
-    "zen-observable": "^0.3.0"
+    "wrap-text": "^1.0.7"
   },
   "browserslist": "chrome 55, edge 17, firefox 53, safari 10.1",
   "browserify": {

src/annotator/guest.js

@@ -14,7 +14,7 @@ import {
   setHighlightsVisible,
 } from './highlighter';
 import * as rangeUtil from './range-util';
-import selections from './selections';
+import { SelectionObserver } from './selection-observer';
 import { normalizeURI } from './util/url';
 
 /**
@@ -121,14 +121,13 @@ export default class Guest extends Delegator {
         this.selectAnnotations(anns);
       },
     });
-    this.selections = selections(document).subscribe({
-      next: range => {
-        if (range) {
-          this._onSelection(range);
-        } else {
-          this._onClearSelection();
-        }
-      },
+
+    this.selectionObserver = new SelectionObserver(range => {
+      if (range) {
+        this._onSelection(range);
+      } else {
+        this._onClearSelection();
+      }
     });
 
     this.plugins = {};
@@ -329,7 +328,8 @@ export default class Guest extends Delegator {
 
   destroy() {
     this._removeElementEvents();
-    this.selections.unsubscribe();
+
+    this.selectionObserver.disconnect();
     this.adderToolbar.remove();
 
     removeAllHighlights(this.element);

src/annotator/selection-observer.js

+/**
+ * Return the current selection or `null` if there is no selection or it is empty.
+ *
+ * @param {Document} document
+ * @return {Range|null}
+ */
+function selectedRange(document) {
+  const selection = document.getSelection();
+  if (!selection || selection.rangeCount === 0) {
+    return null;
+  }
+  const range = selection.getRangeAt(0);
+  if (range.collapsed) {
+    return null;
+  }
+  return range;
+}
+
+/**
+ * An observer that watches for and buffers changes to the document's current selection.
+ */
+export class SelectionObserver {
+  /**
+   * Start observing changes to the current selection in the document.
+   *
+   * @param {(range: Range|null) => any} callback -
+   *   Callback invoked with the selected region of the document when it has
+   *   changed.
+   * @param {Document} document_ - Test seam
+   */
+  constructor(callback, document_ = document) {
+    let isMouseDown = false;
+
+    this._pendingCallback = null;
+
+    const scheduleCallback = (delay = 10) => {
+      this._pendingCallback = setTimeout(() => {
+        callback(selectedRange(document_));
+      }, delay);
+    };
+
+    /** @param {Event} event */
+    this._eventHandler = event => {
+      if (event.type === 'mousedown') {
+        isMouseDown = true;
+      }
+      if (event.type === 'mouseup') {
+        isMouseDown = false;
+      }
+
+      // If the user makes a selection with the mouse, wait until they release
+      // it before reporting a selection change.
+      if (isMouseDown) {
+        return;
+      }
+
+      this._cancelPendingCallback();
+
+      // Schedule a notification after a short delay. The delay serves two
+      // purposes:
+      //
+      // - If this handler was called as a result of a 'mouseup' event then the
+      //   selection will not be updated until the next tick of the event loop.
+      //   In this case we only need a short delay.
+      //
+      // - If the user is changing the selection with a non-mouse input (eg.
+      //   keyboard or selection handles on mobile) this buffers updates and
+      //   makes sure that we only report one when the update has stopped
+      //   changing. In this case we want a longer delay.
+
+      const delay = event.type === 'mouseup' ? 10 : 100;
+      scheduleCallback(delay);
+    };
+
+    this._document = document_;
+    this._events = ['mousedown', 'mouseup', 'selectionchange'];
+    for (let event of this._events) {
+      document_.addEventListener(event, this._eventHandler);
+    }
+
+    // Report the initial selection.
+    scheduleCallback(1);
+  }
+
+  disconnect() {
+    for (let event of this._events) {
+      this._document.removeEventListener(event, this._eventHandler);
+    }
+    this._cancelPendingCallback();
+  }
+
+  _cancelPendingCallback() {
+    if (this._pendingCallback) {
+      clearTimeout(this._pendingCallback);
+      this._pendingCallback = null;
+    }
+  }
+}

src/annotator/selections.js

-import * as observable from './util/observable';
-
-/** Returns the selected `DOMRange` in `document`. */
-function selectedRange(document) {
-  const selection = document.getSelection();
-  if (!selection.rangeCount || selection.getRangeAt(0).collapsed) {
-    return null;
-  } else {
-    return selection.getRangeAt(0);
-  }
-}
-
-/**
- * Returns an Observable stream of text selections in the current document.
- *
- * New values are emitted when the user finishes making a selection
- * (represented by a `DOMRange`) or clears a selection (represented by `null`).
- *
- * A value will be emitted with the selected range at the time of subscription
- * on the next tick.
- *
- * @return Observable<DOMRange|null>
- */
-export default function selections(document) {
-  // Get a stream of selection changes that occur whilst the user is not
-  // making a selection with the mouse.
-  let isMouseDown;
-  const selectionEvents = observable
-    .listen(document, ['mousedown', 'mouseup', 'selectionchange'])
-    .filter(function (event) {
-      if (event.type === 'mousedown' || event.type === 'mouseup') {
-        isMouseDown = event.type === 'mousedown';
-        return false;
-      } else {
-        return !isMouseDown;
-      }
-    });
-
-  const events = observable.merge([
-    // Add a delay before checking the state of the selection because
-    // the selection is not updated immediately after a 'mouseup' event
-    // but only on the next tick of the event loop.
-    observable.buffer(10, observable.listen(document, ['mouseup'])),
-
-    // Buffer selection changes to avoid continually emitting events whilst the
-    // user drags the selection handles on mobile devices
-    observable.buffer(100, selectionEvents),
-
-    // Emit an initial event on the next tick
-    observable.delay(0, observable.Observable.of({})),
-  ]);
-
-  return events.map(function () {
-    return selectedRange(document);
-  });
-}

src/annotator/test/guest-test.js

-import { Observable } from '../util/observable';
 import Delegator from '../delegator';
 import Guest from '../guest';
 import { $imports } from '../guest';
@@ -37,7 +36,7 @@ describe('Guest', () => {
   let guestConfig;
   let htmlAnchoring;
   let rangeUtil;
-  let selections;
+  let notifySelectionChanged;
 
   const createGuest = (config = {}) => {
     const element = document.createElement('div');
@@ -62,7 +61,7 @@ describe('Guest', () => {
       isSelectionBackwards: sinon.stub(),
       selectionFocusRect: sinon.stub(),
     };
-    selections = null;
+    notifySelectionChanged = null;
 
     sinon.stub(window, 'requestAnimationFrame').yields();
 
@@ -74,6 +73,13 @@ describe('Guest', () => {
       destroy: sinon.stub(),
     };
 
+    class FakeSelectionObserver {
+      constructor(callback) {
+        notifySelectionChanged = callback;
+        this.disconnect = sinon.stub();
+      }
+    }
+
     CrossFrame = sandbox.stub().returns(fakeCrossFrame);
     guestConfig.pluginClasses.CrossFrame = CrossFrame;
 
@@ -82,11 +88,8 @@ describe('Guest', () => {
       './anchoring/html': htmlAnchoring,
       './highlighter': highlighter,
       './range-util': rangeUtil,
-      './selections': () => {
-        return new Observable(function (obs) {
-          selections = obs;
-          return () => {};
-        });
+      './selection-observer': {
+        SelectionObserver: FakeSelectionObserver,
       },
       './delegator': Delegator,
       'scroll-into-view': scrollIntoView,
@@ -451,12 +454,12 @@ describe('Guest', () => {
         width: 5,
         height: 5,
       });
-      return selections.next({});
+      notifySelectionChanged({});
     };
 
     const simulateSelectionWithoutText = () => {
       rangeUtil.selectionFocusRect.returns(null);
-      return selections.next({});
+      notifySelectionChanged({});
     };
 
     it('shows the adder if the selection contains text', () => {
@@ -488,7 +491,7 @@ describe('Guest', () => {
 
     it('hides the adder if the selection is empty', () => {
       createGuest();
-      selections.next(null);
+      notifySelectionChanged(null);
       assert.called(FakeAdder.instance.hide);
     });
 

src/annotator/test/selections-test.js → src/annotator/test/selection-observer-test.js

-import selections from '../selections';
-import * as observable from '../util/observable';
+import { SelectionObserver } from '../selection-observer';
 
-function FakeDocument() {
-  const listeners = {};
+class FakeDocument extends EventTarget {
+  constructor() {
+    super();
+    this.selection = null;
+  }
 
-  return {
-    getSelection: function () {
-      return this.selection;
-    },
-
-    addEventListener: function (name, listener) {
-      listeners[name] = (listeners[name] || []).concat(listener);
-    },
-
-    removeEventListener: function (name, listener) {
-      listeners[name] = listeners[name].filter(function (lis) {
-        return lis !== listener;
-      });
-    },
-
-    dispatchEvent: function (event) {
-      listeners[event.type].forEach(function (fn) {
-        fn(event);
-      });
-    },
-  };
+  getSelection() {
+    return this.selection;
+  }
 }
 
-describe('selections', function () {
+describe('SelectionObserver', () => {
   let clock;
   let fakeDocument;
   let range;
-  let rangeSub;
+  let observer;
   let onSelectionChanged;
 
-  beforeEach(function () {
+  beforeEach(() => {
     clock = sinon.useFakeTimers();
     fakeDocument = new FakeDocument();
     onSelectionChanged = sinon.stub();
 
-    // Subscribe to selection changes, ignoring the initial event
-    const ranges = observable.drop(selections(fakeDocument), 1);
-    rangeSub = ranges.subscribe({ next: onSelectionChanged });
-
-    range = {};
+    range = { collapsed: false };
     fakeDocument.selection = {
       rangeCount: 1,
       getRangeAt: function (index) {
         return index === 0 ? range : null;
       },
     };
+
+    observer = new SelectionObserver(range => {
+      onSelectionChanged(range);
+    }, fakeDocument);
+
+    // Move the clock forwards past the initial event.
+    clock.tick(10);
+    onSelectionChanged.reset();
   });
 
-  afterEach(function () {
-    rangeSub.unsubscribe();
+  afterEach(() => {
+    observer.disconnect();
     clock.restore();
   });
 
-  it('emits the selected range when mouseup occurs', function () {
-    fakeDocument.dispatchEvent({ type: 'mouseup' });
+  it('invokes callback when mouseup occurs', () => {
+    fakeDocument.dispatchEvent(new Event('mouseup'));
     clock.tick(20);
     assert.calledWith(onSelectionChanged, range);
   });
 
-  it('emits an event if there is a selection at the initial subscription', function () {
+  it('invokes callback with initial selection', () => {
     const onInitialSelection = sinon.stub();
-    const ranges = selections(fakeDocument);
-    const sub = ranges.subscribe({ next: onInitialSelection });
-    clock.tick(1);
+    const observer = new SelectionObserver(onInitialSelection, fakeDocument);
+    clock.tick(10);
     assert.called(onInitialSelection);
-    sub.unsubscribe();
+    observer.disconnect();
   });
 
-  describe('when the selection changes', function () {
-    it('emits a selection if the mouse is not down', function () {
-      fakeDocument.dispatchEvent({ type: 'selectionchange' });
+  describe('when the selection changes', () => {
+    it('invokes callback if mouse is not down', () => {
+      fakeDocument.dispatchEvent(new Event('selectionchange'));
       clock.tick(200);
       assert.calledWith(onSelectionChanged, range);
     });
 
-    it('does not emit a selection if the mouse is down', function () {
-      fakeDocument.dispatchEvent({ type: 'mousedown' });
-      fakeDocument.dispatchEvent({ type: 'selectionchange' });
+    it('does not invoke callback if mouse is down', () => {
+      fakeDocument.dispatchEvent(new Event('mousedown'));
+      fakeDocument.dispatchEvent(new Event('selectionchange'));
       clock.tick(200);
       assert.notCalled(onSelectionChanged);
     });
 
-    it('does not emit a selection until there is a pause since the last change', function () {
-      fakeDocument.dispatchEvent({ type: 'selectionchange' });
+    it('does not invoke callback until there is a pause since the last change', () => {
+      fakeDocument.dispatchEvent(new Event('selectionchange'));
       clock.tick(90);
-      fakeDocument.dispatchEvent({ type: 'selectionchange' });
+      fakeDocument.dispatchEvent(new Event('selectionchange'));
       clock.tick(90);
       assert.notCalled(onSelectionChanged);
       clock.tick(20);

src/annotator/util/observable.js

-/**
- * Functions (aka. 'operators') for generating and manipulating streams of
- * values using the Observable API.
- */
-
-import Observable from 'zen-observable';
-
-/**
- * Returns an observable of events emitted by a DOM event source
- * (eg. an Element, Document or Window).
- *
- * @param {EventTarget} src - The event source.
- * @param {Array<string>} eventNames - List of events to subscribe to
- */
-export function listen(src, eventNames) {
-  return new Observable(function (observer) {
-    const onNext = function (event) {
-      observer.next(event);
-    };
-
-    eventNames.forEach(function (event) {
-      src.addEventListener(event, onNext);
-    });
-
-    return function () {
-      eventNames.forEach(function (event) {
-        src.removeEventListener(event, onNext);
-      });
-    };
-  });
-}
-
-/**
- * Delay events from a source Observable by `delay` ms.
- */
-export function delay(delay, src) {
-  return new Observable(function (obs) {
-    let timeouts = [];
-    const sub = src.subscribe({
-      next: function (value) {
-        const t = setTimeout(function () {
-          timeouts = timeouts.filter(function (other) {
-            return other !== t;
-          });
-          obs.next(value);
-        }, delay);
-        timeouts.push(t);
-      },
-    });
-    return function () {
-      timeouts.forEach(clearTimeout);
-      sub.unsubscribe();
-    };
-  });
-}
-
-/**
- * Buffers events from a source Observable, waiting for a pause of `delay`
- * ms with no events before emitting the last value from `src`.
- *
- * @template T
- * @param {number} delay
- * @param {Observable<T>} src
- * @return {Observable<T>}
- */
-export function buffer(delay, src) {
-  return new Observable(function (obs) {
-    let lastValue;
-    let timeout;
-
-    function onNext() {
-      obs.next(lastValue);
-    }
-
-    const sub = src.subscribe({
-      next: function (value) {
-        lastValue = value;
-        clearTimeout(timeout);
-        timeout = setTimeout(onNext, delay);
-      },
-    });
-
-    return function () {
-      sub.unsubscribe();
-      clearTimeout(timeout);
-    };
-  });
-}
-
-/**
- * Merges multiple streams of values into a single stream.
- *
- * @param {Array<Observable>} sources
- * @return Observable
- */
-export function merge(sources) {
-  return new Observable(function (obs) {
-    const subs = sources.map(function (src) {
-      return src.subscribe({
-        next: function (value) {
-          obs.next(value);
-        },
-      });
-    });
-
-    return function () {
-      subs.forEach(function (sub) {
-        sub.unsubscribe();
-      });
-    };
-  });
-}
-
-/** Drop the first `n` events from the `src` Observable. */
-export function drop(src, n) {
-  let count = 0;
-  return src.filter(function () {
-    ++count;
-    return count > n;
-  });
-}
-
-export { Observable };

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

-import * as observable from '../observable';
-
-describe('observable', function () {
-  describe('delay()', function () {
-    let clock;
-
-    beforeEach(function () {
-      clock = sinon.useFakeTimers();
-    });
-
-    afterEach(function () {
-      clock.restore();
-    });
-
-    it('defers events', function () {
-      const received = [];
-      const obs = observable.delay(50, observable.Observable.of('foo'));
-      obs.forEach(function (v) {
-        received.push(v);
-      });
-      assert.deepEqual(received, []);
-      clock.tick(100);
-      assert.deepEqual(received, ['foo']);
-    });
-
-    it('delivers events in sequence', function () {
-      const received = [];
-      const obs = observable.delay(10, observable.Observable.of(1, 2));
-      obs.forEach(function (v) {
-        received.push(v);
-      });
-      clock.tick(20);
-      assert.deepEqual(received, [1, 2]);
-    });
-  });
-});

yarn.lock

@@ -8180,8 +8180,3 @@ yeast@0.1.2:
   version "0.1.2"
   resolved "https://registry.yarnpkg.com/yeast/-/yeast-0.1.2.tgz#008e06d8094320c372dbc2f8ed76a0ca6c8ac419"
   integrity sha1-AI4G2AlDIMNy28L47XagymyKxBk=
-
-zen-observable@^0.3.0:
-  version "0.3.0"
-  resolved "https://registry.yarnpkg.com/zen-observable/-/zen-observable-0.3.0.tgz#137bb5e31be7b83ed5eda9c14f9305cb3cdaa725"
-  integrity sha1-E3u14xvnuD7V7anBT5MFyzzapyU=