jupyter windowing: somewhat preserve scroll position when cells are a…

…dded/removed

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

@@ -155,9 +155,7 @@ export const CellList: React.FC<CellListProps> = (props: CellListProps) => {
     let scrollHeight: number = 0;
     for (const tm of [0, 1, 100, 250, 500, 1000]) {
       if (!is_mounted.current) return;
-      if (use_windowed_list) {
-        // TODO -- virtuoso
-      } else {
+      if (!use_windowed_list) {
         const elt = cell_list_node.current;
         if (elt != null && elt.scrollHeight !== scrollHeight) {
           // dynamically rendering actually changed something
@@ -253,10 +251,10 @@ export const CellList: React.FC<CellListProps> = (props: CellListProps) => {
       }
     } else if (scroll.startsWith("list")) {
       if (scroll == "list up") {
-        const index = virtuosoRangeRef.current.startIndex;
+        const index = virtuosoRangeRef.current?.startIndex;
         virtuosoRef.current?.scrollToIndex({ index: index + 1, align: "end" });
       } else if (scroll == "list down") {
-        const index = virtuosoRangeRef.current.endIndex;
+        const index = virtuosoRangeRef.current?.endIndex;
         virtuosoRef.current?.scrollToIndex({
           index: index + 1,
           align: "start",
@@ -355,6 +353,17 @@ export const CellList: React.FC<CellListProps> = (props: CellListProps) => {
     startIndex: 0,
     endIndex: 0,
   });
+  const lastScrollStateRef = useRef<{
+    id?: string;
+    index: number;
+    offset: number;
+  }>({
+    index: 0,
+    offset: 0,
+    id: "",
+  });
+  const cellListRef = useRef<any>(cell_list);
+  cellListRef.current = cell_list;
   const virtuosoScroll = useVirtuosoScrollHook(
     use_windowed_list
       ? {
@@ -364,6 +373,10 @@ export const CellList: React.FC<CellListProps> = (props: CellListProps) => {
               ? `${name}${frameActions.current?.frame_id}`
               : undefined,
           onScroll: (scrollState) => {
+            lastScrollStateRef.current = {
+              ...scrollState,
+              id: cellListRef.current?.get(scrollState.index - 1),
+            };
             setTimeout(() => {
               frameActions.current?.set_scrollTop(scrollState);
             }, 0);
@@ -376,6 +389,33 @@ export const CellList: React.FC<CellListProps> = (props: CellListProps) => {
       : { disabled: true }
   );
 
+  useEffect(() => {
+    if (!use_windowed_list) return;
+    if (lastScrollStateRef.current == null) {
+      return;
+    }
+    const { offset, id } = lastScrollStateRef.current;
+    if (!id) {
+      return;
+    }
+    const index = cellListRef.current?.indexOf(id);
+    if (index == null) {
+      return;
+    }
+    // index + 1 because of iframe
+    // the offset+1 is I think compensating for a bug maybe in virtuoso or our use of it.
+    virtuosoRef.current?.scrollToIndex({
+      index: index + 1,
+      offset: offset + 1,
+    });
+    requestAnimationFrame(() => {
+      virtuosoRef.current?.scrollToIndex({
+        index: index + 1,
+        offset: offset + 1,
+      });
+    });
+  }, [cell_list]);
+
   const iframeOnScrolls = useMemo(() => {
     return {};
   }, []);

src/packages/frontend/jupyter/main.tsx

@@ -7,7 +7,7 @@
 Top-level react component, which ties everything together
 */
 
-import { CSS, React, useRedux, Rendered } from "../app-framework";
+import { redux, CSS, React, useRedux, useRef, Rendered } from "../app-framework";
 import * as immutable from "immutable";
 
 import { A, ErrorDisplay } from "../components";
@@ -186,7 +186,23 @@ export const JupyterEditor: React.FC<Props> = React.memo((props: Props) => {
 
   const kernel_error: undefined | string = useRedux([name, "kernel_error"]);
 
-  const editor_settings = useRedux(["account", "editor_settings"]);
+  // We use react-virtuoso, which is an amazing library for
+  // doing windowing on dynamically sized content... like
+  // what comes up with Jupyter notebooks.
+  // We do have to ensure that this can be easily disabled
+  // by users, due to situations like this
+  //   https://github.com/sagemathinc/cocalc/issues/4727
+  // e.g., where maybe they want to use Javascript across all
+  // cells, or they want to preserve state in iframes, which
+  // requires keeping things rendered.
+  // NOTE: we get this once from the account store and do NOT
+  // load it again, since we didn't implement switching between
+  // rendering modes on the fly and such a switch will crash for sure.
+  const useWindowedListRef = useRef<boolean>(
+    !redux
+      .getStore("account")
+      .getIn(["editor_settings", "disable_jupyter_virtualization"])
+  );
 
   const { usage, expected_cell_runtime } = useKernelUsage(name);
 
@@ -308,23 +324,6 @@ export const JupyterEditor: React.FC<Props> = React.memo((props: Props) => {
     );
   }
 
-  function use_windowed_list(): boolean {
-    // We use react-virtuoso, which is an amazing library for
-    // doing windowing on dynamically sized content... like
-    // what comes up with Jupyter notebooks.
-    // We do have to ensure that this can be easily disabled
-    // by users, due to situations like this
-    //   https://github.com/sagemathinc/cocalc/issues/4727
-    // e.g., where maybe they want to use Javascript across all
-    // cells, or they want to preserve state in iframes, which
-    // requires keeping things rendered.
-
-    // TODO -- virtuoso -- make this a switch that is in the View
-    // menu, whose value is stored in metadata.  This could also
-    // have a parameter for "increaseViewportBy".
-    return !editor_settings?.get("disable_jupyter_virtualization");
-  }
-
   function render_cells() {
     if (
       cell_list == null ||
@@ -367,7 +366,7 @@ export const JupyterEditor: React.FC<Props> = React.memo((props: Props) => {
         scroll={scroll}
         cell_toolbar={cell_toolbar}
         trust={trust}
-        use_windowed_list={use_windowed_list()}
+        use_windowed_list={useWindowedListRef.current}
       />
     );
   }

src/packages/frontend/project/warnings/disk-space.tsx

@@ -56,7 +56,7 @@ export const DiskSpaceWarning: React.FC<{ project_id: string }> = ({
     <Alert bsStyle="danger" style={ALERT_STYLE}>
       <Icon name="exclamation-triangle" /> WARNING: This project is running out
       of disk space: only {disk_free} MB out of {quotas.disk_quota} MB
-      available.
+      available.{" "}
       <a onClick={() => actions?.set_active_tab("settings")}>
         Increase the "Disk Space" quota
       </a>

src/packages/next/components/account/config/editor/jupyter.tsx

@@ -10,7 +10,7 @@ const desc = {
   ask_jupyter_kernel: `Each time you create a new Jupyter notebook, by default it will ask which kernel to use.
 If you disable this option, then new notebooks will open using the kernel that you most recently explicitly
 selected.  You can of course change the kernel of any notebook at any time in the Kernel dropdown menu.`,
-  disable_jupyter_virtualization: `By default Jupyter notebooks are rendered using "virtualization", so only the visible cells are actually rendered.   If you select this option, then we instead render entire notebook. This is potentially much slower but may address some issues.`,
+  disable_jupyter_virtualization: `By default Jupyter notebooks are rendered using "virtualization", so only the visible cells are actually rendered.   If you select this option, then we instead render entire notebook. This is potentially much slower but may address some issues.  You must close and open your notebook to see the change.`,
   jupyter_classic: `CoCalc includes a mode where it embeds
 the classical Jupyter notebook in an iframe and installs a plugin to enable realtime collaboration.
 However, collaboration does not work as well as in the default Jupyter editor.`,
@@ -64,8 +64,9 @@ register({
           desc={desc.disable_jupyter_virtualization}
           label={
             <>
-              Render entire notebook rather than rendering just the visible part
-              of it using <A href="https://virtuoso.dev/">react-virtuoso</A>.
+              No virtualization -- render entire notebook rather than
+              rendering just the visible part of it using{" "}
+              <A href="https://virtuoso.dev/">react-virtuoso</A>.
             </>
           }
         />{" "}