Copilot commented on code in PR #8499:
URL: https://github.com/apache/texera/pull/8499#discussion_r3985231392
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts:
##########
@@ -822,13 +822,42 @@ 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();
Review Comment:
This calls `onFocusOut()` directly, so the tests still pass if the new
`@HostListener("focusout")` is removed or miswired; in that case a real blur
never drains the deferred rebuild. Please cover the user path in the existing
TestBed/rendered spec by dispatching a bubbling `focusout` event from the
component host or a child control, similar to the host-listener coverage
elsewhere in the frontend.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -389,30 +407,54 @@ 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.
+ */
+ @HostListener("focusout")
+ public onFocusOut(): void {
+ if (this.rebuildDeferred) {
+ this.later(() => this.rebuildFormOrDefer(true), 0);
Review Comment:
Recheck the pending flag inside the deferred callback. If a rebuild was
already pending and the user leaves the text field by clicking an expose
checkbox, `focusout` queues this callback, then the checkbox's binding event
rebuilds immediately and clears `rebuildDeferred`; this queued callback still
rebuilds the form a second time. Guarding at execution time prevents that stale
callback from duplicating `readConfig()` and change detection.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
