yangzhang75 commented on code in PR #8437:
URL: https://github.com/apache/texera/pull/8437#discussion_r3943267758
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -175,6 +240,149 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
this.workflowActionService.disableWorkflowModification();
}
+ //
---------------------------------------------------------------------------
+ // Inputs: the exposed properties, rendered as their operators' own fields
+ //
---------------------------------------------------------------------------
+
+ /** Whether the cursor is currently inside one of this page's inputs. */
+ 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;
+ }
+
+ private readConfig(): void {
+ this.parameters = this.formBindingService.resolveFields();
+ this.buildForm();
+ }
+
+ /**
+ * Build the form from the operators' JSON schemas (FormlyJsonschema),
keeping the one field per
+ * exposed property. Each input gets its own form keyed by binding id.
+ */
+ private buildForm(): void {
+ this.formsRebuilt.next();
+ this.rendered = this.visibleFields
+ .map(field => this.renderField(field))
+ .filter((r): r is RenderedField => r !== undefined);
+ }
+
+ private renderField(resolved: ResolvedField): RenderedField | undefined {
+ const { binding } = resolved;
+ const schema = this.operatorSchemaFor(binding.operatorID);
+ if (!schema) {
+ return undefined;
+ }
+ const operatorType =
this.workflowActionService.getTexeraGraph().getOperator(binding.operatorID)?.operatorType;
+ const full = this.formlyJsonschema.toFieldConfig(cloneDeep(schema) as
never, {
+ map: (mapped, source) => {
+ // Render the exact custom widget the operator property panel would
(file/model/dataset
+ // pickers, image/audio uploaders, ...), shared via
customFormlyFieldType so an exposed
+ // property shows its real control instead of degrading to a text box.
+ const customType = customFormlyFieldType({
+ key: mapped.key,
+ operatorType,
+ description: (source as { description?: string })?.description,
+ currentType: mapped.type,
+ });
+ // Canvas-only widgets (code editor, drag-reorder) do not work here;
an older workflow may
+ // already carry one, so leave it to formly's default editable control
rather than a widget
+ // that cannot function on a form.
+ if (customType && !CANVAS_ONLY_FORMLY_TYPES.has(customType)) {
+ mapped.type = customType;
+ }
+ return mapped;
+ },
+ });
+ const source = (full.fieldGroup ?? []).find(child => child.key ===
binding.propertyKey);
+ if (!source) {
+ return undefined;
+ }
+
+ const field = cloneDeep(source);
+ // The schema's own title ("Attributes", "Limit", "File") -- the reader's
title when unnamed.
+ // Falls back to this, not the lower-camel key ("fileName"), which would
read inconsistently.
+ const schemaLabel = (source.props?.label as string) || binding.propertyKey;
+ field.key = binding.id;
+ field.props = {
+ ...(field.props ?? {}),
+ label: binding.displayName || schemaLabel,
+ // The schema's own description is the operator author's note to whoever
wired the operator
+ // up; it is not guidance to a form reader, and formly shows it once per
scalar field. Drop it
+ // here so it does not appear unbidden under the input.
+ description: "",
+ };
+
+ const form = new FormGroup({});
+ const model: Record<string, unknown> = { [binding.id]:
cloneDeep(resolved.value) };
Review Comment:
Fixed. The field's model is now seeded with the operator's other properties
(`task` included) as read-only context, so the picker loads the right models
and labels the field. Only the bound property is written back, and the context
is cloned so the widget cannot reach the real operator. The remaining piece --
a form-view mode that also hides/locks the task selector so a task change
cannot desync -- touches the shared widget and is tracked in #8439.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -121,6 +162,28 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
}
this.wid = wid;
this.load(wid);
+
+ // 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.
+ this.workflowCompilingService
+ .getCompilationStateInfoChangedStream()
+ .pipe(debounceTime(FORM_DEBOUNCE_TIME_MS), untilDestroyed(this))
+ .subscribe(() => {
+ if (this.isTypingInTheForm()) {
+ return;
Review Comment:
Acknowledged; left as an edge for now (the next compilation refreshes it).
The pending-rebuild-on-blur fix is tracked in #8439.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -121,6 +162,28 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
}
this.wid = wid;
this.load(wid);
+
+ // 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.
+ this.workflowCompilingService
+ .getCompilationStateInfoChangedStream()
+ .pipe(debounceTime(FORM_DEBOUNCE_TIME_MS), untilDestroyed(this))
+ .subscribe(() => {
+ if (this.isTypingInTheForm()) {
+ return;
+ }
+ this.readConfig();
+ });
+
+ // Exposing or un-exposing a property (from the panel, or a co-editor)
changes the definition;
+ // the inputs above have to follow at once, which is the whole point of
editing side by side.
+
this.workflowActionService.formBindingChanged$.pipe(untilDestroyed(this)).subscribe(()
=> {
Review Comment:
Intended: this stream only fires for a co-editor once #8351 moves
formBinding into the shared model, so the subscription is here to work the
moment that lands. The comment now says it fires for this client today and for
co-editors after #8351, and I added the `isTypingInTheForm()` skip so a remote
change cannot clobber a half-entered value.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -175,6 +240,149 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
this.workflowActionService.disableWorkflowModification();
}
+ //
---------------------------------------------------------------------------
+ // Inputs: the exposed properties, rendered as their operators' own fields
+ //
---------------------------------------------------------------------------
+
+ /** Whether the cursor is currently inside one of this page's inputs. */
+ 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;
+ }
+
+ private readConfig(): void {
+ this.parameters = this.formBindingService.resolveFields();
+ this.buildForm();
+ }
+
+ /**
+ * Build the form from the operators' JSON schemas (FormlyJsonschema),
keeping the one field per
+ * exposed property. Each input gets its own form keyed by binding id.
+ */
+ private buildForm(): void {
+ this.formsRebuilt.next();
+ this.rendered = this.visibleFields
+ .map(field => this.renderField(field))
+ .filter((r): r is RenderedField => r !== undefined);
+ }
+
+ private renderField(resolved: ResolvedField): RenderedField | undefined {
+ const { binding } = resolved;
+ const schema = this.operatorSchemaFor(binding.operatorID);
+ if (!schema) {
+ return undefined;
+ }
+ const operatorType =
this.workflowActionService.getTexeraGraph().getOperator(binding.operatorID)?.operatorType;
+ const full = this.formlyJsonschema.toFieldConfig(cloneDeep(schema) as
never, {
+ map: (mapped, source) => {
+ // Render the exact custom widget the operator property panel would
(file/model/dataset
+ // pickers, image/audio uploaders, ...), shared via
customFormlyFieldType so an exposed
+ // property shows its real control instead of degrading to a text box.
+ const customType = customFormlyFieldType({
+ key: mapped.key,
+ operatorType,
+ description: (source as { description?: string })?.description,
+ currentType: mapped.type,
+ });
+ // Canvas-only widgets (code editor, drag-reorder) do not work here;
an older workflow may
+ // already carry one, so leave it to formly's default editable control
rather than a widget
+ // that cannot function on a form.
+ if (customType && !CANVAS_ONLY_FORMLY_TYPES.has(customType)) {
+ mapped.type = customType;
+ }
+ return mapped;
+ },
+ });
+ const source = (full.fieldGroup ?? []).find(child => child.key ===
binding.propertyKey);
+ if (!source) {
+ return undefined;
+ }
+
+ const field = cloneDeep(source);
+ // The schema's own title ("Attributes", "Limit", "File") -- the reader's
title when unnamed.
+ // Falls back to this, not the lower-camel key ("fileName"), which would
read inconsistently.
+ const schemaLabel = (source.props?.label as string) || binding.propertyKey;
+ field.key = binding.id;
+ field.props = {
+ ...(field.props ?? {}),
+ label: binding.displayName || schemaLabel,
+ // The schema's own description is the operator author's note to whoever
wired the operator
+ // up; it is not guidance to a form reader, and formly shows it once per
scalar field. Drop it
+ // here so it does not appear unbidden under the input.
+ description: "",
+ };
+
+ const form = new FormGroup({});
+ const model: Record<string, unknown> = { [binding.id]:
cloneDeep(resolved.value) };
+ if (this.canEdit) {
+ form.valueChanges
+ .pipe(debounceTime(FORM_DEBOUNCE_TIME_MS),
takeUntil(this.formsRebuilt), untilDestroyed(this))
+ .subscribe(() => {
+ // Formly emits the schema's empty default while building the
control, before any edit;
+ // writing that back silently wiped the operator's real value (both
views edit one
+ // workflow). So only accept a dirtied form, or a value that differs
from the operator's
+ // without being emptier (some controls set values without marking
dirty).
+ const next = model[binding.id];
+ const current =
this.formBindingService.readValue(binding.operatorID, binding.propertyKey);
+ const isEmpty = (v: unknown) => v === undefined || v === null || v
=== "";
+ const unchanged = JSON.stringify(next ?? null) ===
JSON.stringify(current ?? null);
+ if (unchanged || (!form.dirty && isEmpty(next) &&
!isEmpty(current))) {
+ return;
+ }
+ // Write straight onto the operator (the same edit the canvas makes)
and refresh this
+ // card's snapshot, which the template reads.
+ this.formBindingService.writeValue(binding, next);
+ this.parameters = this.formBindingService.resolveFields();
+ const refreshed = this.parameters.find(p => p.binding.id ===
binding.id);
+ const card = this.rendered.find(r => r.resolved.binding.id ===
binding.id);
+ if (refreshed && card) {
+ card.resolved = refreshed;
+ }
+ this.cdr.detectChanges();
+ });
+ } else {
+ // A read-only viewer sees the author's values and can run with them,
but cannot change them:
+ // disable the control so it renders non-editable, and wire no
write-back at all.
+ form.disable();
Review Comment:
Fixed. `form.disable()` on the still-empty group was a no-op -- formly
builds its controls after it, and a control added to a disabled group comes
back enabled (verified: `control.disabled` false, and the group re-enables
itself). Switched to `field.props.disabled = true` (formly honours it, and it
cascades to a nested property's sub-fields); the read-only card also carries
`pointer-events: none`, so the extra elements a widget draws of its own (a file
picker's Browse button, an uploader) are inert too.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -175,6 +240,149 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
this.workflowActionService.disableWorkflowModification();
}
+ //
---------------------------------------------------------------------------
+ // Inputs: the exposed properties, rendered as their operators' own fields
+ //
---------------------------------------------------------------------------
+
+ /** Whether the cursor is currently inside one of this page's inputs. */
+ 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;
+ }
+
+ private readConfig(): void {
+ this.parameters = this.formBindingService.resolveFields();
+ this.buildForm();
+ }
+
+ /**
+ * Build the form from the operators' JSON schemas (FormlyJsonschema),
keeping the one field per
+ * exposed property. Each input gets its own form keyed by binding id.
+ */
+ private buildForm(): void {
+ this.formsRebuilt.next();
+ this.rendered = this.visibleFields
+ .map(field => this.renderField(field))
+ .filter((r): r is RenderedField => r !== undefined);
+ }
+
+ private renderField(resolved: ResolvedField): RenderedField | undefined {
+ const { binding } = resolved;
+ const schema = this.operatorSchemaFor(binding.operatorID);
+ if (!schema) {
+ return undefined;
+ }
+ const operatorType =
this.workflowActionService.getTexeraGraph().getOperator(binding.operatorID)?.operatorType;
+ const full = this.formlyJsonschema.toFieldConfig(cloneDeep(schema) as
never, {
+ map: (mapped, source) => {
+ // Render the exact custom widget the operator property panel would
(file/model/dataset
+ // pickers, image/audio uploaders, ...), shared via
customFormlyFieldType so an exposed
+ // property shows its real control instead of degrading to a text box.
+ const customType = customFormlyFieldType({
+ key: mapped.key,
+ operatorType,
+ description: (source as { description?: string })?.description,
+ currentType: mapped.type,
+ });
+ // Canvas-only widgets (code editor, drag-reorder) do not work here;
an older workflow may
+ // already carry one, so leave it to formly's default editable control
rather than a widget
+ // that cannot function on a form.
+ if (customType && !CANVAS_ONLY_FORMLY_TYPES.has(customType)) {
+ mapped.type = customType;
+ }
+ return mapped;
+ },
+ });
+ const source = (full.fieldGroup ?? []).find(child => child.key ===
binding.propertyKey);
+ if (!source) {
+ return undefined;
+ }
+
+ const field = cloneDeep(source);
+ // The schema's own title ("Attributes", "Limit", "File") -- the reader's
title when unnamed.
+ // Falls back to this, not the lower-camel key ("fileName"), which would
read inconsistently.
+ const schemaLabel = (source.props?.label as string) || binding.propertyKey;
+ field.key = binding.id;
+ field.props = {
+ ...(field.props ?? {}),
+ label: binding.displayName || schemaLabel,
+ // The schema's own description is the operator author's note to whoever
wired the operator
+ // up; it is not guidance to a form reader, and formly shows it once per
scalar field. Drop it
+ // here so it does not appear unbidden under the input.
+ description: "",
+ };
+
+ const form = new FormGroup({});
+ const model: Record<string, unknown> = { [binding.id]:
cloneDeep(resolved.value) };
+ if (this.canEdit) {
+ form.valueChanges
+ .pipe(debounceTime(FORM_DEBOUNCE_TIME_MS),
takeUntil(this.formsRebuilt), untilDestroyed(this))
+ .subscribe(() => {
+ // Formly emits the schema's empty default while building the
control, before any edit;
+ // writing that back silently wiped the operator's real value (both
views edit one
+ // workflow). So only accept a dirtied form, or a value that differs
from the operator's
+ // without being emptier (some controls set values without marking
dirty).
+ const next = model[binding.id];
+ const current =
this.formBindingService.readValue(binding.operatorID, binding.propertyKey);
+ const isEmpty = (v: unknown) => v === undefined || v === null || v
=== "";
+ const unchanged = JSON.stringify(next ?? null) ===
JSON.stringify(current ?? null);
+ if (unchanged || (!form.dirty && isEmpty(next) &&
!isEmpty(current))) {
+ return;
+ }
+ // Write straight onto the operator (the same edit the canvas makes)
and refresh this
+ // card's snapshot, which the template reads.
+ this.formBindingService.writeValue(binding, next);
+ this.parameters = this.formBindingService.resolveFields();
+ const refreshed = this.parameters.find(p => p.binding.id ===
binding.id);
+ const card = this.rendered.find(r => r.resolved.binding.id ===
binding.id);
+ if (refreshed && card) {
+ card.resolved = refreshed;
+ }
+ this.cdr.detectChanges();
+ });
+ } else {
+ // A read-only viewer sees the author's values and can run with them,
but cannot change them:
+ // disable the control so it renders non-editable, and wire no
write-back at all.
+ form.disable();
+ }
+
+ return { resolved, fields: [field], form, model };
+ }
+
+ private operatorSchemaFor(operatorID: string): object | undefined {
+ const graph = this.workflowActionService.getTexeraGraph();
+ if (!graph.hasOperator(operatorID)) {
+ return undefined;
+ }
+ try {
+ // Prefer the per-instance schema: it carries the upstream column names,
so an attribute
+ // picker renders as a dropdown of real columns rather than a text box.
+ return this.dynamicSchemaService.getDynamicSchema(operatorID).jsonSchema;
+ } catch {
+ try {
+ return
this.operatorMetadataService.getOperatorSchema(graph.getOperator(operatorID).operatorType).jsonSchema;
+ } catch {
+ return undefined;
+ }
+ }
+ }
+
+ /**
+ * The inputs a reader is offered. Broken bindings (the operator was
deleted, or the property key
+ * no longer exists) are left out, since filling one in could not affect a
run; the author's view
+ * of them, to repair them, is added by the authoring PR.
+ */
+ public get visibleFields(): ResolvedField[] {
Review Comment:
There is no binding-level `hidden` on `FormFieldBinding` today -- `hidden`
lives on the per-sub-field `FormFieldOverride`, which #8438 honours in the
sub-field walk. So there is nothing at the binding level to filter yet; when
authoring adds a way to hide a whole input, the filter goes in with it.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -121,6 +162,28 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
}
this.wid = wid;
this.load(wid);
+
+ // 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.
+ this.workflowCompilingService
+ .getCompilationStateInfoChangedStream()
+ .pipe(debounceTime(FORM_DEBOUNCE_TIME_MS), untilDestroyed(this))
+ .subscribe(() => {
+ if (this.isTypingInTheForm()) {
+ return;
+ }
+ this.readConfig();
+ });
+
+ // Exposing or un-exposing a property (from the panel, or a co-editor)
changes the definition;
+ // the inputs above have to follow at once, which is the whole point of
editing side by side.
+
this.workflowActionService.formBindingChanged$.pipe(untilDestroyed(this)).subscribe(()
=> {
Review Comment:
Added -- the formBindingChanged path now has the same `isTypingInTheForm()`
skip as the compilation path, and the comment notes it goes live for co-editors
once #8351 lands.
--
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]