Skip to content

Commit

Permalink
Remove unused code in Guest class
Browse files Browse the repository at this point in the history 
 - Remove several methods in `Guest` which were never called in the
   application. This includes the `deleteAnnotation` method which was
   used in a few tests that should have been calling `detach` instead.

 - Remove untested error paths in plugin loading in `Guest`. These were needed
   when the plugins could be defined by the consumer of Annotator but
   this is no longer the case. All plugins are purely internal.

 - Remove logic for setting `annotator` property on root element which
   was never read anywhere
  • Loading branch information
@robertknight
robertknight committed Sep 22, 2020
1 parent f2afb68 commit add1b4e
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 114 deletions.
63 changes: 4 additions & 59 deletions src/annotator/guest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,23 +210,10 @@ export default class Guest extends Delegator {
}

addPlugin(name, options) {
if (this.plugins[name]) {
console.error('You cannot have more than one instance of any plugin.');
} else {
const Klass = this.options.pluginClasses[name];
if (typeof Klass === 'function') {
this.plugins[name] = new Klass(this.element[0], options);
this.plugins[name].annotator = this;
this.plugins[name].pluginInit?.();
} else {
console.error(
'Could not load ' +
name +
' plugin. Have you included the appropriate <script> tag?'
);
}
}
return this; // allow chaining
const Klass = this.options.pluginClasses[name];
this.plugins[name] = new Klass(this.element[0], options);
this.plugins[name].annotator = this;
this.plugins[name].pluginInit?.();
}

// Get the document info
Expand Down Expand Up @@ -330,8 +317,6 @@ export default class Guest extends Delegator {
$(element).remove();
});

this.element.data('annotator', null);

for (let name of Object.keys(this.plugins)) {
this.plugins[name].destroy();
}
Expand Down Expand Up @@ -538,41 +523,6 @@ export default class Guest extends Delegator {
return this.createAnnotation({ $highlight: true });
}

// Create a blank comment (AKA "page note")
createComment() {
const annotation = {};

const prepare = info => {
annotation.document = info.metadata;
annotation.uri = info.uri;
annotation.target = [{ source: info.uri }];
};

this.getDocumentInfo()
.then(prepare)
.then(() => this.publish('beforeAnnotationCreated', [annotation]));

return annotation;
}

// Public: Deletes the annotation by removing the highlight from the DOM.
// Publishes the 'annotationDeleted' event on completion.
//
// annotation - An annotation Object to delete.
//
// Returns deleted annotation.
deleteAnnotation(annotation) {
if (annotation.highlights) {
for (let h of annotation.highlights) {
if (h.parentNode !== null) {
$(h).replaceWith(h.childNodes);
}
}
}

this.publish('annotationDeleted', [annotation]);
}

showAnnotations(annotations) {
const tags = annotations.map(a => a.$tag);
this.crossframe?.call('showAnnotations', tags);
Expand All @@ -584,11 +534,6 @@ export default class Guest extends Delegator {
this.crossframe?.call('toggleAnnotationSelection', tags);
}

updateAnnotations(annotations) {
const tags = annotations.map(a => a.$tag);
this.crossframe?.call('updateAnnotations', tags);
}

focusAnnotations(annotations) {
const tags = annotations.map(a => a.$tag);
this.crossframe?.call('focusAnnotations', tags);
Expand Down
61 changes: 6 additions & 55 deletions src/annotator/test/guest-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -572,47 +572,6 @@ describe('Guest', () => {
});
});

describe('#createComment', () => {
it('adds metadata to the annotation object', () => {
const guest = createGuest();
sinon.stub(guest, 'getDocumentInfo').returns(
Promise.resolve({
metadata: { title: 'hello' },
uri: 'http://example.com/',
})
);

const annotation = guest.createComment();

return timeoutPromise().then(() => {
assert.equal(annotation.uri, 'http://example.com/');
assert.deepEqual(annotation.document, { title: 'hello' });
});
});

it('adds a single target with a source property', () => {
const guest = createGuest();
sinon.stub(guest, 'getDocumentInfo').returns(
Promise.resolve({
metadata: { title: 'hello' },
uri: 'http://example.com/',
})
);

const annotation = guest.createComment();

return timeoutPromise().then(() =>
assert.deepEqual(annotation.target, [{ source: 'http://example.com/' }])
);
});

it('triggers a beforeAnnotationCreated event', done => {
const guest = createGuest();
guest.subscribe('beforeAnnotationCreated', () => done());
guest.createComment();
});
});

describe('#anchor', () => {
let el;
let range;
Expand Down Expand Up @@ -823,39 +782,31 @@ describe('Guest', () => {
const guest = createGuest();
const annotation = {};
guest.anchors.push({ annotation });

guest.detach(annotation);

assert.equal(guest.anchors.length, 0);
});

it('updates the bucket bar plugin', () => {
const guest = createGuest();
guest.plugins.BucketBar = { update: sinon.stub() };
const annotation = {};

guest.anchors.push({ annotation });
guest.detach(annotation);
assert.calledOnce(guest.plugins.BucketBar.update);
});

it('publishes the "annotationDeleted" event', () => {
const guest = createGuest();
const annotation = {};
const publish = sandbox.stub(guest, 'publish');

guest.deleteAnnotation(annotation);
guest.detach(annotation);

assert.calledOnce(publish);
assert.calledWith(publish, 'annotationDeleted', [annotation]);
assert.calledOnce(guest.plugins.BucketBar.update);
});

it('removes any highlights associated with the annotation', () => {
const guest = createGuest();
const annotation = {};
const highlights = [document.createElement('span')];
const { removeHighlights } = highlighter;

guest.anchors.push({ annotation, highlights });
guest.deleteAnnotation(annotation);

guest.detach(annotation);

assert.calledOnce(removeHighlights);
assert.calledWith(removeHighlights, highlights);
Expand Down

0 comments on commit add1b4e

Please sign in to comment.