yangzhang75 commented on code in PR #8499:
URL: https://github.com/apache/texera/pull/8499#discussion_r3985296437
##########
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:
Right, and that is exactly the click that motivated the fix (leaving a text
field by ticking a box), so it would have rebuilt twice every time. The queued
callback now re-checks rebuildDeferred before rebuilding. Test added for the
sequence: held, focusout queued, the tick box change rebuilds and clears, the
callback fires, readConfig has run exactly once. Checked to fail with the
re-check removed.
##########
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:
Done: the rendered spec dispatches a bubbling FocusEvent("focusout") from a
control inside the page (the name input) with a rebuild held, and asserts the
rebuild ran once and the hold cleared. Removing the @HostListener decorator
makes that test fail (checked); the direct-construction tests keep calling
onFocusOut() for the branch logic.
--
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]
