Commit 2568a483 authored by Robert Knight's avatar Robert Knight

Extract the top-level thread list into its own component

 * Extract thread list into its own component for better encapsulation
   and easier testing

 * Rename `annotation-card` to `thread-list__card` and move it to
   the component styling file for `thread-list`.

   Unfortunately a couple of visual effects still require it
   to be referenced in annotation.scss

 * Remove ng-show hack in thread list

   Remove the "ng-show" attribute which was added as a hack for reasons
   which are no longer applicable. See
   https://github.com/hypothesis/h/issues/2642#issuecomment-150629305
   for original context.

 * Remove unused js-hover class and the code that supports it

   It turns out that this class is no longer referenced in code or
   applicable styling.
parent 26327ae3
......@@ -41,16 +41,8 @@ function AnnotationViewerController (
$location.path('/stream').search('q', query);
};
function thread() {
return rootThread.thread(annotationUI.getState());
}
annotationUI.subscribe(function () {
$scope.virtualThreadList = {
visibleThreads: thread().children,
offscreenUpperHeight: '0px',
offscreenLowerHeight: '0px',
};
$scope.rootThread = rootThread.thread(annotationUI.getState());
});
$scope.setCollapsed = function (id, collapsed) {
......
......@@ -163,6 +163,7 @@ module.exports = angular.module('h', [
.directive('spinner', require('./directive/spinner'))
.directive('statusButton', require('./directive/status-button'))
.directive('tagEditor', require('./directive/tag-editor'))
.directive('threadList', require('./directive/thread-list'))
.directive('timestamp', require('./directive/timestamp'))
.directive('topBar', require('./directive/top-bar'))
.directive('windowScroll', require('./directive/window-scroll'))
......
'use strict';
var angular = require('angular');
var EventEmitter = require('tiny-emitter');
var inherits = require('inherits');
var immutable = require('seamless-immutable');
var events = require('../../events');
var threadList = require('../thread-list');
var util = require('./util');
var annotFixtures = immutable({
annotation: {$$tag: 't1', id: '1', text: 'text'},
reply: {
$$tag: 't2',
id: '2',
references: ['1'],
text: 'areply',
},
highlight: {$highlight: true, $$tag: 't3', id: '3'},
});
var threadFixtures = immutable({
thread: {
children: [{
id: annotFixtures.annotation.id,
annotation: annotFixtures.annotation,
children: [{
id: annotFixtures.reply.id,
annotation: annotFixtures.reply,
children: [],
visible: true,
}],
visible: true,
},{
id: annotFixtures.highlight.id,
annotation: annotFixtures.highlight,
}],
},
});
var fakeVirtualThread;
function FakeVirtualThreadList($scope, $window, rootThread) {
fakeVirtualThread = this; // eslint-disable-line consistent-this
var thread = rootThread;
this.setRootThread = function (_thread) {
thread = _thread;
};
this.notify = function () {
this.emit('changed', {
offscreenLowerHeight: 10,
offscreenUpperHeight: 20,
visibleThreads: thread.children,
});
};
this.detach = sinon.stub();
this.yOffsetOf = function () {
return 42;
};
}
inherits(FakeVirtualThreadList, EventEmitter);
describe('threadList', function () {
function createThreadList(inputs) {
var defaultInputs = {
thread: threadFixtures.thread,
onClearSelection: sinon.stub(),
onForceVisible: sinon.stub(),
onFocus: sinon.stub(),
onSelect: sinon.stub(),
onSetCollapsed: sinon.stub(),
};
var element = util.createDirective(document, 'threadList',
Object.assign({}, defaultInputs, inputs));
return element;
}
before(function () {
angular.module('app', [])
.directive('threadList', threadList);
});
beforeEach(function () {
angular.mock.module('app', {
VirtualThreadList: FakeVirtualThreadList,
});
});
it('displays the children of the root thread', function () {
var element = createThreadList();
fakeVirtualThread.notify();
element.scope.$digest();
var children = element[0].querySelectorAll('annotation-thread');
assert.equal(children.length, 2);
});
describe('when a new annotation is created', function () {
var scrollSpy;
beforeEach(function () {
scrollSpy = sinon.stub(window, 'scroll');
});
afterEach(function () {
scrollSpy.restore();
});
it('scrolls the annotation into view', function () {
var element = createThreadList();
var annot = annotFixtures.annotation;
element.scope.$broadcast(events.BEFORE_ANNOTATION_CREATED, annot);
assert.called(scrollSpy);
});
it('does not scroll the annotation into view if it is a reply', function () {
var element = createThreadList();
var reply = annotFixtures.reply;
element.scope.$broadcast(events.BEFORE_ANNOTATION_CREATED, reply);
assert.notCalled(scrollSpy);
});
it('does not scroll the annotation into view if it is a highlight', function () {
var element = createThreadList();
var highlight = annotFixtures.highlight;
element.scope.$broadcast(events.BEFORE_ANNOTATION_CREATED, highlight);
assert.notCalled(scrollSpy);
});
it('clears the selection', function () {
var inputs = { onClearSelection: sinon.stub() };
var element = createThreadList(inputs);
element.scope.$broadcast(events.BEFORE_ANNOTATION_CREATED,
annotFixtures.annotation);
assert.called(inputs.onClearSelection);
});
});
it('calls onFocus() when the user hovers an annotation', function () {
var inputs = {
onFocus: {
args: ['annotation'],
callback: sinon.stub(),
},
};
var element = createThreadList(inputs);
fakeVirtualThread.notify();
element.scope.$digest();
var annotation = element[0].querySelector('.thread-list__card');
util.sendEvent(annotation, 'mouseover');
assert.calledWithMatch(inputs.onFocus.callback,
sinon.match(annotFixtures.annotation));
});
it('calls onSelect() when a user clicks an annotation', function () {
var inputs = {
onSelect: {
args: ['annotation'],
callback: sinon.stub(),
},
};
var element = createThreadList(inputs);
fakeVirtualThread.notify();
element.scope.$digest();
var annotation = element[0].querySelector('.thread-list__card');
util.sendEvent(annotation, 'click');
assert.calledWithMatch(inputs.onSelect.callback,
sinon.match(annotFixtures.annotation));
});
});
'use strict';
var events = require('../events');
var metadata = require('../annotation-metadata');
/**
* Component which displays a virtualized list of annotation threads.
*/
var scopeTimeout = require('../util/scope-timeout');
/**
* Returns the height of the thread for an annotation if it exists in the view
* or undefined otherwise.
*/
function getThreadHeight(id) {
var threadElement = document.getElementById(id);
if (!threadElement) {
return null;
}
// Get the height of the element inside the border-box, excluding
// top and bottom margins.
var elementHeight = threadElement.getBoundingClientRect().height;
var style = window.getComputedStyle(threadElement);
// Get the bottom margin of the element. style.margin{Side} will return
// values of the form 'Npx', from which we extract 'N'.
var marginHeight = parseFloat(style.marginTop) +
parseFloat(style.marginBottom);
return elementHeight + marginHeight;
}
// @ngInject
function ThreadListController($scope, VirtualThreadList) {
// `visibleThreads` keeps track of the subset of all threads matching the
// current filters which are in or near the viewport and the view then renders
// only those threads, using placeholders above and below the visible threads
// to reserve space for threads which are not actually rendered.
var self = this;
var visibleThreads = new VirtualThreadList($scope, window, this.thread);
visibleThreads.on('changed', function (state) {
self.virtualThreadList = {
visibleThreads: state.visibleThreads,
offscreenUpperHeight: state.offscreenUpperHeight + 'px',
offscreenLowerHeight: state.offscreenLowerHeight + 'px',
};
scopeTimeout($scope, function () {
state.visibleThreads.forEach(function (thread) {
var height = getThreadHeight(thread.id);
if (!height) {
return;
}
visibleThreads.setThreadHeight(thread.id, height);
});
}, 50);
});
/**
* Return the vertical scroll offset for the document in order to position the
* annotation thread with a given `id` or $$tag at the top-left corner
* of the view.
*/
function scrollOffset(id) {
// Note: This assumes that the element occupies the entire height of the
// containing document. It would be preferable if only the contents of the
// <thread-list> itself scrolled.
var maxYOffset = document.body.clientHeight - window.innerHeight;
return Math.min(maxYOffset, visibleThreads.yOffsetOf(id));
}
/** Scroll the annotation with a given ID or $$tag into view. */
function scrollIntoView(id) {
var estimatedYOffset = scrollOffset(id);
window.scroll(0, estimatedYOffset);
// As a result of scrolling the sidebar, the target scroll offset for
// annotation `id` might have changed as a result of:
//
// 1. Heights of some cards above `id` changing from an initial estimate to
// an actual measured height after the card is rendered.
// 2. The height of the document changing as a result of any cards heights'
// changing. This may affect the scroll offset if the original target
// was near to the bottom of the list.
//
// So we wait briefly after the view is scrolled then check whether the
// estimated Y offset changed and if so, trigger scrolling again.
scopeTimeout($scope, function () {
var newYOffset = scrollOffset(id);
if (newYOffset !== estimatedYOffset) {
scrollIntoView(id);
}
}, 200);
}
$scope.$on(events.BEFORE_ANNOTATION_CREATED, function (event, annotation) {
if (annotation.$highlight || metadata.isReply(annotation)) {
return;
}
self.onClearSelection();
scrollIntoView(annotation.$$tag);
});
this.$onChanges = function (changes) {
if (changes.thread) {
visibleThreads.setRootThread(changes.thread.currentValue);
}
};
this.$onDestroy = function () {
visibleThreads.detach();
};
}
module.exports = function () {
return {
bindToController: true,
controller: ThreadListController,
controllerAs: 'vm',
restrict: 'E',
scope: {
/** The root thread to be displayed by the thread list. */
thread: '<',
showDocumentInfo: '<',
/**
* Called when the user clicks a link to show an annotation that does not
* match the current filter.
*/
onForceVisible: '&',
/** Called when the user focuses an annotation by hovering it. */
onFocus: '&',
/** Called when a user selects an annotation. */
onSelect: '&',
/** Called when a user toggles the expansion state of an annotation thread. */
onChangeCollapsed: '&',
/** Called to clear the current selection. */
onClearSelection: '&',
},
template: require('../../../templates/client/thread_list.html'),
};
};
......@@ -62,15 +62,8 @@ module.exports = class StreamController
update: (q) -> $location.search({q: q})
}
thread = ->
rootThread.thread(annotationUI.getState())
annotationUI.subscribe( ->
$scope.virtualThreadList = {
visibleThreads: thread().children,
offscreenUpperHeight: '0px',
offscreenLowerHeight: '0px',
};
$scope.rootThread = rootThread.thread(annotationUI.getState())
);
# Sort the stream so that the newest annotations are at the top
......
......@@ -36,16 +36,6 @@ function FakeRootThread() {
}
inherits(FakeRootThread, EventEmitter);
function FakeVirtualThreadList() {
this.setRootThread = sinon.stub();
this.setThreadHeight = sinon.stub();
this.detach = sinon.stub();
this.yOffsetOf = function () {
return 100;
};
}
inherits(FakeVirtualThreadList, EventEmitter);
describe('WidgetController', function () {
var $rootScope;
var $scope;
......@@ -122,7 +112,6 @@ describe('WidgetController', function () {
search: sinon.stub(),
};
$provide.value('VirtualThreadList', FakeVirtualThreadList);
$provide.value('annotationMapper', fakeAnnotationMapper);
$provide.value('crossframe', fakeCrossFrame);
$provide.value('drafts', fakeDrafts);
......@@ -307,47 +296,6 @@ describe('WidgetController', function () {
});
});
describe('when a new annotation is created', function () {
var windowScroll;
beforeEach(function () {
$scope.clearSelection = sinon.stub();
windowScroll = sinon.stub(window, 'scroll');
});
afterEach(function () {
windowScroll.restore();
});
/**
* It should clear any selection that exists in the sidebar before
* creating a new annotation. Otherwise the new annotation with its
* form open for the user to type in won't be visible because it's
* not part of the selection.
*/
it('clears the selection', function () {
$rootScope.$broadcast('beforeAnnotationCreated', {});
assert.called($scope.clearSelection);
});
it('does not clear the selection if the new annotation is a highlight', function () {
$rootScope.$broadcast('beforeAnnotationCreated', {$highlight: true});
assert.notCalled($scope.clearSelection);
});
it('does not clear the selection if the new annotation is a reply', function () {
$rootScope.$broadcast('beforeAnnotationCreated', {
references: ['parent-id'],
});
assert.notCalled($scope.clearSelection);
});
it('scrolls the viewport to the new annotation', function () {
$rootScope.$broadcast('beforeAnnotationCreated', {$$tag: '123'});
assert.called(windowScroll);
});
});
describe('direct linking messages', function () {
beforeEach(function () {
......
......@@ -4,7 +4,6 @@ var SearchClient = require('./search-client');
var events = require('./events');
var memoize = require('./util/memoize');
var metadata = require('./annotation-metadata');
var scopeTimeout = require('./util/scope-timeout');
var tabCounts = require('./tab-counts');
var uiConstants = require('./ui-constants');
......@@ -36,46 +35,16 @@ function groupIDFromSelection(selection, results) {
// @ngInject
module.exports = function WidgetController(
$scope, annotationUI, crossframe, annotationMapper, drafts,
features, groups, rootThread, settings, streamer, streamFilter, store,
VirtualThreadList
features, groups, rootThread, settings, streamer, streamFilter, store
) {
/**
* Returns the height of the thread for an annotation if it exists in the view
* or undefined otherwise.
*/
function getThreadHeight(id) {
var threadElement = document.getElementById(id);
if (!threadElement) {
return null;
}
// Get the height of the element inside the border-box, excluding
// top and bottom margins.
var elementHeight = threadElement.getBoundingClientRect().height;
var style = window.getComputedStyle(threadElement);
// Get the bottom margin of the element. style.margin{Side} will return
// values of the form 'Npx', from which we extract 'N'.
var marginHeight = parseFloat(style.marginTop) +
parseFloat(style.marginBottom);
return elementHeight + marginHeight;
}
function thread() {
return rootThread.thread(annotationUI.getState());
}
// `visibleThreads` keeps track of the subset of all threads matching the
// current filters which are in or near the viewport and the view then renders
// only those threads, using placeholders above and below the visible threads
// to reserve space for threads which are not actually rendered.
var visibleThreads = new VirtualThreadList($scope, window, thread());
var unsubscribeAnnotationUI = annotationUI.subscribe(function () {
var state = annotationUI.getState();
visibleThreads.setRootThread(thread());
$scope.rootThread = thread();
$scope.selectedTab = state.selectedTab;
var counts = tabCounts(state.annotations, {
......@@ -92,28 +61,6 @@ module.exports = function WidgetController(
$scope.$on('$destroy', unsubscribeAnnotationUI);
visibleThreads.on('changed', function (state) {
$scope.virtualThreadList = {
visibleThreads: state.visibleThreads,
offscreenUpperHeight: state.offscreenUpperHeight + 'px',
offscreenLowerHeight: state.offscreenLowerHeight + 'px',
};
scopeTimeout($scope, function () {
state.visibleThreads.forEach(function (thread) {
var height = getThreadHeight(thread.id);
if (!height) {
return;
}
visibleThreads.setThreadHeight(thread.id, height);
});
}, 50);
});
$scope.$on('$destroy', function () {
visibleThreads.detach();
});
function annotationExists(id) {
return annotationUI.getState().annotations.some(function (annot) {
return annot.id === id;
......@@ -346,13 +293,6 @@ module.exports = function WidgetController(
$scope.focus = focusAnnotation;
$scope.scrollTo = scrollToAnnotation;
$scope.hasFocus = function (annotation) {
if (!annotation || !annotationUI.getState().focusedAnnotationMap) {
return false;
}
return annotation.$$tag in annotationUI.getState().focusedAnnotationMap;
};
$scope.selectedAnnotationCount = function () {
var selection = annotationUI.getState().selectedAnnotationMap;
if (!selection) {
......@@ -404,46 +344,4 @@ module.exports = function WidgetController(
$scope.topLevelThreadCount = function () {
return thread().totalChildren;
};
/**
* Return the vertical scroll offset for the document in order to position the
* annotation thread with a given `id` or $$tag at the top-left corner
* of the view.
*/
function scrollOffset(id) {
var maxYOffset = document.body.clientHeight - window.innerHeight;
return Math.min(maxYOffset, visibleThreads.yOffsetOf(id));
}
/** Scroll the annotation with a given ID or $$tag into view. */
function scrollIntoView(id) {
var estimatedYOffset = scrollOffset(id);
window.scroll(0, estimatedYOffset);
// As a result of scrolling the sidebar, the target scroll offset for
// annotation `id` might have changed as a result of:
//
// 1. Heights of some cards above `id` changing from an initial estimate to
// an actual measured height after the card is rendered.
// 2. The height of the document changing as a result of any cards heights'
// changing. This may affect the scroll offset if the original target
// was near to the bottom of the list.
//
// So we wait briefly after the view is scrolled then check whether the
// estimated Y offset changed and if so, trigger scrolling again.
scopeTimeout($scope, function () {
var newYOffset = scrollOffset(id);
if (newYOffset !== estimatedYOffset) {
scrollIntoView(id);
}
}, 200);
}
$scope.$on(events.BEFORE_ANNOTATION_CREATED, function (event, data) {
if (data.$highlight || (data.references && data.references.length > 0)) {
return;
}
$scope.clearSelection();
scrollIntoView(data.$$tag);
});
};
@import "mixins/icons";
//ANNOTATION CARD////////////////////////////////
.annotation-card {
box-shadow: 0px 1px 1px 0px rgba(0, 0, 0, 0.10);
border-radius: 2px;
cursor: pointer;
padding: $layout-h-margin;
background-color: $white;
}
.annotation-card:hover {
box-shadow: 0px 2px 3px 0px rgba(0, 0, 0, 0.15);
.thread-list__card:hover {
.annotation-header__user {
color: $grey-7;
}
......@@ -26,7 +14,7 @@
// When hovering a top-level annotation, show the footer in a hovered state.
// When hovering a reply (at any level), show the reply's own footer in
// a hovered state and also the footer of the top-level annotation.
.annotation-card:hover > .annotation,
.thread-list__card:hover > .annotation,
.annotation:hover {
.annotation-replies__link,
.annotation-replies__count,
......
......@@ -27,6 +27,7 @@ $base-line-height: 20px;
@import './simple-search';
@import './spinner';
@import './tags-input';
@import './thread-list';
@import './tooltip';
@import './top-bar';
......@@ -108,20 +109,6 @@ body {
}
}
.thread-list {
& > * {
// Default spacing between items in the annotation card list
margin-bottom: .72em;
}
}
.thread-list__spacer {
// This is a hidden element which is used to reserve space for off-screen
// threads, so it should not occupy any space other than that set via its
// 'height' inline style property.
margin: 0;
}
.annotation-unavailable-message {
display: flex;
flex-direction: column;
......
.thread-list {
& > * {
// Default spacing between items in the annotation card list
margin-bottom: .72em;
}
}
.thread-list__card {
box-shadow: 0px 1px 1px 0px rgba(0, 0, 0, 0.10);
border-radius: 2px;
cursor: pointer;
padding: $layout-h-margin;
background-color: $white;
&:hover {
box-shadow: 0px 2px 3px 0px rgba(0, 0, 0, 0.15);
}
}
.thread-list__spacer {
// This is a hidden element which is used to reserve space for off-screen
// threads, so it should not occupy any space other than that set via its
// 'height' inline style property.
margin: 0;
}
<ul class="thread-list">
<li class="thread-list__spacer"
ng-style="{height: vm.virtualThreadList.offscreenUpperHeight}"></li>
<li id="{{child.id}}"
class="thread-list__card"
ng-mouseenter="vm.onFocus({annotation: child.annotation})"
ng-click="vm.onSelect({annotation: child.annotation})"
ng-mouseleave="vm.onFocus({annotation: null})"
ng-repeat="child in vm.virtualThreadList.visibleThreads track by child.id">
<annotation-thread
thread="child"
show-document-info="vm.showDocumentInfo"
on-change-collapsed="vm.onChangeCollapsed({id: id, collapsed: collapsed})"
on-force-visible="vm.onForceVisible({thread: thread})">
</annotation-thread>
</li>
<li class="thread-list__spacer"
ng-style="{height: vm.virtualThreadList.offscreenLowerHeight}"></li>
</ul>
......@@ -8,14 +8,7 @@
total-orphans="totalOrphans">
</selection-tabs>
<!-- Annotation thread view
(See gh2642 for rationale for 'ng-show="true"')
-->
<ul class="thread-list ng-hide"
ng-show="true"
window-scroll="loadMore(20)">
<search-status-bar
<search-status-bar
ng-show="!isLoading()"
ng-if="!isStream"
filter-active="!!search.query()"
......@@ -27,8 +20,9 @@
selected-tab="selectedTab"
total-annotations="totalAnnotations"
total-notes="totalNotes">
</search-status-bar>
<li class="annotation-unavailable-message"
</search-status-bar>
<div class="annotation-unavailable-message"
ng-if="selectedAnnotationUnavailable()">
<div class="annotation-unavailable-message__icon"></div>
<p class="annotation-unavailable-message__label">
......@@ -43,27 +37,20 @@
You do not have permission to view this annotation.
</span>
</p>
</li>
<li class="thread-list__spacer"
ng-style="{height: virtualThreadList.offscreenUpperHeight}"></li>
<li id="{{child.id}}"
class="annotation-card"
ng-class="{'js-hover': hasFocus(child.annotation)}"
ng-mouseenter="focus(child.annotation)"
ng-click="scrollTo(child.annotation)"
ng-mouseleave="focus()"
ng-repeat="child in virtualThreadList.visibleThreads track by child.id">
<annotation-thread
thread="child"
show-document-info="::!isSidebar"
</div>
<span window-scroll="loadMore(20)">
<thread-list
on-change-collapsed="setCollapsed(id, collapsed)"
on-force-visible="forceVisible(thread)">
</annotation-thread>
</li>
<li class="thread-list__spacer"
ng-style="{height: virtualThreadList.offscreenLowerHeight}"></li>
<loggedout-message ng-if="isSidebar && shouldShowLoggedOutMessage()"
on-clear-selection="clearSelection()"
on-focus="focus(annotation)"
on-force-visible="forceVisible(thread)"
on-select="scrollTo(annotation)"
show-document-info="::!isSidebar"
thread="rootThread">
</thread-list>
</span>
<loggedout-message ng-if="isSidebar && shouldShowLoggedOutMessage()"
on-login="login()" ng-cloak>
</loggedout-message>
</ul>
<!-- / Thread view -->
</loggedout-message>
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