Merge pull request #439 from evidentpoint/crossframe-sidebar-hide-show

Use CrossFrame to show/hide the Sidebar

src/annotator/guest.coffee

@@ -39,6 +39,8 @@ module.exports = class Guest extends Delegator
     ".annotator-hl click":               "onHighlightClick"
     ".annotator-hl mouseover":           "onHighlightMouseover"
     ".annotator-hl mouseout":            "onHighlightMouseout"
+    "click":                             "onElementClick"
+    "touchstart":                        "onElementTouchStart"
 
   options:
     Document: {}
@@ -336,6 +338,7 @@ module.exports = class Guest extends Delegator
     targets.then(-> self.publish('beforeAnnotationCreated', [annotation]))
     targets.then(-> self.anchor(annotation))
 
+    @crossframe?.call('showSidebar') unless annotation.$highlight
     annotation
 
   createHighlight: ->
@@ -374,6 +377,7 @@ module.exports = class Guest extends Delegator
   showAnnotations: (annotations) ->
     tags = (a.$tag for a in annotations)
     @crossframe?.call('showAnnotations', tags)
+    @crossframe?.call('showSidebar')
 
   toggleAnnotationSelection: (annotations) ->
     tags = (a.$tag for a in annotations)
@@ -421,6 +425,19 @@ module.exports = class Guest extends Delegator
     else
       this.showAnnotations annotations
 
+  onElementClick: (event) ->
+    if !@selectedTargets?.length
+      @crossframe?.call('hideSidebar')
+
+  onElementTouchStart: (event) ->
+    # Mobile browsers do not register click events on
+    # elements without cursor: pointer. So instead of
+    # 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
+      @crossframe?.call('hideSidebar')
+
   onHighlightMouseover: (event) ->
     return unless @visibleHighlights
     annotation = $(event.currentTarget).data('annotation')

src/annotator/sidebar.coffee

@@ -47,30 +47,14 @@ module.exports = class Sidebar extends Host
       @onHelpRequest = serviceConfig.onHelpRequest
 
     this._setupSidebarEvents()
-    this._setupDocumentEvents()
-
-  _setupDocumentEvents: ->
-    sidebarTrigger(document.body, => this.show())
-
-    @element.on 'click', (event) =>
-      if !@selectedTargets?.length
-        this.hide()
-
-    # Mobile browsers do not register click events on
-    # elements without cursor: pointer. So instead of
-    # 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.
-    @element.on 'touchstart', (event) =>
-      if !@selectedTargets?.length
-        this.hide()
-
-    return this
 
   _setupSidebarEvents: ->
     annotationCounts(document.body, @crossframe)
-    features.init(@crossframe);
+    sidebarTrigger(document.body, => this.show())
+    features.init(@crossframe)
 
+    @crossframe.on('showSidebar', => this.show())
+    @crossframe.on('hideSidebar', => this.hide())
     @crossframe.on(events.LOGIN_REQUESTED, =>
       if @onLoginRequest
         @onLoginRequest()
@@ -206,11 +190,3 @@ module.exports = class Sidebar extends Host
 
   isOpen: ->
     !@frame.hasClass('annotator-collapsed')
-
-  createAnnotation: (annotation = {}) ->
-    super
-    this.show() unless annotation.$highlight
-
-  showAnnotations: (annotations) ->
-    super
-    this.show()

src/annotator/test/guest-test.coffee

@@ -86,6 +86,7 @@ describe 'Guest', ->
     fakeCrossFrame = {
       onConnect: sinon.stub()
       on: sinon.stub()
+      call: sinon.stub()
       sync: sinon.stub()
       destroy: sinon.stub()
     }
@@ -280,6 +281,24 @@ describe 'Guest', ->
 
         emitGuestEvent('getDocumentInfo', assertComplete)
 
+  describe 'document events', ->
+
+    guest = null
+
+    beforeEach ->
+      guest = createGuest()
+
+    it 'emits "hideSidebar" on cross frame when the user taps or clicks in the page', ->
+      methods =
+        'click': 'onElementClick'
+        'touchstart': 'onElementTouchStart'
+
+      for event in ['click', 'touchstart']
+        sandbox.spy(guest, methods[event])
+        guest.element.trigger(event)
+        assert.called(guest[methods[event]])
+        assert.calledWith(guest.plugins.CrossFrame.call, 'hideSidebar')
+
   describe 'when the selection changes', ->
     it 'shows the adder if the selection contains text', ->
       guest = createGuest()

src/annotator/test/sidebar-test.coffee

@@ -32,6 +32,20 @@ describe 'Sidebar', ->
     emitEvent = (event, args...) ->
       fn(args...) for [evt, fn] in fakeCrossFrame.on.args when event == evt
 
+    describe 'on "show" event', ->
+      it 'shows the frame', ->
+        target = sandbox.stub(Sidebar.prototype, 'show')
+        sidebar = createSidebar()
+        emitEvent('showSidebar')
+        assert.called(target)
+
+    describe 'on "hide" event', ->
+      it 'hides the frame', ->
+        target = sandbox.stub(Sidebar.prototype, 'hide')
+        sidebar = createSidebar()
+        emitEvent('hideSidebar')
+        assert.called(target)
+
     describe 'on LOGIN_REQUESTED event', ->
       it 'calls the onLoginRequest callback function if one was provided', ->
         onLoginRequest = sandbox.stub()
@@ -195,19 +209,6 @@ describe 'Sidebar', ->
       sidebar.onSwipe({type: 'swiperight'})
       assert.calledOnce(hide)
 
-  describe 'document events', ->
-
-    sidebar = null
-
-    beforeEach ->
-      sidebar = createSidebar({})
-
-    it 'closes the sidebar when the user taps or clicks in the page', ->
-      for event in ['click', 'touchstart']
-        sidebar.show()
-        sidebar.element.trigger(event)
-        assert.isFalse(sidebar.isOpen())
-
   describe 'destruction', ->
     sidebar = null
 

src/sidebar/frame-sync.js

@@ -149,6 +149,15 @@ function FrameSync($rootScope, $window, Discovery, annotationUI, bridge) {
     bridge.on('sidebarOpened', function () {
       $rootScope.$broadcast('sidebarOpened');
     });
+
+    // These merely relay calls
+    bridge.on('showSidebar', function () {
+      bridge.call('showSidebar');
+    });
+
+    bridge.on('hideSidebar', function () {
+      bridge.call('hideSidebar');
+    });
   }
 
   /**

src/sidebar/test/frame-sync-test.js

@@ -249,4 +249,18 @@ describe('FrameSync', function () {
       assert.called(onSidebarOpened);
     });
   });
+
+  describe ('on a relayed bridge call', function() {
+    it ('calls "showSidebar"', function() {
+      fakeBridge.emit('showSidebar');
+
+      assert.calledWith(fakeBridge.call, 'showSidebar');
+    });
+
+    it ('calls "hideSidebar"', function() {
+      fakeBridge.emit('hideSidebar');
+
+      assert.calledWith(fakeBridge.call, 'hideSidebar');
+    });
+  });
 });