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 <noreply@anthropic.com>
This commit is contained in:
1 parent
922774778b
commit
06b1007015
3 files changed
+48
-4
No files matched your search
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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') {
|
||||
|
||||
+29
-2
@@ -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);
|
||||
Reference in new issue
Block a user