Skip to content

Commit ec5eb13

Browse files
author
Alejandro Celaya
committed
Include document info when creating page notes via experimental button
1 parent aaeb9a5 commit ec5eb13

File tree

9 files changed

+130
-18
lines changed

9 files changed

+130
-18
lines changed

src/annotator/guest.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,13 @@ export class Guest
643643

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

646+
// Expose document info to the sidebar on demand, so that annotation
647+
// creation can be triggered from there (e.g. via the experimental new note
648+
// button)
649+
this._sidebarRPC.on('getDocumentInfo', async callback =>
650+
callback(await this.getDocumentInfo()),
651+
);
652+
646653
this._sidebarRPC.on(
647654
'loadAnnotations',
648655
async (annotations: AnnotationData[]) => {

src/annotator/test/guest-test.js

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { delay } from '@hypothesis/frontend-testing';
1+
import { delay, waitFor } from '@hypothesis/frontend-testing';
2+
import sinon from 'sinon';
23

34
import { EventEmitter } from '../../shared/event-emitter';
45
import { DrawError } from '../draw-tool';
@@ -736,6 +737,30 @@ describe('Guest', () => {
736737
});
737738
});
738739

740+
describe('on "getDocumentInfo" event', () => {
741+
it('gets guest document info', async () => {
742+
const callback = sinon.stub();
743+
744+
createGuest();
745+
emitSidebarEvent('getDocumentInfo', callback);
746+
747+
await waitFor(() => callback.called);
748+
749+
assert.calledWith(
750+
callback,
751+
sinon.match({
752+
uri: 'https://example.com/test.pdf',
753+
metadata: {
754+
title: 'Test title',
755+
documentFingerprint: 'test-fingerprint',
756+
},
757+
}),
758+
);
759+
assert.called(fakeIntegration.uri);
760+
assert.called(fakeIntegration.getMetadata);
761+
});
762+
});
763+
739764
describe('on "featureFlagsUpdated" event', () => {
740765
it('updates active feature flags', () => {
741766
const flagsUpdated = sinon.stub();

src/sidebar/components/SidebarTabs.tsx

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,15 @@ import {
88
} from '@hypothesis/frontend-shared';
99
import classnames from 'classnames';
1010
import type { ComponentChildren } from 'preact';
11+
import { useCallback } from 'preact/hooks';
1112

1213
import { pluralize } from '../../shared/pluralize';
1314
import type { SidebarSettings } from '../../types/config';
1415
import type { TabName } from '../../types/sidebar';
1516
import { applyTheme } from '../helpers/theme';
1617
import { withServices } from '../service-context';
1718
import type { AnnotationsService } from '../services/annotations';
19+
import type { FrameSyncService } from '../services/frame-sync';
1820
import { useSidebarStore } from '../store';
1921
import ThreadList from './ThreadList';
2022
import { useRootThread } from './hooks/use-root-thread';
@@ -99,6 +101,7 @@ export type SidebarTabsProps = {
99101
// injected
100102
settings: SidebarSettings;
101103
annotationsService: AnnotationsService;
104+
frameSync: FrameSyncService;
102105
};
103106

104107
/**
@@ -108,6 +111,7 @@ function SidebarTabs({
108111
annotationsService,
109112
isLoading,
110113
settings,
114+
frameSync,
111115
}: SidebarTabsProps) {
112116
const { rootThread, tabCounts } = useRootThread();
113117
const store = useSidebarStore();
@@ -143,6 +147,11 @@ function SidebarTabs({
143147
}
144148
const tabCountsSummary = tabCountsSummaryPieces.join(', ');
145149

150+
const createPageNoteWithDocumentMeta = useCallback(async () => {
151+
const { metadata } = await frameSync.getDocumentInfo();
152+
annotationsService.createPageNote(metadata);
153+
}, [annotationsService, frameSync]);
154+
146155
return (
147156
<>
148157
<div aria-live="polite" role="status" className="sr-only">
@@ -201,7 +210,7 @@ function SidebarTabs({
201210
<div className="flex justify-end">
202211
<Button
203212
data-testid="new-note-button"
204-
onClick={() => annotationsService.createPageNote()}
213+
onClick={createPageNoteWithDocumentMeta}
205214
variant="primary"
206215
style={applyTheme(['ctaBackgroundColor'], settings)}
207216
>
@@ -244,4 +253,8 @@ function SidebarTabs({
244253
);
245254
}
246255

247-
export default withServices(SidebarTabs, ['annotationsService', 'settings']);
256+
export default withServices(SidebarTabs, [
257+
'annotationsService',
258+
'frameSync',
259+
'settings',
260+
]);

src/sidebar/components/test/SidebarTabs-test.js

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ describe('SidebarTabs', () => {
1111
// mock services
1212
let fakeAnnotationsService;
1313
let fakeSettings;
14+
let fakeFrameSync;
1415
let fakeStore;
1516
let fakeUseRootThread;
1617

@@ -19,6 +20,7 @@ describe('SidebarTabs', () => {
1920
<SidebarTabs
2021
annotationsService={fakeAnnotationsService}
2122
settings={fakeSettings}
23+
frameSync={fakeFrameSync}
2224
isLoading={false}
2325
{...props}
2426
/>,
@@ -46,6 +48,9 @@ describe('SidebarTabs', () => {
4648
fakeSettings = {
4749
enableExperimentalNewNoteButton: false,
4850
};
51+
fakeFrameSync = {
52+
getDocumentInfo: sinon.stub().resolves({ metadata: {} }),
53+
};
4954
fakeStore = {
5055
selectTab: sinon.stub(),
5156
isWaitingToAnchorAnnotations: sinon.stub().returns(false),
@@ -151,13 +156,17 @@ describe('SidebarTabs', () => {
151156
assert.deepEqual(button.prop('style'), { backgroundColor: '#00f' });
152157
});
153158

154-
it('should add a new page note on click', () => {
159+
it('should add a new page note on click', async () => {
155160
fakeSettings.enableExperimentalNewNoteButton = true;
156161
fakeStore.selectedTab.returns('note');
157162

158163
const wrapper = createComponent();
159-
wrapper.find('Button[data-testid="new-note-button"]').props().onClick();
164+
await wrapper
165+
.find('Button[data-testid="new-note-button"]')
166+
.props()
167+
.onClick();
160168

169+
assert.calledOnce(fakeFrameSync.getDocumentInfo);
161170
assert.calledOnce(fakeAnnotationsService.createPageNote);
162171
});
163172
});

src/sidebar/services/annotations.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { ModerationStatus } from '@hypothesis/annotation-ui';
22

33
import { generateHexString } from '../../shared/random';
4-
import type { AnnotationData } from '../../types/annotator';
4+
import type { AnnotationData, DocumentMetadata } from '../../types/annotator';
55
import type {
66
APIAnnotationData,
77
Annotation,
@@ -211,7 +211,7 @@ export class AnnotationsService {
211211
* Create a new empty "page note" annotation and add it to the store. If the
212212
* user is not logged in, open the `loginPrompt` panel instead.
213213
*/
214-
createPageNote() {
214+
createPageNote(document?: DocumentMetadata) {
215215
const topLevelFrame = this._store.mainFrame();
216216
if (!this._store.isLoggedIn()) {
217217
this._store.openSidebarPanel('loginPrompt');
@@ -227,7 +227,8 @@ export class AnnotationsService {
227227
},
228228
],
229229
uri: topLevelFrame.uri,
230-
};
230+
document,
231+
} satisfies Partial<AnnotationData>;
231232
this.create(pageNoteAnnotation);
232233
}
233234

src/sidebar/services/frame-sync.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
isMessageEqual,
1212
} from '../../shared/messaging';
1313
import type { Message } from '../../shared/messaging';
14+
import { promiseWithResolvers } from '../../shared/promise-with-resolvers';
1415
import type {
1516
AnnotationData,
1617
DocumentInfo,
@@ -704,6 +705,20 @@ export class FrameSyncService {
704705
});
705706
}
706707

708+
async getDocumentInfo(): Promise<DocumentInfo> {
709+
// Get the guest for the main frame. This assumes that the annotation is
710+
// anchored in the main frame.
711+
const guest = this._guestRPC.get(null);
712+
if (!guest) {
713+
throw new Error('No guest connected');
714+
}
715+
716+
const { promise, resolve } = promiseWithResolvers<DocumentInfo>();
717+
guest.call('getDocumentInfo', resolve);
718+
719+
return promise;
720+
}
721+
707722
// Only used to cleanup tests
708723
destroy() {
709724
this._portFinder.destroy();

src/sidebar/services/test/annotations-test.js

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -314,18 +314,22 @@ describe('AnnotationsService', () => {
314314
assert.isNull(getLastAddedAnnotation());
315315
});
316316

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

320-
const annotation = getLastAddedAnnotation();
322+
const annotation = getLastAddedAnnotation();
321323

322-
assert.equal(annotation.uri, 'http://www.example.com');
323-
assert.deepEqual(annotation.target, [
324-
{
325-
source: 'http://www.example.com',
326-
},
327-
]);
328-
});
324+
assert.equal(annotation.uri, 'http://www.example.com');
325+
assert.deepEqual(annotation.target, [
326+
{
327+
source: 'http://www.example.com',
328+
},
329+
]);
330+
assert.deepEqual(annotation.document, document);
331+
},
332+
);
329333
});
330334

331335
describe('delete', () => {

src/sidebar/services/test/frame-sync-test.js

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { delay } from '@hypothesis/frontend-testing';
2+
import sinon from 'sinon';
23

34
import { EventEmitter } from '../../../shared/event-emitter';
45
import { Injector } from '../../../shared/injector';
@@ -1279,4 +1280,36 @@ describe('FrameSyncService', () => {
12791280
assert.equal(err.message, 'Something went wrong');
12801281
});
12811282
});
1283+
1284+
describe('#getDocumentInfo', () => {
1285+
beforeEach(async () => {
1286+
await frameSync.connect();
1287+
});
1288+
1289+
it('rejects if guest is not connected', async () => {
1290+
let err;
1291+
try {
1292+
await frameSync.getDocumentInfo();
1293+
} catch (e) {
1294+
err = e;
1295+
}
1296+
assert.instanceOf(err, Error);
1297+
assert.equal(err.message, 'No guest connected');
1298+
});
1299+
1300+
it('requests document info from guest', async () => {
1301+
await connectGuest();
1302+
1303+
guestRPC().call.callsFake((method, callback) => {
1304+
if (method === 'getDocumentInfo') {
1305+
delay(0).then(() => callback({ title: 'foo' }));
1306+
}
1307+
});
1308+
1309+
const document = await frameSync.getDocumentInfo();
1310+
1311+
assert.calledWith(guestRPC().call, 'getDocumentInfo', sinon.match.func);
1312+
assert.deepEqual(document, { title: 'foo' });
1313+
});
1314+
});
12821315
});

src/types/port-rpc-calls.d.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,11 @@ export type SidebarToGuestCalls = {
202202
* current activity / assignment.
203203
*/
204204
setOutsideAssignmentNoticeVisible(visible: boolean): void;
205+
206+
/**
207+
* Expose the guest document info to the sidebar
208+
*/
209+
getDocumentInfo(callback: (info: DocumentInfo) => void): void;
205210
};
206211

207212
/** Calls that the sidebar makes to the host. */

0 commit comments

Comments
 (0)