This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-8517-0b103f813666b13ab4aa2bad7dd29d06a92685a7 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 7190a8113ca9ec3ae30d7a1dfdbde4c288034e2c Author: yangzhang75 <[email protected]> AuthorDate: Sat Sep 12 20:31:16 2026 +0000 feat(gui): author the Form View's inputs in place (#8517) ### What changes were proposed in this PR? Closes #8027. Part of the Form View stack (parent issue #8011), on main now that #8516 (the edit mode) has merged. This is the second half of what was #8455 (closed, split in two at 2200 lines). The review commit is the branch's single commit, on main. Lets the author shape the exposed inputs where they appear, in the edit mode #8516 adds. - Rename an input or any of its sub-fields, and hide a sub-field, through a new `editable-label-wrapper` formly wrapper: the label is the input, so what the author types is what the reader reads; leaving it empty falls back to the schema label (captured before the stored override is applied, so a renamed sub-field's placeholder and tooltip promise what clearing the box really yields). The control keeps a programmatic name in both modes (a visually hidden `<label for>` while authoring, the static label for readers); a repeated field has no labelable control carrying its id, so its title names the rows as a group (`role="group"` + `aria-labelledby`) instead of pointing a label at nothing. Overrides are stored per binding and reapplied on every rebuild. In a repeated section every row shares one override, so the controls (name box, eye) sit on the first row only; later rows show the same name and hidden state statically and follow the first row's edits at once, and a scalar array's rows are walked as rows, never as the input's root, so the input's own title box appears once above them. - Reorder inputs by drag (CDK drag-drop, mapped by binding id so a card the config no longer holds cannot move the wrong field) or from the keyboard with Move up / Move down on each card, disabled at the ends. - Write per-input help text (saved without rebuilding the form on every keystroke), see "From X" attribution while authoring, and remove an input. A rename is written through on every keystroke (input, not change), so the hidden label and the saved name follow the box as it is typed and nothing is lost if the page is left while the box still has the focus. A step renamed on the canvas or in the live panel (a co-editor's rename included) rebuilds the cards, held while the reader is typing like the compilation path, so the "From X" attribution never goes stale; nothing else emits for a rename. - A binding whose operator no longer exists renders as a removable card carrying the reason for the author and is hidden from readers; it is never deleted silently on a re-read. - A repeated (array) input keeps its title above its rows in reader mode too: the shared array widget prints its label at the bottom beside its add button, so without this the title jumped from above the rows in edit mode to below them on Done. - Keyboard focus is visible on the Move, Remove and eye buttons (`:focus-visible`), and it is never dropped by the edit it triggers: a rename, a hide or help text is presentation only and is shown by the control that took it, and the page does not rebuild on such a write's own announcement: every config write announces on `formBindingChanged$` and the page rebuilds on that stream, so a presentation write is marked while it is made and its announcement skipped (it still reaches the autosave), while structural writes (expose from the panel, remove, reorder) rebuild as before, once (the callers that re-read themselves mark their write too). The test harness's form-binding mock now announces like the real service, so this chain is under test rather than severed; the Move buttons at the ends are `aria-disabled` rather than disabled, so a move that reaches the top or bottom keeps the focus on the button; after Remove the focus goes to the next card's Remove, else the previous one's, else the Inputs heading. The Move and Remove buttons are named with their input (every card has the same three), and the eye is a proper toggle (constant name, state in `aria-pressed`). The diff is about 1650 added lines because 830 of them are spec against 829 of source (of which 246 are stylesheet); the wrapper and the card's author row are one feature, and the wrapper's four files plus the review-driven fixes (once-per-path controls, self-reflected writes) are the bulk of the rest. ### Any related issues, documentation, discussions? Closes #8027. Part of the Form View feature (parent issue #8011). Replaces the second half of #8455. ### How was this PR tested? Unit tests (vitest). Direct-construction tests cover rename and hide overrides reaching the rendered fields (root title and sub-fields, keyed by path), the cards rebuilt when a step's display name changes, the name box writing through on each input event, a repeated section's controls on its first row only with later rows following a rename and a hide, the schema label kept as the fallback of an already renamed sub-field, a scalar array's rows walked as rows (one title box), the page not rebuilding on its own presentation writes though each is announced while a structural announcement still rebuilds, and the wrapper naming a repeated field as a group and fading a hidden field in a follower row, drag and keyboard reorder with the id-mapped indices (including the end stops and a card the config no longer holds), help text saved without a rebuild, broken bindings shown to an author only and kept for explicit removal, and the repeated input's static title. The rendered spec drives the card's author row through the DOM: the provenance line and drag handle, the Move buttons in both directions with their end states, the help-text box writing through, Remove, the broken card's reason, and the drop hand-off to `onDrop`. The editable-label wrapper has its own TestBed spec (real component: decorate, the name box's change renaming, the eye hiding, the reader's label association and the hidden label while authoring). Each new guard was deletion-checked (removing it turns the corresponding test red). eslint, prettier and the production (AOT) build pass; every changed line, template lines included, is statement and function covered. #### video https://github.com/user-attachments/assets/566f8bd7-5596-4085-9317-79974cfdfad2 ### Was this PR authored or co-authored using generative AI tooling? Yes. Generated-by: Claude Code (Claude Fable 5.1, Anthropic). Co-authored with Claude, reviewed line by line by the author before submission. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY Co-authored-by: Claude Fable 5.1 <[email protected]> --- .../editable-label-wrapper.component.html | 94 +++++ .../editable-label-wrapper.component.scss | 114 ++++++ .../editable-label-wrapper.component.spec.ts | 293 ++++++++++++++++ .../editable-label-wrapper.component.ts | 90 +++++ frontend/src/app/common/formly/formly-config.ts | 2 + .../workflow-form/workflow-form.component.html | 150 ++++++-- .../workflow-form/workflow-form.component.scss | 132 +++++++ .../workflow-form/workflow-form.component.spec.ts | 386 ++++++++++++++++++++- .../workflow-form/workflow-form.component.ts | 294 ++++++++++++++-- .../workflow-form/workflow-form.rendered.spec.ts | 153 ++++++++ .../workflow-form/workflow-form.spec-harness.ts | 24 +- 11 files changed, 1674 insertions(+), 58 deletions(-) diff --git a/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.html b/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.html new file mode 100644 index 0000000000..3e667f669f --- /dev/null +++ b/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.html @@ -0,0 +1,94 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. +--> + +<!-- Authoring: the label is the input, so what you type is what the reader reads. Written through on + every keystroke (input, not change): the hidden label below and the saved name follow the box as + it is typed, so nothing is lost if the page is left while the box still has the focus. --> +<div + class="lbl-row" + *ngIf="props.authoring"> + <input + class="lbl-input" + [value]="props.authorName" + [placeholder]="props.schemaLabel" + (input)="onRename($event)" + [attr.aria-label]="'Label shown above this input'" + [title]="'Shown above this box. Empty keeps ' + props.schemaLabel" /> + <!-- An input the reader can only lose by removing it altogether has no eye: offering + one here would be a second place to decide the same thing. --> + <!-- A toggle button: the name stays constant and aria-pressed carries the state (a name that + changed with the state would announce the opposite of what the state says). --> + <button + *ngIf="props.canHide !== false" + type="button" + class="lbl-eye" + [class.off]="props.authorHidden" + (click)="onToggleHidden()" + [attr.aria-pressed]="props.authorHidden === true" + aria-label="Hide from the form" + [title]="props.authorHidden ? 'Hidden from the form' : 'Shown on the form'"> + <i + nz-icon + [nzType]="props.authorHidden ? 'eye-invisible' : 'eye'" + nzTheme="outline"></i> + </button> +</div> + +<!-- The name box above edits the label; it is not the label. The control below still needs a + programmatic name (decorate blanks formly's own), so it gets one that follows the current name, + present for assistive technology only. A repeated field has no labelable control carrying the + field id (the array widget renders rows and buttons), so its name is given to the rows as a + group (role="group" + aria-labelledby) rather than through `for`, which would point at nothing. --> +<label + class="lbl-sr-only" + *ngIf="props.authoring && !props.labelsGroup" + [attr.for]="id"> + {{ props.authorName || props.schemaLabel }} +</label> +<span + class="lbl-sr-only" + *ngIf="props.authoring && props.labelsGroup" + [id]="id + '-label'"> + {{ props.authorName || props.schemaLabel }} +</span> + +<!-- Everyone else just reads it. This wrapper blanks formly's own label (decorate sets label to ""), + so this one has to do that label's job in full: `for` ties it to the control (formly gives the + control the field id), or the reader's input would show a name and announce none. The static + label is also what a later row of a repeated section shows while authoring: it follows the first + row's name box and eye (and fades with them when hidden), without controls of its own. --> +<label + class="lbl-static" + *ngIf="!props.authoring && !props.labelsGroup && (props.authorName || props.schemaLabel)" + [attr.for]="id"> + {{ props.authorName || props.schemaLabel }} +</label> +<span + class="lbl-static" + *ngIf="!props.authoring && props.labelsGroup && (props.authorName || props.schemaLabel)" + [id]="id + '-label'"> + {{ props.authorName || props.schemaLabel }} +</span> + +<div + [class.dimmed]="props.authorHidden" + [attr.role]="props.labelsGroup ? 'group' : null" + [attr.aria-labelledby]="props.labelsGroup ? id + '-label' : null"> + <ng-container #fieldComponent></ng-container> +</div> diff --git a/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.scss b/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.scss new file mode 100644 index 0000000000..b0a1201283 --- /dev/null +++ b/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.scss @@ -0,0 +1,114 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +.lbl-row { + display: flex; + align-items: center; + gap: 6px; + margin-bottom: 2px; +} + +/* A faint dashed box at rest signals the title is editable (authors could not tell it was + a field before); it firms up on hover and turns into a real input on focus. */ +.lbl-input { + flex: 1; + min-width: 0; + border: 1px dashed #dcdcdc; + border-radius: 4px; + background: none; + padding: 1px 5px; + font-size: 13px; + font-weight: 500; + color: rgba(0, 0, 0, 0.85); + outline: none; + cursor: text; + + &::placeholder { + color: rgba(0, 0, 0, 0.45); + font-weight: 400; + } + + &:hover { + border-style: solid; + border-color: #b0b0b0; + } + + &:focus { + border-style: solid; + border-color: #1890ff; + background: #fff; + } +} + +.lbl-eye { + flex: none; + border: 0; + background: none; + cursor: pointer; + line-height: 0; + padding: 3px; + border-radius: 4px; + color: rgba(0, 0, 0, 0.45); + font-size: 14px; + + &:hover { + color: rgba(0, 0, 0, 0.85); + background: #f5f5f5; + } + + &:focus-visible { + outline: 2px solid #1890ff; + outline-offset: 1px; + } + + /* Hidden is a state worth seeing at a glance, so it keeps the accent rather than + fading like the rest of the row. */ + &.off { + color: #1890ff; + } +} + +/* Present for assistive technology only: the visible name box is the label's editor, not the + control's label. Standard visually-hidden recipe: off-layout, clipped, never display:none (which + would remove it from the accessibility tree too). */ +.lbl-sr-only { + position: absolute; + width: 1px; + height: 1px; + padding: 0; + margin: -1px; + overflow: hidden; + clip: rect(0, 0, 0, 0); + white-space: nowrap; + border: 0; +} + +.lbl-static { + display: block; + font-size: 13px; + color: rgba(0, 0, 0, 0.85); + margin-bottom: 2px; +} + +/* A hidden field stays visible to its author, faded, so they can see what they removed + and put it back. The reader never renders it at all. Faded to 0.6, not further: the field is + still operable while hidden, so its text has to stay readable. */ +.dimmed { + opacity: 0.6; +} diff --git a/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.spec.ts b/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.spec.ts new file mode 100644 index 0000000000..4801dcbd8d --- /dev/null +++ b/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.spec.ts @@ -0,0 +1,293 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { FormlyFieldConfig } from "@ngx-formly/core"; +import { EditableLabelWrapperComponent } from "./editable-label-wrapper.component"; + +describe("EditableLabelWrapperComponent", () => { + let component: EditableLabelWrapperComponent; + let fixture: ComponentFixture<EditableLabelWrapperComponent>; + + const state = (overrides: Partial<Parameters<typeof EditableLabelWrapperComponent.decorate>[1]> = {}) => ({ + authoring: true, + name: "My input", + hidden: false, + fallback: "File Key", + ...overrides, + }); + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [EditableLabelWrapperComponent], + }).compileComponents(); + fixture = TestBed.createComponent(EditableLabelWrapperComponent); + component = fixture.componentInstance; + }); + + describe("decorate", () => { + it("appends the wrapper after any existing ones", () => { + const config: FormlyFieldConfig = { key: "k", wrappers: ["form-field"] }; + EditableLabelWrapperComponent.decorate( + config, + state(), + () => {}, + () => {} + ); + expect(config.wrappers).toEqual(["form-field", "editable-label-wrapper"]); + }); + + it("blanks formly's own label so the wrapper does not print it twice", () => { + const config: FormlyFieldConfig = { key: "k", props: { label: "File Key" } }; + EditableLabelWrapperComponent.decorate( + config, + state(), + () => {}, + () => {} + ); + expect(config.props?.["label"]).toBe(""); + }); + + it("maps the naming state and callbacks into props", () => { + const rename = vi.fn(); + const setHidden = vi.fn(); + const config: FormlyFieldConfig = { key: "k" }; + EditableLabelWrapperComponent.decorate(config, state({ name: "Genes", hidden: true }), rename, setHidden); + expect(config.props?.["authoring"]).toBe(true); + expect(config.props?.["authorName"]).toBe("Genes"); + expect(config.props?.["authorHidden"]).toBe(true); + expect(config.props?.["schemaLabel"]).toBe("File Key"); + expect(config.props?.["renameField"]).toBe(rename); + expect(config.props?.["setFieldHidden"]).toBe(setHidden); + }); + + it("defaults canHide to true, and honors an explicit false", () => { + const shown: FormlyFieldConfig = { key: "k" }; + EditableLabelWrapperComponent.decorate( + shown, + state(), + () => {}, + () => {} + ); + expect(shown.props?.["canHide"]).toBe(true); + + const locked: FormlyFieldConfig = { key: "k" }; + EditableLabelWrapperComponent.decorate( + locked, + state({ canHide: false }), + () => {}, + () => {} + ); + expect(locked.props?.["canHide"]).toBe(false); + }); + + it("names a field as a group only when asked (a repeated field), and by a label otherwise", () => { + const plain: FormlyFieldConfig = { key: "k" }; + EditableLabelWrapperComponent.decorate(plain, state()); + expect(plain.props?.["labelsGroup"]).toBe(false); + + const repeated: FormlyFieldConfig = { key: "rows", type: "array" }; + EditableLabelWrapperComponent.decorate(repeated, state({ group: true })); + expect(repeated.props?.["labelsGroup"]).toBe(true); + }); + }); + + describe("handlers", () => { + it("onRename forwards the input's value to renameField and shows it itself", () => { + const rename = vi.fn(); + component.field = { props: { renameField: rename, authorName: "Old" } } as unknown as FormlyFieldConfig; + component.onRename({ target: { value: "New name" } } as unknown as Event); + expect(rename).toHaveBeenCalledWith("New name"); + // The wrapper reflects the edit itself, so the page need not rebuild the form (and drop the focus). + expect(component.props["authorName"]).toBe("New name"); + }); + + it("onRename is inert on a reader mount, where decorate omitted rename", () => { + const config: FormlyFieldConfig = { key: "k", props: { label: "Predicates" } }; + EditableLabelWrapperComponent.decorate(config, { + authoring: false, + name: "Predicate", + hidden: false, + fallback: "Predicates", + canHide: false, + }); + component.field = config; + + expect(config.props?.["label"]).toBe(""); + expect(() => component.onRename({ target: { value: "x" } } as unknown as Event)).not.toThrow(); + }); + + it("onToggleHidden flips the current hidden state through setFieldHidden and shows it itself", () => { + const setHidden = vi.fn(); + component.field = { props: { authorHidden: false, setFieldHidden: setHidden } } as unknown as FormlyFieldConfig; + component.onToggleHidden(); + expect(setHidden).toHaveBeenCalledWith(true); + expect(component.props["authorHidden"]).toBe(true); + component.onToggleHidden(); + expect(setHidden).toHaveBeenLastCalledWith(false); + expect(component.props["authorHidden"]).toBe(false); + }); + }); + + describe("template", () => { + it("while authoring, renders the name input seeded with authorName and the schema label as placeholder", () => { + component.field = { + props: { authoring: true, authorName: "Genes", schemaLabel: "File Key", canHide: true, authorHidden: false }, + } as unknown as FormlyFieldConfig; + fixture.detectChanges(); + + const input = fixture.nativeElement.querySelector("input.lbl-input") as HTMLInputElement; + expect(input).toBeTruthy(); + expect(input.value).toBe("Genes"); + expect(input.placeholder).toBe("File Key"); + // the hide toggle is offered when canHide is not false + expect(fixture.nativeElement.querySelector("button.lbl-eye")).toBeTruthy(); + }); + + it("while authoring, a change in the name box renames and a click on the eye hides", () => { + const rename = vi.fn(); + const setHidden = vi.fn(); + component.field = { + props: { + authoring: true, + authorName: "Genes", + schemaLabel: "File Key", + canHide: true, + authorHidden: false, + renameField: rename, + setFieldHidden: setHidden, + }, + } as unknown as FormlyFieldConfig; + fixture.detectChanges(); + + const input = fixture.nativeElement.querySelector("input.lbl-input") as HTMLInputElement; + input.value = "Marker genes"; + // Written through on each keystroke, not only on commit: a rename must not be lost when the + // page is left while the box still has the focus. + input.dispatchEvent(new Event("input")); + expect(rename).toHaveBeenCalledWith("Marker genes"); + + const eye = fixture.nativeElement.querySelector("button.lbl-eye") as HTMLButtonElement; + // A toggle: constant name, state in aria-pressed; the field fades and the eye flips in place. + expect(eye.getAttribute("aria-label")).toBe("Hide from the form"); + expect(eye.getAttribute("aria-pressed")).toBe("false"); + expect(fixture.nativeElement.querySelector(".dimmed")).toBeNull(); + eye.focus(); + eye.click(); + fixture.detectChanges(); + expect(setHidden).toHaveBeenCalledWith(true); + expect(eye.getAttribute("aria-label")).toBe("Hide from the form"); + expect(eye.getAttribute("aria-pressed")).toBe("true"); + expect(eye.classList.contains("off")).toBe(true); + expect(fixture.nativeElement.querySelector(".dimmed")).not.toBeNull(); + // Nothing was rebuilt, so the eye still has the focus. + expect(document.activeElement).toBe(eye); + }); + + it("while authoring, still names the control for assistive technology, following the current name", () => { + // The visible box edits the label; it is not the control's label. decorate blanks formly's + // own, so without this the control below would have no accessible name while authoring. + component.field = { + id: "formly_3_genes", + props: { authoring: true, authorName: "Genes", schemaLabel: "File Key", canHide: true }, + } as unknown as FormlyFieldConfig; + fixture.detectChanges(); + + const label = fixture.nativeElement.querySelector("label.lbl-sr-only") as HTMLLabelElement; + expect(label.getAttribute("for")).toBe("formly_3_genes"); + expect(label.textContent?.trim()).toBe("Genes"); + // The name box itself is labelled on its own and never points at the control. + expect(fixture.nativeElement.querySelector("input.lbl-input").getAttribute("aria-label")).toBeTruthy(); + + component.field.props!["authorName"] = ""; + fixture.detectChanges(); + expect(label.textContent?.trim()).toBe("File Key"); + }); + + it("hides the eye toggle when canHide is false", () => { + component.field = { + props: { authoring: true, authorName: "Genes", schemaLabel: "File Key", canHide: false }, + } as unknown as FormlyFieldConfig; + fixture.detectChanges(); + expect(fixture.nativeElement.querySelector("button.lbl-eye")).toBeNull(); + }); + + it("for a reader, renders a plain static label from authorName, tied to the control by id", () => { + // decorate blanks formly's own label, so this one is the control's only label: without `for` + // the reader's input would show a name but have no accessible name at all. + component.field = { + id: "formly_3_genes", + props: { authoring: false, authorName: "Genes", schemaLabel: "File Key" }, + } as unknown as FormlyFieldConfig; + fixture.detectChanges(); + + expect(fixture.nativeElement.querySelector("input.lbl-input")).toBeNull(); + const label = fixture.nativeElement.querySelector("label.lbl-static") as HTMLLabelElement; + expect(label.textContent?.trim()).toBe("Genes"); + expect(label.getAttribute("for")).toBe("formly_3_genes"); + }); + + it("names a repeated field as a group instead of pointing a label at a control it has not got", () => { + // The array widget renders rows and buttons, none carrying the field id, so `label for` would + // associate nothing. The title becomes the accessible name of the rows as a group, for a reader + // and while authoring alike. + component.field = { + id: "formly_4_predicates", + props: { authoring: false, authorName: "Predicates", schemaLabel: "Predicates", labelsGroup: true }, + } as unknown as FormlyFieldConfig; + fixture.detectChanges(); + + expect(fixture.nativeElement.querySelector("label")).toBeNull(); + const title = fixture.nativeElement.querySelector("span.lbl-static") as HTMLSpanElement; + expect(title.id).toBe("formly_4_predicates-label"); + const group = fixture.nativeElement.querySelector("[role=group]") as HTMLElement; + expect(group.getAttribute("aria-labelledby")).toBe("formly_4_predicates-label"); + + component.field = { + id: "formly_4_predicates", + props: { + authoring: true, + authorName: "Predicates", + schemaLabel: "Predicates", + canHide: false, + labelsGroup: true, + }, + } as unknown as FormlyFieldConfig; + fixture.detectChanges(); + expect(fixture.nativeElement.querySelector("label")).toBeNull(); + expect((fixture.nativeElement.querySelector("span.lbl-sr-only") as HTMLElement).id).toBe( + "formly_4_predicates-label" + ); + expect(fixture.nativeElement.querySelector("[role=group]").getAttribute("aria-labelledby")).toBe( + "formly_4_predicates-label" + ); + }); + + it("fades a hidden field in a static (follower) row too, so a repeated section's rows agree", () => { + // A later row of a repeated section is decorated without controls (authoring false) but carries + // the same hidden state as the first row's eye; it must look hidden the same way. + component.field = { + id: "formly_5_alias", + props: { authoring: false, authorName: "Alias", schemaLabel: "Alias", authorHidden: true }, + } as unknown as FormlyFieldConfig; + fixture.detectChanges(); + expect(fixture.nativeElement.querySelector(".dimmed")).not.toBeNull(); + }); + }); +}); diff --git a/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.ts b/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.ts new file mode 100644 index 0000000000..feebd82aa7 --- /dev/null +++ b/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.ts @@ -0,0 +1,90 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { Component } from "@angular/core"; +import { NgIf } from "@angular/common"; +import { NzIconDirective } from "ng-zorro-antd/icon"; +import { FieldWrapper, FormlyFieldConfig } from "@ngx-formly/core"; +import { merge } from "lodash-es"; + +/** + * Lets an author rename or hide one field of the form in place: the label itself becomes + * the input, so what they type is exactly what the reader sees, where they see it (the + * schema's own labels -- "File Key", "Alias" -- describe the operator, not the reader's + * task). Renders as a plain label for anyone not authoring. + */ +@Component({ + selector: "texera-editable-label-wrapper", + templateUrl: "./editable-label-wrapper.component.html", + styleUrls: ["./editable-label-wrapper.component.scss"], + imports: [NgIf, NzIconDirective], +}) +export class EditableLabelWrapperComponent extends FieldWrapper { + /** Add this wrapper to a field with its naming + callbacks; `fallback` (the schema label) + * is the placeholder, so the author sees what leaving it blank yields. `rename` may be omitted + * for a reader mount (`authoring: false`), which renders a static label and no name input; + * `setHidden` may be omitted only with `canHide: false`, where the hide control is never + * rendered. */ + public static decorate( + config: FormlyFieldConfig, + state: { authoring: boolean; name: string; hidden: boolean; fallback: string; canHide?: boolean; group?: boolean }, + rename?: (name: string) => void, + setHidden?: (hidden: boolean) => void + ): void { + merge(config, { + wrappers: [...(config.wrappers ?? []), "editable-label-wrapper"], + props: { + ...config.props, + // The wrapper draws the label itself; leaving formly's own label on would print + // it twice. + label: "", + authoring: state.authoring, + authorName: state.name, + authorHidden: state.hidden, + canHide: state.canHide !== false, + // A repeated field has no labelable control carrying its id (the array widget renders rows + // and buttons), so a `label for` would point at nothing; it is named as a group instead. + labelsGroup: state.group === true, + schemaLabel: state.fallback, + renameField: rename, + setFieldHidden: setHidden, + }, + }); + } + + public onRename(event: Event): void { + // The wrapper reflects the edit itself (the hidden label follows authorName), so the page does + // not rebuild the form for a rename and the author keeps the focus where it is. + const value = (event.target as HTMLInputElement).value; + this.props["authorName"] = value; + // Optional-chained for the same reason as setFieldHidden below: a reader mount omits rename and + // never renders the input this handles, but the handler should not depend on that. + this.props["renameField"]?.(value); + } + + public onToggleHidden(): void { + // Same as onRename: the eye, the pressed state and the faded field follow authorHidden here, so + // no rebuild is needed and the focus stays on the eye. + const hidden = !this.props["authorHidden"]; + this.props["authorHidden"] = hidden; + // Optional-chained: decorate may omit setHidden when canHide is false, and although this handler + // is unreachable then (the hide control is not rendered), a plain call would couple that to luck. + this.props["setFieldHidden"]?.(hidden); + } +} diff --git a/frontend/src/app/common/formly/formly-config.ts b/frontend/src/app/common/formly/formly-config.ts index 61cc13e639..2b42d1239b 100644 --- a/frontend/src/app/common/formly/formly-config.ts +++ b/frontend/src/app/common/formly/formly-config.ts @@ -27,6 +27,7 @@ import { PresetWrapperComponent } from "./preset-wrapper/preset-wrapper.componen import { DatasetFileSelectorComponent } from "../../workspace/component/dataset-file-selector/dataset-file-selector.component"; import { CollabWrapperComponent } from "./collab-wrapper/collab-wrapper/collab-wrapper.component"; import { ExposePropertyWrapperComponent } from "./expose-property-wrapper/expose-property-wrapper.component"; +import { EditableLabelWrapperComponent } from "./editable-label-wrapper/editable-label-wrapper.component"; import { FormlyRepeatDndComponent } from "./repeat-dnd/repeat-dnd.component"; import { UiUdfParametersComponent } from "../../workspace/component/ui-udf-parameters/ui-udf-parameters.component"; import { DatasetVersionSelectorComponent } from "../../workspace/component/dataset-version-selector/dataset-version-selector.component"; @@ -94,6 +95,7 @@ export const TEXERA_FORMLY_CONFIG = { { name: "preset-wrapper", component: PresetWrapperComponent }, { name: "collab-wrapper", component: CollabWrapperComponent }, { name: "expose-property-wrapper", component: ExposePropertyWrapperComponent }, + { name: "editable-label-wrapper", component: EditableLabelWrapperComponent }, ], }; diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html index bad8ee81e6..b0425bc0f6 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html @@ -201,12 +201,18 @@ </section> <!-- The inputs an author exposed, each rendered as its operator's own field. --> + <!-- The heading takes the focus (tabindex -1, not in the tab order) when the last card is removed + and there is no neighbouring card to hand the focus to. --> <div class="pc-section-head"> - <span class="label">Inputs</span> + <span + class="label" + tabindex="-1" + >Inputs</span + > <span class="hint" *ngIf="authoring" - >Click a step in the workflow to add more</span + >Drag to reorder. Click a step in the workflow to add more</span > </div> @@ -217,30 +223,130 @@ has no inputs to fill in." }} </div> - <div class="params"> + <div + class="params" + cdkDropList + (cdkDropListDropped)="onDrop($event)"> <section class="card param" [class.read-only]="!canEdit" - *ngFor="let r of rendered; trackBy: trackByRendered"> - <!-- The operator's own field, so a file property gets the real file picker and an - attribute property a column dropdown. --> - <form - [formGroup]="r.form" - class="param-form"> - <formly-form - [model]="r.model" - [fields]="r.fields" - [form]="r.form"></formly-form> - </form> + *ngFor="let r of rendered; let i = index; trackBy: trackByRendered" + cdkDrag + [cdkDragDisabled]="!authoring"> + <!-- Author row: the drag handle, the "From <step>" provenance line, and the keyboard way to + reorder. The provenance is hidden for a broken input, whose operatorLabel is the raw id; + the broken card below says what happened. --> + <div + class="field-top" + *ngIf="authoring"> + <i + nz-icon + nzType="holder" + class="grip" + cdkDragHandle + aria-hidden="true"></i> + <div + class="field-help" + *ngIf="!r.resolved.brokenReason"> + From {{ r.resolved.operatorLabel }} + </div> + <span class="field-top-spacer"></span> + <!-- The drag handle is pointer-only and hidden from assistive tech (CDK drag-drop has no + keyboard path), so these two are how a keyboard author reorders. Same reorder as the + drag. At the ends they are aria-disabled, not disabled: a move that reaches the top or + bottom would otherwise disable the very button that has the focus and drop it, and the + handler already moves nothing off either end. Named with the input, since every card + has the same two buttons. --> + <button + type="button" + class="move" + [attr.aria-disabled]="i === 0 ? true : null" + [attr.aria-label]="'Move ' + cardName(r) + ' up'" + title="Move up" + (click)="onMoveBinding(r, -1)"> + <i + nz-icon + nzType="up" + aria-hidden="true"></i> + </button> + <button + type="button" + class="move" + [attr.aria-disabled]="i === rendered.length - 1 ? true : null" + [attr.aria-label]="'Move ' + cardName(r) + ' down'" + title="Move down" + (click)="onMoveBinding(r, 1)"> + <i + nz-icon + nzType="down" + aria-hidden="true"></i> + </button> + </div> - <!-- The one line of guidance a reader gets, when the author wrote one. The operator - schema's own field descriptions are dropped (author notes, not reader guidance), so - this is the only help text under an input. --> - <p - *ngIf="r.resolved.binding.helpText" - class="param-help-text"> - {{ r.resolved.binding.helpText }} - </p> + <!-- A broken input (its operator removed) can never be filled; only an author reaches it, + only to remove it. --> + <div + class="broken" + *ngIf="r.resolved.brokenReason"> + {{ r.resolved.brokenReason }} + </div> + + <ng-container *ngIf="!r.resolved.brokenReason"> + <!-- The operator's own field, so a file property gets the real file picker and an + attribute property a column dropdown. --> + <form + [formGroup]="r.form" + class="param-form"> + <formly-form + [model]="r.model" + [fields]="r.fields" + [form]="r.form"></formly-form> + </form> + + <!-- The one line of guidance a reader gets, when the author wrote one. Hidden while + authoring, where the help text is edited in its own box below. --> + <p + *ngIf="!authoring && r.resolved.binding.helpText" + class="param-help-text"> + {{ r.resolved.binding.helpText }} + </p> + </ng-container> + + <!-- Author edits the input in place, right under what it looks like. The name is renamed by + clicking the title above (the same gesture that renames every nested box); here it is + the help text and the way to take the input off the form. --> + <div + class="edit" + *ngIf="authoring"> + <label + class="field wide" + *ngIf="!r.resolved.brokenReason"> + <span class="label">Help text</span> + <input + [value]="r.resolved.binding.helpText || ''" + (input)="onEditHelpText(r.resolved, $any($event.target).value)" /> + </label> + <div class="edit-foot"> + <span + class="hint" + *ngIf="!r.resolved.brokenReason" + >Set on {{ r.resolved.operatorLabel }}: {{ r.resolved.binding.propertyKey }}</span + > + <span class="spacer"></span> + <button + type="button" + class="remove" + [attr.data-binding]="r.resolved.binding.id" + [attr.aria-label]="'Remove ' + cardName(r)" + (click)="onRemoveBinding(r.resolved)"> + <i + nz-icon + nzType="delete" + aria-hidden="true"></i> + Remove + </button> + </div> + </div> </section> </div> diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss index 54f962672f..fac1e8e948 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss @@ -921,6 +921,138 @@ $shell: #fafafa; } } +.field-top { + display: flex; + align-items: flex-start; + gap: 10px; +} + +.field-top-spacer { + flex: 1; +} + +.grip { + color: rgba(0, 0, 0, 0.3); + cursor: grab; + padding-top: 3px; +} + +/* Move up / Move down: quiet icon buttons at the row's right edge, the keyboard route the drag + handle does not give. Disabled at the ends rather than hidden, so the row does not reflow as a + card reaches the top or bottom. */ +.move { + appearance: none; + border: 0; + background: none; + padding: 2px 3px; + border-radius: 3px; + color: $text-2; + cursor: pointer; + font: inherit; + line-height: 1; + + &:hover:not([aria-disabled="true"]) { + color: $text; + } + + /* At the top or bottom the button stays focusable (aria-disabled, not disabled) so a move that + reaches the end does not drop the keyboard focus; it only looks inert and does nothing. */ + &[aria-disabled="true"] { + color: rgba(0, 0, 0, 0.15); + cursor: default; + } + + &:focus-visible { + outline: 2px solid $blue; + outline-offset: 1px; + } +} + +.field-help { + font-size: 13px; + color: $text-2; + margin-top: 2px; +} + +.broken { + margin-top: 4px; + font-size: 13px; + color: #874d00; +} + +/* A dragged card lifts; the gap it left fades. */ +.cdk-drag-preview { + box-shadow: 0 6px 20px rgba(0, 0, 0, 0.14); +} + +.cdk-drag-placeholder { + opacity: 0.35; +} + +.edit { + margin-top: 14px; + padding-top: 14px; + border-top: 1px dashed $border; + display: grid; + grid-template-columns: 1fr; + gap: 12px 18px; + + .field { + display: flex; + flex-direction: column; + gap: 4px; + + .label { + font-size: 12px; + color: $text-2; + } + + input { + border: 1px solid $border; + border-radius: 6px; + padding: 6px 10px; + font-size: 13px; + + &:focus { + border-color: $blue; + outline: none; + } + } + } + + .field.wide { + grid-column: 1 / -1; + } + + .edit-foot { + grid-column: 1 / -1; + display: flex; + align-items: center; + + .spacer { + flex: 1; + } + + .remove { + background: none; + border: 1px solid $border; + border-radius: 6px; + padding: 4px 10px; + color: #ff4d4f; + cursor: pointer; + + &:hover { + border-color: #ff4d4f; + } + + &:focus-visible { + outline: 2px solid $blue; + outline-offset: 2px; + } + } + } +} + .respick { margin-top: 26px; padding: 15px 18px; diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts index e199a2187f..b6d780ac44 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts @@ -1009,6 +1009,41 @@ describe("WorkflowFormComponent", () => { return component.rendered[0].fields[0] as any; }; + // The shared array widget prints its label at the bottom beside its add button, so a repeated + // input's title would sit above the rows in edit mode and jump below them on Done. A reader gets + // the same static title above instead, and the widget's own label is blanked. + it("gives a repeated input its title above in reader mode, not the array widget's bottom label", () => { + build(formViewWorkflow).ngOnInit(); + h.formlyJsonschema.toFieldConfig = () => ({ + fieldGroup: [ + { + key: "predicates", + type: "array", + props: { label: "Predicates" }, + fieldArray: () => ({ fieldGroup: [{ key: "alias", props: { label: "Alias" } }] }), + }, + ], + }); + + const field = expose({ id: "p", operatorID: "op-1", propertyKey: "predicates", displayName: "Predicate" }); + + expect(field.wrappers).toContain("editable-label-wrapper"); + expect(field.props.authoring).toBe(false); + expect(field.props.authorName).toBe("Predicate"); + expect(field.props.schemaLabel).toBe("Predicates"); + expect(field.props.label).toBe(""); + }); + + it("leaves a scalar input's label to formly in reader mode", () => { + build(formViewWorkflow).ngOnInit(); + + const field = expose({ id: "n", operatorID: "op-1", propertyKey: "n_hvg", displayName: "How many" }); + + // Above the control already, with formly's required marker; nothing to wrap. + expect(field.wrappers ?? []).not.toContain("editable-label-wrapper"); + expect(field.props.label).toBe("How many"); + }); + it("renames and hides an overridden sub-field of an object property", () => { build(formViewWorkflow).ngOnInit(); @@ -1051,6 +1086,128 @@ describe("WorkflowFormComponent", () => { expect(alias.resetOnHide).toBe(false); }); + it("while authoring, gives a repeated section's controls to its first row only; later rows follow", () => { + // Every row shares one override, so a name box and an eye on each row would be that many copies + // of one control, none following the others. The first row carries the controls; later rows + // show the same name and hidden state statically and follow the first row's edits at once. + build(formViewWorkflow).ngOnInit(); + component.authoring = true; + h.formlyJsonschema.toFieldConfig = () => ({ + fieldGroup: [ + { + key: "predicates", + type: "array", + props: { label: "Predicates" }, + fieldArray: () => ({ fieldGroup: [{ key: "alias", props: { label: "Alias" } }] }), + }, + ], + }); + + const field = expose({ + id: "p", + operatorID: "op-1", + propertyKey: "predicates", + displayName: "Predicates", + overrides: { alias: { displayName: "Renamed", hidden: true } }, + }); + const first = field.fieldArray({}).fieldGroup[0]; + const second = field.fieldArray({}).fieldGroup[0]; + + expect(first.props.authoring).toBe(true); + expect(first.props.renameField).toBeTypeOf("function"); + expect(first.props.setFieldHidden).toBeTypeOf("function"); + expect(second.props.authoring).toBe(false); // static: no box, no eye + expect(second.props.renameField).toBeUndefined(); + expect(second.props.authorName).toBe("Renamed"); + expect(second.props.authorHidden).toBe(true); + + first.props.renameField("New alias"); + first.props.setFieldHidden(false); + expect(second.props.authorName).toBe("New alias"); + expect(second.props.authorHidden).toBe(false); + + // A rebuild replaces the rows: the old followers go with them and the new rows register anew, + // so an edit on the rebuilt first row reaches the rebuilt rows, not the stale ones. + const rebuilt = expose({ + id: "p", + operatorID: "op-1", + propertyKey: "predicates", + displayName: "Predicates", + overrides: { alias: { displayName: "Renamed", hidden: true } }, + }); + const rebuiltFirst = rebuilt.fieldArray({}).fieldGroup[0]; + const rebuiltSecond = rebuilt.fieldArray({}).fieldGroup[0]; + rebuiltFirst.props.renameField("Again"); + expect(rebuiltSecond.props.authorName).toBe("Again"); + expect(second.props.authorName).toBe("New alias"); + }); + + it("keeps the schema's own label as the name box's fallback when the sub-field is already renamed", () => { + // The placeholder and the "Empty keeps ..." tooltip promise what clearing the box yields; an + // empty name deletes the override, so that is the schema label, not the old override. + build(formViewWorkflow).ngOnInit(); + component.authoring = true; + + const field = expose({ + id: "n", + operatorID: "op-1", + propertyKey: "nested", + displayName: "Nested", + overrides: { sub: { displayName: "Renamed sub" } }, + }); + + const sub = field.fieldGroup[0]; + expect(sub.props.authorName).toBe("Renamed sub"); + expect(sub.props.schemaLabel).toBe("Sub"); + }); + + it("walks a scalar array's rows as rows, so the input's title box appears once, above them", () => { + build(formViewWorkflow).ngOnInit(); + component.authoring = true; + h.formlyJsonschema.toFieldConfig = () => ({ + fieldGroup: [ + { + key: "tags", + type: "array", + props: { label: "Tags" }, + fieldArray: () => ({ type: "input", props: { label: "Tag" } }), + }, + ], + }); + + const field = expose({ id: "t", operatorID: "op-1", propertyKey: "tags", displayName: "Tags" }); + const row = field.fieldArray({}); + + expect(field.props.authoring).toBe(true); // the input itself carries the one title box... + expect(field.props.labelsGroup).toBe(true); // ...naming the rows as a group + expect(row.wrappers ?? []).not.toContain("editable-label-wrapper"); // no second title box per row + expect(row.props.description).toBe(""); + }); + + it("does not rebuild the form on its own presentation writes, though each is announced; a structural change still does", () => { + // Every config write announces on formBindingChanged$ (the harness emits like the real service). + // A name, a hide flag or help text is already shown by the control that took it, and the eye is + // a button (the typing hold does not cover it), so a rebuild would replace the control mid-click + // and drop the focus. Expose/remove/reorder rebuild as before. + build(formViewWorkflow).ngOnInit(); + component.authoring = true; + const field = expose({ id: "n", operatorID: "op-1", propertyKey: "nested", displayName: "Nested" }); + const rebuild = vi.spyOn(component as any, "readConfig"); + const sub = field.fieldGroup[0]; + + sub.props.setFieldHidden(true); + sub.props.renameField("Other"); + field.props.renameField("Whole input"); + component.onEditHelpText(resolved("n", "N", {}), "help"); + + expect(h.formBindingService.setFieldOverride).toHaveBeenCalledTimes(2); + expect(h.formBindingService.updateBinding).toHaveBeenCalledTimes(2); + expect(rebuild).not.toHaveBeenCalled(); + + h.formBindingChanged.next(undefined); // e.g. the panel exposing a property + expect(rebuild).toHaveBeenCalledTimes(1); + }); + it("drops the schema's own descriptions on the field and its sub-fields", () => { build(formViewWorkflow).ngOnInit(); @@ -1128,6 +1285,17 @@ describe("WorkflowFormComponent", () => { expect(rebuild).toHaveBeenCalled(); }); + it("rebuilds the inputs when a step is renamed, so each card's attribution follows", () => { + // A rename in the live panel (or a co-editor's) reaches no other stream: no compilation, no + // config change. Without this the "From ..." line on the step's cards would keep the old name. + build(formViewWorkflow).ngOnInit(); + const rebuild = vi.spyOn(component as any, "readConfig"); + + h.displayNameChanged.next({}); + + expect(rebuild).toHaveBeenCalledTimes(1); + }); + it("holds a rebuild while someone is typing and runs it once the focus leaves", async () => { build(formViewWorkflow).ngOnInit(); const typing = vi.spyOn(component as any, "isTypingInTheForm").mockReturnValue(true); @@ -1150,7 +1318,7 @@ describe("WorkflowFormComponent", () => { build(formViewWorkflow).ngOnInit(); vi.spyOn(component as any, "isTypingInTheForm").mockReturnValue(true); const rebuild = vi.spyOn(component as any, "readConfig"); - workflowActionService.formBindingChanged$.next(undefined); + h.formBindingChanged.next(undefined); component.onFocusOut(); // tabbed to the next input: still typing when the check runs await new Promise(r => setTimeout(r, 10)); @@ -1175,11 +1343,11 @@ describe("WorkflowFormComponent", () => { build(formViewWorkflow).ngOnInit(); const typing = vi.spyOn(component as any, "isTypingInTheForm").mockReturnValue(true); const rebuild = vi.spyOn(component as any, "readConfig"); - workflowActionService.formBindingChanged$.next(undefined); // held + h.formBindingChanged.next(undefined); // held component.onFocusOut(); // queued typing.mockReturnValue(false); - workflowActionService.formBindingChanged$.next(undefined); // the tick box's own change: rebuilds now + h.formBindingChanged.next(undefined); // the tick box's own change: rebuilds now expect(rebuild).toHaveBeenCalledTimes(1); await new Promise(r => setTimeout(r, 10)); // the queued callback fires @@ -1190,7 +1358,7 @@ describe("WorkflowFormComponent", () => { build(formViewWorkflow).ngOnInit(); const before = formBindingService.resolveFields.mock.calls.length; - workflowActionService.formBindingChanged$.next(undefined); + h.formBindingChanged.next(undefined); expect(formBindingService.resolveFields.mock.calls.length).toBeGreaterThan(before); }); @@ -1203,7 +1371,7 @@ describe("WorkflowFormComponent", () => { const typing = vi.spyOn(component as any, "isTypingInTheForm").mockReturnValue(true); const rebuild = vi.spyOn(component as any, "readConfig"); - workflowActionService.formBindingChanged$.next(undefined); + h.formBindingChanged.next(undefined); expect(rebuild).not.toHaveBeenCalled(); typing.mockReturnValue(false); @@ -2052,6 +2220,49 @@ describe("WorkflowFormComponent", () => { expect(h.workflowActionService.disableWorkflowModification).toHaveBeenCalled(); }); + it("shows broken inputs to an author but never to a reader", () => { + build(formViewWorkflow).ngOnInit(); + (component as any).parameters = [resolved("b", "B", { brokenReason: "gone" })]; + + expect(component.visibleFields).toEqual([]); + component.authoring = true; + expect(component.visibleFields.length).toBe(1); + }); + + it("renders a broken input as an empty card carrying its reason", () => { + build(formViewWorkflow).ngOnInit(); + component.authoring = true; + h.formBindingService.resolveFields.mockReturnValue([resolved("b", "B", { brokenReason: "gone" })]); + + (component as any).readConfig(); + + expect(component.rendered[0].fields).toEqual([]); + expect(component.rendered[0].resolved.brokenReason).toBe("gone"); + }); + + it("keeps an input whose operator is gone, in either mode, for the author to remove explicitly", () => { + // Re-reading the config must not rewrite it. A broken input reaches the author as a card with + // its reason (the two tests above) and leaves only through onRemoveBinding; dropping it on the + // way in would be a silent config write and would hide where the input went. + build(formViewWorkflow).ngOnInit(); + (component as any).loading = false; + h.hasOperatorIds.add("op-1"); + h.formBindingService.getConfig.mockReturnValue({ + instruction: undefined, + fields: [ + { id: "gone", operatorID: "removed" }, + { id: "b", operatorID: "op-1" }, + ], + }); + + (component as any).readConfig(); + component.authoring = true; + (component as any).readConfig(); + + expect(h.formBindingService.setFields).not.toHaveBeenCalled(); + expect(h.formBindingService.removeBinding).not.toHaveBeenCalled(); + }); + it("lists the final steps and the viewed and chosen intermediate steps, with their shown state", () => { build(formViewWorkflow).ngOnInit(); component.authoring = true; @@ -2168,6 +2379,99 @@ describe("WorkflowFormComponent", () => { expect(read).not.toHaveBeenCalled(); }); + it("routes a removal through the binding service and re-reads", () => { + build(formViewWorkflow).ngOnInit(); + const read = vi.spyOn(component as any, "readConfig"); + + component.onRemoveBinding(resolved("n", "N", {})); + + expect(h.formBindingService.removeBinding).toHaveBeenCalledWith("n"); + expect(read).toHaveBeenCalledTimes(1); + }); + + it("saves help text without rebuilding the form (no readConfig on every keystroke)", () => { + build(formViewWorkflow).ngOnInit(); + const read = vi.spyOn(component as any, "readConfig"); + + component.onEditHelpText(resolved("n", "N", {}), "help"); + + expect(h.formBindingService.updateBinding).toHaveBeenCalledWith("n", { helpText: "help" }); + expect(read).not.toHaveBeenCalled(); + }); + + it("reorders the saved field the dragged card names, not the raw rendered index", () => { + build(formViewWorkflow).ngOnInit(); + // rendered is shorter than the saved fields: 'b' rendered no card (its schema was unavailable). + component.rendered = [{ resolved: { binding: { id: "a" } } }, { resolved: { binding: { id: "c" } } }] as any; + h.formBindingService.getConfig.mockReturnValue({ + instruction: undefined, + fields: [{ id: "a" }, { id: "b" }, { id: "c" }], + }); + + // Drag rendered[1] ("c", saved index 2) to the top (onto rendered[0] "a", saved index 0). + component.onDrop({ previousIndex: 1, currentIndex: 0 } as any); + + expect(h.formBindingService.reorder).toHaveBeenCalledWith(2, 0); + }); + + it("moves a card one place from the keyboard through the same reorder as the drag", () => { + build(formViewWorkflow).ngOnInit(); + // Three cards, but 'b' is not among the saved fields' neighbours in the same order (a saved + // field that rendered no card sits between), so the move has to resolve by id, as the drag does. + const cards = [ + { resolved: { binding: { id: "a" } } }, + { resolved: { binding: { id: "b" } } }, + { resolved: { binding: { id: "c" } } }, + ] as any; + component.rendered = cards; + h.formBindingService.getConfig.mockReturnValue({ + instruction: undefined, + fields: [{ id: "a" }, { id: "hidden" }, { id: "b" }, { id: "c" }], + }); + + component.onMoveBinding(cards[1], -1); // 'b' (saved 2) up onto 'a' (saved 0) + expect(h.formBindingService.reorder).toHaveBeenCalledWith(2, 0); + + // The move re-reads the config, which rebuilds `rendered` from the (mocked, empty) resolved + // fields; put the cards back to move again. + component.rendered = cards; + component.onMoveBinding(cards[1], 1); // 'b' (saved 2) down onto 'c' (saved 3) + expect(h.formBindingService.reorder).toHaveBeenCalledWith(2, 3); + }); + + it("moves nothing off either end", () => { + build(formViewWorkflow).ngOnInit(); + const cards = [{ resolved: { binding: { id: "a" } } }, { resolved: { binding: { id: "b" } } }] as any; + component.rendered = cards; + h.formBindingService.getConfig.mockReturnValue({ + instruction: undefined, + fields: [{ id: "a" }, { id: "b" }], + }); + + component.onMoveBinding(cards[0], -1); + component.onMoveBinding(cards[1], 1); + + expect(h.formBindingService.reorder).not.toHaveBeenCalled(); + }); + + it("drops a reorder whose card names a field the config no longer holds", () => { + build(formViewWorkflow).ngOnInit(); + // A card left over from a config that has since changed: its binding is gone from the saved + // fields, so neither end of the drag resolves. Reordering on those -1s would move the wrong + // field, so the drag is dropped instead. + component.rendered = [{ resolved: { binding: { id: "gone" } } }, { resolved: { binding: { id: "a" } } }] as any; + h.formBindingService.getConfig.mockReturnValue({ + instruction: undefined, + fields: [{ id: "a" }], + }); + const read = vi.spyOn(component as any, "readConfig"); + + component.onDrop({ previousIndex: 0, currentIndex: 1 } as any); + + expect(h.formBindingService.reorder).not.toHaveBeenCalled(); + expect(read).not.toHaveBeenCalled(); + }); + it("saves the instruction as the author types, and previews on demand", () => { build(formViewWorkflow).ngOnInit(); component.instructionTitle = "T"; @@ -2183,5 +2487,77 @@ describe("WorkflowFormComponent", () => { expect(component.instructionMode).toBe("preview"); expect(render).toHaveBeenCalled(); }); + + it("wires the editable title to rename the input and a sub-field to rename or hide it", () => { + build(formViewWorkflow).ngOnInit(); + component.authoring = true; + h.hasOperatorIds.add("op-1"); + h.formBindingService.resolveFields.mockReturnValue([ + resolved("n", "N", { + binding: { id: "n", operatorID: "op-1", propertyKey: "nested", displayName: "N", overrides: {} }, + }), + ]); + + (component as any).readConfig(); + const root = component.rendered[0].fields[0] as any; + const read = vi.spyOn(component as any, "readConfig"); + + // The input's own title renames the whole binding. + root.props.renameField("New name"); + expect(h.formBindingService.updateBinding).toHaveBeenCalledWith("n", { displayName: "New name" }); + + // A sub-field's editable label renames it and its eye hides it, both keyed by path. + const sub = root.fieldGroup[0]; + sub.props.renameField("Sub name"); + expect(h.formBindingService.setFieldOverride).toHaveBeenCalledWith("n", "sub", { displayName: "Sub name" }); + sub.props.setFieldHidden(true); + expect(h.formBindingService.setFieldOverride).toHaveBeenCalledWith("n", "sub", { hidden: true }); + + // Presentation only, shown by the wrapper itself: no rebuild, so the name box or the eye the + // author is on is not replaced under their focus. + expect(read).not.toHaveBeenCalled(); + }); + + it("hands the focus to the next card's Remove after a removal, else the previous one's", async () => { + build(formViewWorkflow).ngOnInit(); + const focused: string[] = []; + // Only the focus targets are answered; the name-width measuring the page also runs off the host + // gets null, as the harness gives it. + (component as any).host = { + nativeElement: { + contains: () => true, + querySelector: (selector: string) => + selector.startsWith(".remove[") ? { focus: () => focused.push(selector) } : null, + }, + }; + const cards = [ + { resolved: { binding: { id: "a" } } }, + { resolved: { binding: { id: "b" } } }, + { resolved: { binding: { id: "c" } } }, + ] as any; + + component.rendered = cards; + component.onRemoveBinding(cards[1].resolved); + await new Promise(r => setTimeout(r, 10)); + expect(h.formBindingService.removeBinding).toHaveBeenCalledWith("b"); + expect(focused).toEqual(['.remove[data-binding="c"]']); + + // The last card has no next: its predecessor takes the focus. + component.rendered = cards; + component.onRemoveBinding(cards[2].resolved); + await new Promise(r => setTimeout(r, 10)); + expect(focused[1]).toBe('.remove[data-binding="b"]'); + }); + + it("names a card's controls with the author's name, else the property key", () => { + build(formViewWorkflow).ngOnInit(); + + expect( + component.cardName({ resolved: { binding: { displayName: "File", propertyKey: "fileName" } } } as any) + ).toBe("File"); + expect(component.cardName({ resolved: { binding: { displayName: "", propertyKey: "fileName" } } } as any)).toBe( + "fileName" + ); + }); }); }); diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts index aa4e63942a..c297a829f0 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts @@ -34,7 +34,9 @@ import { MarkdownService } from "ngx-markdown"; import { asapScheduler, EMPTY, forkJoin, merge, Observable, Subject, timer } from "rxjs"; import { catchError, concatMap, debounceTime, finalize, observeOn, switchMap, takeUntil, tap } from "rxjs/operators"; +import { CdkDragDrop, DragDropModule } from "@angular/cdk/drag-drop"; import { USER_WORKFLOW, USER_WORKSPACE } from "../../../app-routing.constant"; +import { EditableLabelWrapperComponent } from "../../../common/formly/editable-label-wrapper/editable-label-wrapper.component"; import { FormFieldBinding, Workflow, WorkflowContent } from "../../../common/type/workflow"; import { ComputingUnitStatusService } from "../../../common/service/computing-unit/computing-unit-status/computing-unit-status.service"; import { ComputingUnitState } from "../../../common/type/computing-unit-connection.interface"; @@ -115,12 +117,11 @@ interface ResultChoice { * visualisation, or a compact "no result yet" -- reading the canvas's view-result set and never * writing it. A reader can also click a step on the embedded preview to open its property panel * read-only: the panel writes nothing to the shared workflow and its content is inert. With write - * access, an Edit toggle turns the page into in-place authoring: write the instruction, pick which - * extra results to feature, and click a step to open its panel live -- its writes turn on and its - * tick boxes choose what the form exposes. Editing the exposed inputs themselves in place (rename, - * hide a sub-field, reorder, remove) is the next PR. A view, not a new object: every graph edit goes - * through the same shared graph the operator canvas edits, and the form-binding config is local - * until #8351 shares it. + * access, an Edit toggle turns the page into in-place authoring: rename an input or its sub-fields, + * hide a sub-field, reorder inputs by drag, expose a new one by clicking a step (the panel goes + * live and its writes turn on), remove one, write the instruction, and pick which extra results to + * feature. A view, not a new object: every graph edit goes through the same shared graph the + * operator canvas edits, and the form-binding config is local until #8351 shares it. */ @UntilDestroy() @Component({ @@ -132,6 +133,7 @@ interface ResultChoice { FormsModule, ReactiveFormsModule, FormlyModule, + DragDropModule, NzAvatarModule, NzIconModule, NzButtonModule, @@ -154,8 +156,8 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { public autoSaveState = ""; /** Write access: only then does a filled-in value write back, and only then does the page save. */ public canEdit = false; - /** Edit mode: a writer authoring the form in place (write the instruction, pick results, open a - * step's panel live to expose settings). Off, the page is the read-only form a reader sees. */ + /** Edit mode: a writer authoring the form in place (rename/hide/reorder/expose/remove inputs, + * edit the instruction, pick results). Off, the page is the read-only form a reader sees. */ public authoring = false; /** While authoring, the instruction is edited as raw markdown ("write") or shown rendered * ("preview"); a reader always sees it rendered. */ @@ -244,6 +246,13 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { private dirtySinceLastEnqueue = false; /** A rebuild of the inputs that arrived while the reader was typing, held until the typing ends. */ private rebuildDeferred = false; + /** Set while this page makes a presentation write, so the write's own announcement does not rebuild + * the form under the control that took it (see reflectLocally). */ + private reflectingLocally = false; + /** The later rows of a repeated section, per input and sub-field path (followerKey): they show the + * path's name and hidden state statically and follow the first row's controls, so the rows agree + * without a rebuild. Rebuilt with the form (applyFieldOverrides). */ + private followersByPath = new Map<string, FormlyFieldConfig[]>(); /** * Operator positions as loaded, kept only as a fallback: a save writes the live positions @@ -493,7 +502,22 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { // a co-editor's -- so, like the compilation path, the rebuild is held while the reader is typing // (a remote change would otherwise throw away a half-entered value under the cursor) and runs // the moment the typing ends. - this.workflowActionService.formBindingChanged$ + this.workflowActionService.formBindingChanged$.pipe(untilDestroyed(this)).subscribe(() => { + // A presentation write this page just made (see reflectLocally) is already shown by the + // control that took it; rebuilding on its own announcement would replace that control + // mid-click and drop the focus. The announcement still reaches the autosave. + if (this.reflectingLocally) { + return; + } + this.rebuildFormOrDefer(true); + }); + + // A step renamed (in the live panel, or by a co-editor) changes the "From ..." attribution on + // every card that belongs to it, and nothing else emits for a rename: no compilation, no config + // change. Rebuild the inputs from it too, held while the reader is typing like the other two. + this.workflowActionService + .getTexeraGraph() + .getOperatorDisplayNameChangedStream() .pipe(untilDestroyed(this)) .subscribe(() => this.rebuildFormOrDefer(true)); } @@ -698,7 +722,13 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { })); } - /** Re-read the saved form config and rebuild everything derived from it. */ + /** + * Re-read the saved form config and rebuild everything derived from it. An input whose operator + * has since been deleted is NOT dropped here: resolveFields marks it broken, a reader never sees + * it (visibleFields), and an author sees it as an empty card with the reason and removes it + * explicitly. Deleting it silently on entering edit mode would be a config write nobody asked for, + * and would leave the author guessing where an input went. + */ private readConfig(): void { const config = this.formBindingService.getConfig(); this.parameters = this.formBindingService.resolveFields(); @@ -772,6 +802,12 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { private renderField(resolved: ResolvedField): RenderedField | undefined { const { binding } = resolved; + // A broken input (its operator gone) has no schema to build a field from. Only an author ever + // sees it (visibleFields drops it for readers), rendered as an empty card so the author can + // remove it; a reader never reaches here for one. + if (resolved.brokenReason) { + return { resolved, fields: [], form: new FormGroup({}), model: {} }; + } const schema = this.operatorSchemaFor(binding.operatorID); if (!schema) { return undefined; @@ -858,7 +894,7 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { field.props = { ...(field.props ?? {}), disabled: true }; } - this.applyFieldOverrides(field, binding); + this.applyFieldOverrides(field, binding, schemaLabel); return { resolved, fields: [field], form, model }; } @@ -901,20 +937,94 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { * (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. + * + * One set of controls per path: a repeated section's rows all share one override, and a name box + * and an eye on every row would be that many copies of one control, none following the others. + * The first row this walk meets for a path carries the controls; later rows show the same name + * and hidden state statically and follow the controls' edits (followersByPath), so the rows agree + * without a rebuild. A scalar array's rows are walked as rows, never as the input's root, so the + * input's own title box appears once, above them. */ - private applyFieldOverrides(field: FormlyFieldConfig, binding: FormFieldBinding): void { - const walk = (node: FormlyFieldConfig, path: string): void => { + private applyFieldOverrides(field: FormlyFieldConfig, binding: FormFieldBinding, schemaLabel: string): void { + for (const key of [...this.followersByPath.keys()]) { + if (key.startsWith(binding.id + "/")) { + this.followersByPath.delete(key); + } + } + const controlsAt = new Set<string>(); + const walk = (node: FormlyFieldConfig, path: string, root: boolean): 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: "" }; + if (root && this.authoring) { + // Author mode, the input itself: its name is renamed in place by clicking the title, like + // every nested field. No eye here -- a whole input leaves via Remove, not a hide toggle. The + // editable label becomes the single title, so formly's own label is cleared to avoid printing + // it twice. A repeated input has no labelable control of its own, so its title names it as a + // group instead. + EditableLabelWrapperComponent.decorate( + node, + { + authoring: true, + name: binding.displayName ?? "", + hidden: false, + fallback: schemaLabel, + canHide: false, + group: node.type === "array", + }, + name => this.onBindingNamed(binding.id, name) + ); + node.props = { ...(node.props ?? {}), label: "" }; + } else if (root && node.type === "array") { + // Reader mode, a repeated input: the shared array widget prints its label at the BOTTOM, + // beside its add button (the canvas panel's convention), while every other widget and the + // author's editable title sit above. Left alone, the title would jump from above the rows + // in edit mode to below them on Done. Give it the same static title above instead; the + // wrapper blanks the widget's own label and names the rows as a group. + EditableLabelWrapperComponent.decorate(node, { + authoring: false, + name: binding.displayName ?? "", + hidden: false, + fallback: schemaLabel, + canHide: false, + group: true, + }); + } // 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] ?? {}; + // The schema's own label, read BEFORE the override replaces it: it is the name box's + // placeholder and tooltip, and what clearing the box yields, since an empty name deletes the + // override (setFieldOverride). + const schemaOwnLabel = (node.props?.label as string) || path; if (override.displayName) { node.props = { ...(node.props ?? {}), label: override.displayName }; } - if (override.hidden) { + if (this.authoring) { + // An author edits the sub-field's label where it appears and keeps hidden fields on + // screen (faded, via the wrapper) so they can be brought back, rather than removed from + // the DOM as they are for a reader. The first node met for a path carries the controls; + // any later row of a repeated section shows the same state and follows the controls. + const carriesControls = !controlsAt.has(path); + controlsAt.add(path); + EditableLabelWrapperComponent.decorate( + node, + { + authoring: carriesControls, + name: override.displayName ?? "", + hidden: override.hidden === true, + fallback: schemaOwnLabel, + group: node.type === "array", + }, + carriesControls ? name => this.onSubFieldNamed(binding.id, path, name) : undefined, + carriesControls ? hidden => this.onSubFieldHiddenAt(binding.id, path, hidden) : undefined + ); + if (!carriesControls) { + const key = WorkflowFormComponent.followerKey(binding.id, path); + this.followersByPath.set(key, [...(this.followersByPath.get(key) ?? []), node]); + } + } else if (override.hidden) { node.hide = true; // Hidden means "not shown", not "cleared". Formly 7's resetFieldOnHide extra defaults to // true, so a field that renders hidden has its value stripped from the model -- and this @@ -939,8 +1049,9 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { const children = row.fieldGroup ?? []; if (children.length === 0) { // A scalar array (a list of strings): the builder returns a leaf row with no sub-fields, - // so decorate the row itself, mirroring the leaf case of the non-function branch below. - walk(row, path); + // so decorate the row itself -- as a row, not as the input's root, or the input's title + // box would appear on every row under the one already at the top. + walk(row, path, false); } else { // An object row: not walked as a root (that reprints the array's group title), but its // own schema description (the items.description) still renders once per row via the @@ -949,7 +1060,7 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { row.props = { ...(row.props ?? {}), description: "" }; } for (const child of children) { - walk(child, WorkflowFormComponent.childPath(path, child.key)); + walk(child, WorkflowFormComponent.childPath(path, child.key), false); } return row; }; @@ -958,12 +1069,13 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { const arrayItem = WorkflowFormComponent.arrayItemOf(node); const children = node.fieldGroup ?? arrayItem?.fieldGroup ?? []; for (const child of children) { - walk(child, WorkflowFormComponent.childPath(path, child.key)); + walk(child, WorkflowFormComponent.childPath(path, child.key), false); } // A scalar array (e.g. a list of strings) has a row template with no sub-fields of its own; - // decorate it directly so its schema description is dropped like every other field's. + // decorate it directly (as a row, see above) so its schema description is dropped like every + // other field's. if (arrayItem && !arrayItem.fieldGroup) { - walk(arrayItem, path); + walk(arrayItem, path, false); } else if (arrayItem) { // A static object-array template: its sub-fields are walked above, but the template // container's own items.description still renders once per row, so drop just that (not @@ -971,7 +1083,12 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { arrayItem.props = { ...(arrayItem.props ?? {}), description: "" }; } }; - walk(field, ""); + walk(field, "", true); + } + + /** Followers are kept per input and path; "/" cannot occur in a binding id (a uuid). */ + private static followerKey(bindingId: string, path: string): string { + return bindingId + "/" + path; } private operatorSchemaFor(operatorID: string): object | undefined { @@ -994,11 +1111,13 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { /** * 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 remove them, comes with the input-authoring PR. + * no longer exists) are left out, since filling one in could not affect a run; an author sees them + * (below), to repair or remove them. */ public get visibleFields(): ResolvedField[] { - return this.parameters.filter(field => !field.brokenReason); + // A reader never sees a broken input (its operator is gone, so filling it could not affect the + // run); an author sees it, to repair or remove it. + return this.authoring ? this.parameters : this.parameters.filter(field => !field.brokenReason); } public trackByRendered(_: number, rendered: RenderedField): string { @@ -1197,6 +1316,83 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { this.readConfig(); } + /** + * Only presentation is editable here (the input's shown name and its help text). Which operator + * property an input drives is decided by ticking it in the property panel, so there is nothing to + * type and no way to point an input at a property that does not exist. + */ + public onEditHelpText(resolved: ResolvedField, value: string): void { + // Help text is presentation only and does not change which inputs the form has, so it is NOT + // followed by readConfig: rebuilding the whole form on every keystroke would churn every card + // (heavy file/model widgets included) and jump the cursor. Like the instruction, it is saved to + // the config and reflected on the next full re-read. (A binding's shown name is edited through + // the editable title, not here -- see onBindingNamed.) + this.reflectLocally(() => this.formBindingService.updateBinding(resolved.binding.id, { helpText: value })); + } + + /** + * Take an input off the form. The card goes with it, so the keyboard focus that was on its Remove + * button is handed to the next card's Remove (else the previous card's, else the Inputs heading) + * once the list has rebuilt; dropped focus would send a keyboard author back to the top of the page. + */ + public onRemoveBinding(resolved: ResolvedField): void { + const at = this.rendered.findIndex(card => card.resolved.binding.id === resolved.binding.id); + const neighbour = this.rendered[at + 1] ?? this.rendered[at - 1]; + // Re-read once, here (the write's own announcement would rebuild a second time), so the focus + // hand-off below lands on the one rebuilt list. + this.reflectLocally(() => this.formBindingService.removeBinding(resolved.binding.id)); + this.readConfig(); + this.later(() => this.focusAfterRemoval(neighbour?.resolved.binding.id), 0); + } + + private focusAfterRemoval(neighbourId: string | undefined): void { + const host: HTMLElement = this.host.nativeElement; + const target = + (neighbourId ? host.querySelector<HTMLElement>(`.remove[data-binding="${neighbourId}"]`) : null) ?? + host.querySelector<HTMLElement>(".pc-section-head .label"); + target?.focus(); + } + + /** The name a card's controls are announced with: the author's name for the input, else its key. */ + public cardName(card: RenderedField): string { + return card.resolved.binding.displayName || card.resolved.binding.propertyKey; + } + + public onDrop(event: CdkDragDrop<unknown>): void { + this.moveRenderedCard(event.previousIndex, event.currentIndex); + } + + /** + * Keyboard counterpart of the drag: the Move up / Move down buttons on an author's card step it + * one place. CDK drag-drop offers no keyboard path of its own and the drag handle is decorative, + * so without these a keyboard-only author could not reorder at all. + */ + public onMoveBinding(card: RenderedField, delta: -1 | 1): void { + const at = this.rendered.indexOf(card); + this.moveRenderedCard(at, at + delta); + } + + /** + * Move the card at one rendered position onto another. The positions are indices into `rendered`, + * which can be shorter than the saved fields (a binding whose operator is live but whose schema is + * momentarily unavailable renders no card), so reordering the saved fields by those raw indices + * could move the wrong one. Translate both ends to the saved field they name, by binding id, and + * reorder those. A position off either end, or a card the config no longer holds, moves nothing. + */ + private moveRenderedCard(fromIndex: number, toIndex: number): void { + const fields = this.formBindingService.getConfig().fields; + const movedId = this.rendered[fromIndex]?.resolved.binding.id; + const targetId = this.rendered[toIndex]?.resolved.binding.id; + const from = fields.findIndex(f => f.id === movedId); + const to = fields.findIndex(f => f.id === targetId); + if (from === -1 || to === -1) { + return; + } + // Re-read once, here: the write's own announcement would rebuild the form a second time. + this.reflectLocally(() => this.formBindingService.reorder(from, to)); + this.readConfig(); + } + public onInstructionChange(): void { this.formBindingService.updateConfig({ instruction: { title: this.instructionTitle, body: this.instructionBody }, @@ -1218,7 +1414,10 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { */ public onToggleResult(choice: ResultChoice): void { if (this.authoring && this.canEdit) { - this.formBindingService.toggleShownResult(choice.operatorID, this.terminalOperatorIds()); + // Re-read once, here: the write's own announcement would rebuild the form a second time. + this.reflectLocally(() => + this.formBindingService.toggleShownResult(choice.operatorID, this.terminalOperatorIds()) + ); this.readConfig(); return; } @@ -1241,6 +1440,51 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { } } + /** + * Renaming the input itself, from its own title. Like the help text, a name or a hide flag is + * presentation only: the wrapper that took the edit already shows it, so the form is NOT rebuilt + * here. A rebuild would replace the very control the author is on (the name box, the eye) and drop + * the keyboard focus with it; the stored override is applied on the next full re-read (Done). + */ + private onBindingNamed(bindingId: string, value: string): void { + this.reflectLocally(() => this.formBindingService.updateBinding(bindingId, { displayName: value })); + } + + private onSubFieldNamed(bindingId: string, path: string, value: string): void { + this.reflectLocally(() => this.formBindingService.setFieldOverride(bindingId, path, { displayName: value })); + // The later rows of a repeated section show this name statically; keep them in step with the + // box on the first row, so the rows agree without a rebuild. + for (const node of this.followersByPath.get(WorkflowFormComponent.followerKey(bindingId, path)) ?? []) { + node.props = { ...(node.props ?? {}), authorName: value }; + } + } + + private onSubFieldHiddenAt(bindingId: string, path: string, hidden: boolean): void { + this.reflectLocally(() => this.formBindingService.setFieldOverride(bindingId, path, { hidden })); + for (const node of this.followersByPath.get(WorkflowFormComponent.followerKey(bindingId, path)) ?? []) { + node.props = { ...(node.props ?? {}), authorHidden: hidden }; + } + } + + /** + * Make a presentation write (a name, a hide flag, help text, a result pick) without the rebuild + * its own announcement would trigger. The control that took the edit already shows it, and the + * announcement (formBindingChanged$) would otherwise rebuild the whole form synchronously inside + * the click -- the eye is a button, so the typing hold does not cover it -- replacing that control, + * dropping the keyboard focus and reverting a value typed elsewhere within its write debounce. The + * announcement still reaches the autosave; only this page's own rebuild is skipped. Structural + * writes (expose, remove, reorder) rebuild as before, through the announcement or through their + * caller's own re-read. + */ + private reflectLocally(write: () => void): void { + this.reflectingLocally = true; + try { + write(); + } finally { + this.reflectingLocally = false; + } + } + // --------------------------------------------------------------------------- // Running the same workflow the canvas runs, through the same execute/kill service. The canvas // wraps its run with completion-email options (executeWorkflowWithEmailNotification); this page diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts index 06e5605c3d..fba4071a25 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts @@ -18,6 +18,7 @@ */ import { DatePipe } from "@angular/common"; +import { CdkDropList } from "@angular/cdk/drag-drop"; import { FormGroup } from "@angular/forms"; import { ComponentFixture, TestBed } from "@angular/core/testing"; import { By } from "@angular/platform-browser"; @@ -36,6 +37,7 @@ import { LockOutline, MinusOutline, PlusOutline, + UpOutline, } from "@ant-design/icons-angular/icons"; import { EMPTY, of, Subject } from "rxjs"; @@ -199,6 +201,7 @@ describe("WorkflowFormComponent (rendered template)", () => { // Author-mode writes the rendered controls reach. updateConfig: vi.fn(), toggleShownResult: vi.fn(), + removeBinding: vi.fn(), }, }, { provide: FormlyJsonschema, useValue: { toFieldConfig: () => ({ fieldGroup: [] }) } }, @@ -269,6 +272,7 @@ describe("WorkflowFormComponent (rendered template)", () => { LockOutline, MinusOutline, PlusOutline, + UpOutline, ], }, DatePipe, @@ -558,6 +562,155 @@ describe("WorkflowFormComponent (rendered template)", () => { expect(after[0].getAttribute("aria-pressed")).toBe("true"); }); + it("offers Move up / Move down on an author's cards, named with the input and inert at the ends", () => { + fixture.detectChanges(); + finishLoad(); + const c = fixture.componentInstance; + c.canEdit = true; + c.authoring = true; + (c as any).parameters = [{ binding: { id: "b1" } }, { binding: { id: "b2" } }]; + c.rendered = [ + { + resolved: { binding: { id: "b1", displayName: "File", propertyKey: "fileName" }, operatorLabel: "Scan" }, + fields: [], + form: new FormGroup({}), + model: {}, + }, + { + resolved: { binding: { id: "b2", propertyKey: "predicate" }, operatorLabel: "Filter" }, + fields: [], + form: new FormGroup({}), + model: {}, + }, + ] as any; + const move = vi.spyOn(c, "onMoveBinding"); + fixture.detectChanges(); + + const buttons = Array.from(fixture.nativeElement.querySelectorAll(".param .move")) as HTMLButtonElement[]; + // Every card has the same two buttons, so each is named with its input (the author's name, else + // the property key). + expect(buttons.map(b => b.getAttribute("aria-label"))).toEqual([ + "Move File up", + "Move File down", + "Move predicate up", + "Move predicate down", + ]); + // First card cannot move up, last card cannot move down: marked inert for assistive tech, but + // NOT disabled, so the button that has the focus after a move to the end keeps it. + expect(buttons.map(b => b.getAttribute("aria-disabled"))).toEqual(["true", null, null, "true"]); + expect(buttons.map(b => b.disabled)).toEqual([false, false, false, false]); + + // Each button carries its own direction: the first card's Move down, the second card's Move up. + buttons[1].click(); + expect(move).toHaveBeenCalledWith(c.rendered[0], 1); + buttons[2].click(); + expect(move).toHaveBeenCalledWith(c.rendered[1], -1); + // An inert end button still takes the click; the handler moves nothing off the end. + buttons[0].focus(); + buttons[0].click(); + expect(move).toHaveBeenCalledWith(c.rendered[0], -1); + expect(document.activeElement).toBe(buttons[0]); + }); + + it("hands the focus to the Inputs heading when the last card is removed", async () => { + fixture.detectChanges(); + finishLoad(); + const c = fixture.componentInstance; + c.canEdit = true; + c.authoring = true; + c.rendered = [ + { + resolved: { binding: { id: "b1", displayName: "File", propertyKey: "fileName" }, operatorLabel: "Scan" }, + fields: [], + form: new FormGroup({}), + model: {}, + }, + ] as any; + fixture.detectChanges(); + + const remove = el(".param .remove") as HTMLButtonElement; + expect(remove.getAttribute("aria-label")).toBe("Remove File"); + expect(remove.getAttribute("data-binding")).toBe("b1"); + remove.focus(); + // The real handler runs: the binding is removed and the re-read (resolveFields -> []) takes the + // card away, so there is no neighbour to hand the focus to. + remove.click(); + fixture.detectChanges(); + await new Promise(r => setTimeout(r, 10)); + + expect(el(".param")).toBeNull(); + expect(document.activeElement).toBe(el(".pc-section-head .label")); + }); + + it("gives an author's card its provenance, drag handle, help-text box and Remove, and drops the reader's help line", () => { + fixture.detectChanges(); + finishLoad(); + const c = fixture.componentInstance; + c.canEdit = true; + c.authoring = true; + c.rendered = [ + { + resolved: { binding: { id: "b1", helpText: "Pick a file", propertyKey: "fileName" }, operatorLabel: "Scan" }, + fields: [], + form: new FormGroup({}), + model: {}, + }, + ] as any; + const help = vi.spyOn(c, "onEditHelpText").mockImplementation(() => {}); + const remove = vi.spyOn(c, "onRemoveBinding").mockImplementation(() => {}); + fixture.detectChanges(); + + expect(el(".param .grip")).not.toBeNull(); + expect(el(".param .field-help")?.textContent?.trim()).toBe("From Scan"); + // While authoring, the help text is edited in its own box rather than shown as the reader's line. + expect(el(".param .param-help-text")).toBeNull(); + const box = el(".param .edit input") as HTMLInputElement; + expect(box.value).toBe("Pick a file"); + box.value = "Pick a CSV"; + box.dispatchEvent(new Event("input")); + expect(help).toHaveBeenCalledWith(c.rendered[0].resolved, "Pick a CSV"); + expect(el(".param .edit-foot .hint")?.textContent).toContain("Set on Scan: fileName"); + (el(".param .remove") as HTMLButtonElement).click(); + expect(remove).toHaveBeenCalledWith(c.rendered[0].resolved); + }); + + it("renders a broken input as its reason plus Remove: no field, no help box, no provenance", () => { + fixture.detectChanges(); + finishLoad(); + const c = fixture.componentInstance; + c.canEdit = true; + c.authoring = true; + c.rendered = [ + { + resolved: { binding: { id: "gone" }, operatorLabel: "gone-op", brokenReason: "This step was removed." }, + fields: [], + form: new FormGroup({}), + model: {}, + }, + ] as any; + fixture.detectChanges(); + + expect(el(".param .broken")?.textContent?.trim()).toBe("This step was removed."); + expect(el(".param form")).toBeNull(); + expect(el(".param .field-help")).toBeNull(); + expect(el(".param .edit input")).toBeNull(); + expect(el(".param .edit-foot .hint")).toBeNull(); + expect(el(".param .remove")).not.toBeNull(); + }); + + it("hands a drop on the card list to onDrop", () => { + fixture.detectChanges(); + finishLoad(); + const c = fixture.componentInstance; + const drop = vi.spyOn(c, "onDrop").mockImplementation(() => {}); + + fixture.debugElement + .query(By.directive(CdkDropList)) + .triggerEventHandler("cdkDropListDropped", { previousIndex: 1, currentIndex: 0 }); + + expect(drop).toHaveBeenCalledWith({ previousIndex: 1, currentIndex: 0 }); + }); + it("renders the run bar with the run button and the computing-unit selector", () => { fixture.detectChanges(); finishLoad(); diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts index cfa68e37b8..dc5cb7609c 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts @@ -53,6 +53,8 @@ export const resolved = (id: string, displayName: string, extra: Partial<Resolve export function setupHarness() { const router = { navigate: vi.fn() }; const workflowChangedStream = new Subject<unknown>(); + // Announces every form-config write (see formBindingChanged$ and the form-binding mock below). + const formBindingChanged = new Subject<unknown>(); // The root-level modification lock as other writers flip it (the execute service after a run, the // computing-unit selector); tests emit `true` to stand in for one of them unlocking the graph. const modificationEnabled = new Subject<boolean>(); @@ -165,8 +167,10 @@ export function setupHarness() { getCurrentHighlightedOperatorIDs: () => highlightedIds, unhighlightOperators, }), - // Exposing or un-exposing a property announces on this stream; the form re-reads its config. - formBindingChanged$: new Subject<unknown>(), + // Every config write announces on this stream (setFormBinding emits it); the form re-reads its + // config on it unless the write is one of its own presentation edits. The form-binding mock's + // writers below emit here, as the real service does, so that chain is under test. + formBindingChanged$: formBindingChanged.asObservable(), }; // Resolves the exposed inputs and reads/writes their values. Tests point `resolveFields` at the // inputs they want rendered; `readValue` seeds the write-back guard. @@ -179,10 +183,17 @@ export function setupHarness() { writeValue: vi.fn(), // A result card's friendly label; the mock returns the operator's display name or its id. operatorLabel: (op: any) => op?.customDisplayName ?? op?.operatorType ?? op?.operatorID, - // Author-mode writes: the component calls these then re-reads config. Spied so a test can - // assert the edit was made without needing a real binding store. - toggleShownResult: vi.fn(), - updateConfig: vi.fn(), + // Author-mode writes. Spied so a test can assert the edit was made without needing a real + // binding store, and each announces on formBindingChanged$ as the real service does (every + // write goes through setFormBinding, which emits), so the page's reaction to its own writes -- + // rebuild, or not, for a presentation edit -- is what the tests see. + updateBinding: vi.fn(() => formBindingChanged.next(undefined)), + setFieldOverride: vi.fn(() => formBindingChanged.next(undefined)), + removeBinding: vi.fn(() => formBindingChanged.next(undefined)), + reorder: vi.fn(() => formBindingChanged.next(undefined)), + toggleShownResult: vi.fn(() => formBindingChanged.next(undefined)), + updateConfig: vi.fn(() => formBindingChanged.next(undefined)), + setFields: vi.fn(), }; // A field per property the tests expose. Real formly json-schema conversion is exercised by the // property panel's own spec; here a deterministic map keeps these tests about the component's @@ -333,6 +344,7 @@ export function setupHarness() { datePipe, config, workflowChangedStream, + formBindingChanged, workflowMetaDataChangedStream, compilationChanged, executionStateStream,
