From ebb878fba12c93abef036f063be58e1bc8dd5ab4 Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Fri, 23 Sep 2016 12:06:31 +0100 Subject: [PATCH] Remove the AnnotationUISync service Its functionality has now been moved into the FrameSync service, except for the logic that was intended to synchronize the "Show Annotation Highlights" logic across multiple frames, since this is clearly broken [1]. The two pieces of functionality related to this we need to support at the moment are: 1. The `showHighlights` config option to set whether highlights are initially visible. 2. The highlight toggle button in the sidebar's outer frame Both of these work without the sidebar app's involvement. [1] See https://github.com/hypothesis/h/issues/3433 and https://github.com/hypothesis/h/pull/3295 --- h/static/scripts/annotation-ui-sync.js | 51 ---------- h/static/scripts/app.js | 1 - h/static/scripts/frame-sync.js | 12 +-- .../scripts/test/annotation-ui-sync-test.js | 97 ------------------- 4 files changed, 1 insertion(+), 160 deletions(-) delete mode 100644 h/static/scripts/annotation-ui-sync.js delete mode 100644 h/static/scripts/test/annotation-ui-sync-test.js diff --git a/h/static/scripts/annotation-ui-sync.js b/h/static/scripts/annotation-ui-sync.js deleted file mode 100644 index b5243196dd9..00000000000 --- a/h/static/scripts/annotation-ui-sync.js +++ /dev/null @@ -1,51 +0,0 @@ -'use strict'; - -/** - * Uses a channel between the sidebar and the attached frames to ensure - * the interface remains in sync. - * - * @name AnnotationUISync - * @param {$window} $window An Angular window service. - * @param {Bridge} bridge - * @param {AnnotationUI} annotationUI An instance of the AnnotatonUI service - * @description - * Listens for incoming events over the bridge concerning the annotation - * interface and updates the applications internal state. It also ensures - * that the messages are broadcast out to other frames. - */ -// @ngInject -function AnnotationUISync($rootScope, $window, annotationUI, bridge) { - - var channelListeners = { - setVisibleHighlights: function (state) { - if (typeof state !== 'boolean') { - state = true; - } - if (annotationUI.getState().visibleHighlights !== state) { - annotationUI.setShowHighlights(state); - bridge.call('setVisibleHighlights', state); - } - }, - }; - - for (var channel in channelListeners) { - if (Object.prototype.hasOwnProperty.call(channelListeners, channel)) { - var listener = channelListeners[channel]; - bridge.on(channel, listener); - } - } - - var onConnect = function (channel, source) { - if (source === $window.parent) { - // The host initializes its own state - return; - } else { - // Synchronize the state of guests - channel.call('setVisibleHighlights', annotationUI.getState().visibleHighlights); - } - }; - - bridge.onConnect(onConnect); -} - -module.exports = AnnotationUISync; diff --git a/h/static/scripts/app.js b/h/static/scripts/app.js index 489bf6d5ea3..f2b4cead67a 100644 --- a/h/static/scripts/app.js +++ b/h/static/scripts/app.js @@ -190,7 +190,6 @@ module.exports = angular.module('h', [ .factory('store', require('./store')) - .value('AnnotationUISync', require('./annotation-ui-sync')) .value('Discovery', require('./discovery')) .value('ExcerptOverflowMonitor', require('./directive/excerpt-overflow-monitor')) .value('VirtualThreadList', require('./virtual-thread-list')) diff --git a/h/static/scripts/frame-sync.js b/h/static/scripts/frame-sync.js index f63c678dacc..8808aaea8c0 100644 --- a/h/static/scripts/frame-sync.js +++ b/h/static/scripts/frame-sync.js @@ -38,8 +38,7 @@ function formatAnnot(ann) { * sidebar. */ // @ngInject -function FrameSync($rootScope, $window, AnnotationUISync, Discovery, - annotationUI, bridge) { +function FrameSync($rootScope, $window, Discovery, annotationUI, bridge) { // List of frames currently connected to the sidebar var frames = []; @@ -133,15 +132,6 @@ function FrameSync($rootScope, $window, AnnotationUISync, Discovery, bridge.on('sidebarOpened', function () { $rootScope.$broadcast('sidebarOpened'); }); - - // Create an instance of the AnnotationUISync class which listens for - // selection/focus messages from the frame and propagates them to the rest - // of the sidebar app. - // - // FIXME: The frame message listeners from AnnotationUISync should be - // extracted and moved here and then the AnnotationUISync class can be - // removed entirely. - new AnnotationUISync($rootScope, $window, annotationUI, bridge); } /** diff --git a/h/static/scripts/test/annotation-ui-sync-test.js b/h/static/scripts/test/annotation-ui-sync-test.js deleted file mode 100644 index 43de0e695f4..00000000000 --- a/h/static/scripts/test/annotation-ui-sync-test.js +++ /dev/null @@ -1,97 +0,0 @@ -'use strict'; - -var angular = require('angular'); - -var annotationUIFactory = require('../annotation-ui'); - -describe('AnnotationUISync', function () { - var sandbox = sinon.sandbox.create(); - var publish; - var fakeBridge; - var annotationUI; - var createAnnotationUISync; - var createChannel = function () { - return { call: sandbox.stub() }; - }; - var PARENT_WINDOW = 'PARENT_WINDOW'; - - before(function () { - angular.module('h', []) - .value('AnnotationUISync', require('../annotation-ui-sync')); - }); - - beforeEach(angular.mock.module('h')); - beforeEach(angular.mock.inject(function (AnnotationUISync, $rootScope) { - var listeners = {}; - publish = function (method) { - var args = [].slice.apply(arguments); - return listeners[method].apply(null, args.slice(1)); - }; - - var fakeWindow = { parent: PARENT_WINDOW }; - fakeBridge = { - on: sandbox.spy(function (method, fn) { listeners[method] = fn; }), - call: sandbox.stub(), - onConnect: sandbox.stub(), - links: [ - { window: PARENT_WINDOW, channel: createChannel() }, - { window: 'ANOTHER_WINDOW', channel: createChannel() }, - { window: 'THIRD_WINDOW', channel: createChannel() }, - ], - }; - - annotationUI = annotationUIFactory($rootScope, {}); - annotationUI.addAnnotations([ - {id: 'id1', $$tag: 'tag1'}, - {id: 'id2', $$tag: 'tag2'}, - {id: 'id3', $$tag: 'tag3'}, - ]); - createAnnotationUISync = function () { - new AnnotationUISync( - $rootScope, fakeWindow, annotationUI, fakeBridge - ); - }; - })); - - afterEach(function () { - sandbox.restore(); - }); - - describe('on bridge connection', function () { - describe('when the source is not the parent window', function () { - it('broadcasts the visibility settings to the channel', function () { - var channel = createChannel(); - fakeBridge.onConnect.callsArgWith(0, channel, {}); - - createAnnotationUISync(); - - assert.calledWith(channel.call, 'setVisibleHighlights', false); - }); - }); - - describe('when the source is the parent window', function () { - it('does nothing', function () { - var channel = { call: sandbox.stub() }; - fakeBridge.onConnect.callsArgWith(0, channel, PARENT_WINDOW); - - createAnnotationUISync(); - assert.notCalled(channel.call); - }); - }); - }); - - - describe('on "setVisibleHighlights" event', function () { - it('updates the annotationUI state', function () { - createAnnotationUISync(); - publish('setVisibleHighlights', true); - assert.equal(annotationUI.getState().visibleHighlights, true); - }); - - it('notifies the other frames of the change', function () { - createAnnotationUISync(); - publish('setVisibleHighlights', true); - assert.calledWith(fakeBridge.call, 'setVisibleHighlights', true); - }); - }); -});