Commit 99c414b4 authored by Robert Knight's avatar Robert Knight

Store tags rather than IDs for pending scroll/hover

Following https://github.com/hypothesis/client/pull/4962, we can now identify
annotations that should be scrolled to/focused after they anchor by their tag
rather than ID, since the annotation in the sidebar's store persists across the
chapter navigation.
parent a0cb5684
...@@ -127,22 +127,19 @@ export class FrameSyncService { ...@@ -127,22 +127,19 @@ export class FrameSyncService {
private _store: SidebarStore; private _store: SidebarStore;
/** /**
* ID of an annotation that should be scrolled to after anchoring completes. * Tag of an annotation that should be scrolled to after anchoring completes.
* *
* This is set when {@link scrollToAnnotation} is called and the document * This is set when {@link scrollToAnnotation} is called and the document
* needs to be navigated to a different URL. This can happen in EPUBs. * needs to be navigated to a different URL. This can happen in EPUBs.
*
* We store an ID rather than a tag because the navigation currently triggers
* a reload of annotations, which will change their tags but not their IDs.
*/ */
private _pendingScrollToId: string | null; private _pendingScrollToTag: string | null;
/** /**
* ID of an annotation that should be hovered after anchoring completes. * Tag of an annotation that should be hovered after anchoring completes.
* *
* See notes for {@link _pendingScrollToId}. * See notes for {@link _pendingScrollToTag}.
*/ */
private _pendingHoverId: string | null; private _pendingHoverTag: string | null;
// Test seam // Test seam
private _window: Window; private _window: Window;
...@@ -166,8 +163,8 @@ export class FrameSyncService { ...@@ -166,8 +163,8 @@ export class FrameSyncService {
this._inFrame = new Set<string>(); this._inFrame = new Set<string>();
this._highlightsVisible = false; this._highlightsVisible = false;
this._pendingScrollToId = null; this._pendingScrollToTag = null;
this._pendingHoverId = null; this._pendingHoverTag = null;
this._setupSyncToGuests(); this._setupSyncToGuests();
this._setupHostEvents(); this._setupHostEvents();
...@@ -376,14 +373,13 @@ export class FrameSyncService { ...@@ -376,14 +373,13 @@ export class FrameSyncService {
anchoringStatusUpdates[$tag] = $orphan ? 'orphan' : 'anchored'; anchoringStatusUpdates[$tag] = $orphan ? 'orphan' : 'anchored';
scheduleAnchoringStatusUpdate(); scheduleAnchoringStatusUpdate();
const [id] = this._store.findIDsForTags([$tag]); if ($tag === this._pendingHoverTag) {
if (id === this._pendingHoverId) { this._pendingHoverTag = null;
this._pendingHoverId = null;
guestRPC.call('hoverAnnotations', [$tag]); guestRPC.call('hoverAnnotations', [$tag]);
} }
if (this._pendingScrollToId) { if (this._pendingScrollToTag) {
if (id === this._pendingScrollToId) { if ($tag === this._pendingScrollToTag) {
this._pendingScrollToId = null; this._pendingScrollToTag = null;
guestRPC.call('scrollToAnnotation', $tag); guestRPC.call('scrollToAnnotation', $tag);
} }
} }
...@@ -532,7 +528,7 @@ export class FrameSyncService { ...@@ -532,7 +528,7 @@ export class FrameSyncService {
* user can hover multiple highlights in the document at once. * user can hover multiple highlights in the document at once.
*/ */
hoverAnnotation(ann: Annotation | null) { hoverAnnotation(ann: Annotation | null) {
this._pendingHoverId = null; this._pendingHoverTag = null;
if (!ann) { if (!ann) {
this._guestRPC.forEach(rpc => rpc.call('hoverAnnotations', [])); this._guestRPC.forEach(rpc => rpc.call('hoverAnnotations', []));
...@@ -551,9 +547,7 @@ export class FrameSyncService { ...@@ -551,9 +547,7 @@ export class FrameSyncService {
!frame || !frame ||
(frame.segment && !annotationMatchesSegment(ann, frame.segment)) (frame.segment && !annotationMatchesSegment(ann, frame.segment))
) { ) {
if (ann.id) { this._pendingHoverTag = ann.$tag;
this._pendingHoverId = ann.id;
}
return; return;
} }
this._guestRPC.forEach(rpc => rpc.call('hoverAnnotations', tags)); this._guestRPC.forEach(rpc => rpc.call('hoverAnnotations', tags));
...@@ -580,10 +574,8 @@ export class FrameSyncService { ...@@ -580,10 +574,8 @@ export class FrameSyncService {
// the annotation to anchor in the new guest frame before we can actually // the annotation to anchor in the new guest frame before we can actually
// scroll to it. // scroll to it.
if (frame.segment && !annotationMatchesSegment(ann, frame.segment)) { if (frame.segment && !annotationMatchesSegment(ann, frame.segment)) {
if (ann.id) { // Schedule scroll once anchoring completes.
// Schedule scroll once anchoring completes. this._pendingScrollToTag = ann.$tag;
this._pendingScrollToId = ann.id;
}
guest.call('navigateToSegment', formatAnnot(ann)); guest.call('navigateToSegment', formatAnnot(ann));
return; return;
} }
......
...@@ -947,7 +947,6 @@ describe('FrameSyncService', () => { ...@@ -947,7 +947,6 @@ describe('FrameSyncService', () => {
// Create an annotation with a CFI that doesn't match `fixtures.epubDocumentInfo`. // Create an annotation with a CFI that doesn't match `fixtures.epubDocumentInfo`.
const ann = createEPUBAnnotation('/4/8'); const ann = createEPUBAnnotation('/4/8');
fakeStore.findIDsForTags.withArgs([ann.$tag]).returns([ann.id]);
// Request hover of annotation. The annotation is marked as hovered in // Request hover of annotation. The annotation is marked as hovered in
// the sidebar, but nothing is sent to the guest since the annotation's // the sidebar, but nothing is sent to the guest since the annotation's
...@@ -1001,7 +1000,6 @@ describe('FrameSyncService', () => { ...@@ -1001,7 +1000,6 @@ describe('FrameSyncService', () => {
// Create an annotation with a CFI that doesn't match `fixtures.epubDocumentInfo`. // Create an annotation with a CFI that doesn't match `fixtures.epubDocumentInfo`.
const ann = createEPUBAnnotation('/4/8'); const ann = createEPUBAnnotation('/4/8');
fakeStore.findIDsForTags.withArgs([ann.$tag]).returns([ann.id]);
// Request a scroll to this annotation, this will require a navigation of // Request a scroll to this annotation, this will require a navigation of
// the guest frame. // the guest frame.
......
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