Skip to content

feat(workflow-form): render Projection's drag-reorder list in the form, with the drag - #8843

Merged
mengw15 merged 2 commits into
apache:mainfrom
yangzhang75:feat/form-projection-drag-reorder
Oct 7, 2026
Merged

mengw15 merged 2 commits into
apache:mainfrom
yangzhang75:feat/form-projection-drag-reorder

Conversation

@yangzhang75

@yangzhang75 yangzhang75 commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Closes #8761. Part of the Form View feature (#8011); from the #8439 refinements (item 2).

An exposed drag-reorder property -- Projection's columns, formly type repeat-section-dnd -- fell back to a plain repeated list in the Form View (CANVAS_ONLY_FORMLY_TYPES): a reader could edit the rows but not reorder them, which is what the widget is for. The fallback assumed the drag had nowhere to attach on a form; the widget carries its own CDK drop list, and the row template it needs comes from the operator's own schema (the per-instance one, with the upstream columns), which the form already builds.

  • repeat-section-dnd leaves CANVAS_ONLY_FORMLY_TYPES; the set now holds the code editor only. On the form the widget renders exactly as on the operator property panel, drag handle and all, and a drop reaches the operator through the card's valueChanges write-back; the panel's reorder callback stays optional (the widget calls it only when the host provides one), so the same widget serves both hosts without a form-specific branch. Nothing is added to the widget's UI.
  • Two defects in the widget itself, found while reviewing this and in reach of the form for the first time: every button now has type="button" -- nz-button sets no type, so inside a form (the card's, the panel's) a typeless button is the submit button and Enter in a row's input clicked it, removing the first row (the fallback the form used before had typed buttons; the panel has had this since the widget exists); and the rows are re-keyed to their positions after a drop, as formly's own remove() does -- formly keys rows by position and writes a cell's value into the model by that key path, so a value typed into a moved row landed on the row that used to be there (the same after a drag on the panel, until the operator was re-selected).

Any related issues, documentation, discussions?

Closes #8761. Raised by @mengw15 on #8436; filed from #8439. Builds on the sub-field walk from #8438. #8760 (proper) also shrinks CANVAS_ONLY_FORMLY_TYPES; whichever lands second deletes the then-empty set. Known limitation, not this PR's code: the Form View rebuilds every card on every compilation, including the one a card's own write triggers, so on the form a drop's write redraws the rows about a second later, and a second drop made in the last 150 ms before that redraw is undone. The canvas panel has no such rebuild. Filed as #8844 and fixed in #8845 (the form rebuilds only when the schema, the set of inputs or a value it does not hold changed); this PR is independent of it. Other follow-ups: a card's model is seeded once and never re-seeded from a co-editor's edit of the same property, so a reorder writes the whole array back over such an edit (pre-existing for every array/object property; a reorder is one more trigger); the HuggingFace and codearea widgets also carry typeless buttons.

How was this PR tested?

Unit tests (vitest). The widget's spec: a drop reorders the model, the field configs and the form controls and then tells the parent; a section without a reorder callback still reorders; a keyless row's children are re-keyed to the row's new position, as formly's own remove() does; every button carries type="button"; the existing rendering tests (one row per entry, draggable rows, remove, add, disabled section) still hold. A second spec stands the widget up under real formly (FormlyModule.forRoot, a form, a model): a drop changes the model and the cells and keeps the rows' keys in step with their positions, and a value typed into a moved row lands on that row's entry. The form's spec: Projection's attributes renders through repeat-section-dnd. custom-formly-type's spec pins the set. Each guard was deletion-checked (the re-keying dropped; the callback called unguarded; the remove button typeless; the type back in the canvas-only set) and turned a named test red. eslint, prettier and the production (AOT) build pass; the full frontend suite passes; every statement, function and branch this PR adds is covered.

Checked on a flag-on instance (main-demo; a Python source with four columns feeding a Projection whose attributes is exposed), with the canvas and the form open side by side: on main the card is a plain list (no handle); with this PR the card shows the drag handles, as the panel does. Dragging the last row to the top: the operator's order follows in the canvas tab's panel within a second and in the saved workflow. An alias typed into the moved row lands on that column, not its neighbour; Enter in an alias input removes no row; a reload shows the same order. A drop made on the canvas panel reaches the form within a second.

Video

Before

Screen.Recording.2026-10-04.at.2.04.31.PM.mov

After

Screen.Recording.2026-10-04.at.2.14.49.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

@github-actions github-actions Bot added feature frontend Changes related to the frontend GUI labels Oct 4, 2026
@github-actions

github-actions Bot commented Oct 4, 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: @anthonychengit, @Yicong-Huang, @eugenegujing
    You can notify them by mentioning @anthonychengit, @Yicong-Huang, @eugenegujing in a comment.

@codecov-commenter

codecov-commenter commented Oct 4, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.60%. Comparing base (4c982a7) to head (6743c1d).

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #8843   +/-   ##
=========================================
  Coverage     92.60%   92.60%           
- Complexity     5044     5045    +1     
=========================================
  Files          1252     1252           
  Lines         53580    53582    +2     
  Branches       6672     6672           
=========================================
+ Hits          49616    49622    +6     
+ Misses         2317     2314    -3     
+ Partials       1647     1646    -1     
Flag Coverage Δ *Carryforward flag
access-control-service 77.38% <ø> (ø) Carriedforward from 2082021
agent-service 99.16% <ø> (ø) Carriedforward from 2082021
amber 88.10% <ø> (+0.01%) ⬆️ Carriedforward from 2082021
computing-unit-managing-service 60.43% <ø> (-0.06%) ⬇️ Carriedforward from 2082021
config-service 87.37% <ø> (ø) Carriedforward from 2082021
file-service 81.53% <ø> (ø) Carriedforward from 2082021
frontend 96.58% <100.00%> (-0.01%) ⬇️
notebook-migration-service 83.73% <ø> (ø) Carriedforward from 2082021
pyamber 98.58% <ø> (+0.06%) ⬆️ Carriedforward from 2082021
workflow-compiling-service 74.09% <ø> (ø) Carriedforward from 2082021

*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.

@yangzhang75 yangzhang75 changed the title feat(workflow-form): render Projection's drag-reorder list in the form, with the drag and a keyboard path feat(workflow-form): render Projection's drag-reorder list in the form, with the drag Oct 4, 2026
@yangzhang75
yangzhang75 force-pushed the feat/form-projection-drag-reorder branch from 7b5aa23 to 60053dc Compare October 4, 2026 21:15
@yangzhang75
yangzhang75 marked this pull request as ready for review October 4, 2026 21:31
@yangzhang75

Copy link
Copy Markdown
Contributor Author

/request_review: @mengw15

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.

Copilot review overview

🟡 Changes recommended

The enabled reorder widget lacks the keyboard Move up/down path required by #8761.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Enables Projection column drag-reordering in Form View and fixes repeat-list state synchronization.

Changes:

  • Enables repeat-section-dnd in Form View.
  • Re-keys rows after reordering and makes callbacks optional.
  • Prevents widget buttons from submitting forms and expands tests.
File Description
custom-formly-type.ts Enables drag-reorder widgets in forms.
custom-formly-type.spec.ts Updates canvas-only type expectations.
workflow-form.component.ts Updates fallback documentation.
workflow-form.component.spec.ts Tests Projection widget selection.
repeat-dnd.component.ts Synchronizes reordered models, fields, controls, and keys.
repeat-dnd.component.spec.ts Expands component behavior coverage.
repeat-dnd.component.html Marks controls as non-submit buttons.
repeat-dnd.component.formly.spec.ts Adds real Formly integration coverage.

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

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.

Copilot review overview

🟢 Approval recommended

The implementation and coverage are sound; remaining feedback concerns only misleading test comments.

Review effort: Balanced
Findings: 2 Low severity

Open (2)
Resolved since last review (1)

…m, with the drag

An exposed drag-reorder property (Projection's columns, formly type
repeat-section-dnd) fell back to a plain repeated list in the Form View:
the rows could be edited but not reordered. The fallback assumed the
drag had nowhere to attach on a form; the widget carries its own drop
list, and the row template comes from the operator schema the form
already builds. Take it out of CANVAS_ONLY_FORMLY_TYPES, which now
holds the code editor only. The widget renders on the form exactly as
on the operator property panel; the panel's reorder callback stays
optional, since the card's form persists a drop through valueChanges.

Two defects of the widget itself, in reach of the form for the first
time: its buttons carried no type, so inside a form (the card's, the
panel's) Enter in a row's input clicked the first one and removed a
row; and the rows kept their build-time keys after a drop, so a value
typed into a moved row landed on the row that used to be there (formly
keys rows by position and writes by that key path, re-keying on its own
remove(); a drop now re-keys the same way).

Closes apache#8761

Co-Authored-By: Claude Fable 5.1 <[email protected]>
@yangzhang75
yangzhang75 force-pushed the feat/form-projection-drag-reorder branch from 60053dc to 7a4c601 Compare October 7, 2026 00:58
@mengw15
mengw15 requested a balanced review from Copilot October 7, 2026 14:31

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.

Copilot review overview

🟢 Approval recommended

The implementation is consistent with Formly’s array-key behavior and has focused unit and integration coverage.

Review effort: Balanced
Findings: None

Resolved since last review (2)

@mengw15 mengw15 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.

LGTM

@mengw15
mengw15 enabled auto-merge October 7, 2026 18:48
@mengw15
mengw15 disabled auto-merge October 7, 2026 18:48
@mengw15
mengw15 enabled auto-merge October 7, 2026 18:49
@mengw15
mengw15 added this pull request to the merge queue Oct 7, 2026
Merged via the queue into apache:main with commit 397de92 Oct 7, 2026
23 checks passed
renovate-bot pushed a commit to renovate-bot/apache-_-texera that referenced this pull request Oct 18, 2026
…nged something they show (apache#8845)

### What changes were proposed in this PR?

Closes apache#8844. Part of the Form View feature (apache#8011); from the apache#8439
refinements. Found on the apache#8761 demo (apache#8843).

A compilation result rebuilt every card on the Form View, including the
compilation a card's own write triggers. Three things followed from
that, all visible with a Projection's drag-reorder list (apache#8761) and with
any tick box: the controls were redrawn under the reader's eyes after
every click (the row buttons flicker); the keyboard focus went with the
replaced node, so a keyboard reader landed on the body about a second
after every Move up / Move down; and a click made in the last 150 ms
before the rebuild was lost, since the rebuild ended that card's pending
debounced write and re-seeded the card from the operator, which still
held the previous state (the row "jumps back"). The canvas's property
panel redraws only when an operator's schema changes.

The form now does the same:

- A compilation rebuilds the cards only when it changed something they
show: a card's operator schema (the column enums compilation writes into
it, which turn an attribute box into a dropdown), the set of inputs (an
operator deleted on the canvas makes its card broken; a property exposed
or removed), or an operator value a card does not hold (a co-editor's
edit), including the operator's other properties, which a card is seeded
with as the context a custom widget reads to decide what it shows (the
HuggingFace picker reads `task`): nothing but a rebuild re-seeds those,
so a sibling changed on the canvas or by a co-editor would have left the
card showing the old one (Copilot). Every property an input is bound to
is left out of that context, this card's own and any other card's on the
same step, since each is compared against the card that shows it:
counting them as context too would make one card's edit a change to
every other card of that step, and a form with two inputs from one step
would rebuild wholesale on every compilation its own typing triggers --
the churn this PR exists to stop (Copilot). The keys are compared
sorted, so a rewrite that only reorders them is not a change, and a
card's own edit, written back or still on its way, is not a reason: the
two agree once it is written. The result picker and the shown results
still follow the graph on every compilation.
- Before any rebuild, from any source, a card's unwritten edit is
written, so a rebuild can no longer lose one.
- An empty string and an absent value count as the same value in the
comparison (formly's blank default for a property the operator never
had), so such a card is not rebuilt on every compilation.

### Any related issues, documentation, discussions?

Closes apache#8844. The drag-reorder list apache#8843 (apache#8761) brings to the form is
where this is plain to see: every drop writes and compiles, and the rows
redraw about a second later; a second drop made just before that redraw
is undone. The cause is the form's own compile handling, present since
the cards were first built from the compiled schema; on main alone a
tick box goes through the same redraw, focus drop and lost second click,
which take a keyboard or a stopwatch to notice. The Before/After below
is therefore recorded on top of apache#8843 (before: apache#8843 alone; after: apache#8843
plus this); this PR does not depend on apache#8843 and applies to main as is.
Known limitation, unchanged: a card still shows a co-editor's edit of
the same property only after the rebuild that edit's compilation
triggers (tracked as a follow-up).

### How was this PR tested?

Unit tests (vitest), the form's spec: a compilation that changes nothing
a card shows leaves the cards alone (same card objects) while the result
picker is still refreshed; a changed operator schema, an operator gone
from the graph, an operator value changed under a card (a co-editor's
edit), another property of a card's operator changed (the widget
context), a changed set of inputs (one exposed; a shown one turned
broken) each rebuild; the same properties in another order do not, nor
does a change to the card's own property while its edit is still on its
way, nor one card's edit when two inputs come from the same step (both
card objects survive it); an author's broken card is left alone; the
card's own edit does not rebuild while pending nor once written;
formly's blank and an absent value count as the same; an unwritten edit
is written, once, before a rebuild from any source replaces the card; a
read-only viewer's card has no write to flush; the typing hold still
holds a needed rebuild until the focus leaves. Each guard was
deletion-checked (every compilation rebuilding; the schema check, the
context check, the exclusion of every bound property from it, that the
exclusion covers the other cards of the step and not only this one, the
context's key sort, the pending check, the broken-set check, the blank
normalisation, the pre-rebuild flush, the pending mark, the pending
reset and the result-picker refresh each removed; a broken card checked
against a schema) and turned a named test red. eslint, prettier and the
production (AOT) build pass; the full frontend suite passes; every
statement, function and branch this PR adds is covered.

Checked on a flag-on instance (main-demo; before = main, and main +
apache#8843; after = the same plus this), a Projection's columns and its Drop
Option exposed on the form. The tick box on main: 0.3 s after a tick the
box still has the focus; 2.8 s later it is a new DOM node and the focus
is on the body; two ticks 700 ms apart end in the state after the first
(the second undone). With this change the box is the same node three
seconds later, still focused, and both ticks land. The drag-reorder rows
on main + apache#8843: after a drop the rows are replaced about a second
later; with this change they are the same nodes, and the order still
reaches a canvas tab open beside the form within a second, a drop made
on the canvas panel still reaches the form (a value the card does not
hold rebuilds it), and the saved workflow carries the order.

#### Video

**Before**


https://github.com/user-attachments/assets/eac466c3-c0c9-4969-b39a-a6da41ec54ff

**After**



https://github.com/user-attachments/assets/3ecd6342-7d6b-405c-bd83-f45415c2bcba



### 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)

Co-authored-by: Claude Fable 5.1 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature frontend Changes related to the frontend GUI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(workflow-form): render Projection's drag-reorder list in the form, with the drag

4 participants