Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Comment thread
yangzhang75 marked this conversation as resolved.
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;
Expand All @@ -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", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
Loading