Copilot commented on code in PR #8438:
URL: https://github.com/apache/texera/pull/8438#discussion_r3943473310
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss:
##########
@@ -183,13 +183,134 @@ $shell: #fafafa;
padding: 40px 0;
}
-/* A section of the page: the workflow preview here, the inputs and results in
later PRs. */
+/* A section of the page: the inputs and the workflow preview here, results in
later PRs. */
.card {
border: 1px solid $border;
border-radius: 8px;
background: #fff;
}
+/* ---------- inputs ---------- */
+
+.pc-section-head {
+ display: flex;
+ align-items: baseline;
+ gap: 12px;
+ margin-bottom: 12px;
+}
+
+.label {
+ font-size: 12px;
+ font-weight: 600;
+ letter-spacing: 0.08em;
+ text-transform: uppercase;
+ color: $text-2;
+}
+
+.empty {
+ padding: 28px;
+ text-align: center;
+ color: $text-2;
+ border: 1px dashed $border;
+ border-radius: 8px;
+}
+
+.params {
+ display: flex;
+ flex-direction: column;
+ gap: 12px;
+}
+
+.param {
+ padding: 16px 18px;
+
+ /* A read-only viewer sees the values but cannot change them. The bound
controls are already
+ disabled through props.disabled; this blocks every other interactive
element a custom widget
+ draws of its own -- a file picker's Browse button, an uploader, a
picker's dropdown -- which
+ do not consult the form's disabled state. */
+ &.read-only {
+ pointer-events: none;
Review Comment:
`pointer-events: none` only blocks pointing-device hit testing; it does not
remove descendants from keyboard focus or prevent keyboard activation. This
leaves unbound controls in custom widgets active for read-only viewers—for
example, the Hugging Face audio file input ignores Formly's disabled control
and immediately POSTs the selected file. Use semantic disabling for all
descendant controls (and retain readable values) rather than relying on this
CSS lock.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -175,6 +247,246 @@ 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 operator =
this.workflowActionService.getTexeraGraph().getOperator(binding.operatorID);
+ const operatorType = operator?.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,
+ };
+
+ const form = new FormGroup({});
+ // Seed the model with the operator's other properties as read-only
context, not just this
+ // input's own value: some custom widgets read a sibling to decide what to
show -- the
+ // HuggingFace model picker reads `task` to load the right models and
label the field. Only
+ // this binding's value is ever written back (see below); the context is
never persisted, and
+ // it is cloned so a widget that mutates it cannot reach through to the
real operator.
+ const model: Record<string, unknown> = {
+ ...cloneDeep(operator?.operatorProperties ?? {}),
+ [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 at the field level, not with form.disable(): formly builds
its controls into the
+ // form after this, and a FormGroup disabled while still empty does not
disable controls added
+ // later (it re-enables itself), so the input stayed editable.
props.disabled is what formly
+ // honours, and it cascades to a nested property's sub-fields. No
write-back is wired either.
+ field.props = { ...(field.props ?? {}), disabled: true };
+ }
+
+ this.applyFieldOverrides(field, binding);
+ return { resolved, fields: [field], form, model };
+ }
+
+ /**
+ * The template for one row of a repeated section. formly's `fieldArray` may
be the template
+ * object or a function that builds one per row; resolve both so an array
property's sub-fields
+ * are reachable (treating the function case as a leaf hid them). @internal,
exported for tests.
+ */
+ public static arrayItemOf(node: FormlyFieldConfig): FormlyFieldConfig |
undefined {
+ const fa = node.fieldArray;
+ if (!fa) {
+ return undefined;
+ }
+ if (typeof fa !== "function") {
+ return fa;
+ }
+ try {
+ return fa(node);
+ } catch {
+ // A builder that needs more context than we can give it tells us
nothing about the row's
+ // shape; better to list no sub-fields than to guess at them.
+ return undefined;
+ }
+ }
+
+ /**
+ * The override path for a child field: the parent path joined with the
child's key, but array
+ * indices are dropped so one override entry covers every row of a repeated
section. @internal,
+ * exported for tests.
+ */
+ public static childPath(parent: string, key: unknown): string {
+ if (typeof key !== "string" || key === "" || /^\d+$/.test(key)) {
+ return parent;
+ }
+ return parent ? parent + "." + key : key;
+ }
+
+ /**
+ * Walk the field and its sub-fields, dropping the operator schema's own
per-field descriptions
+ * (author notes about the operator, not guidance to a form reader) and
applying the author's
+ * stored per-sub-field overrides (rename, hide), keyed by field path. A
repeated section builds
+ * its row template on demand, so its builder is wrapped to decorate every
row formly ever makes.
+ */
+ private applyFieldOverrides(field: FormlyFieldConfig, binding:
FormFieldBinding): void {
+ const walk = (node: FormlyFieldConfig, path: string): void => {
+ // Drop the schema's own description on every field, nested ones
included: on this page the
+ // one piece of guidance is the help text the form's author writes,
rendered once by the card.
+ node.props = { ...(node.props ?? {}), description: "" };
+ // Apply the author's stored overrides so a reader sees each sub-field
renamed and hidden as
+ // set up. The root (path "") carries the binding's own displayName, set
in renderField.
+ if (path) {
+ const override = binding.overrides?.[path] ?? {};
+ if (override.displayName) {
+ node.props = { ...(node.props ?? {}), label: override.displayName };
+ }
+ if (override.hidden) {
+ node.hide = true;
+ }
+ }
+ // A repeated section may build its row template on demand, once per
row. Decorating the
+ // object it returns is pointless -- the next row gets a fresh one. Wrap
the builder instead,
+ // so every row formly ever creates comes out decorated.
+ if (typeof node.fieldArray === "function") {
+ const build = node.fieldArray;
+ node.fieldArray = (f: FormlyFieldConfig) => {
+ const row = build(f);
+ // Walk what is INSIDE each row, never the row container itself: the
container carries the
+ // array property's own name, so decorating it as a root (path "")
printed the group title
+ // a second time above the rows. Its sub-fields keep their own key
paths, the same ones
+ // their overrides are stored under.
+ for (const child of row.fieldGroup ?? []) {
+ walk(child, WorkflowFormComponent.childPath(path, child.key));
+ }
Review Comment:
Function-backed scalar arrays are never passed to `walk`: when the builder
returns a leaf row, `row.fieldGroup ?? []` is empty and the wrapper returns it
unchanged. Its schema description therefore remains visible, unlike the
equivalent object-valued scalar array handled below. Please walk the row itself
when it has no `fieldGroup`.
##########
frontend/src/app/workspace/util/custom-formly-type.ts:
##########
@@ -25,6 +25,15 @@
*/
export const NON_FORM_FIELD_TYPES: ReadonlySet<string> = new Set(["codearea"]);
+/**
+ * Widgets that only work on the operator canvas, so the Form View does not
render them: it falls
+ * back to formly's default control instead. The code editor (also blocked
from exposure by
+ * {@link NON_FORM_FIELD_TYPES}) and the drag-reorder list, whose drag has
nowhere to attach on a
+ * form -- a workflow may still carry an exposed drag-reorder property from
before, and it degrades
+ * to a plain editable list rather than a control that cannot function here.
+ */
+export const CANVAS_ONLY_FORMLY_TYPES: ReadonlySet<string> = new
Set(["codearea", "repeat-section-dnd"]);
Review Comment:
`ui-udf-parameters` remains selectable in Form View, but its renderer hides
Formly labels and displays hard-coded `fieldColumns` headers (`Value`, `Name`,
`Type`). Consequently, applying `displayName` to its nested field configs does
not rename what the reader sees, and hiding a field leaves its hard-coded
column heading. Either fall back to an override-aware Form View renderer for
this type or make the custom component consume the nested labels/visibility.
--
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]
