Commit 1017c930 authored by Nick Stenning's avatar Nick Stenning Committed by GitHub

Merge pull request #3483 from hypothesis/highlight-standalone-reply

Highlight the specific reply on standalone annotation pages for replies
parents 88546abf 8107b000
...@@ -50,6 +50,9 @@ function initialState(settings) { ...@@ -50,6 +50,9 @@ function initialState(settings) {
// by the user even if they do not match the current search filter // by the user even if they do not match the current search filter
forceVisible: {}, forceVisible: {},
// IDs of annotations that should be highlighted
highlighted: [],
filterQuery: null, filterQuery: null,
// Key by which annotations are currently sorted. // Key by which annotations are currently sorted.
...@@ -62,6 +65,7 @@ function initialState(settings) { ...@@ -62,6 +65,7 @@ function initialState(settings) {
var types = { var types = {
SELECT_ANNOTATIONS: 'SELECT_ANNOTATIONS', SELECT_ANNOTATIONS: 'SELECT_ANNOTATIONS',
FOCUS_ANNOTATIONS: 'FOCUS_ANNOTATIONS', FOCUS_ANNOTATIONS: 'FOCUS_ANNOTATIONS',
HIGHLIGHT_ANNOTATIONS: 'HIGHLIGHT_ANNOTATIONS',
SET_HIGHLIGHTS_VISIBLE: 'SET_HIGHLIGHTS_VISIBLE', SET_HIGHLIGHTS_VISIBLE: 'SET_HIGHLIGHTS_VISIBLE',
SET_FORCE_VISIBLE: 'SET_FORCE_VISIBLE', SET_FORCE_VISIBLE: 'SET_FORCE_VISIBLE',
SET_EXPANDED: 'SET_EXPANDED', SET_EXPANDED: 'SET_EXPANDED',
...@@ -113,6 +117,8 @@ function reducer(state, action) { ...@@ -113,6 +117,8 @@ function reducer(state, action) {
return Object.assign({}, state, {forceVisible: action.forceVisible}); return Object.assign({}, state, {forceVisible: action.forceVisible});
case types.SET_EXPANDED: case types.SET_EXPANDED:
return Object.assign({}, state, {expanded: action.expanded}); return Object.assign({}, state, {expanded: action.expanded});
case types.HIGHLIGHT_ANNOTATIONS:
return Object.assign({}, state, {highlighted: action.highlighted});
case types.SET_FILTER_QUERY: case types.SET_FILTER_QUERY:
return Object.assign({}, state, { return Object.assign({}, state, {
filterQuery: action.query, filterQuery: action.query,
...@@ -311,5 +317,18 @@ module.exports = function (settings) { ...@@ -311,5 +317,18 @@ module.exports = function (settings) {
key: key, key: key,
}); });
}, },
/**
* Highlight annotations with the given `ids`.
*
* This is used to indicate the specific annotation in a thread that was
* linked to for example.
*/
highlightAnnotations: function (ids) {
store.dispatch({
type: types.HIGHLIGHT_ANNOTATIONS,
highlighted: ids,
});
},
}; };
}; };
...@@ -58,23 +58,27 @@ function AnnotationViewerController ( ...@@ -58,23 +58,27 @@ function AnnotationViewerController (
this.ready = fetchThread(store, id).then(function (annots) { this.ready = fetchThread(store, id).then(function (annots) {
annotationMapper.loadAnnotations(annots); annotationMapper.loadAnnotations(annots);
var topLevelID = annots.filter(function (annot) { var topLevelAnnot = annots.filter(function (annot) {
return (annot.references || []).length === 0; return (annot.references || []).length === 0;
})[0]; })[0];
if (!topLevelID) { if (!topLevelAnnot) {
return; return;
} }
streamFilter streamFilter
.setMatchPolicyIncludeAny() .setMatchPolicyIncludeAny()
.addClause('/references', 'one_of', topLevelID, true) .addClause('/references', 'one_of', topLevelAnnot.id, true)
.addClause('/id', 'equals', topLevelID, true); .addClause('/id', 'equals', topLevelAnnot.id, true);
streamer.setConfig('filter', { filter: streamFilter.getFilter() }); streamer.setConfig('filter', { filter: streamFilter.getFilter() });
annots.forEach(function (annot) { annots.forEach(function (annot) {
annotationUI.setCollapsed(annot.id, false); annotationUI.setCollapsed(annot.id, false);
}); });
if (topLevelAnnot.id !== id) {
annotationUI.highlightAnnotations([id]);
}
}); });
} }
......
...@@ -28,6 +28,13 @@ var DEFAULT_THREAD_STATE = { ...@@ -28,6 +28,13 @@ var DEFAULT_THREAD_STATE = {
* including any which have been hidden by filters. * including any which have been hidden by filters.
*/ */
totalChildren: 0, totalChildren: 0,
/**
* The highlight state of this annotation:
* undefined - Do not (de-)emphasize this annotation
* 'dim' - De-emphasize this annotation
* 'highlight' - Emphasize this annotation
*/
highlightState: undefined,
}; };
/** /**
...@@ -234,6 +241,8 @@ var defaultOpts = { ...@@ -234,6 +241,8 @@ var defaultOpts = {
* Mapping of annotation IDs to expansion states. * Mapping of annotation IDs to expansion states.
*/ */
expanded: {}, expanded: {},
/** List of highlighted annotation IDs */
highlighted: [],
/** /**
* Less-than comparison function used to compare annotations in order to sort * Less-than comparison function used to compare annotations in order to sort
* the top-level thread. * the top-level thread.
...@@ -291,10 +300,17 @@ function buildThread(annotations, opts) { ...@@ -291,10 +300,17 @@ function buildThread(annotations, opts) {
}); });
} }
// Set the visibility of threads based on whether they match // Set the visibility and highlight states of threads
// the current search filter
thread = mapThread(thread, function (thread) { thread = mapThread(thread, function (thread) {
var highlightState;
if (opts.highlighted.length > 0) {
var isHighlighted = thread.annotation &&
opts.highlighted.indexOf(thread.id) !== -1;
highlightState = isHighlighted ? 'highlight' : 'dim';
}
return Object.assign({}, thread, { return Object.assign({}, thread, {
highlightState: highlightState,
visible: thread.visible && visible: thread.visible &&
thread.annotation && thread.annotation &&
shouldShowThread(thread.annotation), shouldShowThread(thread.annotation),
......
...@@ -62,6 +62,8 @@ function AnnotationThreadController() { ...@@ -62,6 +62,8 @@ function AnnotationThreadController() {
annotation: true, annotation: true,
'annotation--reply': this.thread.depth > 0, 'annotation--reply': this.thread.depth > 0,
'is-collapsed': this.thread.collapsed, 'is-collapsed': this.thread.collapsed,
'is-highlighted': this.thread.highlightState === 'highlight',
'is-dimmed': this.thread.highlightState === 'dim',
}; };
}; };
......
...@@ -68,6 +68,7 @@ function RootThread($rootScope, annotationUI, searchFilter, viewFilter) { ...@@ -68,6 +68,7 @@ function RootThread($rootScope, annotationUI, searchFilter, viewFilter) {
return buildThread(state.annotations, { return buildThread(state.annotations, {
forceVisible: truthyKeys(state.forceVisible), forceVisible: truthyKeys(state.forceVisible),
expanded: state.expanded, expanded: state.expanded,
highlighted: state.highlighted,
selected: truthyKeys(state.selectedAnnotationMap || {}), selected: truthyKeys(state.selectedAnnotationMap || {}),
sortCompareFn: sortFn, sortCompareFn: sortFn,
filterFn: filterFn, filterFn: filterFn,
......
...@@ -218,4 +218,11 @@ describe('annotationUI', function () { ...@@ -218,4 +218,11 @@ describe('annotationUI', function () {
assert.deepEqual(annotationUI.getState().expanded, {}); assert.deepEqual(annotationUI.getState().expanded, {});
}); });
}); });
describe('#highlightAnnotations()', function () {
it('sets the highlighted annotations', function () {
annotationUI.highlightAnnotations(['id1', 'id2']);
assert.deepEqual(annotationUI.getState().highlighted, ['id1', 'id2']);
});
});
}); });
...@@ -58,7 +58,11 @@ describe('AnnotationViewerController', function () { ...@@ -58,7 +58,11 @@ describe('AnnotationViewerController', function () {
$scope: { $scope: {
search: {}, search: {},
}, },
annotationUI: {setCollapsed: sinon.stub(), subscribe: sinon.stub()}, annotationUI: {
setCollapsed: sinon.stub(),
highlightAnnotations: sinon.stub(),
subscribe: sinon.stub()
},
rootThread: {thread: sinon.stub()}, rootThread: {thread: sinon.stub()},
streamer: { setConfig: function () {} }, streamer: { setConfig: function () {} },
store: opts.store, store: opts.store,
...@@ -95,6 +99,17 @@ describe('AnnotationViewerController', function () { ...@@ -95,6 +99,17 @@ describe('AnnotationViewerController', function () {
sinon.match(fakeStore.annots)); sinon.match(fakeStore.annots));
}); });
}); });
it('does not highlight any annotations', function () {
var fakeStore = new FakeStore([
{id: 'test_annotation_id'},
{id: 'test_reply_id', references: ['test_annotation_id']},
]);
var controller = createController({store: fakeStore});
return controller.ctrl.ready.then(function () {
assert.notCalled(controller.annotationUI.highlightAnnotations);
});
});
}); });
describe('the standalone view for a reply', function () { describe('the standalone view for a reply', function () {
...@@ -121,5 +136,17 @@ describe('AnnotationViewerController', function () { ...@@ -121,5 +136,17 @@ describe('AnnotationViewerController', function () {
assert.calledWith(controller.annotationUI.setCollapsed, 'test_annotation_id', false); assert.calledWith(controller.annotationUI.setCollapsed, 'test_annotation_id', false);
}); });
}); });
it('highlights the reply', function () {
var fakeStore = new FakeStore([
{id: 'parent_id'},
{id: 'test_annotation_id', references: ['parent_id']},
]);
var controller = createController({store: fakeStore});
return controller.ctrl.ready.then(function () {
assert.calledWith(controller.annotationUI.highlightAnnotations,
sinon.match(['test_annotation_id']));
});
});
}); });
}); });
...@@ -339,4 +339,23 @@ describe('build-thread', function () { ...@@ -339,4 +339,23 @@ describe('build-thread', function () {
assert.deepEqual(thread[0].children[0].depth, 1); assert.deepEqual(thread[0].children[0].depth, 1);
}); });
}); });
describe('highlighting', function () {
it('does not set highlight state when none are highlighted', function () {
var thread = createThread(SIMPLE_FIXTURE, {}, ['dimmed']);
thread.forEach(function (child) {
assert.equal(child.highlightState, undefined);
});
});
it('highlights annotations', function () {
var thread = createThread(SIMPLE_FIXTURE, {highlighted: ['1']}, ['highlightState']);
assert.equal(thread[0].highlightState, 'highlight');
});
it('dims annotations which are not highlighted', function () {
var thread = createThread(SIMPLE_FIXTURE, {highlighted: ['1']}, ['highlightState']);
assert.equal(thread[1].highlightState, 'dim');
});
});
}); });
...@@ -31,14 +31,15 @@ describe('rootThread', function () { ...@@ -31,14 +31,15 @@ describe('rootThread', function () {
fakeAnnotationUI = { fakeAnnotationUI = {
state: { state: {
annotations: [], annotations: [],
visibleHighlights: false,
focusedAnnotationMap: null,
selectedAnnotationMap: null,
expanded: {}, expanded: {},
forceVisible: {},
filterQuery: null, filterQuery: null,
focusedAnnotationMap: null,
forceVisible: {},
highlighted: [],
selectedAnnotationMap: null,
sortKey: 'Location', sortKey: 'Location',
sortKeysAvailable: ['Location'], sortKeysAvailable: ['Location'],
visibleHighlights: false,
}, },
getState: function () { getState: function () {
...@@ -120,6 +121,16 @@ describe('rootThread', function () { ...@@ -120,6 +121,16 @@ describe('rootThread', function () {
forceVisible: ['id1', 'id2'], forceVisible: ['id1', 'id2'],
})); }));
}); });
it('passes the highlighted set to buildThread()', function () {
fakeAnnotationUI.state = Object.assign({}, fakeAnnotationUI.state, {
highlighted: ['id1', 'id2'],
});
rootThread.thread(fakeAnnotationUI.state);
assert.calledWith(fakeBuildThread, [], sinon.match({
highlighted: ['id1', 'id2'],
}));
});
}); });
describe('when the sort order changes', function () { describe('when the sort order changes', function () {
......
...@@ -49,6 +49,25 @@ $annotation-card-left-padding: 10px; ...@@ -49,6 +49,25 @@ $annotation-card-left-padding: 10px;
position: relative; position: relative;
} }
.annotation.is-dimmed {
// Lighten the username and bodies of dimmed annotations to make other
// annotations which are not dimmed stand out
.annotation-header__user,
.annotation-body {
color: $grey-5;
}
}
.annotation.is-highlighted {
// Slightly darken the username and bodies of highlighted annotations to
// make them stand out next to others, which will have the `is-dimmed` state
// set
.annotation-header__user,
.annotation-body {
color: $grey-7;
}
}
.annotation-link { .annotation-link {
@include font-small; @include font-small;
color: $grey-4; color: $grey-4;
......
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