You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "yangzhang75 (via GitHub)" <gi...@apache.org> on 2026/09/05 17:20:39 UTC
[PR] feat(workflow-form): render the exposed inputs and write values back [texera]
yangzhang75 opened a new pull request, #8437:
URL: https://github.com/apache/texera/pull/8437
### Purpose
Closes #8370. Part of the Form View stack (#8011), stacked on #8436 (PR9).
The Form View page already has the title bar and the collapsible read-only workflow preview. This PR fills in the middle: it renders the inputs an author exposed and writes filled-in values straight back to their operators.
### Changes
- Each exposed binding is turned into its operator's own ngx-formly field, built from the operator's JSON schema (not guessed from the value), so a file property gets the real file picker and an attribute property a column dropdown instead of degrading to a plain text box.
- The widget is decided by the shared `customFormlyFieldType` (extracted in #8436). Two widgets that only work on the operator canvas, the code editor and the drag-reorder list, are collected in a new `CANVAS_ONLY_FORMLY_TYPES` and fall back to formly's default editable control, since they cannot function on a form.
- A changed value writes straight back to its operator, the same edit the canvas makes, guarded so formly's build-time empty default never silently wipes a real value. Only a viewer with write access can edit; a read-only viewer sees the values disabled and the page never persists for them.
- The inputs rebuild on the compilation stream (attribute boxes become dropdowns once upstream columns are known) and on `formBindingChanged$` (exposing or un-exposing a property, a co-editor's change included, is reflected at once), both skipped while the cursor is in a field so an in-progress edit is not thrown away.
- Broken bindings (the operator was deleted, or the key no longer exists) are left out of what a reader sees.
Nested and array sub-field overrides are added in the next PR (#8022); running the workflow and showing results follow after that.
### Tests
- `workflow-form.component.spec.ts`: direct-construction unit tests for rendering, widget selection, the write-back guards, read-only locking, the compilation/binding rebuild triggers, and the typing guard, for both the single-user and the collaboration paths.
- `workflow-form.rendered.spec.ts`: TestBed test standing up the real template so the inputs markup (section head, empty state, input card and form wrapper) is covered.
- `custom-formly-type.spec.ts`: covers the new `CANVAS_ONLY_FORMLY_TYPES`.
- 100% statement and function coverage on the changed source. `ng test` (71 tests here), `ng build gui`, eslint and prettier all pass.
### Was AI used?
Yes, co-authored with Claude (Claude Code).
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-form): render the exposed inputs and write values back [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-form): render the exposed inputs and write values back [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on PR #8437:
URL: https://github.com/apache/texera/pull/8437#issuecomment-5553983140
/request-review: @mengw15
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-form): render the exposed inputs and write values back [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8437:
URL: https://github.com/apache/texera/pull/8437#issuecomment-5553974735
<!-- texera:template-compliance -->
👋 Thanks for opening this pull request, @yangzhang75!
It looks like the pull request description doesn't quite follow our template yet:
- The **What changes were proposed in this PR?** section is missing; please keep the template's headings.
- The **How was this PR tested?** section is missing; please keep the template's headings.
- The **Was this PR authored or co-authored using generative AI tooling?** section is missing; please keep the template's headings.
Filling out the template helps reviewers understand and triage your contribution faster. Please edit the description to complete it. This message will disappear automatically once the template is followed.
You can find the template prompts by editing the description, or see [CONTRIBUTING.md](https://github.com/apache/texera/blob/main/CONTRIBUTING.md) for the full contribution flow.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-form): render the exposed inputs and write values back [texera]
Posted by "mengw15 (via GitHub)" <gi...@apache.org>.
mengw15 commented on code in PR #8437:
URL: https://github.com/apache/texera/pull/8437#discussion_r3941705164
##########
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:
`hidden`'s contract (workflow.ts: "Kept out of the reader's form. The value the author set still applies.") isn't honored here: `resolveFields` carries hidden bindings through, and this filter only drops broken ones, so a binding the author hid renders as an editable field. Nothing in the shipped UI sets `hidden` yet, so it's latent — but it's a one-line filter (plus a test) now, versus a reader editing a value the author meant to pin once the hide toggle arrives.
##########
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:
Next to Copilot's point that this can't fire on a co-editor's change today: right — #8351 (in review) is what moves formBinding into the shared model and makes this stream live for remote edits, so the wiring belongs here. But when it goes live, this path needs the same `isTypingInTheForm()` skip the compilation path has — the description says both are skipped, and only one is. A co-editor exposing a property would rebuild the cards mid-keystroke and throw away the reader's half-entered value.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-form): render the exposed inputs and write values back [texera]
Posted by "mengw15 (via GitHub)" <gi...@apache.org>.
mengw15 commented on code in PR #8437:
URL: https://github.com/apache/texera/pull/8437#discussion_r3943459829
##########
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:
You're right — I pinned FormFieldOverride's doc on the binding level, which has no `hidden`. With #8438 honouring it in the sub-field walk, nothing to filter here.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-form): render the exposed inputs and write values back [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8437:
URL: https://github.com/apache/texera/pull/8437#issuecomment-5553489397
<!-- texera-reviewer-suggestion -->
### Automated Reviewer Suggestions
Based on the `git blame` history of the changed files, we recommend the following reviewers:
- No candidates found from `git blame` history.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-form): render the exposed inputs and write values back [texera]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8437:
URL: https://github.com/apache/texera/pull/8437#discussion_r3941692539
##########
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:
The isolated model omits operator context required by the `huggingface` field type. `HuggingFaceComponent` reads and writes `model.task` and sibling controls such as `modelId` (`hugging-face.component.ts:538-579, 639-679`), but this model contains only the renamed binding-id key. Consequently an exposed `modelId` opens on the default `text-generation` task rather than the operator's task, and changing the widget's task is not written back, allowing an incompatible model/task pair. This widget needs a Form View mode/context that preserves the operator task while exposing only the bound property.
##########
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:
Returning here permanently drops this compilation update; `ReplaySubject(1)` does not emit again when focus later leaves the field. If this was the update that supplied upstream columns, the attribute control remains stale until some unrelated compilation occurs. Record a pending rebuild and execute it on blur instead of discarding the event.
This issue also appears on line 183 of the same file.
##########
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:
Disabling this empty `FormGroup` does not make all rendered custom widgets read-only. Several selected widgets expose controls outside the bound FormControl—for example, dataset selector buttons and Hugging Face file inputs/task selector—and their templates do not consult the form's disabled state, so a read-only viewer can still interact with them. Propagate a read-only flag into every custom widget (or render a non-interactive representation) and verify the actual rendered controls, not only `form.disabled`.
##########
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:
This does not receive a co-editor's binding changes. `WorkflowActionService` explicitly keeps `formBinding` outside the shared model, and `formBindingChanged$` is only emitted by the local `setFormBinding` call (`workflow-action.service.ts:108-117, 781-783`). A collaborator exposing or removing a property therefore leaves this page stale, contrary to the PR's collaboration behavior; the binding definition needs a real cross-client synchronization path before this subscription can provide that guarantee.
This issue also appears on line 321 of the same file.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-form): render the exposed inputs and write values back [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on PR #8437:
URL: https://github.com/apache/texera/pull/8437#issuecomment-5557765328
@mengw15 on your question about per-field guidance: the schema's own field descriptions are the operator author's notes to whoever wires the operator up on the canvas ("Attribute name in the schema", "Multiple string key/value pairs"), not guidance to a form reader, which is why they are dropped. The reader's per-field guidance is the form author's help text, carried on the binding and, as of the latest push, rendered under each input. The workflow-level markdown instruction above the inputs (the next PR, #8023) is the other half.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-form): render the exposed inputs and write values back [texera]
Posted by "mengw15 (via GitHub)" <gi...@apache.org>.
mengw15 merged PR #8437:
URL: https://github.com/apache/texera/pull/8437
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-form): render the exposed inputs and write values back [texera]
Posted by "yangzhang75 (via GitHub)" <gi...@apache.org>.
yangzhang75 commented on PR #8437:
URL: https://github.com/apache/texera/pull/8437#issuecomment-5557544452
Thanks both. Addressed in the latest push (force-pushed), with a follow-up issue (#8439) for the two refinements that touch shared code.
**Fixed here:**
- **Read-only controls stayed interactive** (Copilot). Confirmed and fixed. `form.disable()` on the still-empty FormGroup was a no-op: formly builds its controls after that, and a control added to an already-disabled group comes back enabled (verified in isolation: `control.disabled` false, and the group re-enables itself). Switched to `field.props.disabled = true`, which formly honours and which cascades to a nested property's sub-fields. For the extra elements a custom widget draws of its own (a file picker's Browse button, an uploader) that do not consult the form's disabled state, the read-only card now also carries `pointer-events: none`, so nothing inside it is interactive.
- **HuggingFace model picker lacked operator context** (Copilot). Right: the widget reads `task` from its model or a sibling control, and the isolated per-input model had neither, so it defaulted to text-generation. The field's model is now seeded with the operator's other properties (`task` included) as read-only context, so it 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 or locks the task selector so a task change cannot desync, touches the shared widget and is filed as #8439.
- **Binding-change stream and typing** (mengw15, Copilot). Correct on both: this stream is not live for a co-editor until #8351 moves formBinding into the shared model, and when it goes live it needs the same typing skip the compilation path has. Added the `isTypingInTheForm()` skip here, and the comment now says it fires for this client today and for co-editors once #8351 lands.
- **Per-field guidance** (mengw15's question). The reader's guidance is the author's help text, which was carried on the binding but not rendered. It now renders under each input; the schema's own field descriptions stay dropped, since those are operator-author notes, not reader guidance. The markdown instruction above the inputs is the next PR (#8023).
**Left as-is, with reasons:**
- **Compilation update dropped while typing** (Copilot). A state update that lands mid-typing is skipped and not replayed. It is an edge (the next compilation refreshes it), and the pending-rebuild-on-blur fix is filed in #8439 rather than added to this slice.
- **Hidden bindings** (mengw15). There is no binding-level `hidden` on `FormFieldBinding` today. `hidden` lives on the per-sub-field `FormFieldOverride`, which PR11 (#8438) already 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.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(workflow-form): render the exposed inputs and write values back [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #8437:
URL: https://github.com/apache/texera/pull/8437#issuecomment-5553538837
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/8437?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `96.45390%` with `5 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 94.03%. Comparing base ([`2a3bba1`](https://app.codecov.io/gh/apache/texera/commit/2a3bba15b2a913d9fe2f6188d029b61dce99d08b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`9eb0ea0`](https://app.codecov.io/gh/apache/texera/commit/9eb0ea0977d5db653dc4ad01ab46252a9757422b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on main.
| [Files with missing lines](https://app.codecov.io/gh/apache/texera/pull/8437?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...component/workflow-form/workflow-form.component.ts](https://app.codecov.io/gh/apache/texera/pull/8437?src=pr&el=tree&filepath=frontend%2Fsrc%2Fapp%2Fworkspace%2Fcomponent%2Fworkflow-form%2Fworkflow-form.component.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZnJvbnRlbmQvc3JjL2FwcC93b3Jrc3BhY2UvY29tcG9uZW50L3dvcmtmbG93LWZvcm0vd29ya2Zsb3ctZm9ybS5jb21wb25lbnQudHM=) | 95.12% | [0 Missing and 4 partials :warning: ](https://app.codecov.io/gh/apache/texera/pull/8437?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ponent/workflow-form/workflow-form.spec-harness.ts](https://app.codecov.io/gh/apache/texera/pull/8437?src=pr&el=tree&filepath=frontend%2Fsrc%2Fapp%2Fworkspace%2Fcomponent%2Fworkflow-form%2Fworkflow-form.spec-harness.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZnJvbnRlbmQvc3JjL2FwcC93b3Jrc3BhY2UvY29tcG9uZW50L3dvcmtmbG93LWZvcm0vd29ya2Zsb3ctZm9ybS5zcGVjLWhhcm5lc3MudHM=) | 93.75% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/texera/pull/8437?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #8437 +/- ##
==========================================
Coverage 94.02% 94.03%
Complexity 4820 4820
==========================================
Files 1207 1208 +1
Lines 49207 49329 +122
Branches 5978 5998 +20
==========================================
+ Hits 46268 46387 +119
Misses 1465 1465
- Partials 1474 1477 +3
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/8437/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [access-control-service](https://app.codecov.io/gh/apache/texera/pull/8437/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.00% <ø> (ø)` | | Carriedforward from [985e09e](https://app.codecov.io/gh/apache/texera/commit/985e09e096b9f314f2278bf52d798258c9bb3521?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/8437/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `99.32% <ø> (ø)` | | Carriedforward from [985e09e](https://app.codecov.io/gh/apache/texera/commit/985e09e096b9f314f2278bf52d798258c9bb3521?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [amber](https://app.codecov.io/gh/apache/texera/pull/8437/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `89.91% <ø> (ø)` | | Carriedforward from [985e09e](https://app.codecov.io/gh/apache/texera/commit/985e09e096b9f314f2278bf52d798258c9bb3521?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/8437/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.67% <ø> (ø)` | | Carriedforward from [985e09e](https://app.codecov.io/gh/apache/texera/commit/985e09e096b9f314f2278bf52d798258c9bb3521?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [config-service](https://app.codecov.io/gh/apache/texera/pull/8437/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `87.12% <ø> (ø)` | | Carriedforward from [985e09e](https://app.codecov.io/gh/apache/texera/commit/985e09e096b9f314f2278bf52d798258c9bb3521?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/8437/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `87.91% <ø> (ø)` | | Carriedforward from [985e09e](https://app.codecov.io/gh/apache/texera/commit/985e09e096b9f314f2278bf52d798258c9bb3521?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [frontend](https://app.codecov.io/gh/apache/texera/pull/8437/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `96.77% <96.45%> (+<0.01%)` | :arrow_up: | |
| [notebook-migration-service](https://app.codecov.io/gh/apache/texera/pull/8437/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.57% <ø> (ø)` | | Carriedforward from [985e09e](https://app.codecov.io/gh/apache/texera/commit/985e09e096b9f314f2278bf52d798258c9bb3521?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/8437/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `98.41% <ø> (ø)` | | Carriedforward from [985e09e](https://app.codecov.io/gh/apache/texera/commit/985e09e096b9f314f2278bf52d798258c9bb3521?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/8437/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `77.19% <ø> (ø)` | | Carriedforward from [985e09e](https://app.codecov.io/gh/apache/texera/commit/985e09e096b9f314f2278bf52d798258c9bb3521?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/8437?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org