Commit abe12c47 authored by csillag's avatar csillag

Fix sidebar's focusing on annotation cards

When the user moves the mouse over a highlight in the document,
the corresponding annotation card (in the sidebar) is supposed
to be highlighted, to indicate focus.

Our mechanism for this was broken in 3 different ways:

1. AnnotationUI service's focusAnnotations() method was working
   with annotation IDs. However, annotations being created
   right now don't yet have ID, only tags. So we should be working
   with tags instead. (Like we are, in many other cases
   when cross-frame communication communication is done
   around annotations.)

   I fixed that, and also updated the corresponding tests.

2. In controllers.coffee, when we were trying to check if a given
   annotation has focus, we were using the tag value, while on
   the other hand, it was not the tag, but the id that was put
   into the map. Of course this didn't work.

   Since now we are consistently using the tag, no change
   was needed here.

3. We are putting $$tag:true pairs into the map, but when
   checking for membership, the `in` operator was used.
   That works for lists, but not for maps. For maps, we simply
   have to read out the value belonging to the given key,
   and see if it's true.

After fixing these problems, the feature works again.
parent 9020e2d5
...@@ -13,7 +13,7 @@ createAnnotationUI = -> ...@@ -13,7 +13,7 @@ createAnnotationUI = ->
visibleHighlights: false visibleHighlights: false
# Contains a map of annotation id:true pairs. # Contains a map of annotation tag:true pairs.
focusedAnnotationMap: null focusedAnnotationMap: null
# Contains a map of annotation id:true pairs. # Contains a map of annotation id:true pairs.
...@@ -28,7 +28,7 @@ createAnnotationUI = -> ...@@ -28,7 +28,7 @@ createAnnotationUI = ->
### ###
focusAnnotations: (annotations) -> focusAnnotations: (annotations) ->
selection = {} selection = {}
selection[id] = true for {id} in annotations selection[$$tag] = true for {$$tag} in annotations
@focusedAnnotationMap = value(selection) @focusedAnnotationMap = value(selection)
###* ###*
......
...@@ -251,7 +251,7 @@ class ViewerController ...@@ -251,7 +251,7 @@ class ViewerController
true true
$scope.hasFocus = (annotation) -> $scope.hasFocus = (annotation) ->
annotation?.$$tag in ($scope.focusedAnnotations ? []) !!($scope.focusedAnnotations ? {})[annotation?.$$tag]
angular.module('h') angular.module('h')
.controller('AppController', AppController) .controller('AppController', AppController)
......
...@@ -17,25 +17,25 @@ describe 'AnnotationUI', -> ...@@ -17,25 +17,25 @@ describe 'AnnotationUI', ->
describe '.focusAnnotations()', -> describe '.focusAnnotations()', ->
it 'adds the passed annotations to the focusedAnnotationMap', -> it 'adds the passed annotations to the focusedAnnotationMap', ->
annotationUI.focusAnnotations([{id: 1}, {id: 2}, {id: 3}]) annotationUI.focusAnnotations([{$$tag: 1}, {$$tag: 2}, {$$tag: 3}])
assert.deepEqual(annotationUI.focusedAnnotationMap, { assert.deepEqual(annotationUI.focusedAnnotationMap, {
1: true, 2: true, 3: true 1: true, 2: true, 3: true
}) })
it 'replaces any annotations originally in the map', -> it 'replaces any annotations originally in the map', ->
annotationUI.focusedAnnotationMap = {1: true} annotationUI.focusedAnnotationMap = {1: true}
annotationUI.focusAnnotations([{id: 2}, {id: 3}]) annotationUI.focusAnnotations([{$$tag: 2}, {$$tag: 3}])
assert.deepEqual(annotationUI.focusedAnnotationMap, { assert.deepEqual(annotationUI.focusedAnnotationMap, {
2: true, 3: true 2: true, 3: true
}) })
it 'does not modify the original map object', -> it 'does not modify the original map object', ->
orig = annotationUI.focusedAnnotationMap = {1: true} orig = annotationUI.focusedAnnotationMap = {1: true}
annotationUI.focusAnnotations([{id: 2}, {id: 3}]) annotationUI.focusAnnotations([{$$tag: 2}, {$$tag: 3}])
assert.notEqual(annotationUI.focusedAnnotationMap, orig) assert.notEqual(annotationUI.focusedAnnotationMap, orig)
it 'nulls the map if no annotations are focused', -> it 'nulls the map if no annotations are focused', ->
orig = annotationUI.focusedAnnotationMap = {1: true} orig = annotationUI.focusedAnnotationMap = {$$tag: true}
annotationUI.focusAnnotations([]) annotationUI.focusAnnotations([])
assert.isNull(annotationUI.focusedAnnotationMap) assert.isNull(annotationUI.focusedAnnotationMap)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment