Merge pull request #521 from wjt/many-overlapping-annotations

viewer: don't repeatedly show overlapping annotations
openannotation · Jul 2, 2015 · 2f40f38 · 2f40f38
2 parents 6d7e1e5 + b5211bf
commit 2f40f38
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 9 deletions.
diff --git a/src/ui/viewer.js b/src/ui/viewer.js
@@ -117,21 +117,25 @@ var Viewer = exports.Viewer = Widget.extend({
 
             $(this.options.autoViewHighlights)
                 .on("mouseover." + NS, '.annotator-hl', function (event) {
-                    self._onHighlightMouseover(event);
+                    // If there are many overlapping highlights, still only
+                    // call _onHighlightMouseover once.
+                    if (event.target === this) {
+                        self._onHighlightMouseover(event);
+                    }
                 })
                 .on("mouseleave." + NS, '.annotator-hl', function () {
                     self._startHideTimer();
                 });
 
             $(this.document.body)
                 .on("mousedown." + NS, function (e) {
-                    if (e.which > 1) {
-                        this.mouseDown = true;
+                    if (e.which == 1) {
+                        self.mouseDown = true;
                     }
                 })
                 .on("mouseup." + NS, function (e) {
-                    if (e.which > 1) {
-                        this.mouseDown = false;
+                    if (e.which == 1) {
+                        self.mouseDown = false;
                     }
                 });
         }

diff --git a/test/fixtures/viewer.html b/test/fixtures/viewer.html
@@ -6,4 +6,5 @@
     <li>First item</li>
     <li>Second item</li>
   </ul>
+  <p><span class="annotator-hl three"><span class="annotator-hl four">Fruit flies like bananas.</span></span></p>
 </div>
diff --git a/test/spec/ui/filter_spec.js b/test/spec/ui/filter_spec.js
@@ -125,9 +125,10 @@ describe('ui.filter.Filter', function () {
             };
             annotations = [{text: 'cat'}, {text: 'dog'}, {text: 'car'}];
             plugin.filters = {'text': testFilter};
-            plugin.highlights = {
-                map: function () { return annotations; }
-            };
+            plugin.highlights = $('<span>cat</span><span>dog</span><span>car</span>');
+            plugin.highlights.each(function(i) {
+                $(this).data('annotation', annotations[i]);
+            });
             sandbox.stub(plugin, 'updateHighlights');
             sandbox.stub(plugin, 'resetHighlights');
             sandbox.stub(plugin, 'filterHighlights');

diff --git a/test/spec/ui/viewer_spec.js b/test/spec/ui/viewer_spec.js
@@ -274,7 +274,8 @@ describe('ui.viewer.Viewer', function () {
 
     describe('event handlers', function () {
         var hl = null,
-            clock = null;
+            clock = null,
+            sandbox = null;
 
         beforeEach(function () {
             v = new viewer.Viewer({
@@ -288,9 +289,11 @@ describe('ui.viewer.Viewer', function () {
                 text: "Cats with mats"
             });
             clock = sinon.useFakeTimers();
+            sandbox = sinon.sandbox.create();
         });
 
         afterEach(function () {
+            sandbox.restore();
             clock.restore();
             v.destroy();
         });
@@ -365,6 +368,33 @@ describe('ui.viewer.Viewer', function () {
             });
             assert.isTrue(called, 'event should have propagated to the document');
         });
+
+        // Regression test: Issue #520. Repeatedly showing the dialog is
+        // surprisingly slow when many annotations overlap.
+        it('should only show the viewer once when mousing over overlapping annotations', function() {
+            var hl3 = $(h.fix()).find('.annotator-hl.three');
+            hl3.data('annotation', {
+                text: "Time flies like arrows."
+            });
+
+            var hl4 = $(h.fix()).find('.annotator-hl.four');
+            hl4.data('annotation', {
+                text: "(Is that right?)"
+            });
+
+            sandbox.spy(v, 'load');
+            sandbox.spy(v, 'show');
+
+            hl4.mouseover();
+            clock.tick(200);
+
+            assert.isTrue(v.isShown());
+            assert.isTrue(v.element.html().indexOf("Time flies like arrows.") >= 0);
+            assert.isTrue(v.element.html().indexOf("(Is that right?)") >= 0);
+
+            assert.equal(v.load.callCount, 1);
+            assert.equal(v.show.callCount, 1);
+        });
     });
 });