Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/annotator/guest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,13 @@ export class Guest

this._sidebarRPC.on('deleteAnnotation', (tag: string) => this.detach(tag));

// Expose document info to the sidebar on demand, so that annotation
// creation can be triggered from there (e.g. via the experimental new note
// button)
this._sidebarRPC.on('getDocumentInfo', async callback =>
callback(await this.getDocumentInfo()),
);

this._sidebarRPC.on(
'loadAnnotations',
async (annotations: AnnotationData[]) => {
Expand Down
27 changes: 26 additions & 1 deletion src/annotator/test/guest-test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { delay } from '@hypothesis/frontend-testing';
import { delay, waitFor } from '@hypothesis/frontend-testing';
import sinon from 'sinon';

import { EventEmitter } from '../../shared/event-emitter';
import { DrawError } from '../draw-tool';
Expand Down Expand Up @@ -736,6 +737,30 @@ describe('Guest', () => {
});
});

describe('on "getDocumentInfo" event', () => {
it('gets guest document info', async () => {
const callback = sinon.stub();

createGuest();
emitSidebarEvent('getDocumentInfo', callback);

await waitFor(() => callback.called);

assert.calledWith(
callback,
sinon.match({
uri: 'https://example.com/test.pdf',
metadata: {
title: 'Test title',
documentFingerprint: 'test-fingerprint',
},
}),
);
assert.called(fakeIntegration.uri);
assert.called(fakeIntegration.getMetadata);
});
});

describe('on "featureFlagsUpdated" event', () => {
it('updates active feature flags', () => {
const flagsUpdated = sinon.stub();
Expand Down
17 changes: 15 additions & 2 deletions src/sidebar/components/SidebarTabs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ import {
} from '@hypothesis/frontend-shared';
import classnames from 'classnames';
import type { ComponentChildren } from 'preact';
import { useCallback } from 'preact/hooks';

import { pluralize } from '../../shared/pluralize';
import type { SidebarSettings } from '../../types/config';
import type { TabName } from '../../types/sidebar';
import { applyTheme } from '../helpers/theme';
import { withServices } from '../service-context';
import type { AnnotationsService } from '../services/annotations';
import type { FrameSyncService } from '../services/frame-sync';
import { useSidebarStore } from '../store';
import ThreadList from './ThreadList';
import { useRootThread } from './hooks/use-root-thread';
Expand Down Expand Up @@ -99,6 +101,7 @@ export type SidebarTabsProps = {
// injected
settings: SidebarSettings;
annotationsService: AnnotationsService;
frameSync: FrameSyncService;
};

/**
Expand All @@ -108,6 +111,7 @@ function SidebarTabs({
annotationsService,
isLoading,
settings,
frameSync,
}: SidebarTabsProps) {
const { rootThread, tabCounts } = useRootThread();
const store = useSidebarStore();
Expand Down Expand Up @@ -143,6 +147,11 @@ function SidebarTabs({
}
const tabCountsSummary = tabCountsSummaryPieces.join(', ');

const createPageNoteWithDocumentMeta = useCallback(async () => {
const { metadata } = await frameSync.getDocumentInfo();
annotationsService.createPageNote(metadata);
}, [annotationsService, frameSync]);

return (
<>
<div aria-live="polite" role="status" className="sr-only">
Expand Down Expand Up @@ -201,7 +210,7 @@ function SidebarTabs({
<div className="flex justify-end">
<Button
data-testid="new-note-button"
onClick={() => annotationsService.createPageNote()}
onClick={createPageNoteWithDocumentMeta}
variant="primary"
style={applyTheme(['ctaBackgroundColor'], settings)}
>
Expand Down Expand Up @@ -244,4 +253,8 @@ function SidebarTabs({
);
}

export default withServices(SidebarTabs, ['annotationsService', 'settings']);
export default withServices(SidebarTabs, [
'annotationsService',
'frameSync',
'settings',
]);
13 changes: 11 additions & 2 deletions src/sidebar/components/test/SidebarTabs-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ describe('SidebarTabs', () => {
// mock services
let fakeAnnotationsService;
let fakeSettings;
let fakeFrameSync;
let fakeStore;
let fakeUseRootThread;

Expand All @@ -19,6 +20,7 @@ describe('SidebarTabs', () => {
<SidebarTabs
annotationsService={fakeAnnotationsService}
settings={fakeSettings}
frameSync={fakeFrameSync}
isLoading={false}
{...props}
/>,
Expand Down Expand Up @@ -46,6 +48,9 @@ describe('SidebarTabs', () => {
fakeSettings = {
enableExperimentalNewNoteButton: false,
};
fakeFrameSync = {
getDocumentInfo: sinon.stub().resolves({ metadata: {} }),
};
fakeStore = {
selectTab: sinon.stub(),
isWaitingToAnchorAnnotations: sinon.stub().returns(false),
Expand Down Expand Up @@ -151,13 +156,17 @@ describe('SidebarTabs', () => {
assert.deepEqual(button.prop('style'), { backgroundColor: '#00f' });
});

it('should add a new page note on click', () => {
it('should add a new page note on click', async () => {
fakeSettings.enableExperimentalNewNoteButton = true;
fakeStore.selectedTab.returns('note');

const wrapper = createComponent();
wrapper.find('Button[data-testid="new-note-button"]').props().onClick();
await wrapper
.find('Button[data-testid="new-note-button"]')
.props()
.onClick();

assert.calledOnce(fakeFrameSync.getDocumentInfo);
assert.calledOnce(fakeAnnotationsService.createPageNote);
});
});
Expand Down
7 changes: 4 additions & 3 deletions src/sidebar/services/annotations.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { ModerationStatus } from '@hypothesis/annotation-ui';

import { generateHexString } from '../../shared/random';
import type { AnnotationData } from '../../types/annotator';
import type { AnnotationData, DocumentMetadata } from '../../types/annotator';
import type {
APIAnnotationData,
Annotation,
Expand Down Expand Up @@ -211,7 +211,7 @@ export class AnnotationsService {
* Create a new empty "page note" annotation and add it to the store. If the
* user is not logged in, open the `loginPrompt` panel instead.
*/
createPageNote() {
createPageNote(document?: DocumentMetadata) {
const topLevelFrame = this._store.mainFrame();
if (!this._store.isLoggedIn()) {
this._store.openSidebarPanel('loginPrompt');
Expand All @@ -227,7 +227,8 @@ export class AnnotationsService {
},
],
uri: topLevelFrame.uri,
};
document,
} satisfies Partial<AnnotationData>;
this.create(pageNoteAnnotation);
}

Expand Down
15 changes: 15 additions & 0 deletions src/sidebar/services/frame-sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
isMessageEqual,
} from '../../shared/messaging';
import type { Message } from '../../shared/messaging';
import { promiseWithResolvers } from '../../shared/promise-with-resolvers';
import type {
AnnotationData,
DocumentInfo,
Expand Down Expand Up @@ -704,6 +705,20 @@ export class FrameSyncService {
});
}

async getDocumentInfo(): Promise<DocumentInfo> {
// Get the guest for the main frame. This assumes that the annotation is
// anchored in the main frame.
const guest = this._guestRPC.get(null);
if (!guest) {
throw new Error('No guest connected');
}

const { promise, resolve } = promiseWithResolvers<DocumentInfo>();
guest.call('getDocumentInfo', resolve);

return promise;
}

// Only used to cleanup tests
destroy() {
this._portFinder.destroy();
Expand Down
24 changes: 14 additions & 10 deletions src/sidebar/services/test/annotations-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -314,18 +314,22 @@ describe('AnnotationsService', () => {
assert.isNull(getLastAddedAnnotation());
});

it('should create a new unsaved annotation with page note defaults', () => {
svc.createPageNote();
it.each([undefined, { title: 'foo' }])(
'should create a new unsaved annotation with page note defaults',
document => {
svc.createPageNote(document);

const annotation = getLastAddedAnnotation();
const annotation = getLastAddedAnnotation();

assert.equal(annotation.uri, 'http://www.example.com');
assert.deepEqual(annotation.target, [
{
source: 'http://www.example.com',
},
]);
});
assert.equal(annotation.uri, 'http://www.example.com');
assert.deepEqual(annotation.target, [
{
source: 'http://www.example.com',
},
]);
assert.deepEqual(annotation.document, document);
},
);
});

describe('delete', () => {
Expand Down
33 changes: 33 additions & 0 deletions src/sidebar/services/test/frame-sync-test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { delay } from '@hypothesis/frontend-testing';
import sinon from 'sinon';

import { EventEmitter } from '../../../shared/event-emitter';
import { Injector } from '../../../shared/injector';
Expand Down Expand Up @@ -1279,4 +1280,36 @@ describe('FrameSyncService', () => {
assert.equal(err.message, 'Something went wrong');
});
});

describe('#getDocumentInfo', () => {
beforeEach(async () => {
await frameSync.connect();
});

it('rejects if guest is not connected', async () => {
let err;
try {
await frameSync.getDocumentInfo();
} catch (e) {
err = e;
}
assert.instanceOf(err, Error);
assert.equal(err.message, 'No guest connected');
});

it('requests document info from guest', async () => {
await connectGuest();

guestRPC().call.callsFake((method, callback) => {
if (method === 'getDocumentInfo') {
delay(0).then(() => callback({ title: 'foo' }));
}
});

const document = await frameSync.getDocumentInfo();

assert.calledWith(guestRPC().call, 'getDocumentInfo', sinon.match.func);
assert.deepEqual(document, { title: 'foo' });
});
});
});
5 changes: 5 additions & 0 deletions src/types/port-rpc-calls.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,11 @@ export type SidebarToGuestCalls = {
* current activity / assignment.
*/
setOutsideAssignmentNoticeVisible(visible: boolean): void;

/**
* Expose the guest document info to the sidebar
*/
getDocumentInfo(callback: (info: DocumentInfo) => void): void;
};

/** Calls that the sidebar makes to the host. */
Expand Down