Commit 2d25a77e authored by Lyza Danger Gardner's avatar Lyza Danger Gardner Committed by Lyza Gardner

Add `$cluster` client property to annotation objects

This property is set when an annotation object is initialized in:

* annotator: In `guest`, when a user creates a new annotation or highlight
  via the adder controls
* sidebar: In the `annotations` store module when annotation objects are
  initialized before being added to the store

This property is communicated between the sidebar and the annotator when
exchanging annotation data. In the annotator, the value of this property
is used to set an additional CSS class on drawn anchor highlights
(`<hypothesis-highlight>` `span`s).

The presence of this CSS class will allow subsequent differentiated styling
for highlight clusters.
parent 7bb1e081
import classnames from 'classnames';
import { ListenerCollection } from '../shared/listener-collection'; import { ListenerCollection } from '../shared/listener-collection';
import { PortFinder, PortRPC } from '../shared/messaging'; import { PortFinder, PortRPC } from '../shared/messaging';
import { generateHexString } from '../shared/random'; import { generateHexString } from '../shared/random';
...@@ -543,7 +545,10 @@ export class Guest { ...@@ -543,7 +545,10 @@ export class Guest {
} }
const highlights = /** @type {AnnotationHighlight[]} */ ( const highlights = /** @type {AnnotationHighlight[]} */ (
highlightRange(range) highlightRange(
range,
classnames('hypothesis-highlight', anchor.annotation?.$cluster)
)
); );
highlights.forEach(h => { highlights.forEach(h => {
h._annotation = anchor.annotation; h._annotation = anchor.annotation;
...@@ -656,6 +661,7 @@ export class Guest { ...@@ -656,6 +661,7 @@ export class Guest {
document: info.metadata, document: info.metadata,
target, target,
$highlight: highlight, $highlight: highlight,
$cluster: highlight ? 'user-highlights' : 'user-annotations',
$tag: 'a:' + generateHexString(8), $tag: 'a:' + generateHexString(8),
}; };
......
...@@ -959,6 +959,18 @@ describe('Guest', () => { ...@@ -959,6 +959,18 @@ describe('Guest', () => {
assert.equal(annotation.$highlight, true); assert.equal(annotation.$highlight, true);
}); });
it('sets `$cluster` to `user-highlights` if `highlight` is true', async () => {
const guest = createGuest();
const annotation = await guest.createAnnotation({ highlight: true });
assert.equal(annotation.$cluster, 'user-highlights');
});
it('sets `$cluster` to `user-annotations` if `highlight` is false', async () => {
const guest = createGuest();
const annotation = await guest.createAnnotation({ highlight: false });
assert.equal(annotation.$cluster, 'user-annotations');
});
it('triggers a "createAnnotation" event', async () => { it('triggers a "createAnnotation" event', async () => {
const guest = createGuest(); const guest = createGuest();
...@@ -1125,6 +1137,22 @@ describe('Guest', () => { ...@@ -1125,6 +1137,22 @@ describe('Guest', () => {
]); ]);
}); });
it('provides CSS classes for anchor highlight elements', async () => {
const guest = createGuest();
const annotation = {
$cluster: 'user-annotations',
target: [{ selector: [{ type: 'TextQuoteSelector', exact: 'hello' }] }],
};
fakeIntegration.anchor.resolves(range);
await guest.anchor(annotation);
assert.equal(
highlighter.highlightRange.lastCall.args[1],
'hypothesis-highlight user-annotations'
);
});
it('returns a promise of the anchors for the annotation', () => { it('returns a promise of the anchors for the annotation', () => {
const guest = createGuest(); const guest = createGuest();
const highlights = [document.createElement('span')]; const highlights = [document.createElement('span')];
......
...@@ -37,8 +37,14 @@ type DocumentInfo = { ...@@ -37,8 +37,14 @@ type DocumentInfo = {
* JavaScript, it includes only the information needed to uniquely identify it * JavaScript, it includes only the information needed to uniquely identify it
* within the current session and anchor it in the document. * within the current session and anchor it in the document.
*/ */
export function formatAnnot({ $tag, target, uri }: Annotation): AnnotationData { export function formatAnnot({
$cluster,
$tag,
target,
uri,
}: Annotation): AnnotationData {
return { return {
$cluster,
$tag, $tag,
target, target,
uri, uri,
......
...@@ -17,7 +17,7 @@ class FakeWindow extends EventTarget { ...@@ -17,7 +17,7 @@ class FakeWindow extends EventTarget {
const testAnnotation = annotationFixtures.defaultAnnotation(); const testAnnotation = annotationFixtures.defaultAnnotation();
const fixtures = { const fixtures = {
ann: { $tag: 't1', ...testAnnotation }, ann: { $cluster: 'user-annotations', $tag: 't1', ...testAnnotation },
// New annotation received from the frame // New annotation received from the frame
newAnnFromFrame: { newAnnFromFrame: {
...@@ -221,6 +221,17 @@ describe('FrameSyncService', () => { ...@@ -221,6 +221,17 @@ describe('FrameSyncService', () => {
}); });
}); });
describe('formatAnnot', () => {
it('formats annotations with only those properties needed by the annotator', () => {
assert.hasAllKeys(formatAnnot(fixtures.ann), [
'$cluster',
'$tag',
'target',
'uri',
]);
});
});
context('when annotations are loaded into the sidebar', () => { context('when annotations are loaded into the sidebar', () => {
beforeEach(() => { beforeEach(() => {
frameSync.connect(); frameSync.connect();
......
...@@ -7,15 +7,18 @@ import { createSelector } from 'reselect'; ...@@ -7,15 +7,18 @@ import { createSelector } from 'reselect';
import { hasOwn } from '../../../shared/has-own'; import { hasOwn } from '../../../shared/has-own';
import * as metadata from '../../helpers/annotation-metadata'; import * as metadata from '../../helpers/annotation-metadata';
import { isSaved } from '../../helpers/annotation-metadata'; import { isHighlight, isSaved } from '../../helpers/annotation-metadata';
import { countIf, toTrueMap, trueKeys } from '../../util/collections'; import { countIf, toTrueMap, trueKeys } from '../../util/collections';
import { createStoreModule, makeAction } from '../create-store'; import { createStoreModule, makeAction } from '../create-store';
import { routeModule } from './route'; import { routeModule } from './route';
import { sessionModule } from './session';
/** /**
* @typedef {'anchored'|'orphan'|'timeout'} AnchorStatus * @typedef {'anchored'|'orphan'|'timeout'} AnchorStatus
* @typedef {import('../../../types/api').Annotation} Annotation * @typedef {import('../../../types/api').Annotation} Annotation
* @typedef {import('../../../types/shared').HighlightCluster} HighlightCluster
* @typedef {import('../../../types/shared').ClientAnnotationData} ClientAnnotationData
* @typedef {import('../../../types/api').SavedAnnotation} SavedAnnotation * @typedef {import('../../../types/api').SavedAnnotation} SavedAnnotation
*/ */
...@@ -71,18 +74,20 @@ function findByTag(annotations, tag) { ...@@ -71,18 +74,20 @@ function findByTag(annotations, tag) {
} }
/** /**
* Set custom private fields on an annotation object about to be added to the * Merge client annotation data into the annotation object about to be added to
* store's collection of `annotations`. * the store's collection of `annotations`.
* *
* `annotation` may either be new (unsaved) or a persisted annotation retrieved * `annotation` may either be new (unsaved) or a persisted annotation retrieved
* from the service. * from the service.
* *
* @param {Omit<Annotation, '$anchorTimeout'>} annotation * @param {Omit<Annotation, '$anchorTimeout'>} annotation
* @param {string} tag - The `$tag` value that should be used for this * @param {string} tag - The `$tag` value that should be used for this if it
* if it doesn't have a `$tag` already * doesn't have a `$tag` already
* @return {Annotation} - annotation with local (`$*`) fields set * @param {string|null} currentUserId - The account id of the currently-auth'd
* user, if any
* @return {Annotation} - API annotation data with client annotation data merged
*/ */
function initializeAnnotation(annotation, tag) { function initializeAnnotation(annotation, tag, currentUserId) {
let orphan = annotation.$orphan; let orphan = annotation.$orphan;
if (!annotation.id) { if (!annotation.id) {
...@@ -90,11 +95,21 @@ function initializeAnnotation(annotation, tag) { ...@@ -90,11 +95,21 @@ function initializeAnnotation(annotation, tag) {
orphan = false; orphan = false;
} }
return Object.assign({}, annotation, { let $cluster = /** @type {HighlightCluster} */ ('other-content');
$anchorTimeout: false, if (annotation.user === currentUserId) {
$tag: annotation.$tag || tag, $cluster = isHighlight(annotation) ? 'user-highlights' : 'user-annotations';
$orphan: orphan, }
});
return Object.assign(
{},
annotation,
/** @type {ClientAnnotationData} */ ({
$anchorTimeout: false,
$cluster,
$tag: annotation.$tag || tag,
$orphan: orphan,
})
);
} }
const initialState = { const initialState = {
...@@ -129,7 +144,7 @@ const initialState = { ...@@ -129,7 +144,7 @@ const initialState = {
const reducers = { const reducers = {
/** /**
* @param {State} state * @param {State} state
* @param {{ annotations: Annotation[], currentAnnotationCount: number }} action * @param {{ annotations: Annotation[], currentAnnotationCount: number, currentUserId: string|null }} action
*/ */
ADD_ANNOTATIONS(state, action) { ADD_ANNOTATIONS(state, action) {
const updatedIDs = new Set(); const updatedIDs = new Set();
...@@ -160,7 +175,9 @@ const reducers = { ...@@ -160,7 +175,9 @@ const reducers = {
updatedTags.add(existing.$tag); updatedTags.add(existing.$tag);
} }
} else { } else {
added.push(initializeAnnotation(annot, 't' + nextTag)); added.push(
initializeAnnotation(annot, 't' + nextTag, action.currentUserId)
);
++nextTag; ++nextTag;
} }
} }
...@@ -296,7 +313,7 @@ const reducers = { ...@@ -296,7 +313,7 @@ const reducers = {
function addAnnotations(annotations) { function addAnnotations(annotations) {
/** /**
* @param {import('redux').Dispatch} dispatch * @param {import('redux').Dispatch} dispatch
* @param {() => { annotations: State, route: import('./route').State }} getState * @param {() => { annotations: State, route: import('./route').State, session: import('./session').State }} getState
*/ */
return function (dispatch, getState) { return function (dispatch, getState) {
const added = annotations.filter(annot => { const added = annotations.filter(annot => {
...@@ -305,10 +322,13 @@ function addAnnotations(annotations) { ...@@ -305,10 +322,13 @@ function addAnnotations(annotations) {
); );
}); });
const profile = sessionModule.selectors.profile(getState().session);
dispatch( dispatch(
makeAction(reducers, 'ADD_ANNOTATIONS', { makeAction(reducers, 'ADD_ANNOTATIONS', {
annotations, annotations,
currentAnnotationCount: getState().annotations.annotations.length, currentAnnotationCount: getState().annotations.annotations.length,
currentUserId: profile.userid,
}) })
); );
......
...@@ -3,9 +3,10 @@ import * as metadata from '../../../helpers/annotation-metadata'; ...@@ -3,9 +3,10 @@ import * as metadata from '../../../helpers/annotation-metadata';
import { createStore } from '../../create-store'; import { createStore } from '../../create-store';
import { annotationsModule } from '../annotations'; import { annotationsModule } from '../annotations';
import { routeModule } from '../route'; import { routeModule } from '../route';
import { sessionModule } from '../session';
function createTestStore() { function createTestStore() {
return createStore([annotationsModule, routeModule], [{}]); return createStore([annotationsModule, routeModule, sessionModule], [{}]);
} }
// Tests for some of the functionality in this store module are currently in // Tests for some of the functionality in this store module are currently in
...@@ -44,7 +45,7 @@ describe('sidebar/store/modules/annotations', () => { ...@@ -44,7 +45,7 @@ describe('sidebar/store/modules/annotations', () => {
]); ]);
}); });
it('assigns a local tag to annotations', () => { it('assigns a $tag to annotations', () => {
const annotA = Object.assign(fixtures.defaultAnnotation(), { id: 'a1' }); const annotA = Object.assign(fixtures.defaultAnnotation(), { id: 'a1' });
const annotB = Object.assign(fixtures.defaultAnnotation(), { id: 'a2' }); const annotB = Object.assign(fixtures.defaultAnnotation(), { id: 'a2' });
...@@ -57,6 +58,46 @@ describe('sidebar/store/modules/annotations', () => { ...@@ -57,6 +58,46 @@ describe('sidebar/store/modules/annotations', () => {
assert.deepEqual(tags, ['t1', 't2']); assert.deepEqual(tags, ['t1', 't2']);
}); });
it('assigns a $cluster to annotations', () => {
const getClusters = () =>
store.getState().annotations.annotations.map(a => a.$cluster);
const userHighlight = Object.assign(fixtures.defaultAnnotation(), {
id: 'a1',
user: 'acct:jondoe@hypothes.is',
});
const userAnnotation = Object.assign(fixtures.defaultAnnotation(), {
id: 'a2',
user: 'acct:jondoe@hypothes.is',
text: 'content', // This will ensure this is treated as an annotation instead of a highlight
});
const otherContent = Object.assign(fixtures.defaultAnnotation(), {
id: 'a3',
user: 'acct:someone-else@hypothes.is',
text: 'content',
});
store.updateProfile({ userid: 'acct:jondoe@hypothes.is' });
store.addAnnotations([userHighlight, userAnnotation, otherContent]);
assert.deepEqual(getClusters(), [
'user-highlights',
'user-annotations',
'other-content',
]);
store.clearAnnotations();
store.updateProfile({ userid: null });
store.addAnnotations([userHighlight, userAnnotation, otherContent]);
assert.deepEqual(getClusters(), [
'other-content',
'other-content',
'other-content',
]);
});
it('updates annotations with matching IDs in the store', () => { it('updates annotations with matching IDs in the store', () => {
const annot = fixtures.defaultAnnotation(); const annot = fixtures.defaultAnnotation();
store.addAnnotations([annot]); store.addAnnotations([annot]);
...@@ -69,7 +110,7 @@ describe('sidebar/store/modules/annotations', () => { ...@@ -69,7 +110,7 @@ describe('sidebar/store/modules/annotations', () => {
assert.equal(updatedAnnot.text, 'update'); assert.equal(updatedAnnot.text, 'update');
}); });
it('updates annotations with matching tags in the store', () => { it('updates annotations with matching $tags in the store', () => {
const annot = fixtures.newAnnotation(); const annot = fixtures.newAnnotation();
annot.$tag = 'local-tag'; annot.$tag = 'local-tag';
store.addAnnotations([annot]); store.addAnnotations([annot]);
......
/**
* Clusters provide the client application a mechanism for categorizing
* annotations so that their drawn anchor highlights may be styled distinctively
* in the annotated document. An annotation can only belong to one cluster.
*/
export type HighlightCluster =
| 'other-content' // default cluster: content not belonging to the current user
| 'user-annotations' // An annotation belonging to the current user
| 'user-highlights'; // A highlight (highlights are private; they always belong to the current user)
/** /**
* Annotation properties not present on API objects, but added by the client * Annotation properties not present on API objects, but added by the client
*/ */
export type ClientAnnotationData = { export type ClientAnnotationData = {
$cluster?: HighlightCluster;
/** /**
* Client-side identifier: set even if annotation does not have a * Client-side identifier: set even if annotation does not have a
* server-provided `id` (i.e. is unsaved) * server-provided `id` (i.e. is unsaved)
......
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