Trim JSON-editor comments to the load-bearing constraints

Author: fiskus

quilt-sync/ui/js/json-editor-glue.js

@@ -1,23 +1,14 @@
-// Thin wrapper around vanilla-jsoneditor that exposes the three functions
-// consumed by the Leptos commit page via wasm-bindgen.
-//
 // Bundled by esbuild during `trunk build` (see Trunk.toml hook).
 import { createJSONEditor } from "vanilla-jsoneditor/standalone.js";
 
-// Registry keyed by the editor's DOM element, NOT an id string. During the
-// commit page's keep-alive subtree swap (the outer `Transition`), an old and a
-// new editor container are alive at once; keying by id — or resolving via
-// `document.getElementById` — would be ambiguous by construction and let a
-// replaced subtree's cleanup kill its successor's editor. Element identity never
-// is. A WeakMap needs no manual eviction to avoid leaks, and `destroy` still
-// deletes its entry so the same element is never destroyed twice.
+// Keyed by DOM element, never by id: the commit page's `Transition` keeps an
+// old and new editor container alive at once, so an id (or `getElementById`)
+// could resolve to the wrong one and let a stale cleanup destroy the live
+// editor. Element identity can't. WeakMap avoids manual eviction.
 const editors = new WeakMap();
 
 function mountEditor(target, textarea, initialValue) {
   if (!target || editors.has(target)) return;
-  // Hide the textarea fallback belonging to THIS dialog. The textarea element
-  // is passed in (not looked up by id) so we always hide the current dialog's
-  // fallback, never another dialog's during a swap.
   if (textarea && textarea.parentElement)
     textarea.parentElement.style.display = "none";
   const editor = createJSONEditor({
@@ -29,12 +20,8 @@ function mountEditor(target, textarea, initialValue) {
         if (updatedContent.json !== undefined)
           textarea.value = JSON.stringify(updatedContent.json);
         else textarea.value = updatedContent.text ?? "";
-        // Mirror the edit into the (hidden) textarea AND notify Leptos: a
-        // programmatic `value` assignment does not fire an `input` event, so
-        // dispatch one so the commit page's live-validation signal tracks
-        // metadata edits made through the JSON editor. The textarea is captured
-        // from this closure, so the write always targets the CURRENT dialog's
-        // textarea, never the first `#metadata` match in the document.
+        // Assigning `.value` does not fire `input`; dispatch one so the
+        // commit page's live-validation signal sees edits made in the editor.
         textarea.dispatchEvent(new Event("input", { bubbles: true }));
       },
       navigationBar: false,

quilt-sync/ui/src/pages/commit.rs

@@ -861,12 +861,8 @@ fn field_violation_view(
 
 // ── JSON editor integration ──
 
-// The wasm-bindgen boundary passes the editor's own DOM element (and, for
-// create, the dialog's textarea) rather than an id string. Keying the JS-side
-// registry by element identity is what makes the keep-alive `Transition` swap
-// safe: an old and a new container are alive at once, so an id key (and
-// `getElementById`) would be ambiguous and let an old subtree's cleanup destroy
-// the new subtree's editor. See json-editor-glue.js.
+// The boundary passes DOM elements, not id strings, so the JS registry can key
+// by element identity — see the `Transition`-safety note in json-editor-glue.js.
 #[wasm_bindgen::prelude::wasm_bindgen]
 extern "C" {
     #[wasm_bindgen(js_namespace = ["window"], js_name = "__getJsonEditorValue")]
@@ -883,9 +879,8 @@ extern "C" {
     fn destroy_json_editor_js(target: &web_sys::HtmlElement);
 }
 
-/// Read the committed metadata at submit: prefer the JSON editor's live value,
-/// keyed by its own element; fall back to this dialog's textarea when the editor
-/// never mounted or is empty.
+/// Read the committed metadata at submit: the editor's live value, or this
+/// dialog's textarea when the editor never mounted or is empty.
 fn get_json_editor_value(
     editor_ref: NodeRef<html::Div>,
     textarea_ref: NodeRef<html::Textarea>,
@@ -908,15 +903,9 @@ fn JsonEditor(
     textarea_ref: NodeRef<html::Textarea>,
     initial_value: String,
 ) -> impl IntoView {
-    // Mount once BOTH the editor container and this dialog's textarea are in the
-    // DOM. The textarea appears earlier in the view than the editor `<div>`, so
-    // rather than assume a load order (and silently skip when the textarea's
-    // `NodeRef` is not yet set — the same silent-failure class as the old
-    // `getElementById` miss), the mount is driven off `on_load` for both nodes
-    // and fires on whichever loads last. `on_load` guarantees each node is
-    // connected, and mounting targets exactly those elements — never a global
-    // lookup — so a concurrent keep-alive swap can never resolve to the wrong
-    // dialog's nodes.
+    // Mount needs both the editor div and the textarea loaded, and their load
+    // order isn't guaranteed, so drive it off both `on_load`s and fire on
+    // whichever lands last (the guard makes the other a no-op).
     let mounted = StoredValue::new(false);
     let init = StoredValue::new(initial_value);
     let try_mount = move || {
@@ -934,9 +923,6 @@ fn JsonEditor(
     node_ref.on_load(move |_| try_mount());
     textarea_ref.on_load(move |_| try_mount());
 
-    // Destroy exactly this element's editor. Because the registry is keyed by
-    // element, a replaced subtree's cleanup can never kill its successor's
-    // instance regardless of mount/cleanup ordering.
     on_cleanup(move || {
         if let Some(editor) = node_ref.get_untracked() {
             destroy_json_editor_js(&editor);