feat(gui): author the Form View's inputs in place - #8517
Merged
Conversation
This was referenced Sep 11, 2026
Contributor
Automated Reviewer SuggestionsBased on the
|
yangzhang75
force-pushed
the
formview-pr15b
branch
from
September 11, 2026 18:56
afcf147 to
f72d4f6
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8517 +/- ##
============================================
- Coverage 95.90% 95.89% -0.01%
Complexity 459 459
============================================
Files 586 588 +2
Lines 33635 33802 +167
Branches 4182 4217 +35
============================================
+ Hits 32257 32415 +158
Misses 811 811
- Partials 567 576 +9
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
yangzhang75
force-pushed
the
formview-pr15b
branch
11 times, most recently
from
September 12, 2026 00:49
ab0c176 to
b405661
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Array labels, fallback naming, stale operator names, and group accessibility require correction.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds in-place Form View authoring for input labels, visibility, ordering, help text, result selection, and edit-mode safeguards.
Changes:
- Adds editable Formly labels and authoring controls.
- Adds result selection, ordered persistence, and graph locking.
- Expands accessibility and unit-test coverage.
File summaries
| File | Description |
|---|---|
workflow-action.service.ts |
Persists result selections. |
workflow-action.service.spec.ts |
Tests binding persistence. |
joint-ui.service.ts |
Hides locked-preview actions. |
joint-ui.service.spec.ts |
Tests hidden actions. |
form-binding.service.ts |
Manages shown results. |
form-binding.service.spec.ts |
Tests result toggling. |
workflow-form.spec-harness.ts |
Expands test stubs. |
workflow-form.rendered.spec.ts |
Tests rendered authoring UI. |
workflow-form.component.ts |
Implements authoring behavior. |
workflow-form.component.spec.ts |
Tests authoring and persistence. |
workflow-form.component.scss |
Styles authoring controls. |
workflow-form.component.html |
Renders authoring UI. |
workflow-editor.component.ts |
Enforces preview restrictions. |
workflow-editor.component.spec.ts |
Tests locked previews. |
workflow-editor.component.html |
Passes structure lock. |
context-menu.component.ts |
Adds structural-action gating. |
context-menu.component.spec.ts |
Tests locked menu actions. |
context-menu.component.html |
Applies menu guards. |
property-editor.component.ts |
Preserves teardown behavior. |
menu.component.ts |
Uses injectable file saving. |
menu.component.spec.ts |
Updates export tests. |
workflow.ts |
Defines result-selection state. |
formly-config.ts |
Registers editable wrapper. |
editable-label-wrapper.component.ts |
Implements label editing. |
editable-label-wrapper.component.spec.ts |
Tests wrapper behavior. |
editable-label-wrapper.component.scss |
Styles editable labels. |
editable-label-wrapper.component.html |
Renders labels and visibility toggle. |
Review details
Suppressed comments (3)
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:922
- When a sub-field already has a custom name,
node.props.labelis overwritten with that name before it is captured as the fallback. Clearing the name therefore leaves the old custom name as the placeholder/programmatic label until a rebuild, instead of immediately falling back to the schema label. Capture the schema label before applying the override.
fallback: (node.props?.label as string) || path,
frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.html:68
- This wrapper is used for repeated inputs, but
ArrayTypeComponentrenders no element with the Formly fieldid; therefore thisforattribute is dangling and the repeated rows are not programmatically associated with their visible title. The authoring hidden label has the same assumption for array/object fields. Group fields need group semantics such as afieldset/legend(or anaria-labelledbygroup), while scalar fields can retainlabel[for].
<label
class="lbl-static"
*ngIf="!props.authoring && (props.authorName || props.schemaLabel)"
[attr.for]="id">
{{ props.authorName || props.schemaLabel }}
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:884
- This
!pathcheck also matches scalar-array row templates: the existingwalk(row, path)calls pass the root array's empty path into every generated item. In author mode, each scalar row therefore receives another editable binding title wired to rename the whole array, duplicating the title above the array. Distinguish the actual root node from an item template (or only clear item descriptions without recursively applying root decoration).
if (!path && this.authoring) {
- Files reviewed: 27/27 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
yangzhang75
force-pushed
the
formview-pr15b
branch
2 times, most recently
from
September 12, 2026 05:40
df28263 to
431acf7
Compare
renovate-bot
pushed a commit
to renovate-bot/apache-_-texera
that referenced
this pull request
Sep 12, 2026
…uthoring (apache#8516) ### What changes were proposed in this PR? Closes apache#8026. Part of the Form View stack (parent issue apache#8011), on main. This is the first half of what was apache#8455 (closed, split in two at 2200 lines); the second half, authoring the inputs themselves, is apache#8517 stacked on this one. Four commits: the first is the feature, the second and third are test-only follow-ups (the Windows CI leg and the template coverage report), and the last opens the result picker to everyone with one saved list of shown results (the design changes asked for in review, mengw15's list shape included). The review commit is the branch tip. Adds the edit mode of the page and the page-level authoring it enables. - `toggleAuthoring`: with write access, Edit turns the page into in-place authoring; Done returns it to the reader state. Edit mode is the only state that enables workflow modification here (`applyEditability`), and entering it is refused without write access at the method itself, not only by hiding the button. - The inspect panel goes live in edit mode: it acts as an editor (`actsAsEditor`), its expose tick boxes are on (`exposeChoosing`), and `inert` comes off, so the author opens a step on the embedded preview and chooses which of its settings people fill in right there. It stays inert and read-only for everyone else, as before. - The embedded preview's right-click menu carries the structure lock. The menu's re-shaping commands followed the modification flag alone, so re-enabling it for the panel would have offered cut, paste, delete and disable on the structure-locked preview. `texera-context-menu` now takes `[structureLocked]` from the editor and gates those on modification AND no lock (`canModify`); copy, the result toggles, execute-to and export do not re-shape the graph and are unchanged. - Write the author's instruction in place: the heading is edited in the header (a row rather than one button, so the input is not nested in a control) and the body as markdown with Write / Preview tabs. A reader still sees it rendered, only when there is text. - Pick which results to show, for everyone: the picker lists the final steps and the intermediate steps that have view-result on the canvas, and rebuilds live when an eye is toggled or the graph's shape or names change (a step deleted, disabled, given a downstream link, or renamed, a co-editor's edit included), from the graph's own streams rather than only after the debounced compilation. What shows is one saved list, `shownResultIds` (the shape mengw15 proposed): absent, every final step shows, as on the canvas; once the author has chosen, exactly the listed steps show, and `[]` means none, a choice the earlier pair of opposite-signed lists (featured / turned off) could not store and could contradict each other on. In edit mode a pill adds the step to or removes it from that list; the first choice starts the list from the final steps at that moment, so nothing the author saw disappears. The accepted cost: a step that becomes final after the author has chosen does not appear by itself. A definition whose only content is that list, an empty one included, is still saved. A listed step whose eye has since been turned off stays offered in edit mode only, so the author can take it off the list; a reader is not offered a pill that could never turn on. Anyone else, a writer merely viewing included, gets the same picker but changes only their own view of the page; nothing is written, so a reader without write access can choose too, and the choice lasts for the page. A disabled step is neither offered nor shown, eye or no eye: the compiled plan leaves it out. - Edit mode is the only state that unlocks the graph from this page, and only while no run is in flight (the canvas rule, kept so that entering edit mode mid-run cannot undo it). The lock is a root-level flag with writers that know nothing of this page: the execute service unlocks it whenever a run ends (completed, failed, killed, reset), and the computing-unit selector embedded here unlocks it when it finds no run on the chosen unit. Rather than chase each caller, the page clamps at the stream they all report to (`getWorkflowModificationEnabledStream`): whenever the flag turns on while the page must stay locked, it is turned off again, so a writer merely viewing can never reach the preview's view-result command. In edit mode the canvas rule stands: locked while running, unlocked when the run ends. The clamp runs a microtask after the unlocking call, never inside it: `enableWorkflowModification` enables undo/redo after it emits, and the stream still has other subscribers to reach, so a nested disable would leave them on the stale "true"; run afterwards, the disable is the last word and every consumer sees one locked state. The execute service flips the lock before it emits the new state; by the time the clamp looks, the execution-state handler has re-applied the same rule with the final state, so in edit mode the unlock stands. The step panel follows the same rule (`panelLive`), not edit mode alone: the property frame does not consult the lock before its own writes (the version sync on mount, the schema defaults ajv fills in, the editing marker), so a step selected while a run is in flight stays a read-only, inert mount even in edit mode and turns live when the run ends. Done dismisses the step panel before leaving edit mode, while the frame is still an editor: that is the only state in which the property editor clears the "currently editing" marker co-editors see. - The embedded preview grows no editing buttons: no link tools on hover (remove, breakpoint), and a selected operator unfolds its state and port counts but not its delete, chat and add/remove-port buttons. None of them can act on the structure-locked preview, so they only suggested it could be edited. - Open canvas saves first and hands over only once the save has completed: the switch is a full-page load, which aborts a request still in flight. Saves go out one at a time, in order: two persists in flight at once can reach the backend out of order and the older content would win, so the switch's save waits for an autosave already on its way, and the page hands over only once the queue has drained: that save and any asked for while it was in flight (the page stays interactive until the hand-over) have completed; each request carries the workflow as it was when the save was asked for, so the last one enqueued is the latest. The drain outlives the page: the final save on the way out joins the same queue rather than racing an autosave still in flight, and the queue is closed after it. A failed save keeps the author on the form with the error shown and does not stop the queue; a reader with nothing to save goes straight through. A save's response feeds back the server-owned metadata (the timestamp, and the normalised name when nothing changed) but never undoes a rename made while it was in flight, and repaints nothing once the page is gone. (apache#8456 does the save-then-navigate part on the canvas side.) - Keyboard focus is visible on the pill and tab buttons (`:focus-visible`). The picker's pills are tracked by step (`trackBy`): a toggle re-reads the config and rebuilds the choices as new objects, and re-created buttons would have dropped the keyboard focus from the pill just pressed. The instruction body carries an `aria-label`, since its placeholder is gone as soon as there is text. - `29c54bb85` (test only, workspace menu): the menu's export test module-mocked the CommonJS `file-saver` package with `vi.mock`. Under the Angular unit-test builder that call is not reliably hoisted (Vitest warns about it on every platform), and with this stack's shared-chunk graph it stopped applying on the Windows leg (7 of the last 8 Windows runs of the unsplit PR, while main passes). The export now goes through the existing injectable `FileSaverService`, as the dashboard downloads already do, and the spec stubs that with TestBed. No behaviour change. - `695c2fb4f` (test only, Form View rendered spec): the spec swapped the property panel for a stub by overriding the page's imports, which JIT-recompiles the page; a JIT template has no mapping back to the `.component.html`, so the page's template read as 0% covered on codecov from apache#8442 on (the 10 "missing" template lines there, 132 on the unsplit PR). The real panel's template is blanked instead, with its lifecycle hooks switched off, so the page stays AOT-compiled and its template is measured again. The edit-mode markup this PR adds is then covered through the DOM: Edit / Done, the Write / Preview tabs, heading and body writing through, the picker's pills and empty hint. The diff is about 1400 added lines because 725 of them are spec against 694 of source (of which 192 are stylesheet); the two test-only commits and the review-driven redesign of the picker are a large part of it, and the feature itself is under 700 lines. Not in this PR: renaming, hiding, reordering, help text and removal of the exposed inputs, and the author's view of a broken input. Those are apache#8517. ### Any related issues, documentation, discussions? Closes apache#8026. Part of the Form View feature (parent issue apache#8011). Replaces the first half of apache#8455. ### How was this PR tested? Unit tests (vitest). Direct-construction tests cover the authoring gate (enter, leave, refused without write access, always allowed to leave), the result picker's range (final steps until the author chooses, then exactly the saved list, `[]` included; viewed and listed intermediates; disabled steps left out), its live rebuild on an eye toggle, the edit-mode toggle writing the default (the saved list started from the final steps on the first choice), a viewer's own toggle writing nothing and giving way on entering edit mode, a reader not offered a saved pick whose eye is off, the lock clamped back whenever anything else unlocks the graph outside edit mode, unlocked in edit mode once a run has ended in the execute service's real order (unlock before state), and kept locked when edit mode is entered mid-run, the switch's save queued behind an autosave in flight with navigation after both, the final save on the way out drained behind an autosave in flight after the page is gone, a failed save not stopping the queue, a failed final save still reported without throwing, an older save's response not undoing a rename made meanwhile and no metadata repaint after the page is gone, a shown step dropped from the cards and the picker the moment the graph's shape makes it unavailable, a pill renamed the moment its step's display name changes, the hand-over waiting for a save queued behind the switch's and staying put when that save fails, the step panel dismissed on Done while the frame is still an editor, the instruction writes going through the binding service, and the switch's save-then-navigate order (navigates on complete, stays on error, straight through for a reader). The rendered spec covers the edit-mode header shape (title input outside the toggle, `aria-controls`), the panel turning live in edit mode (editor on, tick boxes on, inert off), and the panel staying read-only while a run is in flight even in edit mode, turning live when the run ends. The context menu's spec renders it under a structure lock with modification enabled and checks cut, paste, delete, disable and enable are off while copy and the result toggle stay; the editor's spec opens the right-click menu on a locked editor and checks the lock arrived, and checks a locked preview grows no link tools on hover or on add, and keeps a selected operator's delete, chat and port buttons hidden while its state still unfolds, while the canvas shows all of them. The binding service's spec checks the first choice starts the list from the default handed in, later choices flip within the list ignoring the default, and turning off the only shown step leaves `[]` rather than an absent list; the action service's spec checks a definition whose only content is `shownResultIds: []` is still carried in the saved content. 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 (5830 tests). #### Video https://github.com/user-attachments/assets/ca2f8e99-357a-48f7-95cb-6bf4e57c1bc4 ### 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 <noreply@anthropic.com> Co-authored-by: Meng Wang <mengw15@uci.edu> Co-authored-by: mengw15 <125719918+mengw15@users.noreply.github.com>
yangzhang75
force-pushed
the
formview-pr15b
branch
from
September 12, 2026 18:12
431acf7 to
056803e
Compare
yangzhang75
marked this pull request as ready for review
September 12, 2026 18:12
Contributor
Author
|
/request-review: @mengw15 |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Scalar arrays, label fallback, hide-toggle focus preservation, and array accessibility contain unresolved defects.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 4
- Review effort level: Balanced
mengw15
reviewed
Sep 12, 2026
On top of the edit mode, an author shapes the exposed inputs where they appear: rename an input or any of its sub-fields and hide a sub-field through a new editable-label formly wrapper (the label is the input; the control keeps a programmatic name), reorder by drag or from the keyboard with Move up / Move down, write per-input help text, and remove an input. A binding whose operator no longer exists renders as a removable card for the author and is hidden from readers; a repeated input keeps its title above its rows in reader mode too. Closes apache#8027. A rename is written through on every keystroke, so the saved name and the hidden label follow the box as it is typed; a step renamed on the canvas or in the live panel rebuilds the cards (held while typing) so the "From X" attribution never goes stale. Every config write announces on formBindingChanged$ and the page rebuilds on it; a presentation write (a name, a hide flag, help text, a result pick) is marked while it is made so its own announcement does not rebuild the form under the control that took it (the eye is a button, outside the typing hold), while structural writes still rebuild, once. In a repeated section the controls sit on the first row only and later rows follow them; a scalar array's rows are never the input's root; the name box falls back to the schema label captured before the override; and a repeated field is named as a group, having no labelable control of its own. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY
yangzhang75
force-pushed
the
formview-pr15b
branch
from
September 12, 2026 19:27
056803e to
0f1254b
Compare
mengw15
approved these changes
Sep 12, 2026
mengw15
left a comment
mengw15
left a comment
Contributor
There was a problem hiding this comment.
LGTM, thanks for addressing the comments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
editable-label-wrapperformly 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.: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 onformBindingChanged$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 arearia-disabledrather 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 inaria-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
Screen.Recording.2026-09-12.at.12.17.43.PM.mov
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.ai/code/session_01FVvP3ttj22f9LB4p9u2anY