Merge pull request #343 from ausboss/fix/wheel-in-text-box

Zoom the graph over a text box that has nothing to scroll

CONTRIBUTING.md

@@ -193,7 +193,7 @@ example_workflows/  # example workflows (regular workflow JSON, not API JSON)
   drag). Messages to the user go through `showToast`, never `alert()`.
 - The mouse wheel over a node zooms the graph, on every part of its face.
   `js/canvas_passthrough/` hands the wheel on from the pack's panels, and
-  keeps it only for a box that can still scroll or a field being typed in,
+  keeps it only for a box that can still scroll, typed in or not,
   so a panel adds no wheel listener of its own
   (`tests/canvas_passthrough.test.mjs` lists the few that exist). After a
   change to a node face, `scripts/dev/wheel_sweep.mjs` turns the wheel at

js/callout/index.js

@@ -649,7 +649,6 @@ function startEdit(state) {
     if (event.target !== area) event.preventDefault();
   });
   wrap.addEventListener("pointerdown", (event) => event.stopPropagation());
-  wrap.addEventListener("wheel", (event) => event.stopPropagation(), { passive: true });
   // Clicking another node on the canvas takes the keyboard from the text box.
   // That is how the link is picked, so the edit stays open then; a click on
   // empty canvas, or on the note itself, ends it.

js/canvas_passthrough/index.js

@@ -5,7 +5,7 @@
 // zooming stopped over Compare, the crop previews, the video viewers and every
 // control on a card. One listener here hands those gestures back, the same way
 // ComfyUI does for its own text boxes. A panel that wants the wheel for itself
-// (a scrolling list, a focused text field, an editor) still keeps it.
+// (a scrolling list, a long text box) keeps it while it can still scroll.
 //
 // A file dragged in from the desktop, or a picture's card from the Assets
 // panel, has the same problem in the classic renderer: dropped on a node's
@@ -31,7 +31,7 @@ function onWheel(event) {
   const root = panelRoot(target);
   const lgCanvas = canvas();
   if (!root || !lgCanvas || event.defaultPrevented) return;
-  if (panelKeepsWheel(target, event, { root, host: panelHost(target), styleOf: (el) => getComputedStyle(el), activeElement: document.activeElement })) return;
+  if (panelKeepsWheel(target, event, { root, host: panelHost(target), styleOf: (el) => getComputedStyle(el) })) return;
   event.preventDefault();
   lgCanvas.processMouseWheel(event);
 }

js/shared/canvas_passthrough.mjs

@@ -9,7 +9,9 @@
 // in js/canvas_passthrough/ wires them to the page.
 
 // The wheel belongs to the panel while it can still scroll that way (a long
-// text box, a list of rows), or while a text field has the keyboard.
+// text box, a list of rows). A text box with the cursor in it is no
+// exception: when it has nothing left to scroll, the wheel zooms the graph,
+// as it does over ComfyUI's own text boxes.
 export function canScrollFurther(element, delta, axis = "y") {
   if (!element || !delta) return false;
   const horizontal = axis === "x";
@@ -25,10 +27,11 @@ export function canScrollFurther(element, delta, axis = "y") {
 // and the panel's own root? `styleOf` is getComputedStyle, passed in so this
 // stays testable without a page. `host` is the frontend's wrapper around the
 // panel: a pointer on the wrapper itself is on nothing inside the panel.
-export function panelKeepsWheel(target, event, { root, host, styleOf, activeElement }) {
+// Ctrl + wheel, which is also how a trackpad pinch arrives, always zooms the
+// graph; left to a scrolling box it would zoom the whole page.
+export function panelKeepsWheel(target, event, { root, host, styleOf }) {
+  if (event.ctrlKey || event.metaKey) return false;
   for (let element = target; element && element !== root && element !== host; element = element.parentElement) {
-    const tag = String(element.tagName ?? "").toUpperCase();
-    if ((tag === "TEXTAREA" || tag === "INPUT") && element === activeElement) return true;
     const style = styleOf(element);
     const overflow = style?.overflowY;
     if ((overflow === "auto" || overflow === "scroll") && canScrollFurther(element, event.deltaY)) return true;

tests/canvas_passthrough.test.mjs

@@ -27,12 +27,19 @@ test("the graph gets the wheel unless the panel has a use for it", () => {
   const list = { tagName: "DIV", parentElement: root, ...box() };
   const field = { tagName: "INPUT", parentElement: root };
   const styleOf = (el) => ({ overflowY: el === list ? "auto" : "visible" });
-  const ctx = (extra = {}) => ({ root, styleOf, activeElement: null, ...extra });
+  const ctx = (extra = {}) => ({ root, styleOf, ...extra });
   assert.equal(panelKeepsWheel(stage, { deltaY: -100 }, ctx()), false, "plain picture stage");
   assert.equal(panelKeepsWheel(list, { deltaY: 100 }, ctx()), true, "list with room to scroll");
   assert.equal(panelKeepsWheel(list, { deltaY: -100 }, ctx()), false, "list at its top, wheel up zooms");
   assert.equal(panelKeepsWheel(field, { deltaY: 100 }, ctx()), false, "idle field");
-  assert.equal(panelKeepsWheel(field, { deltaY: 100 }, ctx({ activeElement: field })), true, "field being typed in");
+  // A text box with the cursor in it and nothing to scroll: the graph zooms
+  // (issue #115, the prompt box in a workflow).
+  assert.equal(panelKeepsWheel(field, { deltaY: 100 }, ctx({ activeElement: field })), false, "field being typed in, nothing to scroll");
+  const prompt = { tagName: "TEXTAREA", parentElement: root, ...box() };
+  const scrolls = (el) => ({ overflowY: el === prompt || el === list ? "auto" : "visible" });
+  assert.equal(panelKeepsWheel(prompt, { deltaY: 100 }, ctx({ styleOf: scrolls })), true, "long text still scrolls");
+  assert.equal(panelKeepsWheel(prompt, { deltaY: -100 }, ctx({ styleOf: scrolls })), false, "long text at its top, wheel up zooms");
+  assert.equal(panelKeepsWheel(list, { deltaY: 100, ctrlKey: true }, ctx()), false, "Ctrl + wheel and pinch always zoom the graph");
 });
 
 test("middle button and Ctrl+Shift drag go to the graph; ordinary clicks do not", () => {
@@ -131,7 +138,6 @@ test("only the listed parts of the pack listen for the wheel", () => {
   // face of the node.
   const js = join(dirname(fileURLToPath(import.meta.url)), "..", "js");
   const listed = [
-    "callout/index.js wrap", // the text box while a note is being edited
     "canvas_passthrough/index.js window", // hands the wheel to the graph
     "image_folder/index.js root", // the copy question, a window over the page
     "image_folder/index.js root", // the folder list, a window over the page