Commit 0a28a16d authored by Robert Knight's avatar Robert Knight

Change `createAnnotation` argument to just a `highlight` flag

`createAnnotation` used to accept a partially filled-out annotation
object with arbitrary properties. However the argument was only ever
used to set the `$highlight` flag. Therefore we can simplify the
interface by making the method accept just a `highlight` flag instead.

 - Simplify `createAnnotation` argument

 - Add tests for handling of `highlight` option to `createAnnotation`

 - Remove the `createHighlight` method in guest as it was just a trivial
   wrapper around `createAnnotation({ highlight: true })`
parent fa2f4135
......@@ -128,7 +128,7 @@ export default class Guest extends Delegator {
},
onHighlight: async () => {
this.setVisibleHighlights(true);
await this.createHighlight();
await this.createAnnotation({ highlight: true });
/** @type {Selection} */ (document.getSelection()).removeAllRanges();
},
onShowAnnotations: anns => {
......@@ -532,25 +532,22 @@ export default class Guest extends Delegator {
* Create a new annotation that is associated with the selected region of
* the current document.
*
* @param {Partial<AnnotationData>} annotation - Initial properties of the
* new annotation, other than `target`, `document` and `uri` which are
* set by this method.
* @return {Promise<AnnotationData>} - The updated annotation with selectors
* and metadata populated.
* @param {object} options
* @param {boolean} [options.highlight] - If true, the new annotation has
* the `$highlight` flag set, causing it to be saved immediately without
* prompting for a comment.
* @return {Promise<AnnotationData>} - The new annotation
*/
async createAnnotation(annotation = {}) {
async createAnnotation({ highlight = false } = {}) {
const ranges = this.selectedRanges ?? [];
this.selectedRanges = null;
const info = await this.getDocumentInfo();
annotation.document = info.metadata;
annotation.uri = info.uri;
const root = this.element;
const rangeSelectors = await Promise.all(
ranges.map(range => this.anchoring.describe(root, range))
);
annotation.target = rangeSelectors.map(selectors => ({
const target = rangeSelectors.map(selectors => ({
source: info.uri,
// In the Hypothesis API the field containing the selectors is called
......@@ -558,26 +555,25 @@ export default class Guest extends Delegator {
selector: selectors,
}));
/** @type {AnnotationData} */
const annotation = {
uri: info.uri,
document: info.metadata,
target,
$highlight: highlight,
// nb. `$tag` is assigned by `AnnotationSync`.
$tag: '',
};
this.publish('beforeAnnotationCreated', [annotation]);
this.anchor(/** @type {AnnotationData} */ (annotation));
this.anchor(annotation);
if (!annotation.$highlight) {
this.crossframe?.call('openSidebar');
}
return /** @type {AnnotationData} */ (annotation);
}
/**
* Create a new annotation with the `$highlight` flag set.
*
* This flag indicates that the sidebar should save the new annotation
* automatically and not show a form for the user to enter a comment about it.
*
* @return {Promise<AnnotationData>}
*/
createHighlight() {
return this.createAnnotation({ $highlight: true });
return annotation;
}
/**
......
......@@ -629,9 +629,8 @@ describe('Guest', () => {
describe('#createAnnotation', () => {
it('adds document metadata to the annotation', async () => {
const guest = createGuest();
const annotation = {};
await guest.createAnnotation(annotation);
const annotation = await guest.createAnnotation();
assert.equal(annotation.uri, fakeDocumentMeta.uri());
assert.deepEqual(annotation.document, fakeDocumentMeta.metadata);
......@@ -658,22 +657,42 @@ describe('Guest', () => {
]);
});
it('merges properties of input object into returned annotation', async () => {
it('sets `$tag` to a falsey value', async () => {
const guest = createGuest();
let annotation = { foo: 'bar' };
const annotation = await guest.createAnnotation();
assert.notOk(annotation.$tag);
});
annotation = await guest.createAnnotation(annotation);
it('sets falsey `$highlight` if `highlight` is false', async () => {
const guest = createGuest();
const annotation = await guest.createAnnotation();
assert.notOk(annotation.$highlight);
});
assert.equal(annotation.foo, 'bar');
it('sets `$highlight` to true if `highlight` is true', async () => {
const guest = createGuest();
const annotation = await guest.createAnnotation({ highlight: true });
assert.equal(annotation.$highlight, true);
});
it('opens sidebar if `highlight` is false', async () => {
const guest = createGuest();
await guest.createAnnotation();
assert.calledWith(fakeCrossFrame.call, 'openSidebar');
});
it('does not open sidebar if `highlight` is true', async () => {
const guest = createGuest();
await guest.createAnnotation({ highlight: true });
assert.notCalled(fakeCrossFrame.call);
});
it('triggers a "beforeAnnotationCreated" event', async () => {
const guest = createGuest();
const callback = sinon.stub();
guest.subscribe('beforeAnnotationCreated', callback);
const annotation = {};
await guest.createAnnotation(annotation);
const annotation = await guest.createAnnotation();
assert.calledWith(callback, annotation);
});
......
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