Skip to content

C
coopwire-hypothesis
Commit 5408f2f3 authored by Robert Knight's avatar Robert Knight
Browse files
Options

Remove `updateViewModel` 

The `updateViewModel()` function was responsible for computing several
pieces of data (links, document domain and title) used by the annotation
template.

Since we can now rely on vm.annotation being immutable, we can just
compute this data lazily when we need it and use memoization to avoid
unnecessary recalculation.
parent 0df06b8a
Hide whitespace changes
Inline Side-by-side
Showing with 53 additions and 87 deletions
h/static/scripts/directive/annotation.js
View file @ 5408f2f3
/* jshint node: true */
'use strict';
var angular = require('angular');
var annotationMetadata = require('../annotation-metadata');
var events = require('../events');
var memoize = require('../util/memoize');
var persona = require('../filter/persona');
var isNew = annotationMetadata.isNew;
......@@ -47,21 +46,6 @@ function updateModel(annotation, changes, permissions) {
});
}
/** Update the view model from the domain model changes. */
function updateViewModel($scope, vm) {
if (vm.annotation.links) {
vm.linkInContext = vm.annotation.links.incontext ||
vm.annotation.links.html ||
'';
vm.linkHTML = vm.annotation.links.html || '';
} else {
vm.linkInContext = '';
vm.linkHTML = '';
}
vm.documentMeta = annotationMetadata.domainAndTitle(vm.annotation);
}
// @ngInject
function AnnotationController(
$document, $q, $rootScope, $scope, $timeout, $window, annotationUI,
......@@ -138,13 +122,6 @@ function AnnotationController(
*/
newlyCreatedByHighlightButton = vm.annotation.$highlight || false;
// Call `onAnnotationUpdated()` whenever the "annotationUpdated" event is
// emitted. This event is emitted after changes to the annotation are
// successfully saved to the server, and also when changes to the
// annotation made by another client are received by this client from the
// server.
$rootScope.$on(events.ANNOTATION_UPDATED, onAnnotationUpdated);
// When a new annotation is created, remove any existing annotations that
// are empty
$rootScope.$on(events.BEFORE_ANNOTATION_CREATED, deleteIfNewAndEmpty);
......@@ -171,8 +148,6 @@ function AnnotationController(
// log in.
saveNewHighlight();
updateView();
// If this annotation is not a highlight and if it's new (has just been
// created by the annotate button) or it has edits not yet saved to the
// server - then open the editor on AnnotationController instantiation.
......@@ -183,16 +158,6 @@ function AnnotationController(
}
}
function updateView() {
updateViewModel($scope, vm, permissions);
}
function onAnnotationUpdated(event, updatedDomainModel) {
if (updatedDomainModel.id === vm.annotation.id) {
updateView();
}
}
function deleteIfNewAndEmpty() {
if (isNew(vm.annotation) && !vm.state().text && vm.state().tags.length === 0) {
vm.revert();
......@@ -233,7 +198,6 @@ function AnnotationController(
save(vm.annotation).then(function(model) {
model.$$tag = vm.annotation.$$tag;
$rootScope.$emit(events.ANNOTATION_CREATED, model);
updateView();
});
} else {
// User isn't logged in, save to drafts.
......@@ -410,8 +374,6 @@ function AnnotationController(
drafts.remove(vm.annotation);
if (isNew(vm.annotation)) {
$rootScope.$emit(events.ANNOTATION_DELETED, vm.annotation);
} else {
updateView();
}
};
......@@ -499,6 +461,17 @@ function AnnotationController(
return isReply(vm.annotation);
};
vm.links = function () {
if (vm.annotation.links) {
return {incontext: vm.annotation.links.incontext ||
vm.annotation.links.html ||
'',
html: vm.annotation.links.html};
} else {
return {incontext: '', html: ''};
}
};
/**
* Sets whether or not the controls for expanding/collapsing the body of
* lengthy annotations should be shown.
......@@ -543,6 +516,11 @@ function AnnotationController(
};
};
var documentMeta = memoize(annotationMetadata.domainAndTitle);
vm.documentMeta = function () {
return documentMeta(vm.annotation);
};
init();
}
......
h/static/scripts/directive/test/annotation-test.js
View file @ 5408f2f3
......@@ -11,6 +11,12 @@ var util = require('./util');
var inject = angular.mock.inject;
var fakeDocumentMeta = {
domain: 'docs.io',
titleLink: 'http://docs.io/doc.html',
titleText: 'Dummy title',
};
/**
* Returns the annotation directive with helpers stubbed out.
*/
......@@ -19,10 +25,13 @@ function annotationDirective() {
var annotation = proxyquire('../annotation', {
angular: testUtil.noCallThru(angular),
'../filter/document-domain': noop,
'../filter/document-title': noop,
'../filter/persona': {
username: noop,
},
'../annotation-metadata': {
domainAndTitle: function (annot) {
return fakeDocumentMeta;
},
}
});
......@@ -684,6 +693,14 @@ describe('annotation', function() {
});
});
describe('#documentMeta()', function () {
it('returns the domain, title link and text for the annotation', function () {
var annot = fixtures.defaultAnnotation();
var controller = createDirective(annot).controller;
assert.deepEqual(controller.documentMeta(), fakeDocumentMeta);
});
});
describe('saving a new annotation', function() {
var annotation;
......@@ -853,30 +870,6 @@ describe('annotation', function() {
});
});
describe('onAnnotationUpdated()', function() {
it('updates vm.annotation', function() {
var parts = createDirective();
var updatedModel = {
id: parts.annotation.id,
links: {html: 'http://hyp.is/new-link'}
};
parts.controller.annotation = updatedModel;
$rootScope.$emit(events.ANNOTATION_UPDATED, updatedModel);
assert.equal(parts.controller.linkHTML, 'http://hyp.is/new-link');
});
it('doesn\'t update if a different annotation was updated', function() {
var parts = createDirective();
var updatedModel = {
id: 'different annotation id',
links: {html: 'http://hyp.is/new-link'},
};
$rootScope.$emit(events.ANNOTATION_UPDATED, updatedModel);
assert.notEqual(parts.controller.linkHTML, 'http://hyp.is/new-link');
});
});
describe('when another new annotation is created', function () {
it('removes the current annotation if empty', function () {
var annotation = fixtures.newEmptyAnnotation();
......@@ -973,7 +966,7 @@ describe('annotation', function() {
});
describe('annotation links', function () {
it('linkInContext uses the in-context links when available', function () {
it('uses the in-context links when available', function () {
var annotation = Object.assign({}, fixtures.defaultAnnotation(), {
links: {
html: 'https://test.hypothes.is/a/deadbeef',
......@@ -981,22 +974,20 @@ describe('annotation', function() {
},
});
var controller = createDirective(annotation).controller;
assert.equal(controller.linkInContext, annotation.links.incontext);
assert.equal(controller.links().incontext, annotation.links.incontext);
});
it('linkInContext falls back to the HTML link when in-context links are missing', function () {
it('falls back to the HTML link when in-context links are missing', function () {
var annotation = Object.assign({}, fixtures.defaultAnnotation(), {
links: {
html: 'https://test.hypothes.is/a/deadbeef',
},
});
var controller = createDirective(annotation).controller;
assert.equal(controller.linkInContext, annotation.links.html);
assert.equal(controller.links().html, annotation.links.html);
});
it('linkHTML uses the HTML link when available', function () {
it('uses the HTML link when available', function () {
var annotation = Object.assign({}, fixtures.defaultAnnotation(), {
links: {
html: 'https://test.hypothes.is/a/deadbeef',
......@@ -1004,22 +995,19 @@ describe('annotation', function() {
},
});
var controller = createDirective(annotation).controller;
assert.equal(controller.linkHTML, annotation.links.html);
assert.equal(controller.links().html, annotation.links.html);
});
it('linkInContext is blank when unknown', function () {
it('in-context link is blank when unknown', function () {
var annotation = fixtures.defaultAnnotation();
var controller = createDirective(annotation).controller;
assert.equal(controller.linkInContext, '');
assert.equal(controller.links().incontext, '');
});
it('linkHTML is blank when unknown', function () {
it('HTML is blank when unknown', function () {
var annotation = fixtures.defaultAnnotation();
var controller = createDirective(annotation).controller;
assert.equal(controller.linkHTML, '');
assert.equal(controller.links().html, '');
});
});
});
......
h/templates/client/annotation.html
View file @ 5408f2f3
......@@ -28,14 +28,14 @@
</span>
<i class="h-icon-border-color" ng-show="vm.isHighlight() && !vm.editing()" title="This is a highlight. Click 'edit' to add a note or tag."></i>
<span ng-if="::vm.showDocumentInfo">
<span class="annotation-citation" ng-if="vm.documentMeta.titleLink">
on "<a ng-href="{{vm.documentMeta.titleLink}}">{{vm.documentMeta.titleText}}</a>"
<span class="annotation-citation" ng-if="vm.documentMeta().titleLink">
on "<a ng-href="{{vm.documentMeta().titleLink}}">{{vm.documentMeta().titleText}}</a>"
</span>
<span class="annotation-citation" ng-if="!vm.documentMeta.titleLink">
on "{{vm.documentMeta.titleText}}"
<span class="annotation-citation" ng-if="!vm.documentMeta().titleLink">
on "{{vm.documentMeta().titleText}}"
</span>
<span class="annotation-citation-domain"
ng-if="vm.documentMeta.domain">({{vm.documentMeta.domain}})</span>
ng-if="vm.documentMeta().domain">({{vm.documentMeta().domain}})</span>
</span>
</span>
</span>
......@@ -45,7 +45,7 @@
<timestamp
class-name="'annotation-header__timestamp'"
timestamp="vm.updated()"
href="vm.linkHTML"
href="vm.links().html"
ng-if="!vm.editing() && vm.updated()"></timestamp>
</header>
......@@ -170,7 +170,7 @@
</button>
<annotation-share-dialog
group="vm.group()"
uri="vm.linkInContext"
uri="vm.links().incontext"
is-private="vm.state().isPrivate"
is-open="vm.showShareDialog"
on-close="vm.showShareDialog = false">
......
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