Commit b44864c7 authored by csillag's avatar csillag

Simplify visual diff code

Remove the unnecessary per-target hasDiff flag.

Extract the code for calculating the hasDiff/showDiff flags
into a separate method.
parent b8ab3ef2
...@@ -128,6 +128,18 @@ AnnotationController = [ ...@@ -128,6 +128,18 @@ AnnotationController = [
@action = 'view' @action = 'view'
@editing = false @editing = false
###*
# @ngdoc method
# @name annotation.AnnotationController.diffFromTarget
# @description Calculates the visual diff flags from the targets
###
this.diffFromTargets = (targets) ->
hasDiff = targets.filter((t) -> t.diffHTML?).length > 0
shouldShowDiff = hasDiff and
targets.filter((t) -> t.diffHTML? and not t.diffCaseOnly).length > 0
{hasDiff, shouldShowDiff}
###* ###*
# @ngdoc method # @ngdoc method
# @name annotation.AnnotationController#save # @name annotation.AnnotationController#save
...@@ -215,16 +227,15 @@ AnnotationController = [ ...@@ -215,16 +227,15 @@ AnnotationController = [
@annotation.tags = ({text} for text in (model.tags or [])) @annotation.tags = ({text} for text in (model.tags or []))
# Calculate the visual diff flags # Calculate the visual diff flags
@hasDiff = false diffFlags = this.diffFromTargets(@annotation.target)
for t in @annotation.target or [] @hasDiff = diffFlags.hasDiff
if t.diffHTML?
@hasDiff = t.hasDiff = true
unless t.diffCaseOnly
@showDiff ?= true
else
t.hasDiff = false
if @hasDiff if @hasDiff
@showDiff ?= false # We don't want to override the showDiff value manually changed
# by the user, that's why we use a conditional assignment here,
# instead of directly setting showDiff to the calculated value
@showDiff ?= diffFlags.shouldShowDiff
else
@showDiff = undefined
updateTimestamp = (repeat=false) => updateTimestamp = (repeat=false) =>
@timestamp = timeHelpers.toFuzzyString model.updated @timestamp = timeHelpers.toFuzzyString model.updated
......
...@@ -44,14 +44,14 @@ ...@@ -44,14 +44,14 @@
<section class="annotation-target" <section class="annotation-target"
ng-repeat="target in vm.annotation.target track by $index"> ng-repeat="target in vm.annotation.target track by $index">
<blockquote class="annotation-quote" <blockquote class="annotation-quote"
ng-hide="target.hasDiff && vm.showDiff" ng-hide="target.diffHTML && vm.showDiff"
ng-bind-html="selector.exact" ng-bind-html="selector.exact"
ng-repeat="selector in target.selector ng-repeat="selector in target.selector
| filter : {'type': 'TextQuoteSelector'} | filter : {'type': 'TextQuoteSelector'}
track by $index"></blockquote> track by $index"></blockquote>
<blockquote class="annotation-quote" <blockquote class="annotation-quote"
ng-bind-html="target.diffHTML" ng-bind-html="target.diffHTML"
ng-show="target.hasDiff && vm.showDiff"></blockquote> ng-show="target.diffHTML && vm.showDiff"></blockquote>
</section> </section>
<div class="small pull-right" <div class="small pull-right"
ng-show="vm.hasDiff"> ng-show="vm.hasDiff">
......
...@@ -148,9 +148,6 @@ describe 'h.directives.annotation', -> ...@@ -148,9 +148,6 @@ describe 'h.directives.annotation', ->
assert.isTrue(controller.hasDiff) assert.isTrue(controller.hasDiff)
assert.isTrue(controller.showDiff) assert.isTrue(controller.showDiff)
it 'sets `hasDiff` on the target to true', ->
assert.isTrue(targets[0].hasDiff)
describe 'when thre are only upper case/lower case difference between the text in the single target, and what was saved in a selector', -> describe 'when thre are only upper case/lower case difference between the text in the single target, and what was saved in a selector', ->
targets = null targets = null
...@@ -166,9 +163,6 @@ describe 'h.directives.annotation', -> ...@@ -166,9 +163,6 @@ describe 'h.directives.annotation', ->
assert.isTrue(controller.hasDiff) assert.isTrue(controller.hasDiff)
assert.isFalse(controller.showDiff) assert.isFalse(controller.showDiff)
it 'sets `hasDiff` on the target to true', ->
assert.isTrue(targets[0].hasDiff)
describe 'when there are multiple targets, some with differences in text', -> describe 'when there are multiple targets, some with differences in text', ->
targets = null targets = null
...@@ -187,11 +181,6 @@ describe 'h.directives.annotation', -> ...@@ -187,11 +181,6 @@ describe 'h.directives.annotation', ->
it 'sets `showDiff` to true', -> it 'sets `showDiff` to true', ->
assert.isTrue(controller.showDiff) assert.isTrue(controller.showDiff)
it 'sets `hasDiff` to true on targets with any differences', ->
assert.isFalse(targets[0].hasDiff)
assert.isTrue(targets[1].hasDiff)
assert.isTrue(targets[2].hasDiff)
it 'preserves the `showDiff` value on update', -> it 'preserves the `showDiff` value on update', ->
controller.showDiff = false controller.showDiff = false
annotation.target = annotation.target.slice(1) annotation.target = annotation.target.slice(1)
......
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