fixes for focus'ing to enable resetting of all navigations if desired(eg. zooming into portal)

7 files changed, 58 insertions, 43 deletions

diff --git a/src/Utils.ts b/src/Utils.ts
index 770cef283..b6a59118a 100644
--- a/src/Utils.ts
+++ b/src/Utils.ts
@@ -193,9 +193,9 @@ export namespace Utils {
 
     export function scrollIntoView(targetY: number, targetHgt: number, scrollTop: number, contextHgt: number) {
         if (scrollTop + contextHgt < targetY + targetHgt * 1.1) {
-            return targetY + targetHgt * 1.1 - contextHgt;
+            return Math.ceil(targetY + targetHgt * 1.1 - contextHgt);
         } else if (scrollTop > targetY - targetHgt * .1) {
-            return Math.max(0, targetY - targetHgt * .1);
+            return Math.max(0, Math.floor(targetY - targetHgt * .1));
         }
     }
 
diff --git a/src/client/util/DocumentManager.ts b/src/client/util/DocumentManager.ts
index 5d3f8af57..816f7f6be 100644
--- a/src/client/util/DocumentManager.ts
+++ b/src/client/util/DocumentManager.ts
@@ -144,6 +144,10 @@ export class DocumentManager {
     ): Promise<void> => {
         const getFirstDocView = LightboxView.LightboxDoc ? DocumentManager.Instance.getLightboxDocumentView : DocumentManager.Instance.getFirstDocumentView;
         const docView = getFirstDocView(targetDoc, originatingDoc);
+        const highlight = () => {
+            const finalDocView = getFirstDocView(targetDoc);
+            finalDocView && Doc.linkFollowHighlight(finalDocView.rootDoc);
+        };
         const focusAndFinish = (didFocus: boolean) => {
             if (originatingDoc?.isPushpin) {
                 if (!didFocus || targetDoc.hidden) {
@@ -153,13 +157,10 @@ export class DocumentManager {
                 targetDoc.hidden && (targetDoc.hidden = undefined);
                 docView?.select(false);
             }
+            highlight();
             finished?.();
             return false;
         };
-        const highlight = () => {
-            const finalDocView = getFirstDocView(targetDoc);
-            finalDocView && Doc.linkFollowHighlight(finalDocView.rootDoc);
-        };
         let annotatedDoc = Cast(targetDoc.annotationOn, Doc, null);
         const contextDocs = docContext ? await DocListCastAsync(docContext.data) : undefined;
         const contextDoc = contextDocs?.find(doc => Doc.AreProtosEqual(doc, targetDoc) || Doc.AreProtosEqual(doc, annotatedDoc)) ? docContext : undefined;
@@ -172,7 +173,12 @@ export class DocumentManager {
                 first.focus(targetDoc, false);
             }
         } else if (docView && (targetDocContextView || !targetDocContext)) {  // we have a docView already and aren't forced to create a new one ... just focus on the document.  TODO move into view if necessary otherwise just highlight?      
-            docView.props.focus(docView.rootDoc, willZoom, undefined, (didFocus: boolean) => focusAndFinish(didFocus));
+            docView.props.focus(docView.rootDoc, willZoom, undefined, (didFocus: boolean) =>
+                new Promise<boolean>(res => {
+                    focusAndFinish(didFocus);
+                    res();
+                })
+            );
         } else {
             if (!targetDocContext) { // we don't have a view and there's no context specified ... create a new view of the target using the dockFunc or default
                 createViewFunc(Doc.BrushDoc(targetDoc), finished); // bcz: should we use this?: Doc.MakeAlias(targetDoc)));
@@ -190,7 +196,12 @@ export class DocumentManager {
                         const findView = (delay: number) => {
                             const retryDocView = getFirstDocView(targetDoc);  // test again for the target view snce we presumably created the context above by focusing on it
                             if (retryDocView) {   // we found the target in the context
-                                retryDocView.props.focus(targetDoc, willZoom, undefined, focusAndFinish); // focus on the target in the context
+                                retryDocView.props.focus(targetDoc, willZoom, undefined, (didFocus: boolean) =>
+                                    new Promise<boolean>(res => {
+                                        focusAndFinish(didFocus);
+                                        res();
+                                    })
+                                ); // focus on the target in the context
                                 highlight();
                             } else if (delay > 1500) {
                                 // we didn't find the target, so it must have moved out of the context.  Go back to just creating it.
diff --git a/src/client/util/LinkManager.ts b/src/client/util/LinkManager.ts
index 8a6ed9944..58ccfe645 100644
--- a/src/client/util/LinkManager.ts
+++ b/src/client/util/LinkManager.ts
@@ -103,25 +103,29 @@ export class LinkManager {
     public static FollowLink = (linkDoc: Opt<Doc>, sourceDoc: Doc, docViewProps: DocumentViewSharedProps, altKey: boolean) => {
         const batch = UndoManager.StartBatch("follow link click");
         // open up target if it's not already in view ...
-        const createViewFunc = (doc: Doc, followLoc: string, finished: Opt<() => void>) => {
-            const targetFocusAfterDocFocus = (didFocus: boolean) => {
+        const createViewFunc = (doc: Doc, followLoc: string, finished?: Opt<() => void>) => {
+            const createTabForTarget = (didFocus: boolean) => new Promise<boolean>(res => {
                 const where = StrCast(sourceDoc.followLinkLocation) || followLoc;
-                const hackToCallFinishAfterFocus = () => {
-                    finished && setTimeout(finished); // finished() needs to be called right after hackToCallFinishAfterFocus(), but there's no callback for that so we use the hacky timeout.
-                    return false; // we must return false here so that the zoom to the document is not reversed.  If it weren't for needing to call finished(), we wouldn't need this function at all since not having it is equivalent to returning false
-                };
-                const addTab = docViewProps.addDocTab(doc, where);
-                addTab && setTimeout(() => {
+                docViewProps.addDocTab(doc, where);
+                setTimeout(() => {
                     const targDocView = DocumentManager.Instance.getFirstDocumentView(doc);
-                    targDocView?.props.focus(doc, BoolCast(sourceDoc.followLinkZoom, false), undefined, hackToCallFinishAfterFocus);
-                }); //  add the target and focus on it.
-                return where !== "inPlace" || addTab; // return true to reset the initial focus&zoom (return false for 'inPlace' since resetting the initial focus&zoom will negate the zoom into the target)
-            };
+                    if (targDocView) {
+                        targDocView.props.focus(doc, BoolCast(sourceDoc.followLinkZoom, false), undefined, (didFocus: boolean) => {
+                            finished?.();
+                            res(true);
+                            return new Promise<boolean>(res2 => res2());
+                        });
+                    } else {
+                        res(where !== "inPlace"); // return true to reset the initial focus&zoom (return false for 'inPlace' since resetting the initial focus&zoom will negate the zoom into the target)
+                    }
+                });
+            });
+
             if (!sourceDoc.followLinkZoom) {
-                targetFocusAfterDocFocus(false);
+                createTabForTarget(false);
             } else {
                 // first focus & zoom onto this (the clicked document).  Then execute the function to focus on the target
-                docViewProps.focus(sourceDoc, BoolCast(sourceDoc.followLinkZoom, true), 1, targetFocusAfterDocFocus);
+                docViewProps.focus(sourceDoc, BoolCast(sourceDoc.followLinkZoom, true), 1, createTabForTarget);
             }
         };
         LinkManager.traverseLink(linkDoc, sourceDoc, createViewFunc, BoolCast(sourceDoc.followLinkZoom, false), docViewProps.ContainingCollectionDoc, batch.end, altKey ? true : undefined);
diff --git a/src/client/views/collections/CollectionStackingView.tsx b/src/client/views/collections/CollectionStackingView.tsx
index 770aea362..9c12d5020 100644
--- a/src/client/views/collections/CollectionStackingView.tsx
+++ b/src/client/views/collections/CollectionStackingView.tsx
@@ -192,7 +192,7 @@ export class CollectionStackingView extends CollectionSubView<StackingDocument,
             }
         }
         return this.props.styleProvider?.(doc, props, property);
-    };
+    }
     getDisplayDoc(doc: Doc, width: () => number) {
         const dataDoc = (!doc.isTemplateDoc && !doc.isTemplateForField && !doc.PARAMS) ? undefined : this.props.DataDoc;
         const height = () => this.getDocHeight(doc);
diff --git a/src/client/views/collections/collectionFreeForm/CollectionFreeFormView.tsx b/src/client/views/collections/collectionFreeForm/CollectionFreeFormView.tsx
index 7ae4137d9..eeb4e61aa 100644
--- a/src/client/views/collections/collectionFreeForm/CollectionFreeFormView.tsx
+++ b/src/client/views/collections/collectionFreeForm/CollectionFreeFormView.tsx
@@ -49,6 +49,7 @@ import { CurrentUserUtils } from "../../../util/CurrentUserUtils";
 import { StyleProp, StyleLayers } from "../../StyleProvider";
 import { DocumentDecorations } from "../../DocumentDecorations";
 import { FieldViewProps } from "../../nodes/FieldView";
+import { reset } from "colors";
 
 export const panZoomSchema = createSchema({
     _panX: "number",
@@ -908,8 +909,8 @@ export class CollectionFreeFormView extends CollectionSubView<PanZoomDocument, P
             const layoutdoc = Doc.Layout(doc);
             const savedState = { px: NumCast(this.Document._panX), py: NumCast(this.Document._panY), s: this.Document[this.scaleFieldKey], pt: this.Document._viewTransition };
             const newState = HistoryUtil.getState();
-            const { px, py } = layoutdoc.annotationOn ? savedState : willZoom ? this.setZoomToDoc(layoutdoc, scale) : this.setPanIntoView(doc);
-            if (!layoutdoc.annotationOn) {   // only pan and zoom to focus on a document if the document is not an annotation in an annotation overlay collection
+            const { px, py } = layoutdoc.annotationOn || this.rootDoc._isGroup ? savedState : willZoom ? this.setZoomToDoc(layoutdoc, scale) : this.setPanIntoView(doc);
+            if (!layoutdoc.annotationOn && !this.rootDoc._isGroup) {   // only pan and zoom to focus on a document if the document is not an annotation in an annotation overlay collection
                 newState.initializers![this.Document[Id]] = { panX: px, panY: py };
                 HistoryUtil.pushState(newState);
             }
@@ -921,22 +922,21 @@ export class CollectionFreeFormView extends CollectionSubView<PanZoomDocument, P
             Doc.BrushDoc(this.rootDoc);
             !doc.hidden && Doc.linkFollowHighlight(doc);
 
-            // focus the collection in its parent view.  the parent view after focusing determines whether to reset the view change within the collection
-            this.props.focus(this.rootDoc, willZoom, scale, (didFocus: boolean) => {
-                setTimeout(() => {
-                    if (afterFocus?.(didFocus || didMove)) {
-                        doc.hidden && Doc.UnHighlightDoc(doc);
-                        this.Document._panX = savedState.px;
-                        this.Document._panY = savedState.py;
-                        this.Document[this.scaleFieldKey] = savedState.s;
-                        this.Document._viewTransition = savedState.pt;
-                    } else {
-                        doc.hidden && Doc.UnHighlightDoc(doc);
-                    }
-                }, focusSpeed);
-                return false;
-            })
-        };
+            // focus on this collection within its parent view.  the parent view after focusing determines whether to reset the view change within the collection
+            const endFocus = async (moved: boolean) => {
+                doc.hidden && Doc.UnHighlightDoc(doc);
+                const resetView = afterFocus ? await afterFocus(moved) : false;
+                if (resetView) {
+                    this.Document._panX = savedState.px;
+                    this.Document._panY = savedState.py;
+                    this.Document[this.scaleFieldKey] = savedState.s;
+                    this.Document._viewTransition = savedState.pt;
+                }
+                return resetView;
+            };
+            this.props.focus(this.rootDoc._isGroup ? doc : this.rootDoc, willZoom, scale, (didFocus: boolean) =>
+                new Promise<boolean>(res => setTimeout(async () => res(await endFocus(didMove || didFocus)), focusSpeed)));
+        }
     }
 
     setPanIntoView = (doc: Doc) => {
diff --git a/src/client/views/nodes/DocumentView.tsx b/src/client/views/nodes/DocumentView.tsx
index d20866f94..3fc39d554 100644
--- a/src/client/views/nodes/DocumentView.tsx
+++ b/src/client/views/nodes/DocumentView.tsx
@@ -42,7 +42,7 @@ import { RadialMenu } from './RadialMenu';
 import React = require("react");
 import { LinkDocPreview } from "./LinkDocPreview";
 
-export type DocAfterFocusFunc = (notFocused: boolean) => boolean;
+export type DocAfterFocusFunc = (notFocused: boolean) => Promise<boolean>;
 export type DocFocusFunc = (doc: Doc, willZoom?: boolean, scale?: number, afterFocus?: DocAfterFocusFunc) => void;
 export type StyleProviderFunc = (doc: Opt<Doc>, props: Opt<DocumentViewProps | FieldViewProps>, property: string) => any;
 export interface DocComponentView {
diff --git a/src/client/views/nodes/PDFBox.tsx b/src/client/views/nodes/PDFBox.tsx
index 4b926bb6f..989be1ab9 100644
--- a/src/client/views/nodes/PDFBox.tsx
+++ b/src/client/views/nodes/PDFBox.tsx
@@ -80,7 +80,7 @@ export class PDFBox extends ViewBoxAnnotatableComponent<FieldViewProps, PdfDocum
         }
     }
 
-    scrollFocus = (doc: Doc, smooth: boolean, afterFocus?: DocAfterFocusFunc) => { this._pdfViewer?.scrollFocus(doc, smooth, afterFocus); }
+    scrollFocus = (doc: Doc, smooth: boolean, afterFocus?: DocAfterFocusFunc) => this._pdfViewer?.scrollFocus(doc, smooth, afterFocus);
     getAnchor = () => this.rootDoc;
     componentWillUnmount() { this._selectReactionDisposer?.(); }
     componentDidMount() {