fix(nodes2): stop the Debug node inflating to fit its payload
fillHost means the user owns the node's height and the widget scrolls whatever overflows. That rests on `height: 100%` resolving -- and a percentage height against an INDEFINITE container computes to `auto`. Legacy gives the host a definite box so it resolves. Nodes 2.0 lays nodes out from content, so it does not: the flex column sizes to content, .wp-dbg-pre (flex: 1 1 0%; min-height: 0) is handed its full content height instead of being compressed, overflow: auto never engages, and the node inflates to the whole payload -- which the layout store then syncs back into node.size. Measured with node.size[1] pinned at 400: payload legacy host / pre Nodes 2.0 host / pre 3 vars 354u 305/305 338u 289/289 20 vars 354u 305/429 scrolls 477u 429/429 never scrolls 60 vars 354u 305/1089 scrolls 1138u 1089/1089 never scrolls One mechanism, three reported symptoms: the dead scrollbar, the node that cannot be dragged shorter, and the absurdly tall node. Pinning a pixel height alone is NOT enough. The Nodes 2.0 parent is .flex.flex-col.*:flex-1, and that `*:flex-1` hits every child including us; with flex-basis 0% and flex-grow 1 the main-axis size comes from flexing and an explicit height is ignored. Measured: host pinned to 308px still rendered 1024px. So opt out of flexing too. Height comes from node.size[1], which is authoritative in both renderers, via an error-correcting step rather than a modelled chrome constant -- compared against size[1] + header, because the host's own sync notes that layout size excludes the title. Width: legacy gets its floor free from computeSize, which Nodes 2.0 never calls, so the node could be squeezed to MIN_NODE_WIDTH (225) against a widget declaring 494. Its resize interaction reads the floor from the node element's inline min-width, so publish ours there -- writing node.size[0] does not work, the layout store owns the rendered size and syncs one-way. After, Nodes 2.0 matches legacy: size[1] holds at 400 for every payload, the pre scrolls (283/1089), and dragging shrinks to a 390x348 floor instead of 225. Only WP_Debug sets fillHost, so nothing else is affected.
This commit is contained in:
DumiFlex
@@ -23,6 +23,7 @@
|
||||
* other extensions can't read or collide with our internals.
|
||||
*/
|
||||
|
||||
import { isVueNodes } from "./renderer";
|
||||
import {
|
||||
nodeCollapseAnimating,
|
||||
nodeCollapseAnimToken,
|
||||
@@ -142,11 +143,6 @@ const VUE_SLOT_MERGED_TAIL_CLASS = "wp-slot-merged-tail";
|
||||
* stale snapshot of the slot name. */
|
||||
const VUE_SLOT_LABEL_CLASS = "wp-slot-label";
|
||||
|
||||
function isVueNodes(): boolean {
|
||||
return (globalThis as { LiteGraph?: { vueNodesMode?: boolean } })
|
||||
.LiteGraph?.vueNodesMode === true;
|
||||
}
|
||||
|
||||
/** The node's Vue-rendered element, or null (legacy renderer, node not
|
||||
* mounted yet, or no id). */
|
||||
function vueNodeEl(node: CollapseTargetNode): HTMLElement | null {
|
||||
|
||||
@@ -0,0 +1,17 @@
|
||||
/**
|
||||
* Which node renderer ComfyUI is currently using.
|
||||
*
|
||||
* ComfyUI 0.28 ships two: the legacy canvas painter and "Nodes 2.0", a Vue
|
||||
* renderer that draws each node as real DOM. They differ in ways that matter
|
||||
* to us — `computeSize` is never consulted, `node.size[0]` is a floor rather
|
||||
* than a width, and slots are DOM rows instead of canvas paint — so a handful
|
||||
* of places need to branch.
|
||||
*
|
||||
* Read live, never cache: the setting (`Comfy.VueNodes.Enabled`) is
|
||||
* user-togglable, and while the switch currently forces a reload, nothing
|
||||
* about our code should depend on that staying true.
|
||||
*/
|
||||
export function isVueNodes(): boolean {
|
||||
return (globalThis as { LiteGraph?: { vueNodesMode?: boolean } })
|
||||
.LiteGraph?.vueNodesMode === true;
|
||||
}
|
||||
@@ -1,5 +1,6 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { describe, it, expect, afterEach } from "vitest";
|
||||
import {
|
||||
createDomWidgetHost,
|
||||
emptyBundleInstance,
|
||||
emptyContextValue,
|
||||
emptyInjectorRowsValue,
|
||||
@@ -97,3 +98,78 @@ describe("parseWidgetJsonWithRecovery for ContextWidgetValue.bundles", () => {
|
||||
expect(bundles[0]).toEqual(bundleEntry);
|
||||
});
|
||||
});
|
||||
|
||||
describe("createDomWidgetHost — fillHost under the Vue renderer", () => {
|
||||
// `fillHost` relies on `height: 100%` resolving. Under Nodes 2.0 the host's
|
||||
// ancestor chain is content-sized, so it resolves to `auto` and the widget
|
||||
// grows to its payload instead of scrolling it — measured: with node.size[1]
|
||||
// pinned at 400, a 60-entry snapshot inflated the node to 1200 and the
|
||||
// scroller never engaged. These pin the renderer branch.
|
||||
function makeHostNode(id: string) {
|
||||
const widgets: Record<string, unknown>[] = [];
|
||||
return {
|
||||
id,
|
||||
size: [500, 400] as [number, number],
|
||||
addDOMWidget(name: string, type: string, el: HTMLElement, opts?: Record<string, unknown>) {
|
||||
const w = { element: el, options: opts, name, type };
|
||||
widgets.push(w);
|
||||
return w;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function mountNodeEl(id: string): HTMLElement {
|
||||
const el = document.createElement("div");
|
||||
el.setAttribute("data-node-id", id);
|
||||
document.body.appendChild(el);
|
||||
return el;
|
||||
}
|
||||
|
||||
const Stub = { render: () => null };
|
||||
|
||||
afterEach(() => {
|
||||
const lg = (globalThis as { LiteGraph?: Record<string, unknown> }).LiteGraph;
|
||||
if (lg) delete lg.vueNodesMode;
|
||||
document.body.innerHTML = "";
|
||||
});
|
||||
|
||||
it("publishes our min-width on the node element — the host's own resize hook", async () => {
|
||||
// useNodeResize reads the floor from the node element's inline min-width
|
||||
// and only falls back to MIN_NODE_WIDTH (225) when it is absent.
|
||||
(globalThis as { LiteGraph?: Record<string, unknown> }).LiteGraph = { vueNodesMode: true };
|
||||
const node = makeHostNode("41");
|
||||
const nodeEl = mountNodeEl("41");
|
||||
const host = createDomWidgetHost(node as never, "w", Stub as never, {
|
||||
fillHost: true,
|
||||
minWidth: () => 372,
|
||||
});
|
||||
nodeEl.appendChild(host.widget.element);
|
||||
|
||||
await new Promise((r) => requestAnimationFrame(() => r(null)));
|
||||
// 372 snapped to the 10px node grid.
|
||||
expect(nodeEl.style.minWidth).toBe("370px");
|
||||
// And we opt out of the parent's `*:flex-1`, or the pinned height is
|
||||
// ignored in the main axis.
|
||||
expect(host.widget.element.style.flexGrow).toBe("0");
|
||||
expect(host.widget.element.style.flexBasis).toBe("auto");
|
||||
host.unmount();
|
||||
});
|
||||
|
||||
it("leaves the legacy renderer completely alone", async () => {
|
||||
(globalThis as { LiteGraph?: Record<string, unknown> }).LiteGraph = { vueNodesMode: false };
|
||||
const node = makeHostNode("42");
|
||||
const nodeEl = mountNodeEl("42");
|
||||
const host = createDomWidgetHost(node as never, "w", Stub as never, {
|
||||
fillHost: true,
|
||||
minWidth: () => 372,
|
||||
});
|
||||
nodeEl.appendChild(host.widget.element);
|
||||
|
||||
await new Promise((r) => requestAnimationFrame(() => r(null)));
|
||||
expect(nodeEl.style.minWidth).toBe("");
|
||||
expect(host.widget.element.style.flexGrow).toBe("");
|
||||
// Legacy gives the host a definite box, so the percentage still resolves.
|
||||
expect(host.widget.element.style.height).toBe("100%");
|
||||
host.unmount();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { createApp, type App, type Component } from "vue";
|
||||
|
||||
import { nodeCollapseAnimating } from "../extension/_stashes";
|
||||
import { isVueNodes } from "../extension/renderer";
|
||||
import { installClipboardShield } from "./clipboard-shield";
|
||||
|
||||
export interface DomWidget {
|
||||
@@ -244,6 +245,139 @@ export function createDomWidgetHost<P extends Record<string, unknown>>(
|
||||
});
|
||||
if (resizeObserver) queueMicrotask(() => resizeObserver.observe(inner));
|
||||
|
||||
/* ── fillHost under the Vue renderer ──────────────────────────────────
|
||||
*
|
||||
* `fillHost` means the user owns the node's height and the widget fills it,
|
||||
* scrolling any overflow. That relies on `height: 100%` above resolving to a
|
||||
* definite length — and a percentage height against an INDEFINITE container
|
||||
* computes to `auto`. The legacy renderer gives our host a definite box, so
|
||||
* it resolves; Nodes 2.0 lays nodes out from their content, so it does not.
|
||||
*
|
||||
* The consequence is not subtle. With `height: auto` the flex column sizes
|
||||
* to content, so `.wp-dbg-pre` (`flex: 1 1 0%; min-height: 0`) is handed its
|
||||
* full content height instead of being compressed, `overflow: auto` never
|
||||
* engages, and the node inflates to fit the whole payload — overwriting
|
||||
* `node.size[1]`. Measured with `node.size[1]` pinned at 400:
|
||||
*
|
||||
* payload legacy host / pre Nodes 2.0 host / pre
|
||||
* 3 vars 354u 305/305 338u 289/289
|
||||
* 20 vars 354u 305/429 scrolls 477u 429/429 never scrolls
|
||||
* 60 vars 354u 305/1089 scrolls 1138u 1089/1089 never scrolls
|
||||
*
|
||||
* That one mechanism is the dead scrollbar, the node that cannot be dragged
|
||||
* shorter, and the absurdly tall node — all three.
|
||||
*
|
||||
* So under Vue, pin a real pixel height taken from `node.size[1]`, which is
|
||||
* authoritative in both renderers. The chrome (header, slots, padding) is
|
||||
* measured rather than assumed: it differs per node type and per host
|
||||
* version. Also pin `min-width`, because Nodes 2.0 ignores `computeSize` and
|
||||
* would otherwise let the node be squeezed far below the width the widget
|
||||
* says it needs (measured floor 225u against a declared 494u).
|
||||
*/
|
||||
let fillObserver: ResizeObserver | null = null;
|
||||
let fillDisposed = false;
|
||||
if (options.fillHost) {
|
||||
const sized = node as unknown as { id?: string | number; size?: [number, number] | number[] };
|
||||
const syncFill = (): void => {
|
||||
if (!isVueNodes()) {
|
||||
// Legacy sizes us itself. Undo any pin, in case the renderer was
|
||||
// switched under a live widget.
|
||||
if (host.style.height !== "100%") host.style.height = "100%";
|
||||
host.style.removeProperty("flex-grow");
|
||||
host.style.removeProperty("flex-shrink");
|
||||
host.style.removeProperty("flex-basis");
|
||||
return;
|
||||
}
|
||||
const nodeH = sized.size?.[1];
|
||||
if (typeof nodeH !== "number" || nodeH <= 0) return;
|
||||
const nodeEl = typeof document === "undefined" || sized.id == null
|
||||
? null
|
||||
: document.querySelector<HTMLElement>(`[data-node-id="${String(sized.id)}"]`);
|
||||
if (!nodeEl) return;
|
||||
// Width floor. Legacy gets this free: litegraph asks `computeSize`, which
|
||||
// folds in the widget's `computeLayoutSize().minWidth`, and refuses to
|
||||
// drag narrower. Nodes 2.0 never calls `computeSize`, so the node could
|
||||
// be squeezed to its global floor — `MIN_NODE_WIDTH = 225` — against a
|
||||
// widget declaring it needs 494, turning the content into an unreadable
|
||||
// column.
|
||||
//
|
||||
// Its resize interaction reads the floor off the NODE element's inline
|
||||
// style and only falls back to the global default:
|
||||
//
|
||||
// const minWidth = parseFloat(
|
||||
// nodeElement.style.getPropertyValue('min-width') || '0'
|
||||
// ) || MIN_NODE_WIDTH
|
||||
//
|
||||
// so publishing our minimum there is the sanctioned hook, and it gets
|
||||
// the west-corner position compensation for free. Writing `node.size[0]`
|
||||
// instead does NOT work: the layout store owns the rendered size and
|
||||
// syncs one-way into litegraph, so the model and the view disagree.
|
||||
const wantMinW = minWidthGetter ? snapToGrid(minWidthGetter()) : 0;
|
||||
const minPx = wantMinW > 0 ? `${wantMinW}px` : "";
|
||||
if (nodeEl.style.minWidth !== minPx) nodeEl.style.minWidth = minPx;
|
||||
// Opt out of the parent's `*:flex-1`. Written as longhands because the
|
||||
// `flex` shorthand is not implemented by every CSSOM.
|
||||
if (host.style.flexGrow !== "0") host.style.flexGrow = "0";
|
||||
if (host.style.flexShrink !== "0") host.style.flexShrink = "0";
|
||||
if (host.style.flexBasis !== "auto") host.style.flexBasis = "auto";
|
||||
|
||||
// Error-correcting step rather than a chrome constant: nudge our height
|
||||
// by however far the node's RENDERED height is from the height the model
|
||||
// asked for. `offsetHeight` is layout pixels — the same unit as
|
||||
// `node.size` — so no zoom maths is involved, and the loop settles in a
|
||||
// frame or two without needing to model the header, slots and padding.
|
||||
const nodeLayoutH = nodeEl.offsetHeight;
|
||||
const hostLayoutH = host.offsetHeight;
|
||||
if (nodeLayoutH <= 0 || hostLayoutH <= 0) return;
|
||||
// `node.size[1]` is the body height EXCLUDING the title — the host's own
|
||||
// sync comments say so ("layout.size.height is the content height
|
||||
// without title"). The DOM element includes the header, so compare
|
||||
// against `size[1] + header`, or the correction would chase the header
|
||||
// height downward on every pass.
|
||||
const headerEl = nodeEl.querySelector<HTMLElement>(".lg-node-header");
|
||||
const headerH = headerEl?.offsetHeight ?? 0;
|
||||
const error = (nodeH + headerH) - nodeLayoutH;
|
||||
|
||||
|
||||
if (Math.abs(error) <= 1 && host.style.height.endsWith("px")) return;
|
||||
const target = Math.max(minHeight, Math.round(hostLayoutH + error));
|
||||
const px = `${target}px`;
|
||||
if (host.style.height !== px) host.style.height = px;
|
||||
// The Nodes 2.0 parent is `.flex.flex-col.*:flex-1`, and that `*:flex-1`
|
||||
// hits every child including us. `flex: 1 1 0%` makes flex-basis and
|
||||
// flex-grow decide the main-axis size, so an explicit `height` is
|
||||
// ignored — measured: host pinned to 308px still rendered 1024px, and
|
||||
// the layout store then synced that back into `node.size`. The flex
|
||||
// opt-out above is what makes the pinned height actually govern.
|
||||
};
|
||||
// The node's height changes with no DOM event we can hear — a drag writes
|
||||
// `node.size` directly — but the Vue node element resizes when it does, so
|
||||
// observe that. The target is computed from the MODEL, never from the
|
||||
// element, so our own write cannot drive a feedback loop.
|
||||
//
|
||||
// The element does not exist yet at widget-construction time (a restored
|
||||
// workflow mounts its nodes later), so wait for it rather than giving up
|
||||
// on a single lookup — that was the bug that left the wire merge unapplied
|
||||
// after a renderer switch.
|
||||
const attachFillObserver = (framesLeft: number): void => {
|
||||
if (fillDisposed) return;
|
||||
if (typeof ResizeObserver === "undefined" || typeof document === "undefined") return;
|
||||
const nodeEl = sized.id == null
|
||||
? null
|
||||
: document.querySelector<HTMLElement>(`[data-node-id="${String(sized.id)}"]`);
|
||||
if (!nodeEl) {
|
||||
if (framesLeft > 0) requestAnimationFrame(() => attachFillObserver(framesLeft - 1));
|
||||
return;
|
||||
}
|
||||
syncFill();
|
||||
fillObserver = new ResizeObserver(() => syncFill());
|
||||
fillObserver.observe(nodeEl);
|
||||
};
|
||||
// ~10s of frames. Only runs while the element is missing, one selector
|
||||
// lookup per frame, and stops for good once found.
|
||||
queueMicrotask(() => attachFillObserver(600));
|
||||
}
|
||||
|
||||
// Pull-based relayout. Reentrancy flag short-circuits if litegraph's
|
||||
// response to our setSize triggers (via some plugin or observer) a
|
||||
// synchronous re-entry. 32ms release matches rgthree's _tempWidth
|
||||
@@ -323,6 +457,8 @@ export function createDomWidgetHost<P extends Record<string, unknown>>(
|
||||
app,
|
||||
unmount: () => {
|
||||
resizeObserver?.disconnect();
|
||||
fillDisposed = true;
|
||||
fillObserver?.disconnect();
|
||||
app.unmount();
|
||||
},
|
||||
getValue: () => state,
|
||||
|
||||
Reference in New Issue
Block a user