Skip to content

Commit 09f7928

Browse files
bjones1bjones1
committed
Fix: correct handling of IDE and user edits.
 Correctly save and restore cursor location when updating TinyMCE text.
1 parent 510bad3 commit 09f7928

File tree

1 file changed

+103
-102
lines changed

1 file changed

+103
-102
lines changed

‎client/src/CodeMirror-integration.mts‎

Lines changed: 103 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -102,15 +102,8 @@ import { show_toast } from "./show_toast.mjs";
102102
// -----------------------------------------------------------------------------
103103
let current_view: EditorView;
104104
let tinymce_singleton: Editor | undefined;
105-
// When true, this is an IDE edit; when false, it's due to a user edit.
106-
//
107-
// There's an inherent cycle produced by edits; this variable breaks the cycle. There are two paths through the cycle: a user edit and an IDE edit. The cycle must be broken just before it loops in both cases; this variable therefore specifies where to break the cycle. The sequence is:
108-
//
109-
// 1. The user performs an edit **or** `DocBlockWidget.updateDom()` is called; if `is_ide_change` is false, this function returns; otherwise, it performs an edit.
110-
// 2. The `addEventListener("input")` for an indent or `TinyMCE.editor.on("Dirty")` callback is triggered, invoking `on_dirty()`.
111-
// 3. An IDE edit dispatches an `add/delete/updateDocBlock` **or** in `on_dirty()`, if `is_ide_change` is true, `on_dirty()` sets it to false then returns. Otherwise, `on_dirty()` dispatches `updateDocBlock`.
112-
// 5. This transaction invokes `docBlockField.update()`, which creates a `new DocBlockWidget()`. This loops back to step 1.
113-
let is_ide_change = false;
105+
// True if the change was produce by a user edit, not an IDE update.
106+
let is_user_change = false;
114107
// This indicates that a call to `on_dirty` is scheduled, but hasn't run yet.
115108
let on_dirty_scheduled = false;
116109
// True to ignore the next text selection change, since updates to the cursor or
@@ -458,7 +451,8 @@ class DocBlockWidget extends WidgetType {
458451
// different, non-eq content) to reflect this widget."
459452
updateDOM(dom: HTMLElement, view: EditorView): boolean {
460453
// If this change was produced by a user edit, then the DOM was already updated. Stop here.
461-
if (!is_ide_change) {
454+
if (is_user_change) {
455+
is_user_change = false;
462456
return true;
463457
}
464458
(dom.childNodes[0] as HTMLDivElement).innerHTML = this.indent;
@@ -469,10 +463,9 @@ class DocBlockWidget extends WidgetType {
469463
window.MathJax.typesetClear(contents_div);
470464
if (is_tinymce) {
471465
// Save the cursor location before the update, then restore it afterwards.
472-
const bm = tinymce.activeEditor!.selection.getBookmark();
466+
const sel = saveSelection();
473467
tinymce_singleton!.setContent(this.contents);
474-
tinymce_singleton!.save();
475-
tinymce.activeEditor!.selection.moveToBookmark(bm);
468+
restoreSelection(sel);
476469
} else {
477470
contents_div.innerHTML = this.contents;
478471
}
@@ -508,6 +501,97 @@ class DocBlockWidget extends WidgetType {
508501
}
509502
}
510503

504+
const saveSelection = () => {
505+
// Changing the text inside TinyMCE causes it to loose a selection
506+
// tied to a specific node. So, instead store the
507+
// selection as an array of indices in the childNodes
508+
// array of each element: for example, a given selection
509+
// is element 10 of the root TinyMCE div's children
510+
// (selecting an ol tag), element 5 of the ol's children
511+
// (selecting the last li tag), element 0 of the li's
512+
// children (a text node where the actual click landed;
513+
// the offset in this node is placed in
514+
// `selection_offset`.)
515+
const sel = window.getSelection();
516+
let selection_path = [];
517+
const selection_offset = sel?.anchorOffset;
518+
if (sel?.anchorNode) {
519+
// Find a path from the selection back to the
520+
// containing div.
521+
for (
522+
let current_node = sel.anchorNode, is_first = true;
523+
// Continue until we find the div which contains
524+
// the doc block contents: either it's not an
525+
// element (such as a div), ...
526+
current_node.nodeType !== Node.ELEMENT_NODE ||
527+
// or it's not the doc block contents div.
528+
!(current_node as Element).classList.contains(
529+
"CodeChat-doc-contents",
530+
);
531+
current_node = current_node.parentNode!, is_first = false
532+
) {
533+
// Store the index of this node in its' parent
534+
// list of child nodes/children. Use
535+
// `childNodes` on the first iteration, since
536+
// the selection is often in a text node, which
537+
// isn't in the `parents` list. However, using
538+
// `childNodes` all the time causes trouble when
539+
// reversing the selection -- sometimes, the
540+
// `childNodes` change based on whether text
541+
// nodes (such as a newline) are included are
542+
// not after tinyMCE parses the content.
543+
let p = current_node.parentNode!;
544+
selection_path.unshift(
545+
Array.prototype.indexOf.call(
546+
is_first ? p.childNodes : p.children,
547+
current_node,
548+
),
549+
);
550+
}
551+
}
552+
return { selection_path, selection_offset };
553+
};
554+
555+
// Restore the selection produced by `saveSelection`.
556+
const restoreSelection = ({
557+
selection_path,
558+
selection_offset,
559+
}: {
560+
selection_path: number[];
561+
selection_offset?: number;
562+
}) => {
563+
// Copy the selection over to TinyMCE by indexing the
564+
// selection path to find the selected node.
565+
if (selection_path.length && typeof selection_offset === "number") {
566+
let selection_node = tinymce_singleton!.getContentAreaContainer();
567+
for (
568+
;
569+
selection_path.length &&
570+
// If something goes wrong, bail out instead of producing exceptions.
571+
selection_node !== undefined;
572+
selection_node =
573+
// As before, use the more-consistent
574+
// `children` except for the last element,
575+
// where we might be selecting a `text`
576+
// node.
577+
(
578+
selection_path.length > 1
579+
? selection_node.children
580+
: selection_node.childNodes
581+
)[selection_path.shift()!]! as HTMLElement
582+
);
583+
// Use that to set the selection.
584+
tinymce_singleton!.selection.setCursorLocation(
585+
selection_node,
586+
// In case of edits, avoid an offset past the end of the node.
587+
Math.min(
588+
selection_offset,
589+
selection_node.nodeValue?.length ?? Number.MAX_VALUE,
590+
),
591+
);
592+
}
593+
};
594+
511595
// Typeset the provided node; taken from the
512596
// [MathJax docs](https://docs.mathjax.org/en/latest/web/typeset.html#handling-asynchronous-typesetting).
513597
export const mathJaxTypeset = async (
@@ -576,11 +660,7 @@ const on_dirty = (
576660
// The div that's dirty. It must be a child of the doc block div.
577661
event_target: HTMLElement,
578662
) => {
579-
// If this change was produced by an IDE edit, then the underlying state is updated. Stop here.
580-
if (is_ide_change) {
581-
is_ide_change = false;
582-
return;
583-
}
663+
is_user_change = true;
584664

585665
if (on_dirty_scheduled) {
586666
return;
@@ -589,6 +669,7 @@ const on_dirty = (
589669

590670
// Only run this after typesetting is done.
591671
window.MathJax.whenReady(() => {
672+
on_dirty_scheduled = false;
592673
// Find the doc block parent div.
593674
const target = (event_target as HTMLDivElement).closest(
594675
".CodeChat-doc",
@@ -625,8 +706,6 @@ const on_dirty = (
625706
];
626707

627708
current_view.dispatch({ effects });
628-
629-
on_dirty_scheduled = false;
630709
});
631710
};
632711

@@ -748,55 +827,8 @@ export const DocBlockPlugin = ViewPlugin.fromClass(
748827
mathJaxUnTypeset(contents_div);
749828
// The code which moves TinyMCE into this div disturbs
750829
// all the nodes, which causes it to loose a selection
751-
// tied to a specific node. So, instead store the
752-
// selection as an array of indices in the childNodes
753-
// array of each element: for example, a given selection
754-
// is element 10 of the root TinyMCE div's children
755-
// (selecting an ol tag), element 5 of the ol's children
756-
// (selecting the last li tag), element 0 of the li's
757-
// children (a text node where the actual click landed;
758-
// the offset in this node is placed in
759-
// `selection_offset`.)
760-
const sel = window.getSelection();
761-
let selection_path = [];
762-
const selection_offset = sel?.anchorOffset;
763-
if (sel?.anchorNode) {
764-
// Find a path from the selection back to the
765-
// containing div.
766-
for (
767-
let current_node = sel.anchorNode,
768-
is_first = true;
769-
// Continue until we find the div which contains
770-
// the doc block contents: either it's not an
771-
// element (such as a div), ...
772-
current_node.nodeType !== Node.ELEMENT_NODE ||
773-
// or it's not the doc block contents div.
774-
!(current_node as Element).classList.contains(
775-
"CodeChat-doc-contents",
776-
);
777-
current_node = current_node.parentNode!,
778-
is_first = false
779-
) {
780-
// Store the index of this node in its' parent
781-
// list of child nodes/children. Use
782-
// `childNodes` on the first iteration, since
783-
// the selection is often in a text node, which
784-
// isn't in the `parents` list. However, using
785-
// `childNodes` all the time causes trouble when
786-
// reversing the selection -- sometimes, the
787-
// `childNodes` change based on whether text
788-
// nodes (such as a newline) are included are
789-
// not after tinyMCE parses the content.
790-
let p = current_node.parentNode!;
791-
selection_path.unshift(
792-
Array.prototype.indexOf.call(
793-
is_first ? p.childNodes : p.children,
794-
current_node,
795-
),
796-
);
797-
}
798-
}
799-
830+
// tied to a specific node.
831+
const sel = saveSelection();
800832
// With the selection saved, it's safe to replace the
801833
// contenteditable div with the TinyMCE instance (which
802834
// would otherwise wipe the selection).
@@ -822,9 +854,7 @@ export const DocBlockPlugin = ViewPlugin.fromClass(
822854

823855
// Setting the content makes TinyMCE consider it dirty
824856
// -- ignore this "dirty" event.
825-
is_ide_change = true;
826857
tinymce_singleton!.setContent(contents_div.innerHTML);
827-
tinymce_singleton!.save();
828858
contents_div.remove();
829859
// The new div is now a TinyMCE editor. Retypeset this.
830860
mathJaxTypeset(tinymce_div);
@@ -836,34 +866,7 @@ export const DocBlockPlugin = ViewPlugin.fromClass(
836866

837867
// Copy the selection over to TinyMCE by indexing the
838868
// selection path to find the selected node.
839-
if (
840-
selection_path.length &&
841-
typeof selection_offset === "number"
842-
) {
843-
let selection_node =
844-
tinymce_singleton!.getContentAreaContainer();
845-
for (
846-
;
847-
selection_path.length &&
848-
// If something goes wrong, bail out instead of producing exceptions.
849-
selection_node !== undefined;
850-
selection_node =
851-
// As before, use the more-consistent
852-
// `children` except for the last element,
853-
// where we might be selecting a `text`
854-
// node.
855-
(
856-
selection_path.length > 1
857-
? selection_node.children
858-
: selection_node.childNodes
859-
)[selection_path.shift()!]! as HTMLElement
860-
);
861-
// Use that to set the selection.
862-
tinymce_singleton!.selection.setCursorLocation(
863-
selection_node,
864-
selection_offset,
865-
);
866-
}
869+
restoreSelection(sel);
867870
}, 0);
868871
}
869872
return false;
@@ -1084,8 +1087,7 @@ export const CodeMirror_load = async (
10841087
if (target_or_false == null) {
10851088
return false;
10861089
}
1087-
// For some reason, calling this directly omits the most recent edit. Earlier versions of the code didn't have this problem. ???
1088-
on_dirty(target_or_false);
1090+
setTimeout(() => on_dirty(target_or_false));
10891091
});
10901092
},
10911093
})
@@ -1128,7 +1130,6 @@ export const CodeMirror_load = async (
11281130
}
11291131
}
11301132
// Update the view with these changes to the state.
1131-
is_ide_change = true;
11321133
current_view.dispatch({ effects: stateEffects });
11331134
// Resume autosave.
11341135
current_view.dispatch({

0 commit comments

Comments
 (0)