From 06b1007015cd0728399968e8c7913b40f1879adc Mon Sep 17 00:00:00 2001 From: root Date: Mon, 7 Sep 2026 13:03:10 +0200 Subject: [PATCH] Fix double node creation when picking a palette entry by click - TextField: new onSubmit callback (Enter only, independent of value change) - NodePalette: use onSubmit instead of onChange, add per-open picked guard - e2e: new check 13a (type + click row -> exactly one node), wait for Chromium exit in finally with SIGKILL escalation after 5s Co-Authored-By: Claude Fable 5.1 --- src/editor/graph/NodePalette.ts | 9 ++++++++- src/ui/widgets/TextField.ts | 12 +++++++++++- tools/e2e.mjs | 31 +++++++++++++++++++++++++++++-- 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/src/editor/graph/NodePalette.ts b/src/editor/graph/NodePalette.ts index ed0e7018..1807154b 100644 --- a/src/editor/graph/NodePalette.ts +++ b/src/editor/graph/NodePalette.ts @@ -8,6 +8,8 @@ export class NodePalette extends Popup { private readonly search: TextField; private readonly list = new ListView(); private filtered: NodeDef[] = []; + /** Guards against a second pick per opening (e.g. Enter and a row click racing). */ + private picked = false; constructor(private readonly onPick: (def: NodeDef) => void) { super({ padding: 4, gap: 4 }); @@ -15,7 +17,9 @@ export class NodePalette extends Popup { this.search = new TextField('', { placeholder: 'Search nodes…', onInput: () => this.refresh(), - onChange: () => { + // Enter only. NOT onChange: that also fires when the search field loses focus because + // the user clicked a list row, which would add the first match *and* the clicked one. + onSubmit: () => { const first = this.filtered[0]; if (first) this.pick(first); }, @@ -36,6 +40,7 @@ export class NodePalette extends Popup { override open(globalX: number, globalY: number): void { super.open(globalX, globalY); + this.picked = false; ui().focus.focus(this.search); } @@ -49,6 +54,8 @@ export class NodePalette extends Popup { } private pick(def: NodeDef): void { + if (this.picked) return; + this.picked = true; this.close(); this.onPick(def); } diff --git a/src/ui/widgets/TextField.ts b/src/ui/widgets/TextField.ts index 9920fb35..eb8c9fa5 100644 --- a/src/ui/widgets/TextField.ts +++ b/src/ui/widgets/TextField.ts @@ -12,6 +12,12 @@ export interface TextFieldOptions { onInput?: (value: string) => void; /** Fires on Enter / blur when the value changed since editing started. */ onChange?: (value: string) => void; + /** + * Fires on Enter only (before the blur commit), regardless of whether the value changed. + * Use this for "accept" semantics (search boxes, dialogs) instead of `onChange`, which + * also fires when focus is taken away by a click elsewhere. + */ + onSubmit?: (value: string) => void; } /** @@ -39,6 +45,7 @@ export class TextField extends Widget implements TextEditTarget { placeholder: string; onInput?: (value: string) => void; onChange?: (value: string) => void; + onSubmit?: (value: string) => void; constructor(value = '', options: TextFieldOptions = {}) { super(); @@ -46,6 +53,7 @@ export class TextField extends Widget implements TextEditTarget { this.placeholder = options.placeholder ?? ''; this.onInput = options.onInput; this.onChange = options.onChange; + this.onSubmit = options.onSubmit; this.focusable = true; this.cursor = 'text'; this.minHeight = Theme.controlHeight; @@ -174,7 +182,9 @@ export class TextField extends Widget implements TextEditTarget { onBridgeKey(event: KeyboardEvent): boolean { if (event.key === 'Enter') { - ui().focus.blur(); // triggers onBlur → commit + this.onSubmit?.(this.valueText); + // onSubmit may have closed/destroyed us; only blur if we are still the focused widget. + if (ui().focus.focused === this) ui().focus.blur(); // triggers onBlur → commit return true; } if (event.key === 'Escape') { diff --git a/tools/e2e.mjs b/tools/e2e.mjs index 9f568f56..ba021573 100644 --- a/tools/e2e.mjs +++ b/tools/e2e.mjs @@ -362,8 +362,28 @@ try { const mulUndone = await nodeOf('n-mul'); check('undo moves the node back', mulUndone.x === mulBefore.x && mulUndone.y === mulBefore.y, `${mulUndone.x},${mulUndone.y}`); - // 13b. Right-click the empty canvas → palette → search "log" → Enter adds a Log node. + // 13a. Regression: typing a query and then *clicking* a row must add exactly one node. + // (Previously the search field's blur-commit picked the first match and the row click + // picked it again → two nodes.) const gcenter = JSON.parse(await evaluate(`JSON.stringify(__gm.graph.center())`)); + const beforeClickPick = await graphCounts(); + await click(gcenter.x + 250, gcenter.y + 250, 'right'); + await sleep(250); + check('node palette opens (row-click case)', Boolean(await evaluate(`__gm.locate('Search nodes…')`))); + await typeText('stop'); + await sleep(150); + const stopRow = await locate('Stop game'); // unique on screen (nothing else on screen shows this label) + await click(stopRow.x, stopRow.y); + await sleep(250); + const afterClickPick = await graphCounts(); + check('clicking a palette row adds exactly one node', afterClickPick.nodes === beforeClickPick.nodes + 1, JSON.stringify({ beforeClickPick, afterClickPick })); + check('palette closed after row click', !(await evaluate(`__gm.locate('Search nodes…')`))); + // Undo so 13b finds the same empty spot and the expected graph. + await keyPress('z', 'KeyZ', { modifiers: 2, windowsVirtualKeyCode: 90 }); + await sleep(150); + check('undo removes the clicked node again', (await graphCounts()).nodes === beforeClickPick.nodes); + + // 13b. Right-click the empty canvas → palette → search "log" → Enter adds a Log node. const before13 = await graphCounts(); await click(gcenter.x + 250, gcenter.y + 250, 'right'); await sleep(250); @@ -433,7 +453,14 @@ try { console.error(error); failures++; } finally { - chrome.kill('SIGTERM'); + // Wait for chromium to actually exit (a SwiftShader GPU process kept rendering at full CPU + // once when only SIGTERM was sent); escalate to SIGKILL if it does not stop in time. + await new Promise((resolve) => { + if (chrome.exitCode !== null) return resolve(); + const timer = setTimeout(() => { chrome.kill('SIGKILL'); }, 5000); + chrome.once('exit', () => { clearTimeout(timer); resolve(); }); + chrome.kill('SIGTERM'); + }); } if (consoleLog.length) console.log('--- console ---\n' + consoleLog.slice(0, 20).join('\n')); process.exit(failures ? 1 : 0);