Redo the way ToplevelBlockEditable works to be more friendly

packages/codemirror-blocks/src/components/NodeEditable.tsx

@@ -52,127 +52,151 @@ type Props = Omit<ContentEditableProps, "value"> & {
   editor: CMBEditor;
 };
 
-const NodeEditable = (props: Props) => {
-  const element = useRef<HTMLElement>(null);
-  const dispatch: AppDispatch = useDispatch();
-
-  const ast = useSelector(selectors.getAST);
-
-  const { initialValue, isErrored } = useSelector((state: RootState) => {
-    const nodeId = props.target.node ? props.target.node.id : "editing";
-    const isErrored = selectors.getErrorId(state) == nodeId;
-
-    const initialValue =
-      props.value === null ? props.target.getText(ast, props.editor) : "";
-
-    return { isErrored, initialValue };
-  });
-
-  // select and focus the element on mount
-  useEffect(() => {
-    const currentEl = element.current;
-    if (!currentEl) {
-      // element has been unmounted already, nothing to do.
-      return;
-    }
-    selectElement(currentEl, props.isInsertion);
-  }, [props.isInsertion]);
-
-  useEffect(() => {
-    const text = props.value || initialValue || "";
-    const annt = (props.isInsertion ? "inserting" : "editing") + ` ${text}`;
-    say(annt + `.  Use Enter to save, and Alt-Q to cancel`);
-    dispatch(actions.setSelectedNodeIds([]));
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, []);
-
-  const onBlur = (e: React.SyntheticEvent) => {
-    e.stopPropagation();
-    const { target } = props;
-    dispatch((dispatch: AppDispatch, getState: () => RootState) => {
-      // we grab the value directly from the content editable element
-      // to deal with this issue:
-      // https://github.com/lovasoa/react-contenteditable/issues/161
-      const value = element.current?.textContent;
-      const focusedNode = selectors.getFocusedNode(getState());
-      // if there's no insertion value, or the new value is the same as the
-      // old one, preserve focus on original node and return silently
-      if (value === initialValue || !value) {
-        props.onDisableEditable();
-        const nid = focusedNode && focusedNode.nid;
-        dispatch(activateByNid(props.editor, nid));
+export type NodeEditableHandle = {
+  /**
+   * Selects and focuses the text in the node editable component
+   */
+  select: () => void;
+};
+
+const NodeEditable = React.forwardRef<NodeEditableHandle, Props>(
+  (props, ref) => {
+    const element = useRef<HTMLElement>(null);
+    const dispatch: AppDispatch = useDispatch();
+
+    const ast = useSelector(selectors.getAST);
+
+    const { initialValue, isErrored } = useSelector((state: RootState) => {
+      const nodeId = props.target.node ? props.target.node.id : "editing";
+      const isErrored = selectors.getErrorId(state) == nodeId;
+
+      const initialValue =
+        props.value === null ? props.target.getText(ast, props.editor) : "";
+
+      return { isErrored, initialValue };
+    });
+
+    // select and focus the element on mount
+    useEffect(() => {
+      const currentEl = element.current;
+      if (!currentEl) {
+        // element has been unmounted already, nothing to do.
         return;
       }
+      selectElement(currentEl, props.isInsertion);
+    }, [props.isInsertion]);
 
-      const annt = `${props.isInsertion ? "inserted" : "changed"} ${value}`;
-      const result = dispatch(insert(value, target, props.editor, annt));
-      if (result.successful) {
-        dispatch(activateByNid(props.editor, null, { allowMove: false }));
-        props.onChange(null);
-        props.onDisableEditable();
-        dispatch(actions.clearError());
-        say(annt);
-      } else {
-        console.error(result.exception);
-        dispatch(actions.setErrorId(target.node ? target.node.id : "editing"));
+    // expose an imperative handle for selecting the contents
+    // of the rendered element in case it needs to be done after
+    // later. This is only necessary in cases where the component
+    // is rendered to a container that won't be in the document
+    // until later, as is the case with TopLevelNodeEditable
+    React.useImperativeHandle(ref, () => ({
+      select: () => {
         if (element.current) {
-          selectElement(element.current, false);
+          selectElement(element.current, props.isInsertion);
+        }
+      },
+    }));
+
+    useEffect(() => {
+      const text = props.value || initialValue || "";
+      const annt = (props.isInsertion ? "inserting" : "editing") + ` ${text}`;
+      say(annt + `.  Use Enter to save, and Alt-Q to cancel`);
+      dispatch(actions.setSelectedNodeIds([]));
+      // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, []);
+
+    const onBlur = (e: React.SyntheticEvent) => {
+      e.stopPropagation();
+      const { target } = props;
+      dispatch((dispatch: AppDispatch, getState: () => RootState) => {
+        // we grab the value directly from the content editable element
+        // to deal with this issue:
+        // https://github.com/lovasoa/react-contenteditable/issues/161
+        const value = element.current?.textContent;
+        const focusedNode = selectors.getFocusedNode(getState());
+        // if there's no insertion value, or the new value is the same as the
+        // old one, preserve focus on original node and return silently
+        if (value === initialValue || !value) {
+          props.onDisableEditable();
+          const nid = focusedNode && focusedNode.nid;
+          dispatch(activateByNid(props.editor, nid));
+          return;
+        }
+
+        const annt = `${props.isInsertion ? "inserted" : "changed"} ${value}`;
+        const result = dispatch(insert(value, target, props.editor, annt));
+        if (result.successful) {
+          dispatch(activateByNid(props.editor, null, { allowMove: false }));
+          props.onChange(null);
+          props.onDisableEditable();
+          dispatch(actions.clearError());
+          say(annt);
+        } else {
+          console.error(result.exception);
+          dispatch(
+            actions.setErrorId(target.node ? target.node.id : "editing")
+          );
+          if (element.current) {
+            selectElement(element.current, false);
+          }
+        }
+      });
+    };
+
+    const onKeyDown = (e: React.KeyboardEvent<HTMLDivElement>) => {
+      const el = e.target as HTMLDivElement;
+      switch (CodeMirror.keyName(e)) {
+        case "Enter": {
+          // blur the element to trigger handleBlur
+          // which will save the edit
+          element.current?.blur();
+          return;
+        }
+        case "Alt-Q":
+        case "Esc": {
+          el.innerHTML = initialValue;
+          e.stopPropagation();
+          props.onChange(null);
+          props.onDisableEditable();
+          dispatch(actions.clearError());
+          dispatch(activateByNid(props.editor, null, { allowMove: false }));
+          return;
         }
       }
-    });
-  };
-
-  const onKeyDown = (e: React.KeyboardEvent<HTMLDivElement>) => {
-    const el = e.target as HTMLDivElement;
-    switch (CodeMirror.keyName(e)) {
-      case "Enter": {
-        // blur the element to trigger handleBlur
-        // which will save the edit
-        element.current?.blur();
-        return;
-      }
-      case "Alt-Q":
-      case "Esc": {
-        el.innerHTML = initialValue;
-        e.stopPropagation();
-        props.onChange(null);
-        props.onDisableEditable();
-        dispatch(actions.clearError());
-        dispatch(activateByNid(props.editor, null, { allowMove: false }));
-        return;
-      }
-    }
-  };
-
-  const { contentEditableProps, extraClasses, value } = props;
-
-  const classes = (
-    [
-      "blocks-literal",
-      "quarantine",
-      "blocks-editing",
-      "blocks-node",
-      { "blocks-error": isErrored },
-    ] as ClassNamesArgument[]
-  ).concat(extraClasses);
-  const text = value ?? initialValue;
-  return (
-    <ContentEditable
-      {...contentEditableProps}
-      className={classNames(classes)}
-      role="textbox"
-      ref={element}
-      onChange={props.onChange}
-      onBlur={onBlur}
-      onKeyDown={onKeyDown}
-      // trap mousedown, clicks and doubleclicks, to prevent focus change, or
-      // parent nodes from toggling collapsed state
-      onMouseDown={suppressEvent}
-      onClick={suppressEvent}
-      onDoubleClick={suppressEvent}
-      aria-label={text}
-      value={text}
-    />
-  );
-};
+    };
+
+    const { contentEditableProps, extraClasses, value } = props;
+
+    const classes = (
+      [
+        "blocks-literal",
+        "quarantine",
+        "blocks-editing",
+        "blocks-node",
+        { "blocks-error": isErrored },
+      ] as ClassNamesArgument[]
+    ).concat(extraClasses);
+    const text = value ?? initialValue;
+    return (
+      <ContentEditable
+        {...contentEditableProps}
+        className={classNames(classes)}
+        role="textbox"
+        ref={element}
+        onChange={props.onChange}
+        onBlur={onBlur}
+        onKeyDown={onKeyDown}
+        // trap mousedown, clicks and doubleclicks, to prevent focus change, or
+        // parent nodes from toggling collapsed state
+        onMouseDown={suppressEvent}
+        onClick={suppressEvent}
+        onDoubleClick={suppressEvent}
+        aria-label={text}
+        value={text}
+      />
+    );
+  }
+);
 export default NodeEditable;

packages/codemirror-blocks/src/ui/ToplevelBlockEditable.tsx

@@ -1,10 +1,10 @@
-import React, { useEffect, useMemo } from "react";
+import React, { useEffect, useMemo, useRef } from "react";
 import ReactDOM from "react-dom";
 import { useDispatch } from "react-redux";
-import NodeEditable from "../components/NodeEditable";
+import NodeEditable, { NodeEditableHandle } from "../components/NodeEditable";
 import * as actions from "../state/actions";
 import type { AppDispatch } from "../state/store";
-import type { CMBEditor } from "../editor";
+import { CMBEditor } from "../editor";
 import { Quarantine } from "../state/reducers";
 
 type Props = {
@@ -26,28 +26,37 @@ const ToplevelBlockEditable = (props: Props) => {
   const onChange = (text: string) => dispatch(actions.changeQuarantine(text));
   const { from, to, value } = props.quarantine;
 
-  // add a marker to codemirror, with an empty "widget" into which
-  // the react component will be rendered.
-  // We use useMemo to make sure this marker only gets added the first
-  // time this component is rendered.
-  const { container, marker } = useMemo(() => {
+  // create an empty container element to hold the rendered react
+  // component. We use useMemo to make sure this element only gets
+  // created the first time this component is rendered.
+  const { container } = useMemo(() => {
     const container = document.createElement("span");
     container.classList.add("react-container");
-    const marker = props.editor.replaceMarkerWidget(from, to, container);
-    // call endOperation to flush all buffered updates
-    // forcing codemirror to put the marker into the document's DOM
-    // right away, making it immediately focusable/selectable.
-    // SHARED.editor.endOperation();
-    return { container, marker };
-  }, [props.editor, from, to]);
-  // make sure to clear the marker from codemirror
-  // when the component unmounts
+    return { container };
+  }, []);
+
+  const nodeEditableHandle = useRef<NodeEditableHandle>(null);
+
+  // after react has finished rendering, attach the container to codemirror
+  // as a marker widget and select its contents.
+  //
+  // This has to happen after react finishes rendering because codemirror
+  // calls focus() when you call CodeMirror.setBookmark, which triggers
+  // this cryptic react warning: unstable_flushDiscreteUpdates: Cannot
+  // flush updates when React is already rendering
+  // See https://stackoverflow.com/questions/58123011/cryptic-react-error-unstable-flushdiscreteupdates-cannot-flush-updates-when-re
+  // and https://github.com/facebook/react/issues/20141 for more info.
   useEffect(() => {
+    const marker = props.editor.replaceMarkerWidget(from, to, container);
+    nodeEditableHandle.current?.select();
+    // make sure to clear the marker from codemirror
+    // when the component unmounts
     return () => marker.clear();
-  }, [marker]);
+  }, [container, props.editor, from, to]);
 
   return ReactDOM.createPortal(
     <NodeEditable
+      ref={nodeEditableHandle}
       editor={props.editor}
       target={new actions.OverwriteTarget(from, to)}
       value={value}