Commit a0cb5684 authored by Robert Knight's avatar Robert Knight

Handle hovering annotations which are not loaded in guest frame

When a user hovers an annotation card for an EPUB chapter that is different than
the one currently loaded in the guest, and then clicks on that annotation to
navigate the guest to that chapter, ensure that the highlights in the document
are displayed in a hovered state once anchoring completes.

This is handled similar to `FrameSyncService.scrollToAnnotation`, by setting a
flag if the annotation is not loaded in the guest, which triggers a
"hoverAnnotations" RPC call once the annotation is anchored.
parent bb8dd834
...@@ -55,13 +55,6 @@ function SidebarView({ ...@@ -55,13 +55,6 @@ function SidebarView({
const sidebarHasOpened = store.hasSidebarOpened(); const sidebarHasOpened = store.hasSidebarOpened();
const userId = store.profile().userid; const userId = store.profile().userid;
// The local `$tag` of a direct-linked annotation; populated once it
// has anchored: meaning that it's ready to be focused and scrolled to
const linkedAnnotationAnchorTag =
linkedAnnotation && linkedAnnotation.$orphan === false
? linkedAnnotation.$tag
: null;
// If, after loading completes, no `linkedAnnotation` object is present when // If, after loading completes, no `linkedAnnotation` object is present when
// a `linkedAnnotationId` is set, that indicates an error // a `linkedAnnotationId` is set, that indicates an error
const hasDirectLinkedAnnotationError = const hasDirectLinkedAnnotationError =
...@@ -116,21 +109,15 @@ function SidebarView({ ...@@ -116,21 +109,15 @@ function SidebarView({
// When a `linkedAnnotationAnchorTag` becomes available, scroll to it // When a `linkedAnnotationAnchorTag` becomes available, scroll to it
// and focus it // and focus it
useEffect(() => { useEffect(() => {
if (linkedAnnotation && linkedAnnotationAnchorTag) { if (linkedAnnotation && linkedAnnotation.$orphan === false) {
frameSync.hoverAnnotations([linkedAnnotationAnchorTag]); frameSync.hoverAnnotation(linkedAnnotation);
frameSync.scrollToAnnotation(linkedAnnotation); frameSync.scrollToAnnotation(linkedAnnotation);
store.selectTab(directLinkedTab); store.selectTab(directLinkedTab);
} else if (linkedAnnotation) { } else if (linkedAnnotation) {
// Make sure to allow for orphaned annotations (which won't have an anchor) // Make sure to allow for orphaned annotations (which won't have an anchor)
store.selectTab(directLinkedTab); store.selectTab(directLinkedTab);
} }
}, [ }, [directLinkedTab, frameSync, linkedAnnotation, store]);
directLinkedTab,
frameSync,
linkedAnnotation,
linkedAnnotationAnchorTag,
store,
]);
// Connect to the streamer when the sidebar has opened or if user is logged in // Connect to the streamer when the sidebar has opened or if user is logged in
const hasFetchedProfile = store.hasFetchedProfile(); const hasFetchedProfile = store.hasFetchedProfile();
......
...@@ -32,11 +32,8 @@ function ThreadCard({ frameSync, thread }) { ...@@ -32,11 +32,8 @@ function ThreadCard({ frameSync, thread }) {
const focusThreadAnnotation = useMemo( const focusThreadAnnotation = useMemo(
() => () =>
debounce( debounce(
/** @param {string|null} tag */ /** @param {Annotation|null} ann */
tag => { ann => frameSync.hoverAnnotation(ann),
const focusTags = tag ? [tag] : [];
frameSync.hoverAnnotations(focusTags);
},
10 10
), ),
[frameSync] [frameSync]
...@@ -94,7 +91,7 @@ function ThreadCard({ frameSync, thread }) { ...@@ -94,7 +91,7 @@ function ThreadCard({ frameSync, thread }) {
scrollToAnnotation(thread.annotation); scrollToAnnotation(thread.annotation);
} }
}} }}
onMouseEnter={() => focusThreadAnnotation(threadTag ?? null)} onMouseEnter={() => focusThreadAnnotation(thread.annotation ?? null)}
onMouseLeave={() => focusThreadAnnotation(null)} onMouseLeave={() => focusThreadAnnotation(null)}
key={thread.id} key={thread.id}
> >
......
...@@ -26,7 +26,7 @@ describe('SidebarView', () => { ...@@ -26,7 +26,7 @@ describe('SidebarView', () => {
beforeEach(() => { beforeEach(() => {
fakeFrameSync = { fakeFrameSync = {
hoverAnnotations: sinon.stub(), hoverAnnotation: sinon.stub(),
scrollToAnnotation: sinon.stub(), scrollToAnnotation: sinon.stub(),
}; };
fakeLoadAnnotationsService = { fakeLoadAnnotationsService = {
...@@ -148,11 +148,8 @@ describe('SidebarView', () => { ...@@ -148,11 +148,8 @@ describe('SidebarView', () => {
createComponent(); createComponent();
assert.calledOnce(fakeFrameSync.scrollToAnnotation); assert.calledOnce(fakeFrameSync.scrollToAnnotation);
assert.calledWith(fakeFrameSync.scrollToAnnotation, fakeAnnotation); assert.calledWith(fakeFrameSync.scrollToAnnotation, fakeAnnotation);
assert.calledOnce(fakeFrameSync.hoverAnnotations); assert.calledOnce(fakeFrameSync.hoverAnnotation);
assert.calledWith( assert.calledWith(fakeFrameSync.hoverAnnotation, fakeAnnotation);
fakeFrameSync.hoverAnnotations,
sinon.match(['myTag'])
);
}); });
it('selects the correct tab for direct-linked annotations once anchored', () => { it('selects the correct tab for direct-linked annotations once anchored', () => {
......
...@@ -27,7 +27,7 @@ describe('ThreadCard', () => { ...@@ -27,7 +27,7 @@ describe('ThreadCard', () => {
fakeDebounce = sinon.stub().returnsArg(0); fakeDebounce = sinon.stub().returnsArg(0);
fakeFrameSync = { fakeFrameSync = {
hoverAnnotations: sinon.stub(), hoverAnnotation: sinon.stub(),
scrollToAnnotation: sinon.stub(), scrollToAnnotation: sinon.stub(),
}; };
fakeStore = { fakeStore = {
...@@ -84,7 +84,7 @@ describe('ThreadCard', () => { ...@@ -84,7 +84,7 @@ describe('ThreadCard', () => {
wrapper.find(threadCardSelector).simulate('mouseenter'); wrapper.find(threadCardSelector).simulate('mouseenter');
assert.calledWith(fakeFrameSync.hoverAnnotations, sinon.match(['myTag'])); assert.calledWith(fakeFrameSync.hoverAnnotation, fakeThread.annotation);
}); });
it('unfocuses the annotation thread when mouse exits', () => { it('unfocuses the annotation thread when mouse exits', () => {
...@@ -92,7 +92,7 @@ describe('ThreadCard', () => { ...@@ -92,7 +92,7 @@ describe('ThreadCard', () => {
wrapper.find(threadCardSelector).simulate('mouseleave'); wrapper.find(threadCardSelector).simulate('mouseleave');
assert.calledWith(fakeFrameSync.hoverAnnotations, sinon.match([])); assert.calledWith(fakeFrameSync.hoverAnnotation, null);
}); });
['button', 'a'].forEach(tag => { ['button', 'a'].forEach(tag => {
......
...@@ -137,6 +137,13 @@ export class FrameSyncService { ...@@ -137,6 +137,13 @@ export class FrameSyncService {
*/ */
private _pendingScrollToId: string | null; private _pendingScrollToId: string | null;
/**
* ID of an annotation that should be hovered after anchoring completes.
*
* See notes for {@link _pendingScrollToId}.
*/
private _pendingHoverId: string | null;
// Test seam // Test seam
private _window: Window; private _window: Window;
...@@ -158,7 +165,9 @@ export class FrameSyncService { ...@@ -158,7 +165,9 @@ export class FrameSyncService {
this._guestRPC = new Map(); this._guestRPC = new Map();
this._inFrame = new Set<string>(); this._inFrame = new Set<string>();
this._highlightsVisible = false; this._highlightsVisible = false;
this._pendingScrollToId = null; this._pendingScrollToId = null;
this._pendingHoverId = null;
this._setupSyncToGuests(); this._setupSyncToGuests();
this._setupHostEvents(); this._setupHostEvents();
...@@ -367,8 +376,12 @@ export class FrameSyncService { ...@@ -367,8 +376,12 @@ export class FrameSyncService {
anchoringStatusUpdates[$tag] = $orphan ? 'orphan' : 'anchored'; anchoringStatusUpdates[$tag] = $orphan ? 'orphan' : 'anchored';
scheduleAnchoringStatusUpdate(); scheduleAnchoringStatusUpdate();
const [id] = this._store.findIDsForTags([$tag]);
if (id === this._pendingHoverId) {
this._pendingHoverId = null;
guestRPC.call('hoverAnnotations', [$tag]);
}
if (this._pendingScrollToId) { if (this._pendingScrollToId) {
const [id] = this._store.findIDsForTags([$tag]);
if (id === this._pendingScrollToId) { if (id === this._pendingScrollToId) {
this._pendingScrollToId = null; this._pendingScrollToId = null;
guestRPC.call('scrollToAnnotation', $tag); guestRPC.call('scrollToAnnotation', $tag);
...@@ -508,15 +521,41 @@ export class FrameSyncService { ...@@ -508,15 +521,41 @@ export class FrameSyncService {
} }
/** /**
* Mark annotations as hovered. * Mark annotation as hovered.
* *
* This is used to indicate the highlights in the document that correspond * This is used to indicate the highlights in the document that correspond
* to hovered annotations in the sidebar. * to a hovered annotation in the sidebar.
* *
* @param tags - annotation $tags * This function only accepts a single annotation because the user can only
* hover one annotation card in the sidebar at a time. Hover updates in the
* other direction (guest to sidebar) support multiple annotations since a
* user can hover multiple highlights in the document at once.
*/ */
hoverAnnotations(tags: string[]) { hoverAnnotation(ann: Annotation | null) {
this._pendingHoverId = null;
if (!ann) {
this._guestRPC.forEach(rpc => rpc.call('hoverAnnotations', []));
return;
}
const tags = ann ? [ann.$tag] : [];
this._store.hoverAnnotations(tags); this._store.hoverAnnotations(tags);
// If annotation is not currently anchored in a guest, schedule hover for
// when annotation is anchored. This can happen if an annotation is for a
// different chapter of an EPUB than the currently loaded one. See notes in
// `scrollToAnnotation`.
const frame = frameForAnnotation(this._store.frames(), ann);
if (
!frame ||
(frame.segment && !annotationMatchesSegment(ann, frame.segment))
) {
if (ann.id) {
this._pendingHoverId = ann.id;
}
return;
}
this._guestRPC.forEach(rpc => rpc.call('hoverAnnotations', tags)); this._guestRPC.forEach(rpc => rpc.call('hoverAnnotations', tags));
} }
......
...@@ -147,7 +147,7 @@ describe('FrameSyncService', () => { ...@@ -147,7 +147,7 @@ describe('FrameSyncService', () => {
this.setState({ contentInfo: info }); this.setState({ contentInfo: info });
}, },
findIDsForTags: sinon.stub(), findIDsForTags: sinon.stub().returns([]),
hoverAnnotations: sinon.stub(), hoverAnnotations: sinon.stub(),
isLoggedIn: sinon.stub().returns(false), isLoggedIn: sinon.stub().returns(false),
openSidebarPanel: sinon.stub(), openSidebarPanel: sinon.stub(),
...@@ -226,6 +226,29 @@ describe('FrameSyncService', () => { ...@@ -226,6 +226,29 @@ describe('FrameSyncService', () => {
return port1; return port1;
} }
/**
* Create a fake annotation for an EPUB book.
*
* @param {string} cfi - Canonical Fragment Identifier indicating which chapter
* the annotation relates to
*/
function createEPUBAnnotation(cfi) {
return {
id: 'epub-id-1',
$tag: 'epub-tag-1',
target: [
{
selector: [
{
type: 'EPUBContentSelector',
cfi,
},
],
},
],
};
}
describe('#connect', () => { describe('#connect', () => {
it('discovers and connects to the host frame', async () => { it('discovers and connects to the host frame', async () => {
await frameSync.connect(); await frameSync.connect();
...@@ -890,28 +913,64 @@ describe('FrameSyncService', () => { ...@@ -890,28 +913,64 @@ describe('FrameSyncService', () => {
}); });
}); });
describe('#hoverAnnotations', () => { describe('#hoverAnnotation', () => {
beforeEach(async () => { beforeEach(async () => {
frameSync.connect(); frameSync.connect();
await connectGuest(); await connectGuest();
emitGuestEvent('documentInfoChanged', fixtures.htmlDocumentInfo);
}); });
it('should update the focused annotations in the store', () => { it('updates the focused annotations in the store', () => {
frameSync.hoverAnnotations(['a1', 'a2']); frameSync.hoverAnnotation(fixtures.ann);
assert.calledWith( assert.calledWith(
fakeStore.hoverAnnotations, fakeStore.hoverAnnotations,
sinon.match.array.deepEquals(['a1', 'a2']) sinon.match.array.deepEquals([fixtures.ann.$tag])
); );
}); });
it('should focus the associated highlights in the guest', () => { it('focuses the associated highlights in the guest', () => {
frameSync.hoverAnnotations([1, 2]); frameSync.hoverAnnotation(fixtures.ann);
assert.calledWith( assert.calledWith(
guestRPC().call, guestRPC().call,
'hoverAnnotations', 'hoverAnnotations',
sinon.match.array.deepEquals([1, 2]) sinon.match.array.deepEquals([fixtures.ann.$tag])
); );
}); });
it('clears focused annotations in guest if argument is `null`', () => {
frameSync.hoverAnnotation(null);
assert.calledWith(guestRPC().call, 'hoverAnnotations', []);
});
it('defers focusing highlights when annotation is in a different EPUB chapter', async () => {
emitGuestEvent('documentInfoChanged', fixtures.epubDocumentInfo);
// Create an annotation with a CFI that doesn't match `fixtures.epubDocumentInfo`.
const ann = createEPUBAnnotation('/4/8');
fakeStore.findIDsForTags.withArgs([ann.$tag]).returns([ann.id]);
// Request hover of annotation. The annotation is marked as hovered in
// the sidebar, but nothing is sent to the guest since the annotation's
// book chapter is not loaded.
frameSync.hoverAnnotation(ann);
assert.calledWith(
fakeStore.hoverAnnotations,
sinon.match.array.deepEquals([ann.$tag])
);
assert.isFalse(guestRPC().call.calledWith('hoverAnnotations'));
// Simulate annotation anchoring at a later point, after a chapter
// navigation.
emitGuestEvent('syncAnchoringStatus', { $tag: ann.$tag, $orphan: false });
assert.calledWith(guestRPC().call, 'hoverAnnotations', [ann.$tag]);
// After the `hoverAnnotations` call has been sent, the pending-hover
// state internally should be cleared and a later `syncAnchoringStatus`
// event should not re-hover.
guestRPC().call.resetHistory();
emitGuestEvent('syncAnchoringStatus', { $tag: ann.$tag, $orphan: false });
assert.notCalled(guestRPC().call);
});
}); });
describe('#scrollToAnnotation', () => { describe('#scrollToAnnotation', () => {
...@@ -923,23 +982,6 @@ describe('FrameSyncService', () => { ...@@ -923,23 +982,6 @@ describe('FrameSyncService', () => {
frameSync.scrollToAnnotation(fixtures.ann); frameSync.scrollToAnnotation(fixtures.ann);
}); });
function createEPUBAnnotation(cfi) {
return {
id: 'epub-id-1',
$tag: 'epub-tag-1',
target: [
{
selector: [
{
type: 'EPUBContentSelector',
cfi,
},
],
},
],
};
}
it('should scroll to the annotation in the correct guest', async () => { it('should scroll to the annotation in the correct guest', async () => {
await connectGuest(); await connectGuest();
emitGuestEvent('documentInfoChanged', fixtures.htmlDocumentInfo); emitGuestEvent('documentInfoChanged', fixtures.htmlDocumentInfo);
......
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