yangzhang75 commented on code in PR #8845:
URL: https://github.com/apache/texera/pull/8845#discussion_r4198960915
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -838,24 +852,84 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
* exposed property. Each input gets its own form keyed by binding id.
*/
private buildForm(): void {
+ // A rebuild ends every card's write-back subscription, so an edit whose
debounced write has not
+ // fired yet is written now, and the rebuilt card is seeded with it rather
than without it.
+ this.rendered.forEach(card => card.pendingWrite && card.flushWrite?.());
this.formsRebuilt.next();
this.rendered = this.visibleFields
.map(field => this.renderField(field))
.filter((r): r is RenderedField => r !== undefined);
}
+ /**
+ * A compilation result rebuilds the cards only when it changed something
they show: a card's
+ * operator schema (the column enums compilation writes into it, which turn
an attribute box into a
+ * dropdown), the set of inputs (an operator deleted on the canvas makes its
card broken), or an
+ * operator value a card does not hold (a co-editor's edit; the card's own
edit, written back or
+ * still on its way, is not that). Most compilations change none of these --
the one a card's own
+ * write triggers, for one -- and used to rebuild every card regardless: the
controls were redrawn
+ * under the reader's eyes, the focus went with them, and a click made in
the last moments before
+ * the rebuild was lost, since the rebuild cancelled that card's pending
write. The canvas's
+ * property panel redraws only for a schema change; the form does the same.
The result picker and
+ * the shown results still follow the graph either way.
+ */
+ private onCompilationChanged(): void {
+ if (this.cardsNeedRebuild()) {
+ this.rebuildFormOrDefer(false);
+ return;
+ }
+ this.refreshShownResults();
+ this.rebuildResultChoices();
+ this.cdr.markForCheck();
+ }
+
+ private cardsNeedRebuild(): boolean {
+ const fields = this.visibleOf(this.formBindingService.resolveFields());
+ const sameInputs =
+ fields.length === this.rendered.length &&
+ fields.every((field, i) => {
+ const shown = this.rendered[i].resolved;
+ return field.binding.id === shown.binding.id && (field.brokenReason ??
"") === (shown.brokenReason ?? "");
+ });
+ if (!sameInputs) {
+ return true;
+ }
+ return this.rendered.some(card => {
+ const { binding, brokenReason } = card.resolved;
+ if (brokenReason) {
+ return false;
+ }
+ if (this.schemaKeyFor(binding.operatorID) !== card.schemaKey) {
+ return true;
+ }
+ // The card's own edit is on its way to the operator; the two agree once
it is written.
+ if (card.pendingWrite) {
+ return false;
+ }
+ return !sameValue(
+ card.model[binding.id],
+ this.formBindingService.readValue(binding.operatorID,
binding.propertyKey)
+ );
Review Comment:
Confirmed and fixed in e3a35bd0. The rebuild check now also compares the
operator's other properties -- the context the card is seeded with, which only
a rebuild re-seeds -- so a sibling changed on the canvas or by a co-editor
rebuilds the card. The card's own property is left out of that context, so its
pending edit still wins as before, and the keys are compared sorted so a
rewrite that only reorders them is not a change. Three tests cover it (a
sibling change rebuilds, another order does not, a pending own edit still does
not), each deletion-checked.
--
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]
