Merge pull request #2106 from hypothesis/fix-voiceover-sidebar-toggle

Fix sidebar toggle button when using VoiceOver on iOS

src/annotator/delegator.coffee

@@ -244,7 +244,7 @@ Delegator.natives = do ->
   """
   blur focus focusin focusout load resize scroll unload click dblclick
   mousedown mouseup mousemove mouseover mouseout mouseenter mouseleave
-  change select submit keydown keypress keyup error
+  change select submit keydown keypress keyup error touchstart
   """.split(/[^a-z]+/).concat(specials)
 
 

src/annotator/guest.coffee

@@ -10,6 +10,7 @@ highlighter = require('./highlighter')
 rangeUtil = require('./range-util')
 { default: selections } = require('./selections')
 xpathRange = require('./anchoring/range')
+{ closest } = require('./util/dom-element')
 { normalizeURI } = require('./util/url')
 
 animationPromise = (fn) ->
@@ -442,8 +443,15 @@ module.exports = class Guest extends Delegator
     else
       this.showAnnotations annotations
 
+  # Did an event originate from an element in the annotator UI? (eg. the sidebar
+  # frame, or its toolbar)
+  _isEventInAnnotator: (event) ->
+    return closest(event.target, '.annotator-frame') != null
+
+  # Event handlers to close the sidebar when the user clicks in the document.
+  # These really ought to live with the sidebar code.
   onElementClick: (event) ->
-    if !@selectedTargets?.length
+    if !this._isEventInAnnotator(event) and !@selectedTargets?.length
       @crossframe?.call('hideSidebar')
 
   onElementTouchStart: (event) ->
@@ -452,7 +460,7 @@ module.exports = class Guest extends Delegator
     # adding that to every element, we can add the initial
     # touchstart event which is always registered to
     # make up for the lack of click support for all elements.
-    if !@selectedTargets?.length
+    if !this._isEventInAnnotator(event) and !@selectedTargets?.length
       @crossframe?.call('hideSidebar')
 
   onHighlightMouseover: (event) ->

src/annotator/highlighter.js

+import { closest } from './util/dom-element';
+
 const SVG_NAMESPACE = 'http://www.w3.org/2000/svg';
 
 function isCSSPropertySupported(property, value) {
@@ -8,20 +10,6 @@ function isCSSPropertySupported(property, value) {
   return CSS.supports(property, value);
 }
 
-/**
- * Implementation of `element.closest(selector)`. This is used to support browsers
- * (IE 11) that don't have a native implementation.
- */
-function closest(element, selector) {
-  while (element) {
-    if (element.matches(selector)) {
-      return element;
-    }
-    element = element.parentElement;
-  }
-  return null;
-}
-
 /**
  * Return the canvas element underneath a highlight element in a PDF page's
  * text layer.

src/annotator/test/guest-test.coffee

@@ -320,22 +320,46 @@ describe 'Guest', ->
 
   describe 'document events', ->
 
+    fakeSidebarFrame = null
     guest = null
+    rootElement = null
+
+    methods =
+      'click': 'onElementClick'
+      'touchstart': 'onElementTouchStart'
 
     beforeEach ->
+      fakeSidebarFrame = null
       guest = createGuest()
+      rootElement = guest.element[0]
+
+    afterEach ->
+      fakeSidebarFrame?.remove()
 
-    it 'emits "hideSidebar" on cross frame when the user taps or clicks in the page', ->
-      methods =
-        'click': 'onElementClick'
-        'touchstart': 'onElementTouchStart'
+    it 'hides sidebar when the user taps or clicks in the page', ->
 
       for event in ['click', 'touchstart']
         sandbox.spy(guest, methods[event])
-        guest.element.trigger(event)
+
+        rootElement.dispatchEvent(new Event(event))
+
         assert.called(guest[methods[event]])
         assert.calledWith(guest.plugins.CrossFrame.call, 'hideSidebar')
 
+    it 'does not hide sidebar if event occurs inside annotator UI', ->
+      fakeSidebarFrame = document.createElement('div')
+      fakeSidebarFrame.className = 'annotator-frame'
+      rootElement.appendChild(fakeSidebarFrame)
+
+      for event in ['click', 'touchstart']
+        sandbox.spy(guest, methods[event])
+
+        fakeSidebarFrame.dispatchEvent(new Event(event, { bubbles: true }))
+
+        assert.called(guest[methods[event]])
+        assert.notCalled(guest.plugins.CrossFrame.call)
+
+
   describe 'when the selection changes', ->
     container = null
 

src/annotator/util/dom-element.js

+/**
+ * Ponyfills [1] for DOM element methods that are not supported in all browsers.
+ *
+ * [1] https://github.com/sindresorhus/ponyfill
+ */
+
+/**
+ * Implementation of `element.closest(selector)`. This is used to support browsers
+ * (IE 11) that don't have a native implementation.
+ */
+export function closest(element, selector) {
+  while (element) {
+    if (element.matches(selector)) {
+      return element;
+    }
+    element = element.parentElement;
+  }
+  return null;
+}

src/annotator/util/test/dom-element-test.js

+import { closest } from '../dom-element';
+
+describe('annotator/util/dom-element', () => {
+  let container;
+
+  beforeEach(() => {
+    container = document.createElement('div');
+    document.body.appendChild(container);
+  });
+
+  afterEach(() => {
+    container.remove();
+  });
+
+  describe('closest', () => {
+    it('returns closest matching ancestor', () => {
+      container.innerHTML = `
+<div class="outer">
+  <div class="middle">
+    <div class="inner"></div>
+  </div>
+</div>
+`;
+
+      const el = container.querySelector('.inner');
+      const target = container.querySelector('.outer');
+
+      assert.equal(closest(el, '.inner'), el);
+      assert.equal(closest(el, '.outer'), target);
+      assert.isNull(closest(el, '.no-match'));
+    });
+  });
+});