Skip to content

C
coopwire-hypothesis
Commit 9b3ef75c authored by Robert Knight's avatar Robert Knight
Browse files
Options

Merge pull request #2781 from hypothesis/AnnotationController-domain-model-view-model-refactor 

Annotation controller domain model view model refactor
parents 85ba902e 2874e51a
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 440 additions and 598 deletions
h/static/scripts/directive/annotation.js
View file @ 9b3ef75c
This diff is collapsed.
h/static/scripts/directive/test/thread-test.coffee
View file @ 9b3ef75c
......@@ -5,7 +5,6 @@ describe 'thread', ->
$element = null
$scope = null
controller = null
fakeGroups = null
fakeRender = null
fakeAnnotationUI = null
sandbox = null
......@@ -25,9 +24,6 @@ describe 'thread', ->
beforeEach module ($provide) ->
sandbox = sinon.sandbox.create()
fakeGroups = {
focused: sandbox.stub().returns({id: '__world__'})
}
fakeRender = sandbox.spy()
fakeAnnotationUI = {
hasSelectedAnnotations: ->
......@@ -35,7 +31,6 @@ describe 'thread', ->
isAnnotationSelected: (id) ->
selectedAnnotations.indexOf(id) != -1
}
$provide.value 'groups', fakeGroups
$provide.value 'render', fakeRender
$provide.value 'annotationUI', fakeAnnotationUI
return
......@@ -160,20 +155,6 @@ describe 'thread', ->
id: 123
group: 'wibble'
it 'is false for draft annotations not from the focused group', ->
# Set the focused group to one other than the annotation's group.
fakeGroups.focused.returns({id: 'foo'})
# Make the annotation into a "draft" annotation (make isNew() return
# true).
delete controller.container.message.id
assert.isFalse(controller.shouldShow())
it 'is true when the focused group does match', ->
fakeGroups.focused.returns({id: 'wibble'})
assert.isTrue(controller.shouldShow())
describe 'filters messages based on the selection', ->
messageID = 456
......
h/static/scripts/directive/thread.coffee
View file @ 9b3ef75c
......@@ -13,8 +13,8 @@ uuid = require('node-uuid')
# the collapsing behavior.
###
ThreadController = [
'$scope', 'groups', 'annotationUI'
($scope, groups, annotationUI) ->
'$scope', 'annotationUI'
($scope, annotationUI) ->
@container = null
@collapsed = true
@parent = null
......@@ -44,14 +44,6 @@ ThreadController = [
# current system state.
###
this.shouldShow = ->
# Hide "draft" annotations (new annotations that haven't been saved to
# the server yet) that don't belong to the focused group. These draft
# annotations persist across route reloads so they have to be hidden
# here.
group = this.container?.message?.group
if this.isNew() and group and group != groups.focused().id
return false
# when there is a selection, hide unselected annotations
annotationID = this.container?.message?.id
if annotationUI.hasSelectedAnnotations() &&
......
h/static/scripts/drafts.js
View file @ 9b3ef75c
/**
* The drafts service provides temporary storage for unsaved edits
* to new or existing annotations.
* The drafts service provides temporary storage for unsaved edits to new or
* existing annotations.
*
* A draft consists of:
*
* 1. `model` which is the original annotation domain model object which the
* draft is associated with. Domain model objects are never returned from
* the drafts service, they're only used to identify the correct draft to
* return.
*
* 2. `isPrivate` (boolean), `tags` (array of objects) and `text` (string)
* which are the user's draft changes to the annotation. These are returned
* from the drafts service by `drafts.get()`.
*
* A draft consists of a 'model' which is the original annotation
* which the draft is associated with and `changes' which is
* a set of edits to the original annotation.
*/
function DraftStore() {
this._drafts = [];
......@@ -31,21 +39,25 @@ function DraftStore() {
* unsaved drafts exist.
*/
this.unsaved = function unsaved() {
return this._drafts.filter(function (draft) {
return this._drafts.filter(function(draft) {
return !draft.model.id;
}).map(function (draft) {
}).map(function(draft) {
return draft.model;
});
}
};
/** Retrieve the draft changes for an annotation. */
this.get = function get(model) {
for (var i=0; i < this._drafts.length; i++) {
for (var i = 0; i < this._drafts.length; i++) {
if (match(this._drafts[i], model)) {
return this._drafts[i].changes;
return {
isPrivate: this._drafts[i].isPrivate,
tags: this._drafts[i].tags,
text: this._drafts[i].text,
};
}
}
}
};
/**
* Update the draft version for a given annotation, replacing any
......@@ -54,24 +66,26 @@ function DraftStore() {
this.update = function update(model, changes) {
var newDraft = {
model: model,
changes: changes,
isPrivate: changes.isPrivate,
tags: changes.tags,
text: changes.text
};
this.remove(model);
this._drafts.push(newDraft);
}
};
/** Remove the draft version of an annotation. */
this.remove = function remove(model) {
this._drafts = this._drafts.filter(function (draft) {
this._drafts = this._drafts.filter(function(draft) {
return !match(draft, model);
});
}
};
this.discard = function discard() {
this._drafts = [];
}
};
}
module.exports = function () {
module.exports = function() {
return new DraftStore();
};
h/static/scripts/test/drafts-test.js
View file @ 9b3ef75c
......@@ -11,23 +11,25 @@ describe('drafts', function () {
it('should save changes', function () {
var model = {id: 'foo'};
assert.notOk(drafts.get(model));
drafts.update(model, {text: 'edit'});
assert.deepEqual(drafts.get(model), {text: 'edit'});
drafts.update(model, {isPrivate:true, tags:['foo'], text:'edit'});
assert.deepEqual(
drafts.get(model),
{isPrivate: true, tags: ['foo'], text: 'edit'});
});
it('should replace existing drafts', function () {
var model = {id: 'foo'};
drafts.update(model, {text: 'foo'});
drafts.update(model, {text: 'bar'});
assert.deepEqual(drafts.get(model), {text: 'bar'});
drafts.update(model, {isPrivate:true, tags:['foo'], text:'foo'});
drafts.update(model, {isPrivate:true, tags:['foo'], text:'bar'});
assert.equal(drafts.get(model).text, 'bar');
});
it('should replace existing drafts with the same ID', function () {
var modelA = {id: 'foo'};
var modelB = {id: 'foo'};
drafts.update(modelA, {text: 'foo'});
drafts.update(modelB, {text: 'bar'});
assert.deepEqual(drafts.get(modelA), {text: 'bar'});
drafts.update(modelA, {isPrivate:true, tags:['foo'], text:'foo'});
drafts.update(modelB, {isPrivate:true, tags:['foo'], text:'bar'});
assert.equal(drafts.get(modelA).text, 'bar');
});
});
......
h/templates/client/annotation.html
View file @ 9b3ef75c
<header class="annotation-header" ng-if="!vm.annotation.user">
<header class="annotation-header" ng-if="!vm.user()">
<strong>You must be signed in to create annotations.</strong>
</header>
<div ng-if="vm.annotation.user">
<div ng-if="vm.user()">
<header class="annotation-header">
<!-- User -->
<span ng-if="vm.annotation.user">
<span ng-if="vm.user()">
<span>
<a class="annotation-user"
target="_blank"
ng-href="{{vm.baseURI}}u/{{vm.annotation.user}}"
>{{vm.annotation.user | persona}}</a>
ng-href="{{vm.baseURI}}u/{{vm.user()}}"
>{{vm.user() | persona}}</a>
</span>
<span class="annotation-collapsed-replies">
......@@ -26,17 +26,17 @@
target="_blank" ng-if="vm.group() && vm.group().url" href="{{vm.group().url}}">
<i class="h-icon-group"></i><span class="annotation-header__group-name">{{vm.group().name}}</span>
</a>
<span ng-show="vm.isPrivate()"
<span ng-show="vm.isPrivate"
title="This annotation is visible only to you.">
<i class="h-icon-lock"></i><span class="annotation-header__group-name" ng-show="!vm.group().url">Only me</span>
</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 class="annotation-citation"
ng-bind-html="vm.document | documentTitle"
ng-bind-html="vm.document() | documentTitle"
ng-if="!vm.isSidebar">
</span>
<span class="annotation-citation-domain"
ng-bind-html="vm.document | documentDomain"
ng-bind-html="vm.document() | documentDomain"
ng-if="!vm.isSidebar">
</span>
</span>
......@@ -47,15 +47,15 @@
<!-- Timestamp -->
<a class="annotation-timestamp"
target="_blank"
title="{{vm.annotation.updated | moment:'LLLL'}}"
ng-if="!vm.editing() && vm.annotation.updated"
ng-href="{{vm.baseURI}}a/{{vm.annotation.id}}"
title="{{vm.updated() | moment:'LLLL'}}"
ng-if="!vm.editing() && vm.updated()"
ng-href="{{vm.baseURI}}a/{{vm.id()}}"
>{{vm.timestamp}}</a>
</header>
<!-- Excerpts -->
<section class="annotation-quote-list"
ng-repeat="target in vm.annotation.target track by $index"
ng-repeat="target in vm.target() track by $index"
ng-if="vm.hasQuotes()">
<excerpt enabled="vm.feature('truncate_annotations')">
<blockquote class="annotation-quote"
......@@ -71,7 +71,7 @@
<!-- Body -->
<section name="text" class="annotation-body">
<excerpt enabled="vm.feature('truncate_annotations') && !vm.editing()">
<markdown ng-model="vm.annotation.text"
<markdown ng-model="vm.form.text"
read-only="!vm.editing()"
></markdown>
</excerpt>
......@@ -80,7 +80,7 @@
<!-- Tags -->
<div class="annotation-body form-field" ng-if="vm.editing()">
<tags-input ng-model="vm.annotation.tags"
<tags-input ng-model="vm.form.tags"
name="tags"
class="tags"
placeholder="Add tags…"
......@@ -94,9 +94,9 @@
</div>
<div class="annotation-body tags tags-read-only"
ng-if="vm.annotation.tags.length && !vm.editing()">
ng-if="vm.form.tags.length && !vm.editing()">
<ul class="tag-list">
<li class="tag-item" ng-repeat="tag in vm.annotation.tags">
<li class="tag-item" ng-repeat="tag in vm.form.tags">
<a href="/stream?q=tag:'{{tag.text|urlencode}}'" target="_blank">{{tag.text}}</a>
</li>
</ul>
......@@ -135,7 +135,7 @@
when="{'0': '', 'one': '1 reply', 'other': '{} replies'}"></a>
</div>
<div class="annotation-actions" ng-if="!vm.editing() && vm.annotation.id">
<div class="annotation-actions" ng-if="!vm.editing() && vm.id()">
<button class="small btn btn-clean"
ng-click="vm.reply()"
><i class="h-icon-reply btn-icon"></i> Reply</button>
......
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