Skip to content

fix(workflow-form): hold a rebuild that arrives while typing instead of dropping it - #8499

Merged
mengw15 merged 1 commit into
apache:mainfrom
yangzhang75:fix/form-rebuild-while-typing
Sep 11, 2026
Merged

mengw15 merged 1 commit into
apache:mainfrom
yangzhang75:fix/form-rebuild-while-typing

Conversation

@yangzhang75

@yangzhang75 yangzhang75 commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Closes #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 #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 feat(gui): author the Form View in place with an edit mode #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.

#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 #8497. Found while verifying #8455 on a flag-on instance (parent #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 #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.ai/code/session_01FVvP3ttj22f9LB4p9u2anY

@github-actions github-actions Bot added fix frontend Changes related to the frontend GUI labels Sep 11, 2026
@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Backport auto-label report

This fix: PR was checked against each actively-supported release branch. A release/* label nominates a backport target; the branch's release manager approving this PR is what sends the fix there. The required Backport Approvals check stays red until every label below is approved, so each manager either approves or removes their own label — which is why the labels left on a merged PR are exactly the branches it reached.

Release branch Analysis
⚠️ release/v1.3 Not labeled automatically — none of the files this PR modifies exist on this branch (frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts, frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts, frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts). The fix may target code that isn't on this release, or the files were moved/renamed after the branch was cut. Please check and add release/v1.3 by hand if this fix should be backported here — its release manager then has to approve before this PR can merge.
⚠️ release/v1.2 Not labeled automatically — none of the files this PR modifies exist on this branch (frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts, frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts, frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts). The fix may target code that isn't on this release, or the files were moved/renamed after the branch was cut. Please check and add release/v1.2 by hand if this fix should be backported here — its release manager then has to approve before this PR can merge.

Auto-label run.

@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

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

  • No candidates found from git blame history.

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

The deferred callback can trigger duplicate rebuilds, and the HostListener wiring lacks rendered coverage.

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

Pull request overview

Defers Form View rebuilds during text entry while allowing immediate rebuilds for non-text controls.

Changes:

  • Classifies text-like versus click-only inputs.
  • Runs deferred rebuilds after focus leaves text controls.
  • Adds unit coverage for compilation and binding rebuild paths.
File summaries
File Description
workflow-form.component.ts Implements input classification and deferred rebuilding.
workflow-form.component.spec.ts Tests deferred rebuild behavior and checkbox focus.
Review details
  • Files reviewed: 2/2 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.

Comment thread frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts Outdated
@codecov-commenter

codecov-commenter commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.69%. Comparing base (1fbd346) to head (e99b9f2).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #8499   +/-   ##
=========================================
  Coverage     93.69%   93.69%           
  Complexity     4826     4826           
=========================================
  Files          1209     1209           
  Lines         49871    49881   +10     
  Branches       6099     6102    +3     
=========================================
+ Hits          46727    46737   +10     
  Misses         1652     1652           
  Partials       1492     1492           
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 96.16% <100.00%> (+<0.01%) ⬆️
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.

@yangzhang75
yangzhang75 force-pushed the fix/form-rebuild-while-typing branch from c017314 to 05c752c Compare September 11, 2026 02:05
@github-actions github-actions Bot added the fix label Sep 11, 2026
…of dropping it

The Form View skips rebuilding its input cards while the reader is typing, so a rebuild
cannot throw away a half-entered value. Two things were wrong with the 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. And a skipped rebuild was simply dropped: a schema refresh that landed while someone was
typing never reached the cards until something else happened to rebuild them.

A tick box, radio or button is no longer typing (only text-like inputs, textareas, selects and
content-editables are), and a rebuild that does arrive mid-typing is held and runs once the
focus leaves the text control, decided a tick after focusout so tabbing to the next text field
keeps it held.

Closes apache#8497.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY
@yangzhang75
yangzhang75 force-pushed the fix/form-rebuild-while-typing branch from 05c752c to e99b9f2 Compare September 11, 2026 02:30
@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.

🟢 Approval recommended

The focused implementation addresses both reported failure modes with comprehensive regression coverage.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@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 added this pull request to the merge queue Sep 11, 2026
Merged via the queue into apache:main with commit fd09f20 Sep 11, 2026
24 checks passed
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
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix frontend Changes related to the frontend GUI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(workflow-form): a rebuild that arrives while the reader is typing is dropped instead of held (from #8437)

4 participants