fix(frontend): give the workflow editor its own container, not the document's first - #8607
yangzhang75 wants to merge 1 commit into
Conversation
…cument's first
Found in the browser: switching to the Form View and back left the canvas
blank -- nothing to pan, nothing to click -- while the graph itself was
fine, as the Form View's preview went on showing it running.
`WorkflowEditorComponent` found its container with
`document.getElementById("workflow-editor")`, and both views render that
same hardcoded id: the canvas's editor and the Form View's preview are
the same component. While the switch was a full page load the two could
never coexist, so the lookup was always right. Routing between the views
overlaps them for a tick -- the arriving view runs `ngAfterViewInit`
while the departing one is still in the DOM -- and a document-wide
lookup then returns the departing view's container, first in document
order. The paper was built into a div about to be removed, and the
arriving canvas kept an empty one.
Measured in a real browser, at the moment the canvas comes back:
[GETBYID] workflow-editor: 2 in document, returned index 0
before: #workflow-editor 1399x1000, svg=false, cells=0
after: #workflow-editor 1399x1000, svg=true, cells=1
It resolves both elements from its own host now. No run is needed to
reproduce; expanding the Form View's preview once is enough, which is
why watching a run there made it certain.
Deletion-checked: restoring the document-wide lookup turns exactly the
new named test red.
Full frontend suite: 224 files, 6150 passed, 1 skipped, 0 failed. AOT
build, eslint and prettier clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY
Yicong-Huang
added
release/v1.3
Backport auto-label reportThis
|
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8607 +/- ##
=========================================
Coverage 92.78% 92.78%
Complexity 4899 4899
=========================================
Files 1236 1236
Lines 52120 52144 +24
Branches 6405 6409 +4
=========================================
+ Hits 48357 48381 +24
Misses 2186 2186
Partials 1577 1577
*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:
|
|
Closing: the fix belongs in #8581 rather than beside it. The defect is older than #8581 (the lookup predates the The commit is unchanged, now the third on #8581's branch. #8606 stays open for the two other document-wide lookups of the same id, which #8581 does not need. |
3 participants
What changes were proposed in this PR?
WorkflowEditorComponentfound the element it builds its JointJS paper into with a document-wide lookup:Those ids come from this component's own template, so every instance of it renders elements carrying them. The lookup does not return this instance's container; it returns whichever is first in document order. Three places mount this component -- the operator canvas, the Form View's workflow preview, and the Hub's read-only preview -- so "only one is ever in the page" was an assumption, not a fact.
It resolves both from its own host now (
ElementRef, which was already injected and otherwise unused). The two!assertions are also honest for the first time: the divs are this component's own template with no structural directive above them, so they cannot be missing, whereas the document lookup genuinely could return null.What this was reaching for.
elfor the paper is the visible one, but fifteen places read these two fields, and every one of them was acting on the wrong element whenever a second instance existed:setDimensionsfrom the other view's wrapper,classList.add("hide-operator-status")on the other view's div, aResizeObserverwatching it,fromEvent(this.editor, "mousemove")bound to it, and the centring inhandleCenterEventmeasuring it.Before
After
Measured in a browser on that branch, at the moment the canvas comes back from the Form View:
This is not reachable on
mainas it stands: every route that mounts a second editor does so after a full page load, so the instances never coexist and the first match is always the right one. Verified on a local instance running plainmain(957b6c965), where the same steps keep the canvas live and the lookup only ever sees one container. It becomes reachable the moment two coexist, which is what #8581 does by routing between a workflow's two views.The canvas was left with an empty container: nothing to pan, nothing to click, while the graph itself was untouched -- the Form View's preview went on showing it, including a run in progress, right up to the switch.
Any related issues, documentation, discussions?
Closes #8606.
Found while browser-testing #8581, which is blocked on this: with the switch routed and this unfixed, coming back to the canvas from a Form View whose preview has been opened leaves it blank.
Two more document-wide lookups of the same id are deliberately left alone, since neither leaves a stuck state and both want their own reasoning:
MiniMapComponent.updateNavigatorreads it to place the navigator rectangle (self-corrects on the next pointer event) andReportGenerationServicereads it to snapshot the canvas. Both are noted in #8606.Changing the ids to classes, which is the other half of the story (two elements sharing an id in one document is invalid HTML), is not done here: it reaches into the component's SCSS and those two other readers, and this fix is worth having on its own.
How was this PR tested?
workflow-editor.component.spec: a second editor created while a decoy#workflow-editorsits earlier in the document still builds its paper in its own container, and the decoy stays empty. This reproduces the defect deterministically without any routing, so it fails onmaintoday.Deletion-checked: restoring the document-wide lookup turns exactly that one named test red (1 failed, 117 passed).
Verified end to end in a real browser as well, on the #8581 branch where the defect is reachable: switch to the Form View, expand its workflow preview, switch back, and the canvas comes back live rather than blank. No run is needed; watching a run in the preview simply makes it certain, because that is when a reader expands the preview.
Full frontend suite: 224 files, 6119 passed, 1 skipped (pre-existing), 0 failed.
ng build --configuration=production(AOT),eslintandprettier --checkon both changed files: clean.Was this PR authored or co-authored using generative AI tooling?
Yes. Generated-by: Claude Code (Claude Opus 5, Anthropic). Co-authored with Claude; the author reviewed the change line by line and reproduced both the failure and the fix in a running instance before submission.
🤖 Generated with Claude Code
https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY