Skip to content

feat(gui): author the Form View in place with an edit mode - #8455

Closed
yangzhang75 wants to merge 2 commits into
apache:mainfrom
yangzhang75:formview-pr15
Closed

yangzhang75 wants to merge 2 commits into
apache:mainfrom
yangzhang75:formview-pr15

Conversation

@yangzhang75

@yangzhang75 yangzhang75 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Closes #8026. Part of the Form View stack (parent issue #8011), on main now that #8442 (PR14) has merged. Two commits: 871649877 is the feature; 01d0e2d35 is a test-only follow-up for the Windows CI leg (last bullet). The tip is 01d0e2d35.

Adds the authoring side of the page: with write access, an Edit toggle turns the form into in-place authoring.

  • toggleAuthoring: authoring enables graph modification and turns the inspect panel live (the panel acts as an editor, actsAsEditor); leaving it returns the page to the reader state (inert panel, modification disabled). Entering edit mode is refused without write access at the method itself, not only by hiding the button.
  • Rename an exposed 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; leaving it empty falls back to the schema label; the reader's label is tied to its control with for). Overrides are stored per binding and reapplied on every rebuild.
  • Reorder inputs by drag (CDK drag-drop, mapped by binding id so a hidden card cannot desync the order) or from the keyboard with Move up / Move down on each card, remove one, or expose a new one by clicking a step on the preview.
  • Write the author's instruction (markdown, with preview; the title is edited in the header, which is a row rather than one button so the input is not nested in a control) and per-input help text; "From X" attribution shows only while authoring. A binding whose operator no longer exists renders as a removable card for the author and is hidden from readers; it is never deleted silently.
  • 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. The static label is tied to the control with for.
  • Carries the same rebuild-while-typing fix as fix(workflow-form): hold a rebuild that arrives while typing instead of dropping it #8499 (a tick box is not typing; a rebuild that arrives mid-typing is held and runs on focusout) so the tick boxes work here on their own; whichever merges first, the other rebases and the duplicate hunk disappears.
  • 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, so navigating right after firing the save could lose the edit it was meant to carry across. A failed save keeps the author on the form with the error shown; a reader with nothing to save goes straight through. (feat(gui): wire the Form View entry points #8456 does the same on the canvas side.)
  • Pick which extra results to feature: the picker offers the non-terminal view-result steps (plus already-chosen ones) and rebuilds live when an eye is toggled on the canvas. The final step's result always shows, so it is not offered.
  • Keyboard focus is visible on the pill, remove, tab and eye buttons (:focus-visible).
  • The embedded preview's right-click menu carries the structure lock. Edit mode re-enables workflow modification for the property panel, and the menu's re-shaping commands used to follow that flag alone, so the structure-locked preview would have offered cut, paste, delete and disable again. texera-context-menu now takes [structureLocked] from the editor and gates those commands on modification AND no lock (canModify); copy, the result toggles, execute-to and export do not re-shape the graph and are unchanged.
  • While authoring, the control keeps a programmatic name: the visible name box edits the label but is not the label, so a visually hidden <label for> that follows the current name (the author's, else the schema's) sits beside it. Readers keep the static label as before.
  • Graph edits go through the shared graph; formBinding edits stay local until fix(workflow): keep workflowSettings and formBinding in the shared model #8351 shares them.
  • 01d0e2d35 (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: the real saveAs ran and the spy saw 0 calls, on 7 of the last 8 Windows runs of feat(gui): author the Form View in place with an edit mode #8455/feat(gui): wire the Form View entry points #8456 while main and fix(workflow-form): hold a rebuild that arrives while typing instead of dropping it #8499 pass. 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.

The diff is over 1000 lines because it is 813 lines of spec against 1410 of source (of which 444 are stylesheet); the authoring header, wrapper and reorder logic are one feature and do not split further without landing a half-usable edit mode.

Any related issues, documentation, discussions?

Closes #8026. Part of the Form View feature (parent issue #8011).

How was this PR tested?

Unit tests (vitest). Direct-construction tests cover the authoring gate (a reader cannot enter edit mode and the graph stays unmodifiable), rename/hide overrides reaching the rendered fields, drag and keyboard reorder with the id-mapped indices (including the end stops and a card the config no longer holds), broken bindings kept for explicit removal, the result picker's range and its live rebuild on an eye toggle, the instruction/help-text editing, 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 Move buttons, and the panel turning live in edit mode. The editable-label wrapper has its own TestBed spec (real component: decorate, rename, hide toggle, label association for readers and, while authoring, the hidden label following the current name). 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. Each new guard was deletion-checked (removing it turns the corresponding test red). eslint, prettier and the production (AOT) build pass; every changed line is statement and function covered.

video

Screen.Recording.2026-09-10.at.6.05.08.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

@github-actions github-actions Bot added the frontend Changes related to the frontend GUI label Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @PG1204, @Yicong-Huang, @mengw15
    You can notify them by mentioning @PG1204, @Yicong-Huang, @mengw15 in a comment.

@codecov-commenter

codecov-commenter commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 48.56115% with 143 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.47%. Comparing base (1fbd346) to head (01d0e2d).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
...mponent/workflow-form/workflow-form.component.html 0.00% 132 Missing ⚠️
...abel-wrapper/editable-label-wrapper.component.html 76.00% 2 Missing and 4 partials ⚠️
...component/workflow-form/workflow-form.component.ts 95.04% 0 Missing and 5 partials ⚠️

❌ Your patch status has failed because the patch coverage (48.56%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8455      +/-   ##
============================================
- Coverage     93.69%   93.47%   -0.23%     
  Complexity     4826     4826              
============================================
  Files          1209     1211       +2     
  Lines         49871    50110     +239     
  Branches       6099     6135      +36     
============================================
+ Hits          46727    46840     +113     
- Misses         1652     1768     +116     
- Partials       1492     1502      +10     
Flag Coverage Δ *Carryforward flag
access-control-service 80.18% <ø> (ø) Carriedforward from 1fbd346
agent-service 99.32% <ø> (ø) Carriedforward from 1fbd346
amber 89.91% <ø> (ø) Carriedforward from 1fbd346
computing-unit-managing-service 77.14% <ø> (ø) Carriedforward from 1fbd346
config-service 87.12% <ø> (ø) Carriedforward from 1fbd346
file-service 83.65% <ø> (ø) Carriedforward from 1fbd346
frontend 95.64% <48.56%> (-0.52%) ⬇️
notebook-migration-service 83.73% <ø> (ø) Carriedforward from 1fbd346
pyamber 98.47% <ø> (ø) Carriedforward from 1fbd346
workflow-compiling-service 77.19% <ø> (ø) Carriedforward from 1fbd346

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

Adds in-place authoring capabilities to the Workflow Form View, including edit-mode UI, result rendering/curation, and read-only step inspection via the property panel.

Changes:

  • Add hasNonEmptyResult to distinguish empty results from “no result”.
  • Extend WorkflowFormComponent with authoring mode (rename/hide/reorder/expose/remove inputs), results display + picker, and read-only property-panel inspection.
  • Introduce editable-label-wrapper Formly wrapper and adjust property editor components to support embedded read-only usage.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
frontend/src/app/workspace/service/workflow-result/workflow-result.service.ts Adds hasNonEmptyResult helper for UI logic that should ignore empty results.
frontend/src/app/workspace/service/workflow-result/workflow-result.service.spec.ts Adds unit test coverage for hasNonEmptyResult behavior.
frontend/src/app/workspace/service/form-binding/form-binding.service.ts Updates docstring clarifying “featured results” behavior and terminal-step always-shown rule.
frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts Expands harness with result/view-result/highlight mocks needed by new form-view behavior.
frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts Adds rendered-template coverage for results section and stubs heavy children.
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts Core feature work: authoring toggle, input overrides, results rendering, step inspection, and chart fitting.
frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts Adds unit tests for authoring gate, results filtering, view-result-change behavior, inspection panel, etc.
frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss Adds styles for embedded property panel, results cards, and authoring UI.
frontend/src/app/workspace/component/workflow-form/workflow-form.component.html Adds edit-mode header, authoring instruction editor, drag-drop inputs, results section, and results picker.
frontend/src/app/workspace/component/property-editor/property-editor.component.ts Adds persistPlacement and broadcastEditing inputs for embedded/read-only panel usage.
frontend/src/app/workspace/component/property-editor/property-editor.component.spec.ts Adds tests for persistPlacement=false and new broadcastEditing input wiring.
frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts Guards shared-model writes behind broadcastEditing for read-only inspection mode.
frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts Adds tests asserting broadcast/version-sync occur (or not) based on broadcastEditing.
frontend/src/app/common/type/workflow.ts Clarifies resultOperatorIds semantics in the config type doc.
frontend/src/app/common/formly/formly-config.ts Registers the new editable-label-wrapper Formly wrapper.
frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.ts Adds the wrapper component plus decorate() helper for author rename/hide UI.
frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.spec.ts Adds component-level tests for wrapper behavior, handlers, and template branches.
frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.scss Styles for editable label + hide toggle, including “dimmed when hidden” authoring view.
frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.html Template for editable label input + optional hide toggle (authoring) vs static label (reader).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread frontend/src/app/workspace/component/property-editor/property-editor.component.ts Outdated
Comment thread frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts Outdated
Comment thread frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts Outdated
Comment thread frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts Outdated
@yangzhang75

Copy link
Copy Markdown
Contributor Author

/request-review: @mengw15

@github-actions
github-actions Bot requested a review from mengw15 September 11, 2026 02:35
renovate-bot pushed a commit to renovate-bot/apache-_-texera that referenced this pull request Sep 11, 2026
### What changes were proposed in this PR?

Closes apache#8496. A regression from apache#8125: since then, every workflow's
second autosave (and every one after it) fails with 500, silently on the
operator canvas and as "Could not save" on the Form View.

`persistWorkflow` wrote `is_public` from the request. The frontend feeds
the saved row straight back as its metadata, and that row names the flag
`isPublic` while the rest of the frontend calls it `isPublished`, so the
very next save went out without the flag, the update wrote NULL into a
NOT NULL column, and the request failed. A stale `isPublic: false` on a
save could likewise un-publish a published workflow.

- Backend: `saveWorkflowFields` now writes name, description and content
only. Publishing stays with `/public` and `/private`, `default_view`
with `/set-default-view`, and the timestamps are not rewritten, so a
save can never clobber a concurrent change to any of them.
- Frontend: `WorkflowPersistService.persistWorkflow` no longer sends
`isPublic` (the endpoint does not read it, and the value is not reliably
known after the first save), and `WorkflowUtilService.parseWorkflowInfo`
carries a persist response's `isPublic` over to `isPublished`, so
metadata fed back from a save keeps the publish state instead of
dropping it.

The Form View stack is not blocked by this: apache#8455 touches none of these
files, and apache#8456 touches `workflow-persist.service.ts` only in
`createWorkflow` (adding `defaultView`), a different function; a dry-run
merge of the two is clean.

### Any related issues, documentation, discussions?

Closes apache#8496. Found while verifying apache#8455 on a flag-on instance (parent
apache#8011).

### How was this PR tested?

Backend: `WorkflowResourceSpec` gains two tests, a save carrying no flag
neither fails nor changes `is_public` after `/public`, and a save
carrying `false` does not un-publish; the existing default-view save
test was updated to send no flag, as the frontend does.
`WorkflowResourceSpec` and `PublishedCopySchemaSpec` pass (87 tests),
scalafmt clean.

Frontend: the persist spec asserts the save payload carries no
`isPublic` and that the response's `isPublic` comes back as
`isPublished`; `parseWorkflowInfo` gains tests for the carry-over and
for leaving a present `isPublished` alone. Full suite passes (5777),
changed lines fully covered, eslint, prettier and the production (AOT)
build pass.

End to end, against a running stack rebuilt with this change: the exact
second-save payload that returned 500 now returns 200 with `is_public`
unchanged; a create-through-persist with the new payload inserts with
`is_public = false`; and in a headless browser the Form View renames a
workflow twice with every `/api/workflow/persist` answering 200, no
`isPublic` key in any request body, and no "Could not save".

### 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>
renovate-bot pushed a commit to renovate-bot/apache-_-texera that referenced this pull request Sep 11, 2026
…of dropping it (apache#8499)

### What changes were proposed in this PR?

Closes apache#8497. The Form View skips rebuilding its input cards while the
reader is typing, so a rebuild cannot throw away a half-entered value
(from apache#8437). Two things were wrong with that skip.

- It counted any focused `INPUT` as typing, tick boxes included. Ticking
a property in the step panel focuses the tick box, so the rebuild that
should add the card was skipped and the tick looked like it did nothing
until something else rebuilt the page (the way apache#8455's edit mode
surfaces it).
- A skipped rebuild was dropped, not postponed. A schema refresh that
landed while someone was typing (the column names an attribute dropdown
offers, after a compile) never reached the cards until the next compile
or a reload.

Now only text-like inputs, textareas, selects and content-editables
count as typing, and a rebuild that does arrive mid-typing is held and
runs once the focus leaves the text control: a `focusout` host listener
decides one tick later, so tabbing to the next text field keeps it held
while any other focus change runs it. Both subscribers (compilation
state, form-binding changes) go through the one `rebuildFormOrDefer`.

apache#8455 carries the same change so it stays self-contained and testable on
its own; whichever merges first, the other rebases and the duplicate
hunk disappears.

### Any related issues, documentation, discussions?

Closes apache#8497. Found while verifying apache#8455 on a flag-on instance (parent
apache#8011).

### How was this PR tested?

Unit tests (vitest): a focused tick box is not typing; a held rebuild
runs once on focusout for both the compilation and the binding stream;
it stays held when the focus only moves to another text field; a
focusout with nothing held rebuilds nothing; a queued focusout callback
does not rebuild a second time when the control that took the focus
already rebuilt (the tick box case). The rendered spec dispatches a real
bubbling `focusout` from a control inside the page and asserts the host
listener drains the held rebuild. Each was checked to fail with its half
of the fix removed, including removing the `@HostListener` decorator.
Full suite passes (5780), changed lines fully covered, eslint, prettier
and the production (AOT) build pass.

In a headless browser against a dev server built from this branch:
typing a value into an exposed input and tabbing out keeps the value
through the held rebuild, with no console errors and every autosave
answering 200. The tick-box path was exercised the same way on apache#8455,
which carries the identical change: the card follows the tick within a
second, in both directions.

### 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>
@mengw15
mengw15 requested a balanced review from Copilot September 11, 2026 16:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Edit mode exposes structural context-menu mutations and rebuilds can discard pending debounced form edits.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (4)

frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:1132

  • This immediate re-read can discard the user's latest input value. Each card writes through a debounced form.valueChanges, but readConfig() calls buildForm(), whose formsRebuilt.next() unsubscribes that pending debounce. If someone types a value and clicks Done before the debounce expires, the old graph value is rebuilt and the edit is lost. Flush pending card values before rebuilding, or defer mode/structural rebuilds until those writes have committed.
    // Edit mode is what makes operator properties (and the embedded canvas) editable here.
    this.applyEditability();
    this.readConfig();

frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:599

  • A disabled operator can retain its viewResult flag, but execution builds the logical plan from getSubDAG(), which excludes disabled operators, so its result is not materialized. This filter currently offers such an unchosen disabled step as a selectable result. Exclude disabled viewed operators while retaining already-chosen ones so stale selections can still be removed.
    this.resultChoices = this.operators()
      .filter(op => !terminals.has(op.operatorID) && (viewed.has(op.operatorID) || chosen.has(op.operatorID)))

frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:861

  • The stored override is assigned to node.props.label immediately above, so using that value as fallback makes the editor claim that clearing a custom name keeps the same custom name. Capture the schema label before applying override.displayName and pass that original value as the fallback.
              fallback: (node.props?.label as string) || path,

frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.html:55

  • This for is dangling for the reader-mode use introduced here: the wrapper is attached to an array field, while ArrayTypeComponent renders rows and buttons but no element with the array field's id (array.type.ts:20-55). Therefore the visible array title does not label any control or group. Give the title its own id and apply role="group" plus aria-labelledby to the container around fieldComponent.
<label
  class="lbl-static"
  *ngIf="!props.authoring && (props.authorName || props.schemaLabel)"
  [attr.for]="id">
  • Files reviewed: 13/13 changed files
  • Comments generated: 2
  • Review effort level: Balanced

yangzhang75 added a commit to yangzhang75/texera that referenced this pull request Sep 11, 2026
The workspace menu's export test module-mocked the CommonJS file-saver
package (vi.mock("file-saver")). Under the Angular unit-test builder that
call ends up inside the built spec chunk rather than at its top level
(Vitest warns about it on every platform), and once this stack shifted the
shared chunk graph the mock stopped applying on the Windows leg: the real
saveAs ran and the spy saw 0 calls (7 of the last 8 Windows runs on apache#8455
and apache#8456, while main and apache#8499 pass).

Route the download through the existing injectable FileSaverService, as
the dashboard downloads already do, and stub that with TestBed, which does
not depend on mock hoisting. No behaviour change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY
yangzhang75 and others added 2 commits September 11, 2026 10:22
Add an Edit/Done toggle (write access only) that turns the Form View into
in-place authoring without leaving the page: rename an input or its sub-fields
by clicking the title, hide a sub-field (kept faded, not removed), reorder
inputs by drag, expose a new one by clicking a step on the embedded preview (the
property panel goes live with tick boxes), remove an input, write the
instruction (write/preview), and pick which steps report their results. An
author sees broken inputs (their operator was deleted) to repair or remove them;
a reader never does. Leaving edit mode locks everything back to read-only. Every
edit goes through the same shared graph and form-binding config the operator
canvas edits.

Adds a reusable editable-label formly wrapper (renders a plain label for a
reader, an editable input with an optional hide eye for an author).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY
The workspace menu's export test module-mocked the CommonJS file-saver
package (vi.mock("file-saver")). Under the Angular unit-test builder that
call ends up inside the built spec chunk rather than at its top level
(Vitest warns about it on every platform), and once this stack shifted the
shared chunk graph the mock stopped applying on the Windows leg: the real
saveAs ran and the spy saw 0 calls (7 of the last 8 Windows runs on apache#8455
and apache#8456, while main and apache#8499 pass).

Route the download through the existing injectable FileSaverService, as
the dashboard downloads already do, and stub that with TestBed, which does
not depend on mock hoisting. No behaviour change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY
@yangzhang75

Copy link
Copy Markdown
Contributor Author

Closing: at 2200 lines this was too much for one review, so it is split in two, both rebased onto current main (which now carries #8498 and #8499, so the duplicated rebuild-while-typing hunk is gone):

Together they are byte-for-byte the tree that was here at 01d0e2d35, rebased, plus two strengthened tests; every fix from this PR's review threads is in one of the two. #8456 is now stacked on #8517.

yangzhang75 added a commit to yangzhang75/texera that referenced this pull request Sep 11, 2026
The workspace menu's export test module-mocked the CommonJS file-saver
package (vi.mock("file-saver")). Under the Angular unit-test builder that
call ends up inside the built spec chunk rather than at its top level
(Vitest warns about it on every platform), and once this stack shifted the
shared chunk graph the mock stopped applying on the Windows leg: the real
saveAs ran and the spy saw 0 calls (7 of the last 8 Windows runs on apache#8455
and apache#8456, while main and apache#8499 pass).

Route the download through the existing injectable FileSaverService, as
the dashboard downloads already do, and stub that with TestBed, which does
not depend on mock hoisting. No behaviour change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY
yangzhang75 added a commit to yangzhang75/texera that referenced this pull request Sep 11, 2026
The workspace menu's export test module-mocked the CommonJS file-saver
package (vi.mock("file-saver")). Under the Angular unit-test builder that
call ends up inside the built spec chunk rather than at its top level
(Vitest warns about it on every platform), and once this stack shifted the
shared chunk graph the mock stopped applying on the Windows leg: the real
saveAs ran and the spy saw 0 calls (7 of the last 8 Windows runs on apache#8455
and apache#8456, while main and apache#8499 pass).

Route the download through the existing injectable FileSaverService, as
the dashboard downloads already do, and stub that with TestBed, which does
not depend on mock hoisting. No behaviour change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY
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>
renovate-bot pushed a commit to renovate-bot/apache-_-texera that referenced this pull request Sep 12, 2026
### What changes were proposed in this PR?

Closes apache#8027. Part of the Form View stack (parent issue apache#8011), on main
now that apache#8516 (the edit mode) has merged. This is the second half of
what was apache#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 apache#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 apache#8027. Part of the Form View feature (parent issue apache#8011).
Replaces the second half of apache#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 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend Changes related to the frontend GUI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(workflow-form): Form View edit mode, with instruction and result authoring

3 participants