yangzhang75 opened a new pull request, #8607:
URL: https://github.com/apache/texera/pull/8607
### What changes were proposed in this PR?
`WorkflowEditorComponent` found the element it builds its JointJS paper into
with a document-wide lookup:
```ts
this.editor = document.getElementById("workflow-editor")!;
this.editorWrapper = document.getElementById("workflow-editor-wrapper")!;
```
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.** `el` for 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: `setDimensions` from the
other view's wrapper, `classList.add("hide-operator-status")` on the other
view's div, a `ResizeObserver` watching it, `fromEvent(this.editor,
"mousemove")` bound to it, and the centring in `handleCenterEvent` measuring it.
#### Before / after
Not reachable on `main` as 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. It becomes reachable the moment two do
coexist, which is what #8581 does by routing between a workflow's two views
instead of reloading. Measured in a browser on that branch, at the moment the
canvas comes back from the Form View:
```
two in the document, the lookup returns index 0 (the departing view's)
before: #workflow-editor 1399x1000 svg=false cells=0
after: #workflow-editor 1399x1000 svg=true cells=1
```
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.updateNavigator` reads it to place the navigator rectangle
(self-corrects on the next pointer event) and `ReportGenerationService` reads
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-editor` sits 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 on `main` today.
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), `eslint` and `prettier
--check` on 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.com/claude-code)
https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
