Repository navigation
fix(workflow-form): rebuild the cards on compilation only when it changed something they show - #8845
Merged
mengw15 merged 1 commit intoOct 18, 2026
Conversation
Contributor
Backport auto-label reportThis
|
Contributor
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8845 +/- ##
=========================================
Coverage 92.32% 92.32%
Complexity 5045 5045
=========================================
Files 1260 1260
Lines 54026 54067 +41
Branches 6744 6755 +11
=========================================
+ Hits 49879 49918 +39
Misses 2487 2487
- Partials 1660 1662 +2
*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
fix/form-cards-in-place-on-compile
branch
from
October 4, 2026 21:28
e86f1af to
aa24d90
Compare
yangzhang75
marked this pull request as ready for review
October 4, 2026 21:31
Contributor
Author
|
/request-review: @mengw15 |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Sibling operator-property changes can leave contextual widgets displaying stale state.
Review effort: Balanced
Findings: 1
What changed in this PR
Prevents unnecessary Form View card rebuilds after compilation, preserving pending edits and keyboard focus.
Changes:
- Rebuilds cards only when inputs, schemas, or displayed values change.
- Flushes pending writes before rebuilding and adds regression coverage.
| File | Description |
|---|---|
workflow-form.component.ts |
Adds selective rebuilding and pending-write flushing. |
workflow-form.component.spec.ts |
Tests rebuild decisions and edit preservation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
yangzhang75
force-pushed
the
fix/form-cards-in-place-on-compile
branch
from
October 6, 2026 18:21
aa24d90 to
e3a35bd
Compare
yangzhang75
force-pushed
the
fix/form-cards-in-place-on-compile
branch
from
October 6, 2026 19:05
e3a35bd to
e051ba7
Compare
renovate-bot
pushed a commit
to renovate-bot/apache-_-texera
that referenced
this pull request
Oct 7, 2026
…m, with the drag (apache#8843) ### What changes were proposed in this PR? Closes apache#8761. Part of the Form View feature (apache#8011); from the apache#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 apache#8761. Raised by @mengw15 on apache#8436; filed from apache#8439. Builds on the sub-field walk from apache#8438. apache#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 apache#8844 and fixed in apache#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** https://github.com/user-attachments/assets/5a78f6f5-3b27-4b93-955e-bb15b40fd854 **After** https://github.com/user-attachments/assets/b84c79fa-a0a4-4bbc-b734-e75147bdf0d6 ### 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]> Co-authored-by: Meng Wang <[email protected]>
…nged something they show A compilation result rebuilt every card on the Form View, including the compilation a card's own write triggers: the controls were redrawn under the reader's eyes (the row buttons flickered after every Move), the keyboard focus went with the replaced node, 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. The canvas's property panel redraws only when an operator's schema changes. Follow the panel. A compilation rebuilds the cards only when it changed something they show: a card's operator schema (the column enums compilation writes into it), the set of inputs (an operator deleted makes its card broken), or an operator value a card does not hold (a co-editor's edit). A card's own edit, written back or still pending, is not a reason. The result picker and the shown results still follow the graph on every compilation. And before any rebuild, from any source, a card's unwritten edit is written, so a rebuild can no longer lose one. Part of the Form View feature (apache#8011), from the apache#8439 refinements; found on the apache#8761 demo. Co-Authored-By: Claude Fable 5.1 <[email protected]>
yangzhang75
force-pushed
the
fix/form-cards-in-place-on-compile
branch
from
October 17, 2026 00:25
e051ba7 to
1dc5255
Compare
mengw15
enabled auto-merge
October 18, 2026 02:57
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 #8844. Part of the Form View feature (#8011); from the #8439 refinements. Found on the #8761 demo (#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 (#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:
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.Any related issues, documentation, discussions?
Closes #8844. The drag-reorder list #8843 (#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 #8843 (before: #8843 alone; after: #8843 plus this); this PR does not depend on #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 + #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 + #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
Screen.Recording.2026-10-04.at.2.22.52.PM.mov
After
Screen.Recording.2026-10-04.at.2.23.54.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