This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-8499-3488d37034acb70664c6554dd1bea792ad9590a3 in repository https://gitbox.apache.org/repos/asf/texera.git
commit fd09f20ec9cdf17f1c9fe18af735a22d95774d98 Author: yangzhang75 <[email protected]> AuthorDate: Fri Sep 11 02:57:26 2026 +0000 fix(workflow-form): hold a rebuild that arrives while typing instead of dropping it (#8499) ### What changes were proposed in this PR? Closes #8497. The Form View skips rebuilding its input cards while the reader is typing, so a rebuild cannot throw away a half-entered value (from #8437). Two things were wrong with that skip. - It counted any focused `INPUT` as typing, tick boxes included. Ticking a property in the step panel focuses the tick box, so the rebuild that should add the card was skipped and the tick looked like it did nothing until something else rebuilt the page (the way #8455's edit mode surfaces it). - A skipped rebuild was dropped, not postponed. A schema refresh that landed while someone was typing (the column names an attribute dropdown offers, after a compile) never reached the cards until the next compile or a reload. Now only text-like inputs, textareas, selects and content-editables count as typing, and a rebuild that does arrive mid-typing is held and runs once the focus leaves the text control: a `focusout` host listener decides one tick later, so tabbing to the next text field keeps it held while any other focus change runs it. Both subscribers (compilation state, form-binding changes) go through the one `rebuildFormOrDefer`. #8455 carries the same change so it stays self-contained and testable on its own; whichever merges first, the other rebases and the duplicate hunk disappears. ### Any related issues, documentation, discussions? Closes #8497. Found while verifying #8455 on a flag-on instance (parent #8011). ### How was this PR tested? Unit tests (vitest): a focused tick box is not typing; a held rebuild runs once on focusout for both the compilation and the binding stream; it stays held when the focus only moves to another text field; a focusout with nothing held rebuilds nothing; a queued focusout callback does not rebuild a second time when the control that took the focus already rebuilt (the tick box case). The rendered spec dispatches a real bubbling `focusout` from a control inside the page and asserts the host listener drains the held rebuild. Each was checked to fail with its half of the fix removed, including removing the `@HostListener` decorator. Full suite passes (5780), changed lines fully covered, eslint, prettier and the production (AOT) build pass. In a headless browser against a dev server built from this branch: typing a value into an exposed input and tabbing out keeps the value through the held rebuild, with no console errors and every autosave answering 200. The tick-box path was exercised the same way on #8455, which carries the identical change: the card follows the tick within a second, in both directions. ### Was this PR authored or co-authored using generative AI tooling? Yes. Generated-by: Claude Code (Claude Fable 5.1, Anthropic). Co-authored with Claude, reviewed line by line by the author before submission. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY Co-authored-by: Claude Fable 5.1 <[email protected]> --- .../workflow-form/workflow-form.component.spec.ts | 81 +++++++++++++++++-- .../workflow-form/workflow-form.component.ts | 93 +++++++++++++++++----- .../workflow-form/workflow-form.rendered.spec.ts | 17 ++++ 3 files changed, 167 insertions(+), 24 deletions(-) diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts index 7761316867..ccc125eb9b 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts @@ -822,17 +822,64 @@ describe("WorkflowFormComponent", () => { expect(rebuild).toHaveBeenCalled(); }); - it("does not rebuild under the cursor of someone typing", async () => { + it("holds a rebuild while someone is typing and runs it once the focus leaves", async () => { build(formViewWorkflow).ngOnInit(); - vi.spyOn(component as any, "isTypingInTheForm").mockReturnValue(true); + const typing = vi.spyOn(component as any, "isTypingInTheForm").mockReturnValue(true); const rebuild = vi.spyOn(component as any, "readConfig"); h.compilationChanged.next("Succeeded"); await new Promise(r => setTimeout(r, FORM_DEBOUNCE_TIME_MS + 50)); + expect(rebuild).not.toHaveBeenCalled(); + + // The cursor leaves the field: the held rebuild runs, once. Held rather than dropped, or the + // compiled schema would never reach the cards until something else rebuilt them. + typing.mockReturnValue(false); + component.onFocusOut(); + await new Promise(r => setTimeout(r, 10)); + + expect(rebuild).toHaveBeenCalledTimes(1); + }); + + it("keeps a held rebuild held when the focus only moves to another text field", async () => { + build(formViewWorkflow).ngOnInit(); + vi.spyOn(component as any, "isTypingInTheForm").mockReturnValue(true); + const rebuild = vi.spyOn(component as any, "readConfig"); + workflowActionService.formBindingChanged$.next(undefined); + + component.onFocusOut(); // tabbed to the next input: still typing when the check runs + await new Promise(r => setTimeout(r, 10)); + + expect(rebuild).not.toHaveBeenCalled(); + }); + + it("rebuilds nothing on a focusout with no rebuild held", async () => { + build(formViewWorkflow).ngOnInit(); + const rebuild = vi.spyOn(component as any, "readConfig"); + + component.onFocusOut(); + await new Promise(r => setTimeout(r, 10)); expect(rebuild).not.toHaveBeenCalled(); }); + // Leaving the text field by clicking a tick box: focusout queues the held rebuild, then the tick + // box's own change rebuilds at once and clears the hold. The queued callback must notice and + // not rebuild the same cards a second time. + it("does not rebuild twice when the control that took the focus already rebuilt", async () => { + build(formViewWorkflow).ngOnInit(); + const typing = vi.spyOn(component as any, "isTypingInTheForm").mockReturnValue(true); + const rebuild = vi.spyOn(component as any, "readConfig"); + workflowActionService.formBindingChanged$.next(undefined); // held + component.onFocusOut(); // queued + + typing.mockReturnValue(false); + workflowActionService.formBindingChanged$.next(undefined); // the tick box's own change: rebuilds now + expect(rebuild).toHaveBeenCalledTimes(1); + await new Promise(r => setTimeout(r, 10)); // the queued callback fires + + expect(rebuild).toHaveBeenCalledTimes(1); + }); + it("re-reads the config when a property is exposed or un-exposed", () => { build(formViewWorkflow).ngOnInit(); const before = formBindingService.resolveFields.mock.calls.length; @@ -843,15 +890,37 @@ describe("WorkflowFormComponent", () => { }); // Once #8351 makes this stream fire for a co-editor's change, a rebuild under the cursor would - // discard a half-entered value -- so the binding path skips typing, like the compilation path. - it("does not re-read the config on a binding change while the reader is typing", () => { + // discard a half-entered value -- so the binding path holds it while typing, like the + // compilation path, and runs it when the focus leaves. + it("holds a binding-change rebuild while the reader is typing, then runs it on focusout", async () => { build(formViewWorkflow).ngOnInit(); - vi.spyOn(component as any, "isTypingInTheForm").mockReturnValue(true); + const typing = vi.spyOn(component as any, "isTypingInTheForm").mockReturnValue(true); const rebuild = vi.spyOn(component as any, "readConfig"); workflowActionService.formBindingChanged$.next(undefined); - expect(rebuild).not.toHaveBeenCalled(); + + typing.mockReturnValue(false); + component.onFocusOut(); + await new Promise(r => setTimeout(r, 10)); + + expect(rebuild).toHaveBeenCalledTimes(1); + }); + + // The bug this guards against: ticking a property in the step panel focuses the tick box, an + // <input type="checkbox"> inside this page. Counted as typing, the rebuild that should add the + // card was held back, so the tick looked like it did nothing until something else rebuilt. + it("does not count a focused tick box as typing", () => { + build(formViewWorkflow).ngOnInit(); + const box = document.createElement("input"); + box.type = "checkbox"; + document.body.appendChild(box); + (component as any).host = { nativeElement: { contains: () => true, querySelector: () => null } }; + box.focus(); + + expect((component as any).isTypingInTheForm()).toBe(false); + + document.body.removeChild(box); }); it("reports typing when a form field inside the page is focused", () => { diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts index c9396a6069..b6c8f2ea42 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts @@ -67,6 +67,22 @@ import { CoeditorUserIconComponent } from "../menu/coeditor-user-icon/coeditor-u import { CoeditorPresenceService } from "../../service/workflow-graph/model/coeditor-presence.service"; import { SAVE_DEBOUNCE_TIME_IN_MS } from "../workspace.component"; +/** + * Input types that take a click, not text. Focusing one is not "typing", so a rebuild that arrives + * while one has the focus loses nothing and must not be held back (see isTypingInTheForm). + */ +const NON_TEXT_INPUT_TYPES = new Set([ + "checkbox", + "radio", + "button", + "submit", + "reset", + "range", + "color", + "file", + "image", +]); + /** * One rendered input: the resolved binding plus the operator's own formly field for that property. * Building the field from the operator's JSON schema (not guessing from the value) is what gives a @@ -184,6 +200,8 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { /** Set on teardown so deferred callbacks stop touching a view that is gone. */ private destroyed = false; + /** A rebuild of the inputs that arrived while the reader was typing, held until the typing ends. */ + private rebuildDeferred = false; /** * Operator positions as loaded, kept only as a fallback: a save writes the live positions @@ -389,30 +407,60 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { // Attribute boxes become dropdowns only after compilation writes the column enums into each // operator's dynamic schema -- which lands after these cards were built. Rebuild on the // compilation-state stream, a ReplaySubject(1) so a late subscriber (this page reloads fresh - // on every Canvas<->Form switch) gets the current state at once. Skip it while someone is - // typing, so a rebuild does not throw away a half-entered value under the cursor. + // on every Canvas<->Form switch) gets the current state at once. Held, not dropped, while + // someone is typing (see rebuildFormOrDefer), so it neither throws away a half-entered value + // under the cursor nor goes missing. this.workflowCompilingService .getCompilationStateInfoChangedStream() .pipe(debounceTime(FORM_DEBOUNCE_TIME_MS), untilDestroyed(this)) - .subscribe(() => { - if (this.isTypingInTheForm()) { - return; - } - this.readConfig(); - }); + .subscribe(() => this.rebuildFormOrDefer(false)); // Exposing or un-exposing a property in the panel changes the definition; the inputs above have // to follow at once, which is the whole point of editing them side by side. Today this fires for // this client's own edits; once #8351 moves formBinding into the shared model it also fires for - // a co-editor's -- so, like the compilation path, skip the rebuild while the reader is typing, or - // a remote change would throw away a half-entered value under the cursor. - this.workflowActionService.formBindingChanged$.pipe(untilDestroyed(this)).subscribe(() => { - if (this.isTypingInTheForm()) { - return; - } - this.readConfig(); + // a co-editor's -- so, like the compilation path, the rebuild is held while the reader is typing + // (a remote change would otherwise throw away a half-entered value under the cursor) and runs + // the moment the typing ends. + this.workflowActionService.formBindingChanged$ + .pipe(untilDestroyed(this)) + .subscribe(() => this.rebuildFormOrDefer(true)); + } + + /** + * Rebuild the inputs from the config now or, while the reader is typing, hold the rebuild until + * the focus leaves the text control (onFocusOut). Held rather than dropped: the change that asked + * for it (a property exposed in the panel, a schema compiled) still has to reach the page, only + * not under the cursor. Dropping it left an exposed property's card missing until something else + * happened to rebuild, which read as the tick box doing nothing. + */ + private rebuildFormOrDefer(detect: boolean): void { + if (this.isTypingInTheForm()) { + this.rebuildDeferred = true; + return; + } + this.rebuildDeferred = false; + this.readConfig(); + if (detect) { this.cdr.detectChanges(); - }); + } + } + + /** + * focusout fires before the next element takes the focus, so the held rebuild is decided after + * the current tick: a reader who merely tabbed to another text field keeps it held, anyone else + * gets it now. The hold is re-checked when that tick fires: the very click that took the focus + * can be a control whose own change rebuilds at once (the expose tick box), clearing the hold in + * between, and a stale callback that rebuilt regardless would only rebuild the same cards twice. + */ + @HostListener("focusout") + public onFocusOut(): void { + if (this.rebuildDeferred) { + this.later(() => { + if (this.rebuildDeferred) { + this.rebuildFormOrDefer(true); + } + }, 0); + } } private load(wid: number): void { @@ -473,13 +521,22 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { // Inputs: the exposed properties, rendered as their operators' own fields // --------------------------------------------------------------------------- - /** Whether the cursor is currently inside one of this page's inputs. */ + /** + * Whether the reader is mid-way through typing somewhere on this page: the caret is in a control + * that holds text (a text-like input, a textarea, a select, a content-editable). A tick box, radio + * or button also takes the focus when clicked but holds no half-entered value, so it is not typing + * -- a tick box (the step panel's expose boxes, once that panel is live for authoring) is precisely + * the click that has to rebuild the cards at once, and counting it as typing held that rebuild back. + */ private isTypingInTheForm(): boolean { const active = document.activeElement as HTMLElement | null; if (!active || !this.host.nativeElement.contains(active)) { return false; } - return ["INPUT", "TEXTAREA", "SELECT"].includes(active.tagName) || active.isContentEditable; + if (active.tagName === "INPUT") { + return !NON_TEXT_INPUT_TYPES.has((active as HTMLInputElement).type); + } + return ["TEXTAREA", "SELECT"].includes(active.tagName) || active.isContentEditable; } private readConfig(): void { diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts index 4dfa516d6f..ee31478736 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts @@ -527,4 +527,21 @@ describe("WorkflowFormComponent (rendered template)", () => { expect(workflowActionService.clearWorkflow).toHaveBeenCalled(); }); + + // The held rebuild is drained by a real blur: a focusout bubbling up from a control inside the + // page reaches the host listener. Dispatching the DOM event (not calling the handler) is what + // would catch the listener being removed or miswired. + it("runs a held rebuild when a control inside the page loses focus (the focusout host binding)", async () => { + fixture.detectChanges(); + finishLoad(); + const c = fixture.componentInstance; + const rebuild = vi.spyOn(c as any, "readConfig"); + (c as any).rebuildDeferred = true; + + el("input.wf-name")!.dispatchEvent(new FocusEvent("focusout", { bubbles: true })); + await new Promise(r => setTimeout(r, 10)); + + expect(rebuild).toHaveBeenCalledTimes(1); + expect((c as any).rebuildDeferred).toBe(false); + }); });
