Merge pull request #1931 from hypothesis/add-view-annotations-to-toolbar

Add button to adder toolbar to show annotations for selection

src/annotator/adder.js

@@ -51,6 +51,14 @@ function nearestPositionedAncestor(el) {
   return parentEl;
 }
 
+/**
+ * @typedef AdderOptions
+ * @prop {() => any} onAnnotate - Callback invoked when "Annotate" button is clicked
+ * @prop {() => any} onHighlight - Callback invoked when "Highlight" button is clicked
+ * @prop {(annotations: Object[]) => any} onShowAnnotations -
+ *   Callback invoked when  "Show" button is clicked
+ */
+
 /**
  * Container for the 'adder' toolbar which provides controls for the user to
  * annotate and highlight the selected text.
@@ -66,8 +74,8 @@ export class Adder {
    *
    * The adder is initially hidden.
    *
-   * @param {Element} container - The DOM element into which the adder will be created
-   * @param {Object} options - Options object specifying `onAnnotate` and `onHighlight`
+   * @param {HTMLElement} container - The DOM element into which the adder will be created
+   * @param {AdderOptions} options - Options object specifying `onAnnotate` and `onHighlight`
    *        event handlers.
    */
   constructor(container, options) {
@@ -98,6 +106,17 @@ export class Adder {
     this._arrowDirection = 'up';
     this._onAnnotate = options.onAnnotate;
     this._onHighlight = options.onHighlight;
+    this._onShowAnnotations = options.onShowAnnotations;
+
+    /**
+     * Annotation objects associated with the current selection. If non-empty,
+     * a "Show" button appears in the toolbar. Clicking the button calls the
+     * `onShowAnnotations` callback with the current value of `annotationsForSelection`.
+     *
+     * @type {Object[]}
+     */
+    this.annotationsForSelection = [];
+
     this._render();
   }
 
@@ -198,15 +217,18 @@ export class Adder {
       switch (command) {
         case 'annotate':
           this._onAnnotate();
+          this.hide();
           break;
         case 'highlight':
           this._onHighlight();
+          this.hide();
+          break;
+        case 'show':
+          this._onShowAnnotations(this.annotationsForSelection);
           break;
         default:
           break;
       }
-
-      this.hide();
     };
 
     render(
@@ -214,6 +236,7 @@ export class Adder {
         isVisible={this._isVisible}
         arrowDirection={this._arrowDirection}
         onCommand={handleCommand}
+        annotationCount={this.annotationsForSelection.length}
       />,
       this._shadowRoot
     );

src/annotator/components/adder-toolbar.js

@@ -5,7 +5,7 @@ import propTypes from 'prop-types';
 import { useShortcut } from '../../shared/shortcut';
 import SvgIcon from '../../shared/components/svg-icon';
 
-function ToolbarButton({ icon, label, onClick, shortcut }) {
+function ToolbarButton({ badgeCount, icon, label, onClick, shortcut }) {
   useShortcut(shortcut, onClick);
 
   const title = shortcut ? `${label} (${shortcut})` : label;
@@ -16,14 +16,18 @@ function ToolbarButton({ icon, label, onClick, shortcut }) {
       onClick={onClick}
       title={title}
     >
-      <SvgIcon name={icon} />
+      {icon && <SvgIcon name={icon} />}
+      {typeof badgeCount === 'number' && (
+        <span className="annotator-adder-actions__badge">{badgeCount}</span>
+      )}
       <span className="annotator-adder-actions__label">{label}</span>
     </button>
   );
 }
 
 ToolbarButton.propTypes = {
-  icon: propTypes.string.isRequired,
+  badgeCount: propTypes.number,
+  icon: propTypes.string,
   label: propTypes.string.isRequired,
   onClick: propTypes.func.isRequired,
   shortcut: propTypes.string,
@@ -33,7 +37,12 @@ ToolbarButton.propTypes = {
  * The toolbar that is displayed above selected text in the document providing
  * options to create annotations or highlights.
  */
-export default function AdderToolbar({ arrowDirection, isVisible, onCommand }) {
+export default function AdderToolbar({
+  arrowDirection,
+  isVisible,
+  onCommand,
+  annotationCount = 0,
+}) {
   const handleCommand = (event, command) => {
     event.preventDefault();
     event.stopPropagation();
@@ -46,6 +55,7 @@ export default function AdderToolbar({ arrowDirection, isVisible, onCommand }) {
   // the shortcut. This avoids conflicts with browser/OS shortcuts.
   const annotateShortcut = isVisible ? 'a' : null;
   const highlightShortcut = isVisible ? 'h' : null;
+  const showShortcut = isVisible ? 's' : null;
 
   // nb. The adder is hidden using the `visibility` property rather than `display`
   // so that we can compute its size in order to position it before display.
@@ -71,6 +81,17 @@ export default function AdderToolbar({ arrowDirection, isVisible, onCommand }) {
           label="Highlight"
           shortcut={highlightShortcut}
         />
+        {annotationCount > 0 && (
+          <div className="annotator-adder-actions__separator" />
+        )}
+        {annotationCount > 0 && (
+          <ToolbarButton
+            badgeCount={annotationCount}
+            onClick={e => handleCommand(e, 'show')}
+            label="Show"
+            shortcut={showShortcut}
+          />
+        )}
       </hypothesis-adder-actions>
     </hypothesis-adder-toolbar>
   );
@@ -90,8 +111,16 @@ AdderToolbar.propTypes = {
   isVisible: propTypes.bool.isRequired,
 
   /**
-   * Callback invoked with the name ("annotate", "highlight") of the selected
-   * command when a toolbar command is clicked.
+   * Callback invoked with the name ("annotate", "highlight", "show") of the
+   * selected command when a toolbar command is clicked.
    */
   onCommand: propTypes.func.isRequired,
+
+  /**
+   * Number of annotations associated with the selected text.
+   *
+   * If non-zero, a "Show" button is displayed to allow the user to see the
+   * annotations that correspond to the selection.
+   */
+  annotationCount: propTypes.number,
 };

src/annotator/guest.coffee

@@ -20,6 +20,11 @@ animationPromise = (fn) ->
       catch error
         reject(error)
 
+annotationsForSelection = () ->
+  selection = window.getSelection()
+  range = selection.getRangeAt(0)
+  return rangeUtil.itemsForRange(range, (node) -> $(node).data('annotation'))
+
 # A selector which matches elements added to the DOM by Hypothesis (eg. for
 # highlights and annotation UI).
 #
@@ -68,6 +73,8 @@ module.exports = class Guest extends Delegator
         self.setVisibleHighlights(true)
         self.createHighlight()
         document.getSelection().removeAllRanges()
+      onShowAnnotations: (anns) ->
+        self.selectAnnotations(anns)
     })
     this.selections = selections(document).subscribe
       next: (range) ->
@@ -425,6 +432,7 @@ module.exports = class Guest extends Delegator
       .addClass('h-icon-annotate');
 
     {left, top, arrowDirection} = this.adderCtrl.target(focusRect, isBackwards)
+    this.adderCtrl.annotationsForSelection = annotationsForSelection()
     this.adderCtrl.showAt(left, top, arrowDirection)
 
   _onClearSelection: () ->

src/annotator/range-util.js

@@ -129,3 +129,38 @@ export function selectionFocusRect(selection) {
     return textBoxes[textBoxes.length - 1];
   }
 }
+
+/**
+ * Retrieve a set of items associated with nodes in a given range.
+ *
+ * An `item` can be any data that the caller wishes to compute from or associate
+ * with a node. Only unique items, as determined by `Object.is`, are returned.
+ *
+ * @template T
+ * @param {Range} range
+ * @param {(n: Node) => T} itemForNode - Callback returning the item for a given node
+ * @return {T[]} items
+ */
+export function itemsForRange(range, itemForNode) {
+  const checkedNodes = new Set();
+  const items = new Set();
+
+  forEachNodeInRange(range, node => {
+    let current = node;
+    while (current) {
+      if (checkedNodes.has(current)) {
+        break;
+      }
+      checkedNodes.add(current);
+
+      const item = itemForNode(current);
+      if (item) {
+        items.add(item);
+      }
+
+      current = current.parentNode;
+    }
+  });
+
+  return [...items];
+}

src/annotator/test/adder-test.js

@@ -36,6 +36,7 @@ describe('Adder', () => {
     adderCallbacks = {
       onAnnotate: sinon.stub(),
       onHighlight: sinon.stub(),
+      onShowAnnotations: sinon.stub(),
     };
     adderEl = document.createElement('div');
     document.body.appendChild(adderEl);
@@ -84,22 +85,57 @@ describe('Adder', () => {
   });
 
   describe('button handling', () => {
+    const getButton = label =>
+      getContent(adderCtrl).querySelector(`button[title^="${label}"]`);
+
+    const triggerShortcut = key =>
+      document.body.dispatchEvent(new KeyboardEvent('keydown', { key }));
+
+    const showAdder = () => {
+      // nb. `act` is necessary here to flush effect hooks in `AdderToolbar`
+      // which setup shortcut handlers.
+      act(() => {
+        adderCtrl.showAt(0, 0, ARROW_POINTING_UP);
+      });
+    };
+
     it('calls onHighlight callback when Highlight button is clicked', () => {
-      const highlightBtn = getContent(adderCtrl).querySelector(
-        'button[title^="Highlight"]'
-      );
+      const highlightBtn = getButton('Highlight');
       highlightBtn.dispatchEvent(new Event('click'));
       assert.called(adderCallbacks.onHighlight);
     });
 
     it('calls onAnnotate callback when Annotate button is clicked', () => {
-      const annotateBtn = getContent(adderCtrl).querySelector(
-        'button[title^="Annotate"]'
-      );
+      const annotateBtn = getButton('Annotate');
       annotateBtn.dispatchEvent(new Event('click'));
       assert.called(adderCallbacks.onAnnotate);
     });
 
+    it('does not show "Show" button if the selection has no annotations', () => {
+      showAdder();
+      assert.isNull(getButton('Show'));
+    });
+
+    it('shows the "Show" button if the selection has annotations', () => {
+      adderCtrl.annotationsForSelection = ['ann1', 'ann2'];
+      showAdder();
+
+      const showBtn = getButton('Show');
+      assert.ok(showBtn, '"Show" button not visible');
+      assert.equal(showBtn.querySelector('span').textContent, '2');
+    });
+
+    it('calls onShowAnnotations callback when Show button is clicked', () => {
+      adderCtrl.annotationsForSelection = ['ann1'];
+      showAdder();
+      const showBtn = getButton('Show');
+
+      showBtn.click();
+
+      assert.called(adderCallbacks.onShowAnnotations);
+      assert.calledWith(adderCallbacks.onShowAnnotations, ['ann1']);
+    });
+
     it("calls onAnnotate callback when Annotate button's label is clicked", () => {
       const annotateLabel = getContent(adderCtrl).querySelector(
         'button[title^="Annotate"] > span'
@@ -109,28 +145,32 @@ describe('Adder', () => {
     });
 
     it('calls onAnnotate callback when shortcut is pressed if adder is visible', () => {
-      // nb. `act` is necessary here to flush effect hooks in `AdderToolbar`.
-      act(() => {
-        adderCtrl.showAt(100, 100, ARROW_POINTING_UP);
-      });
-      document.body.dispatchEvent(new KeyboardEvent('keydown', { key: 'a' }));
+      showAdder();
+      triggerShortcut('a');
       assert.called(adderCallbacks.onAnnotate);
     });
 
     it('calls onHighlight callback when shortcut is pressed if adder is visible', () => {
-      // nb. `act` is necessary here to flush effect hooks in `AdderToolbar`.
-      act(() => {
-        adderCtrl.showAt(100, 100, ARROW_POINTING_UP);
-      });
-      document.body.dispatchEvent(new KeyboardEvent('keydown', { key: 'h' }));
+      showAdder();
+      triggerShortcut('h');
       assert.called(adderCallbacks.onHighlight);
     });
 
+    it('calls onShowAnnotations callback when shortcut is pressed if adder is visible', () => {
+      adderCtrl.annotationsForSelection = ['ann1'];
+      showAdder();
+      triggerShortcut('s');
+      assert.called(adderCallbacks.onShowAnnotations);
+    });
+
     it('does not call callbacks when adder is hidden', () => {
-      document.body.dispatchEvent(new KeyboardEvent('keydown', { key: 'a' }));
-      document.body.dispatchEvent(new KeyboardEvent('keydown', { key: 'h' }));
+      triggerShortcut('a');
+      triggerShortcut('h');
+      triggerShortcut('s');
+
       assert.notCalled(adderCallbacks.onAnnotate);
       assert.notCalled(adderCallbacks.onHighlight);
+      assert.notCalled(adderCallbacks.onShowAnnotations);
     });
   });
 

src/annotator/test/guest-test.coffee

@@ -15,12 +15,13 @@ scrollIntoView = sinon.stub()
 class FakeAdder
   instance: null
 
-  constructor: ->
+  constructor: (container, options) ->
     FakeAdder::instance = this
 
     this.hide = sinon.stub()
     this.showAt = sinon.stub()
     this.target = sinon.stub()
+    this.options = options
 
 class FakePlugin extends Plugin
   instance: null
@@ -57,6 +58,7 @@ describe 'Guest', ->
 
     FakeAdder::instance = null
     rangeUtil = {
+      itemsForRange: sinon.stub().returns([])
       isSelectionBackwards: sinon.stub()
       selectionFocusRect: sinon.stub()
     }
@@ -335,6 +337,17 @@ describe 'Guest', ->
         assert.calledWith(guest.plugins.CrossFrame.call, 'hideSidebar')
 
   describe 'when the selection changes', ->
+    container = null
+
+    beforeEach ->
+      container = document.createElement('div')
+      container.innerHTML = 'test text'
+      document.body.appendChild(container)
+      window.getSelection().selectAllChildren(container)
+
+    afterEach ->
+      container.remove()
+
     it 'shows the adder if the selection contains text', ->
       guest = createGuest()
       rangeUtil.selectionFocusRect.returns({left: 0, top: 0, width: 5, height: 5})
@@ -344,6 +357,22 @@ describe 'Guest', ->
       selections.next({})
       assert.called FakeAdder::instance.showAt
 
+    it 'sets the annotations associated with the selection', ->
+      guest = createGuest()
+      ann = {}
+      $(container).data('annotation', ann)
+      rangeUtil.selectionFocusRect.returns({left: 0, top: 0, width: 5, height: 5})
+      FakeAdder::instance.target.returns({
+        left: 0, top: 0, arrowDirection: adder.ARROW_POINTING_UP
+      })
+      rangeUtil.itemsForRange.callsFake((range, callback) ->
+        [callback(range.startContainer)]
+      )
+
+      selections.next({})
+
+      assert.deepEqual FakeAdder::instance.annotationsForSelection, [ann]
+
     it 'hides the adder if the selection does not contain text', ->
       guest = createGuest()
       rangeUtil.selectionFocusRect.returns(null)
@@ -356,6 +385,17 @@ describe 'Guest', ->
       selections.next(null)
       assert.called FakeAdder::instance.hide
 
+  describe 'when adder toolbar buttons are clicked', ->
+    # TODO - Add tests for "Annotate" and "Highlight" buttons.
+
+    it 'shows annotations if "Show" is clicked', ->
+      createGuest()
+
+      FakeAdder::instance.options.onShowAnnotations([{ $tag: 'ann1' }]);
+
+      assert.calledWith(fakeCrossFrame.call, 'showSidebar')
+      assert.calledWith(fakeCrossFrame.call, 'showAnnotations', ['ann1'])
+
   describe '#getDocumentInfo()', ->
     guest = null
 

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

@@ -134,4 +134,21 @@ describe('annotator.range-util', function () {
       );
     });
   });
+
+  describe('itemsForRange', () => {
+    it('returns unique items for range', () => {
+      const range = document.createRange();
+      range.setStart(testNode, 0);
+      range.setEnd(testNode, testNode.childNodes.length);
+
+      const data = new Map();
+      data.set(testNode, 'itemA');
+      data.set(testNode.childNodes[0], 'itemB');
+      data.set(testNode.childNodes[1], 'itemB'); // Intentional duplicate.
+      data.set(testNode.childNodes[2], 'itemC');
+      const items = rangeUtil.itemsForRange(range, item => data.get(item));
+
+      assert.deepEqual(items, ['itemA', 'itemB', 'itemC']);
+    });
+  });
 });

src/styles/annotator/adder.scss

+@use '../mixins/focus' as focus;
 @use '../variables' as var;
 
 $adder-transition-duration: 80ms;
@@ -97,20 +98,14 @@ $adder-transition-duration: 80ms;
   flex-direction: row;
 }
 
-.annotator-adder-actions:hover .annotator-adder-actions__button {
-  color: var.$grey-semi !important;
-}
-
-.annotator-adder-actions:hover .annotator-adder-actions__label {
-  color: var.$grey-semi !important;
-}
-
 .annotator-adder-actions__button {
+  @include focus.outline-on-keyboard-focus;
+
   box-shadow: none;
   font-family: h;
   font-size: 18px;
-  background: transparent !important;
-  color: var.$grey-mid !important;
+  background: transparent;
+  color: var.$grey-mid;
   display: flex;
   flex-direction: column;
   align-items: center;
@@ -125,11 +120,20 @@ $adder-transition-duration: 80ms;
   transition: color $adder-transition-duration;
 }
 
-.annotator-adder-actions .annotator-adder-actions__button:hover {
-  color: var.$grey-mid !important;
+.annotator-adder-actions__separator {
+  margin: 5px 5px;
+  border-right: 1px solid var.$grey-4;
+}
 
-  .annotator-adder-actions__label {
-    color: var.$grey-mid !important;
+// The toolbar has full contrast when not hovered. When the toolbar is hovered,
+// the buttons are dimmed except for the one which is currently hovered.
+.annotator-adder-actions:hover {
+  .annotator-adder-actions__button:not(:hover) {
+    color: var.$grey-semi;
+
+    .annotator-adder-actions__badge {
+      background-color: var.$grey-semi;
+    }
   }
 }
 
@@ -137,8 +141,20 @@ $adder-transition-duration: 80ms;
   margin-bottom: 2px;
   margin-top: 4px;
 
-  font-size: 11px;
+  font-size: 12px;
   font-family: sans-serif;
-  color: var.$grey-mid !important;
   transition: color $adder-transition-duration;
 }
+
+.annotator-adder-actions__badge {
+  background-color: var.$grey-mid;
+  border-radius: 3px;
+  color: white;
+
+  // The badge should be vertically aligned with icons in other toolbar buttons
+  // and the label underneath should also align with labels in other buttons.
+  font-family: sans-serif;
+  font-size: 12px;
+  font-weight: bold;
+  padding: 2px 4px;
+}