Merge remote-tracking branch 'origin/master' into api-exec-await

Author: haraldschilly

src/packages/frontend/chat/input.tsx

@@ -87,13 +87,18 @@ export default function ChatInput({
     return input;
   });
   const currentInputRef = useRef<string>(input);
+  const saveOnUnmountRef = useRef<boolean>(true);
 
   const isMountedRef = useIsMountedRef();
-  const lastSavedRef = useRef<string | null>(null);
+  const lastSavedRef = useRef<string>(input);
   const saveChat = useDebouncedCallback(
     (input) => {
-      if (!isMountedRef.current || syncdb == null || !saveOnUnmountRef.current)
+      if (
+        syncdb == null ||
+        (!isMountedRef.current && !saveOnUnmountRef.current)
+      ) {
         return;
+      }
       onChange(input);
       lastSavedRef.current = input;
       // also save to syncdb, so we have undo, etc.
@@ -130,10 +135,9 @@ export default function ChatInput({
     },
   );
 
-  const saveOnUnmountRef = useRef<boolean>(true);
   useEffect(() => {
     return () => {
-      if (!saveOnUnmountRef.current) {
+      if (!isMountedRef.current && !saveOnUnmountRef.current) {
         return;
       }
       // save before unmounting.  This is very important since if a new reply comes in,
@@ -174,10 +178,10 @@ export default function ChatInput({
         date,
       });
       const input = x?.get("input");
-      if (input != null && input !== lastSavedRef.current) {
+      if (input != null && input != lastSavedRef.current) {
         setInput(input);
         currentInputRef.current = input;
-        lastSavedRef.current = null;
+        lastSavedRef.current = input;
       }
     };
     syncdb.on("change", onSyncdbChange);
@@ -213,6 +217,7 @@ export default function ChatInput({
         // no need to save on unmount, since we are saving
         // the correct state below.
         saveOnUnmountRef.current = false;
+        lastSavedRef.current = input;
         setInput(input);
         saveChat(input);
         saveChat.cancel();

src/packages/frontend/components/link-retry.tsx

@@ -104,7 +104,7 @@ const LinkRetry: React.FC<Props> = ({
     if (autoStart) {
       start();
     }
-  }, []);
+  }, [href]);
 
   function renderError() {
     if (!error) return;

src/packages/frontend/editors/slate/elements/codemirror.tsx

@@ -94,7 +94,7 @@ export const SlateCodeMirror: React.FC<Props> = React.memo(
         } else {
           // everything selected, so now select all editor content.
           // NOTE that this only makes sense if we change focus
-          // to the enclosing select editor, thus loosing the
+          // to the enclosing select editor, thus losing the
           // cm editor focus, which is a bit weird.
           ReactEditor.focus(editor);
           selectAll(editor);

src/packages/frontend/frame-editors/jupyter-editor/actions.ts

@@ -233,7 +233,7 @@ export class JupyterEditorActions extends BaseActions<JupyterEditorState> {
   }
 
   print(_id): void {
-    this.jupyter_actions.show_nbconvert_dialog("cocalc-pdf");
+    this.jupyter_actions.show_nbconvert_dialog("cocalc-html");
   }
 
   async format(id: string): Promise<void> {

src/packages/frontend/jupyter/cell-list.tsx

@@ -39,6 +39,7 @@ import { Cell } from "./cell";
 import HeadingTagComponent from "./heading-tag";
 interface IFrameContextType {
   iframeDivRef?: MutableRefObject<any>;
+  cellListDivRef?: MutableRefObject<any>;
   iframeOnScrolls?: { [key: string]: () => void };
 }
 const IFrameContext = createContext<IFrameContextType>({});
@@ -597,85 +598,90 @@ export const CellList: React.FC<CellListProps> = (props: CellListProps) => {
   let body;
 
   const iframeDivRef = useRef<HTMLDivElement>(null);
+  const cellListDivRef = useRef<HTMLDivElement>(null);
   const virtuosoHeightsRef = useRef<{ [index: number]: number }>({});
   if (use_windowed_list) {
     body = (
-      <IFrameContext.Provider value={{ iframeDivRef, iframeOnScrolls }}>
-        <Virtuoso
-          ref={virtuosoRef}
-          onClick={actions != null && complete != null ? on_click : undefined}
-          topItemCount={EXTRA_TOP_CELLS}
-          style={{
-            fontSize: `${font_size}px`,
-            flex: 1,
-            overflowX: "hidden",
-          }}
-          totalCount={
-            cell_list.size +
-            EXTRA_TOP_CELLS /* +EXTRA_TOP_CELLS due to the iframe cell and style cell at the top */ +
-            EXTRA_BOTTOM_CELLS
-          }
-          itemSize={(el) => {
-            // We capture measured heights -- see big coment above the
-            // the DivTempHeight component below for why this is needed
-            // for Jupyter notebooks (but not most things).
-            const h = el.getBoundingClientRect().height;
-            // WARNING: This uses perhaps an internal implementation detail of
-            //  virtuoso, which I hope they don't change, which is that the index of
-            // the elements whose height we're measuring is in the data-item-index
-            // attribute.
-            const data = el.getAttribute("data-item-index");
-            if (data != null) {
-              const index = parseInt(data);
-              virtuosoHeightsRef.current[index] = h;
+      <IFrameContext.Provider
+        value={{ iframeDivRef, cellListDivRef, iframeOnScrolls }}
+      >
+        <div ref={cellListDivRef} className="smc-vfill">
+          <Virtuoso
+            ref={virtuosoRef}
+            onClick={actions != null && complete != null ? on_click : undefined}
+            topItemCount={EXTRA_TOP_CELLS}
+            style={{
+              fontSize: `${font_size}px`,
+              flex: 1,
+              overflowX: "hidden",
+            }}
+            totalCount={
+              cell_list.size +
+              EXTRA_TOP_CELLS /* +EXTRA_TOP_CELLS due to the iframe cell and style cell at the top */ +
+              EXTRA_BOTTOM_CELLS
             }
-            return h;
-          }}
-          itemContent={(index) => {
-            if (index == 0) {
-              return (
-                <div key="iframes" ref={iframeDivRef} style={ITEM_STYLE}>
-                  iframes here
-                </div>
-              );
-            } else if (index == 1) {
+            itemSize={(el) => {
+              // We capture measured heights -- see big coment above the
+              // the DivTempHeight component below for why this is needed
+              // for Jupyter notebooks (but not most things).
+              const h = el.getBoundingClientRect().height;
+              // WARNING: This uses perhaps an internal implementation detail of
+              //  virtuoso, which I hope they don't change, which is that the index of
+              // the elements whose height we're measuring is in the data-item-index
+              // attribute.
+              const data = el.getAttribute("data-item-index");
+              if (data != null) {
+                const index = parseInt(data);
+                virtuosoHeightsRef.current[index] = h;
+              }
+              return h;
+            }}
+            itemContent={(index) => {
+              if (index == 0) {
+                return (
+                  <div key="iframes" ref={iframeDivRef} style={ITEM_STYLE}>
+                    iframes here
+                  </div>
+                );
+              } else if (index == 1) {
+                return (
+                  <div key="styles" ref={iframeDivRef} style={ITEM_STYLE}>
+                    <style>{allStyles}</style>
+                  </div>
+                );
+              } else if (index == cell_list.size + EXTRA_TOP_CELLS) {
+                return BOTTOM_PADDING_CELL;
+              }
+              const id = cell_list.get(index - EXTRA_TOP_CELLS);
+              if (id == null) return null;
+              const h = virtuosoHeightsRef.current[index];
+              if (actions == null) {
+                return render_cell({
+                  id,
+                  isScrolling: false,
+                  index: index - EXTRA_TOP_CELLS,
+                });
+              }
               return (
-                <div key="styles" ref={iframeDivRef} style={ITEM_STYLE}>
-                  <style>{allStyles}</style>
-                </div>
+                <SortableItem id={id} key={id}>
+                  <DivTempHeight height={h ? `${h}px` : undefined}>
+                    {render_cell({
+                      id,
+                      isScrolling: false,
+                      index: index - EXTRA_TOP_CELLS,
+                      isFirst: id === cell_list.get(0),
+                      isLast: id === cell_list.get(-1),
+                    })}
+                  </DivTempHeight>
+                </SortableItem>
               );
-            } else if (index == cell_list.size + EXTRA_TOP_CELLS) {
-              return BOTTOM_PADDING_CELL;
-            }
-            const id = cell_list.get(index - EXTRA_TOP_CELLS);
-            if (id == null) return null;
-            const h = virtuosoHeightsRef.current[index];
-            if (actions == null) {
-              return render_cell({
-                id,
-                isScrolling: false,
-                index: index - EXTRA_TOP_CELLS,
-              });
-            }
-            return (
-              <SortableItem id={id} key={id}>
-                <DivTempHeight height={h ? `${h}px` : undefined}>
-                  {render_cell({
-                    id,
-                    isScrolling: false,
-                    index: index - EXTRA_TOP_CELLS,
-                    isFirst: id === cell_list.get(0),
-                    isLast: id === cell_list.get(-1),
-                  })}
-                </DivTempHeight>
-              </SortableItem>
-            );
-          }}
-          rangeChanged={(visibleRange) => {
-            virtuosoRangeRef.current = visibleRange;
-          }}
-          {...virtuosoScroll}
-        />
+            }}
+            rangeChanged={(visibleRange) => {
+              virtuosoRangeRef.current = visibleRange;
+            }}
+            {...virtuosoScroll}
+          />
+        </div>
       </IFrameContext.Provider>
     );
   } else {

src/packages/frontend/jupyter/output-messages/cached-iframe.tsx

@@ -1,12 +1,18 @@
 /*
 
-It is completely impossible in modern times to move an iframe in the DOM without loosing state,
-as explained here:
-   https://stackoverflow.com/questions/8318264/how-to-move-an-iframe-in-the-dom-without-losing-its-state
+It is completely impossible in modern times to move an iframe's position in the DOM *tree*
+without losing state, as explained here:
 
-An annoying issue is that if you do shift+tab to get docs or hit the TimeTravel button or anything to
-split the Jupyter frame, then the iframes of course get reset.  That was a problem before this though,
-i.e., it's not special to using windowing.
+https://stackoverflow.com/questions/8318264/how-to-move-an-iframe-in-the-dom-without-losing-its-state
+
+That's what we instead use position absolute, and literally move the iframe's position on the screen.
+We never move it in the dom.
+
+Another unsolved issue is that if you do shift+tab to get docs or hit the TimeTravel button
+or anything to split the Jupyter frame, then the iframes of course get reset.  That was
+a problem before this though, i.e., it's not special to using windowing.  However, it
+is a problem we intend to solve, e.g., maybe we put the iframes somewhere else in the
+DOM and can reuse when rendering the notebook after the frame split happens.
 */
 
 import { Button, Tooltip } from "antd";
@@ -18,7 +24,8 @@ import { delay } from "awaiting";
 import useIsMountedRef from "@cocalc/frontend/app-framework/is-mounted-hook";
 import useResizeObserver from "use-resize-observer";
 
-// This is just an initial default height; the actual height of the iframe resizes to the content.
+// This is just an initial default height; the actual height of the iframe should
+// resize to the content.
 const HEIGHT = "70vh";
 
 interface Props {

src/packages/frontend/jupyter/output-messages/iframe.tsx

@@ -15,27 +15,47 @@ import useCounter from "@cocalc/frontend/app-framework/counter-hook";
 import { get_blob_url } from "../server-urls";
 import CachedIFrame from "./cached-iframe";
 import { useIFrameContext } from "@cocalc/frontend/jupyter/cell-list";
+import HTML from "./mime-types/html";
 
 // This impact loading the iframe data from the backend project (via the sha1 hash).
 // Doing retries is useful, e.g., since the project might not be running.
 const MAX_ATTEMPTS = 10;
 const MAX_WAIT = 5000;
 const BACKOFF = 1.3;
 
+const HEIGHT = "70vh";
+const WIDTH = "max(800px,70vw)";
+
 interface Props {
   sha1: string;
   project_id: string;
   cacheId?: string;
+  index?: number;
+  trust?: boolean;
 }
 
 export default function IFrame(props: Props) {
-  const iframeContext = useIFrameContext(); // we only use cached iframe if the iframecontext is setup, e.g., it is in Jupyter notebooks, but not in whiteboards.
-  return iframeContext.iframeDivRef == null || props.cacheId == null ? (
-    <NonCachedIFrame {...props} />
-  ) : (
+  // we only use cached iframe if the iframecontext is setup, e.g., it is in Jupyter notebooks, but not in whiteboards.
+  const iframeContext = useIFrameContext();
+  if (
+    iframeContext.iframeDivRef == null ||
+    props.cacheId == null ||
+    !props.trust
+  ) {
+    return <NonCachedIFrame {...props} />;
+  } else {
+    const src = get_blob_url(props.project_id, "html", props.sha1);
+    return (
+      <HTML
+        id={props.cacheId}
+        index={props.index}
+        trust={props.trust}
+        value={`<iframe src="${src}" style="border:0;height:${HEIGHT};width:${WIDTH}"/>`}
+      />
+    );
     // @ts-ignore
-    <CachedIFrame {...props} />
-  );
+    return <CachedIFrame {...props} />;
+  }
 }
 
 function NonCachedIFrame({ sha1, project_id }: Props) {

src/packages/frontend/jupyter/output-messages/javascript.tsx

@@ -5,7 +5,7 @@
 
 import { List } from "immutable";
 import $ from "jquery";
-import React, { useRef, useState } from "react";
+import { useEffect, useRef, useState } from "react";
 import { is_array } from "@cocalc/util/misc";
 import { javascript_eval } from "./javascript-eval";
 import ShowError from "@cocalc/frontend/components/error";
@@ -24,7 +24,7 @@ export const Javascript: React.FC<JavascriptProps> = (
 
   const [errors, set_errors] = useState<string | undefined>(undefined);
 
-  React.useEffect(() => {
+  useEffect(() => {
     if (value == null || node.current == null) {
       return;
     }

src/packages/frontend/jupyter/output-messages/message.tsx

@@ -56,6 +56,7 @@ interface CellOutputMessageProps {
   actions?: JupyterActions; // optional  - not needed by most messages
   name?: string;
   id?: string; // optional, and not usually needed either; this is the id of the cell.  It is needed for iframe + windowing.
+  index?: number;
   trust?: boolean; // is notebook trusted by the user (if not won't eval javascript)
 }
 
@@ -71,6 +72,7 @@ export const CellOutputMessage: React.FC<CellOutputMessageProps> = React.memo(
         name={props.name}
         trust={props.trust}
         id={props.id}
+        index={props.index}
       />
     );
   },
@@ -137,6 +139,7 @@ export const CellOutputMessages: React.FC<CellOutputMessagesProps> = React.memo(
         v.push(
           <CellOutputMessage
             key={n}
+            index={n}
             message={mesg}
             project_id={project_id}
             directory={directory}